[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 #4 from Mark Hahnenberg <mhahnenberg at apple.com>  2012-03-12 10:14:02 PST ---
(From update of attachment 131073)
View in context: https://bugs.webkit.org/attachment.cgi?id=131073&action=review

I'm not an official reviewer, but I figured I'd give you some suggestions for improvement for when an official reviewer does get around to review this :-)

> Source/JavaScriptCore/wtf/BumpPointerAllocator.h:114
> +        size_t poolSize = std::max(static_cast<size_t>(MINIMUM_BUMP_POOL_SIZE), WTF::pageSize());

What does this change have to do with refactoring CopiedSpace to use WTF::pageSize()?

> Source/JavaScriptCore/wtf/StdLibExtras.h:74
> +#define __ROUND_UP_TO_MULTIPLE_OF__ \

Why not make this a function macro? It's kind of confusing to assume variables names from the surrounding scope where the macro is invoked and to have a return statement in the macro itself. 

Also, naming things with two initial underscores is generally frowned upon because they're typically assumed to be reserved by the compiler.

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