[webkit-reviews] review denied: [Bug 102828] r134080 causes heap problem on linux systems where PAGESIZE != 4096 : [Attachment 183468] Make capacity field static.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jan 18 10:19:38 PST 2013


Mark Hahnenberg <mhahnenberg at apple.com> has denied Balazs Kilvady
<kilvadyb at homejinni.com>'s request for review:
Bug 102828: r134080 causes heap problem on linux systems where PAGESIZE != 4096
https://bugs.webkit.org/show_bug.cgi?id=102828

Attachment 183468: Make capacity field static.
https://bugs.webkit.org/attachment.cgi?id=183468&action=review

------- Additional Comments from Mark Hahnenberg <mhahnenberg at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=183468&action=review


Additionally, there's a function named "sizeFromCapacity" in MarkStackSegment.
That doesn't appear to be called from anywhere, so let's remove it while we're
here.

> Source/JavaScriptCore/heap/MarkStack.cpp:47
> +#if COMPILER(CLANG)

Let's not ignore the warnings.

> Source/JavaScriptCore/heap/MarkStack.cpp:52
> +const size_t MarkStackArray::s_segmentCapacity =
MarkStackSegment::capacityFromSize(MarkStackSegment::blockSize);

If we could use C++11, we could just add constexpr to capacityFromSize and call
it a day. Unfortunately we can't, so we have to use some template magic.

Instead of using the old capacityFromSize static function, let's rework the
code a little bit. From some brief grepping, it appears that this is the only
client of the capacityFromSize function. So in order to get a compile-time
constant value, we can instead use a template. Something along the lines of:

template <size_t size> struct CapacityFromSize {
    static const size_t value = // body of current capacityFromSize
};

Stick that somewhere private in MarkStackArray and grab the ::value when
defining s_segmentCapacity.


More information about the webkit-reviews mailing list