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

[bugzilla-daemon at webkit.org]

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





--- Comment #9 from Myles C. Maxfield <mmaxfield at google.com>  2012-03-12 17:55:39 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().
> 
> This is yucky since pageSize() is not known at compile-time.
> 
> I think we should be using our own fake notion of page size (64KB?) whenever possible and just asserting at run-time that it is both larger than pageSize() and is a (power-of-two) multiple of it.

pageSize() already reads a constant which gets set once: if (!s_pageSize) s_pageSize = systemPageSize(); ... return s_pageSize;

Should I continue this strategy and add another size_t to PageBlock.cpp? Or should I initialize the value in initializeThreading() like you said? Moving it to initializeThreading would make pageSize() slightly faster, as it wouldn't have to check to see if the constant is already initialized. Or, should I use a constant page size like Filip Pizlo said? I think using a constant page size doesn't make as much sense as using the system page size, initializing it once, and caching it.

I'm happy to do whatever you guys decide on; I'm just getting mixed messages.

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