[webkit-reviews] review granted: [Bug 182217] MarkedBlock should have a footer instead of a header : [Attachment 332484] the patch

[bugzilla-daemon at webkit.org]

JF Bastien <jfbastien at apple.com> has granted Filip Pizlo <fpizlo at apple.com>'s
request for review:
Bug 182217: MarkedBlock should have a footer instead of a header
https://bugs.webkit.org/show_bug.cgi?id=182217

Attachment 332484: the patch

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




--- Comment #6 from JF Bastien <jfbastien at apple.com> ---
Comment on attachment 332484
  --> https://bugs.webkit.org/attachment.cgi?id=332484
the patch

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

r=me

> Source/JavaScriptCore/heap/MarkedBlock.cpp:92
> +	   dataLog(RawPointer(this), ": Allocated.\n");

Drop logging or add verbose flag.

> Source/JavaScriptCore/heap/MarkedBlock.h:269
> +	   CountingLock m_lock;

This is 4 bytes, I'd put it next to HeapVersion (also 4) instead of before the
2x 2 byte mark counts.

> Source/JavaScriptCore/heap/MarkedBlock.h:615
>      return true;

Seems like all uses of isAtom() should be succeeded by a size mask (when
accessing the atoms) to Spectre-protect the accesses?