[webkit-reviews] review granted: [Bug 197940] [JSC] Shrink Metadata : [Attachment 370149] Patch

[bugzilla-daemon at webkit.org]

Michael Saboff <msaboff at apple.com> has granted Yusuke Suzuki
<ysuzuki at apple.com>'s request for review:
Bug 197940: [JSC] Shrink Metadata
https://bugs.webkit.org/show_bug.cgi?id=197940

Attachment 370149: Patch

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




--- Comment #30 from Michael Saboff <msaboff at apple.com> ---
Comment on attachment 370149
  --> https://bugs.webkit.org/attachment.cgi?id=370149
Patch

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

r=me

> Source/JavaScriptCore/ChangeLog:13
> +	   If we shrink Metadata, we can get peak memory footprint in Gmail.

Is this better wording: "If we shrink Metadata, we can *reduce the* peak memory
footprint in Gmail."?

> Source/JavaScriptCore/ChangeLog:15
> +	   This patch attempts to shrink Metadata. This patch first focus on
low hanging fruits: it does not include the change removing OSR exit JSValue in
ValueProfile.

I think you want to say "This patch shrinks Metatdata."

> Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:749
> +    metadata.m_modeMetadata.mode = GetByIdMode::ProtoLoad; // This must be
first set. In 64bit architecture, this field is shared with
protoLoadMode.cachedSlot.

Instead of relying on convention here, it may make sense to have a setting
function in GetByIdMetadata.h that sets all values in the proper order.