[Webkit-unassigned] [Bug 80615] CopiedSpace::tryAllocateOversize assumes system page size

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=80615


Geoffrey Garen <ggaren at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #131466|review?                     |review-
               Flag|                            |




--- Comment #7 from Geoffrey Garen <ggaren at apple.com>  2012-03-12 17:35:02 PST ---
(From update of attachment 131466)
View in context: https://bugs.webkit.org/attachment.cgi?id=131466&action=review

> Source/JavaScriptCore/heap/CopiedSpaceInlineMethods.h:174
> +    return reinterpret_cast<CopiedBlock*>(reinterpret_cast<size_t>(ptr) & WTF::pageMask());

This function is hot, so I'd prefer not to turn a constant into two out-of-line function calls.

I'd suggest making WTF::pageSize() and WTF::pageMask() inline function calls that read globals inside WTF. You can initialize these globals inside WTF::initializeThreading().

> Source/JavaScriptCore/wtf/StdLibExtras.h:76
> +#define ROUND_UP_TO_MULTIPLE_OF(divisor, x) \
> +    size_t remainderMask = divisor - 1; \
> +    return (x + remainderMask) & ~remainderMask;

I'd prefer to see this as an inline function. In fact, you've already written that inline function. So now, all you need is to make you function template call your other function.

> Source/JavaScriptCore/wtf/StdLibExtras.h:166
> +inline size_t roundUpToMultipleOf(size_t divisor, size_t x)

This function should ASSERT that "divisor" is a power of two.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.