Bug 132351 – JSProxies should be cacheable

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Hahnenberg]

Whenever we encounter a proxy in an inline cache we should try to cache on the proxy's target instead of giving up.

[Mark Hahnenberg]

Created attachment 230406 [details]
Patch

[WebKit Commit Bot]

Attachment 230406 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/jit/Repatch.cpp:311:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/jit/Repatch.cpp:322:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/Operations.h:264:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 3 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Comment on attachment 230406 [details]
Patch

Lol can has some tests?  I'd add a fako Proxy in jsc.cpp that behaves like JSGLobalObject (maybe we have one already?) and then write some tests that various try and either succeed or fail in caching on it.

[Mark Hahnenberg]

Created attachment 230414 [details]
Patch

[WebKit Commit Bot]

Attachment 230414 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/Operations.h:264:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Geoffrey Garen]

Comment on attachment 230414 [details]
Patch

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.

[Mark Hahnenberg]

Committed r167963: <http://trac.webkit.org/changeset/167963>