[webkit-reviews] review granted: [Bug 205712] [JSC] MarkedBlock::Handle and BlockDirectory should be shrunk : [Attachment 386662] Patch

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 205712: [JSC] MarkedBlock::Handle and BlockDirectory should be shrunk
https://bugs.webkit.org/show_bug.cgi?id=205712

Attachment 386662: Patch

https://bugs.webkit.org/attachment.cgi?id=386662&action=review




--- Comment #2 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 386662
  --> https://bugs.webkit.org/attachment.cgi?id=386662
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=386662&action=review

r=me with a suggestion.

> Source/JavaScriptCore/heap/MarkedBlock.h:557
> +// This function must return size_t instead of unsigned since pointer |p| is
not guaranteed that this is within MarkedBlock.
> +// See MarkedBlock::isAtom which can accept out-of-bound pointers.
>  inline size_t MarkedBlock::atomNumber(const void* p)

Seems like isAtom() is the only case that can potential use atomNumber with a
bad pointer while lots of other clients expect the pointer to be valid.  How
about doing this instead?
1. rename this function to candidateAtomNumber() and change isAtom() to use it.
2. add a new inline function atomNumber() that calls candidateAtomNumber() in
its implementation, ASSERT(!(atomNumber % handle().m_atomsPerCell)), and return
the atomNumber() as an unsigned value.