Bug 182717 – [JSC] cache TaggedTemplate arrays by callsite rather than by contents

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

[Caitlin Potter (:caitp)]

[JSC] cache TaggedTemplate arrays by callsite rather than by contents

[Caitlin Potter (:caitp)]

Created attachment 333641 [details]
[JSC] cache TaggedTemplate arrays by callsite rather than by contents

[Caitlin Potter (:caitp)]

This patch is based on the v8 implementation @ https://chromium.googlesource.com/v8/v8/+/d3ca0d005077085a30e27415827325ea2216a735 (so, some class/file renaming is influenced by that)

First patch:

- Change BytecodeGenerator mapping from TemplateRegistryKey -> RegisterID to TemplateRegistryKey -> JSTemplateRegistryKey, to prevent creating more JS wrappers than necessary (this probably doesn't buy anything valuable, so no reason not to get rid of this in a later version) --- A new constant pool index is always created, because each TemplateRegistryKey (TemplateObjectDescriptor) gets its own array (per callsite).

- Disable DirectEvalCodeCache if tagged templates occur in code block. This ensures each call to `eval()` will create own template arrays, per spec.

- Delete TemplateRegistry and TemplateRegistryKeyTable

Tests aren't updated yet, wanting to see what will fail before fixing those up or adding new ones.

[EWS Watchlist]

Comment on attachment 333641 [details]
[JSC] cache TaggedTemplate arrays by callsite rather than by contents

Attachment 333641 [details] did not pass jsc-ews (mac):
Output: http://webkit-queues.webkit.org/results/6476510

New failing tests:
stress/tagged-template-registry-key.js.ftl-no-cjit-no-inline-validate
stress/tagged-template-registry-key.js.ftl-no-cjit-no-put-stack-validate
stress/tagged-templates-identity.js.ftl-no-cjit-no-inline-validate
stress/tagged-templates-identity.js.ftl-no-cjit-no-put-stack-validate
stress/tagged-template-registry-key.js.no-ftl
stress/tagged-templates-identity.js.no-ftl
stress/template-string-tags-eval.js.dfg-eager
stress/tagged-template-registry-key.js.dfg-maximal-flush-validate-no-cjit
stress/tagged-template-registry-key.js.no-llint
stress/tagged-templates-identity.js.no-llint
stress/template-string-tags-eval.js.no-cjit-collect-continuously
stress/tagged-template-registry-key.js.no-cjit-collect-continuously
stress/tagged-templates-identity.js.no-cjit-collect-continuously
stress/tagged-templates-identity.js.no-cjit-validate-phases
stress/template-string-tags-eval.js.ftl-eager-no-cjit
stress/tagged-template-registry-key.js.no-cjit-validate-phases
stress/tagged-templates-identity.js.dfg-maximal-flush-validate-no-cjit
stress/template-string-tags-eval.js.no-ftl
stress/tagged-templates-identity.js.ftl-eager
stress/template-string-tags-eval.js.dfg-maximal-flush-validate-no-cjit
stress/tagged-template-registry-key.js.ftl-eager
stress/tagged-templates-identity.js.ftl-eager-no-cjit
stress/tagged-template-registry-key.js.ftl-eager-no-cjit
stress/tagged-template-registry-key.js.ftl-no-cjit-b3o1
stress/tagged-templates-identity.js.ftl-no-cjit-b3o1
stress/template-string-tags-eval.js.ftl-no-cjit-validate-sampling-profiler
stress/template-string-tags-eval.js.dfg-eager-no-cjit-validate
stress/template-string-tags-eval.js.ftl-eager
stress/tagged-templates-identity.js.dfg-eager-no-cjit-validate
stress/tagged-template-registry-key.js.dfg-eager-no-cjit-validate
stress/tagged-templates-identity.js.dfg-eager
stress/tagged-templates-identity.js.ftl-no-cjit-small-pool
stress/tagged-template-registry-key.js.default
stress/tagged-templates-identity.js.default
stress/template-string-tags-eval.js.ftl-no-cjit-no-inline-validate
stress/tagged-template-registry-key.js.ftl-eager-no-cjit-b3o1
stress/tagged-templates-identity.js.ftl-eager-no-cjit-b3o1
stress/tagged-template-registry-key.js.dfg-eager
stress/template-string-tags-eval.js.default
stress/tagged-template-registry-key.js.ftl-no-cjit-small-pool
stress/template-string-tags-eval.js.ftl-eager-no-cjit-b3o1
stress/template-string-tags-eval.js.ftl-no-cjit-no-put-stack-validate
stress/template-string-tags-eval.js.ftl-no-cjit-b3o1
stress/template-string-tags-eval.js.ftl-no-cjit-small-pool
stress/template-string-tags-eval.js.no-llint
stress/tagged-template-registry-key.js.ftl-no-cjit-validate-sampling-profiler
stress/template-string-tags-eval.js.no-cjit-validate-phases
stress/tagged-templates-identity.js.ftl-no-cjit-validate-sampling-profiler

[Caitlin Potter (:caitp)]

Created attachment 333671 [details]
Patch

[Yusuke Suzuki]

Comment on attachment 333671 [details]
Patch

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

r=me with test262.yaml update. Since test262 includes tests for this, we should update test262.yaml.

> Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.cpp:75
> +    , m_hasTaggedTemplates(false)

Drop this since it is not used.

> Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:457
> +    unsigned m_hasTaggedTemplates : 1;

Ditto.

[Caitlin Potter (:caitp)]

Created attachment 333693 [details]
Patch

Remove unneeded member from UnlinkedCodeBlock and update test262.yaml

[Yusuke Suzuki]

Comment on attachment 333693 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 333693 [details]
Patch

Clearing flags on attachment: 333693

Committed r228422: <https://trac.webkit.org/changeset/228422>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/37501758>