[webkit-reviews] review granted: [Bug 132351] JSProxies should be cacheable : [Attachment 230414] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Mark Hahnenberg
<mhahnenberg at apple.com>'s request for review:
Bug 132351: JSProxies should be cacheable
https://bugs.webkit.org/show_bug.cgi?id=132351

Attachment 230414: Patch
https://bugs.webkit.org/attachment.cgi?id=230414&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=230414&action=review


r=me

> Source/JavaScriptCore/ChangeLog:15
> +	   This is performance neutral on the benchmarks we track, although
currently we won't

I would remove "although", and just start a new sentence.

> Source/JavaScriptCore/jsc.cpp:611
> +    Structure* structure = Proxy::createStructure(exec->vm(),
exec->lexicalGlobalObject(), jsTarget->prototype());

You could just instantiate JSProxy here.

> Source/JavaScriptCore/jit/Repatch.cpp:746
> +	   if (!structure->propertyAccessesAreCacheable())
> +	       return false;

Let's rearrange this to share the propertyAccessesAreCacheable() check. It's
nice to keep the PureForwardingProxyType thingy only substituting the base, and
not doing anything else.