Add partitionAllocActualSize() for predicting actual size of allocation

Add partitionAllocActualSize() for predicting actual size of allocation

Similar to partitionAllocGetSize() but can be used before the allocation
has been performed.  Falls back to returning the size unchanged for
"impossible" cases, like when PartitionAlloc isn't actually used, or when
the size is too large to allocate.

R=cevans@chromium.org
BUG=

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=167644

[Jens Widell, 2014-02-21 12:07:12]

Attempt at size_t-based API.  Not sure I covered all the corners.

[Chris Evans, 2014-02-21 22:33:36]

https://codereview.chromium.org/172533004/diff/1/Source/wtf/PartitionAlloc.h
File Source/wtf/PartitionAlloc.h (right):

https://codereview.chromium.org/172533004/diff/1/Source/wtf/PartitionAlloc.h#...
Source/wtf/PartitionAlloc.h:569: {
Can we have a comment such as "caller must handle integer overflow", and an
ASSERT?

https://codereview.chromium.org/172533004/diff/1/Source/wtf/PartitionAlloc.h#...
Source/wtf/PartitionAlloc.h:573: ALWAYS_INLINE size_t
partitionAllocPredictSize(PartitionRootGeneric* root, size_t size)
I think we're doing a more accurate job than predicting. Perhaps
partitionAllocActualSize() ?

https://codereview.chromium.org/172533004/diff/1/Source/wtf/PartitionAlloc.h#...
Source/wtf/PartitionAlloc.h:581: if (LIKELY(bucket->numSystemPagesPerSlotSpan))
{
Can we make this into a small function? This check is now duplicated with other
places (at least partitionFreeSlowPath and partitionAllocSlowPath). I think we
could use a simple partitionBucketIsDirectMapped() and this will make the code
much more readable.

https://codereview.chromium.org/172533004/diff/1/Source/wtf/PartitionAlloc.h#...
Source/wtf/PartitionAlloc.h:581: if (LIKELY(bucket->numSystemPagesPerSlotSpan))
{
Can we abstract this to a simple function, partitionBucketIsDirectMapped()? It'd
make things more readable and we can also use it in a couple of places
(partitionAllocSlowPath, partitionAllocFastPath) to de-duplicate.

[Jens Widell, 2014-02-22 09:23:30]

Patch updated.

https://codereview.chromium.org/172533004/diff/1/Source/wtf/PartitionAlloc.h
File Source/wtf/PartitionAlloc.h (right):

https://codereview.chromium.org/172533004/diff/1/Source/wtf/PartitionAlloc.h#...
Source/wtf/PartitionAlloc.h:569: {
On 2014/02/21 22:33:37, Chris Evans wrote:
> Can we have a comment such as "caller must handle integer overflow", and an
> ASSERT?

Assuming (which I hear is a good idea in general) that size_t is at least 32-bit
and unsigned, the calculation here is safe from overflow well above the actual
size limit (INT_MAX - kSystemPageSize).

I've added a comment about the caller being required to check the size limit
(which is stricter than just guarding against overflow,) and assert that the
size is allowed.

https://codereview.chromium.org/172533004/diff/1/Source/wtf/PartitionAlloc.h#...
Source/wtf/PartitionAlloc.h:573: ALWAYS_INLINE size_t
partitionAllocPredictSize(PartitionRootGeneric* root, size_t size)
On 2014/02/21 22:33:37, Chris Evans wrote:
> I think we're doing a more accurate job than predicting. Perhaps
> partitionAllocActualSize() ?

Renamed it.

https://codereview.chromium.org/172533004/diff/1/Source/wtf/PartitionAlloc.h#...
Source/wtf/PartitionAlloc.h:581: if (LIKELY(bucket->numSystemPagesPerSlotSpan))
{
On 2014/02/21 22:33:37, Chris Evans wrote:
> Can we make this into a small function? This check is now duplicated with
other
> places (at least partitionFreeSlowPath and partitionAllocSlowPath). I think we
> could use a simple partitionBucketIsDirectMapped() and this will make the code
> much more readable.

Added such a helper. Also made a constant out of "INT_MAX - kSystemPageSize" for
the maximum size of direct mapped allocations (and thus any allocations, I
guess.)

[Chris Evans, 2014-02-23 07:40:48]

On 2014/02/22 09:23:30, Jens Lindström wrote:
> Patch updated.
> 
> https://codereview.chromium.org/172533004/diff/1/Source/wtf/PartitionAlloc.h
> File Source/wtf/PartitionAlloc.h (right):
> 
>
https://codereview.chromium.org/172533004/diff/1/Source/wtf/PartitionAlloc.h#...
> Source/wtf/PartitionAlloc.h:569: {
> On 2014/02/21 22:33:37, Chris Evans wrote:
> > Can we have a comment such as "caller must handle integer overflow", and an
> > ASSERT?
> 
> Assuming (which I hear is a good idea in general) that size_t is at least
32-bit
> and unsigned, the calculation here is safe from overflow well above the actual
> size limit (INT_MAX - kSystemPageSize).
> 
> I've added a comment about the caller being required to check the size limit
> (which is stricter than just guarding against overflow,) and assert that the
> size is allowed.
> 
>
https://codereview.chromium.org/172533004/diff/1/Source/wtf/PartitionAlloc.h#...
> Source/wtf/PartitionAlloc.h:573: ALWAYS_INLINE size_t
> partitionAllocPredictSize(PartitionRootGeneric* root, size_t size)
> On 2014/02/21 22:33:37, Chris Evans wrote:
> > I think we're doing a more accurate job than predicting. Perhaps
> > partitionAllocActualSize() ?
> 
> Renamed it.
> 
>
https://codereview.chromium.org/172533004/diff/1/Source/wtf/PartitionAlloc.h#...
> Source/wtf/PartitionAlloc.h:581: if
(LIKELY(bucket->numSystemPagesPerSlotSpan))
> {
> On 2014/02/21 22:33:37, Chris Evans wrote:
> > Can we make this into a small function? This check is now duplicated with
> other
> > places (at least partitionFreeSlowPath and partitionAllocSlowPath). I think
we
> > could use a simple partitionBucketIsDirectMapped() and this will make the
code
> > much more readable.
> 
> Added such a helper. Also made a constant out of "INT_MAX - kSystemPageSize"
for
> the maximum size of direct mapped allocations (and thus any allocations, I
> guess.)

LGTM

[Jens Widell, 2014-02-23 07:54:55]

The CQ bit was checked by jl@opera.com

[commit-bot: I haz the power, 2014-02-23 07:55:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jl@opera.com/172533004/70001

[commit-bot: I haz the power, 2014-02-23 09:22:01]

Change committed as 167644