[webkit-reviews] review granted: [Bug 187440] [JSC] Optimize layout of SourceProvider to reduce padding : [Attachment 344528] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Jul 7 14:10:04 PDT 2018


Mark Lam <mark.lam at apple.com> has granted Yusuke Suzuki
<utatane.tea at gmail.com>'s request for review:
Bug 187440: [JSC] Optimize layout of SourceProvider to reduce padding
https://bugs.webkit.org/show_bug.cgi?id=187440

Attachment 344528: Patch

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




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

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

r=me with suggested changes.

> Source/JavaScriptCore/parser/SourceProvider.h:90
> +	   uintptr_t m_id { 0 };

I think m_id does not need to be a uintptr_t.  You can make it a uint32_t and
move it above to save another 8 bytes (down to 64).  If we have more than 4G of
unique SourceProviders, I think we would have other memory problems long before
we get to exhausting the ids.

Also:
1. in SourceProvider::getID(), change nextProviderID into a static uint32_t.
2. in SourceProvider::getID(), add a RELEASE_ASSERT(m_id) to make sure that
nextProviderID does not overflow, and we do not fail silently.	This should
never happen, but it's good to be explicit, and the RELEASE_ASSERT will also
document this constraint.


More information about the webkit-reviews mailing list