[webkit-reviews] review granted: [Bug 216061] [JSC] Cache toString / valueOf / @@toPrimitive for major cases : [Attachment 407811] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 216061: [JSC] Cache toString / valueOf / @@toPrimitive for major cases
https://bugs.webkit.org/show_bug.cgi?id=216061

Attachment 407811: Patch

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




--- Comment #7 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 407811
  --> https://bugs.webkit.org/attachment.cgi?id=407811
Patch

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

Nice. r=me

>
Source/JavaScriptCore/runtime/CachedSpecialPropertyAdaptiveStructureWatchpoint.
h:2
> + * Copyright (C) 2019 Apple Inc. All rights reserved.

2020

> Source/JavaScriptCore/runtime/StructureRareData.cpp:112
> +    WTF::storeStoreFence(); // Store to the field of this struct should be
visible to the concurrent collectors after struct is visible.

from what I said on slack, I don't think this is needed, but double check

> Source/JavaScriptCore/runtime/StructureRareData.cpp:184
> +	       cache.m_presentWatchpoint =
makeUnique<CachedSpecialPropertyAdaptiveInferredPropertyValueWatchpoint>(equivC
ondition, this);

let's call "m_presentWatchpoint" m_equivalenceWatchpoint or something similar

> Source/JavaScriptCore/runtime/StructureRareData.cpp:219
> +		       continue;

continue isn't needed. Did you mean break so we continue the outer loop?

> Source/JavaScriptCore/runtime/StructureRareDataInlines.h:39
> +    Bag<CachedSpecialPropertyAdaptiveStructureWatchpoint> m_missWatchpoints;
> +   
std::unique_ptr<CachedSpecialPropertyAdaptiveInferredPropertyValueWatchpoint>
m_presentWatchpoint;

maybe in the future we can use ObjectPropertyConditionSet? Or is this just to
save memory?