| Summary: | [JSC] cache TaggedTemplate arrays by callsite rather than by contents | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Caitlin Potter (:caitp) <caitp> | ||||||||
| Component: | JavaScriptCore | Assignee: | Caitlin Potter (:caitp) <caitp> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | cdumez, commit-queue, ews-watchlist, fpizlo, mark.lam, saam, webkit-bug-importer, ysuzuki, zan | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Caitlin Potter (:caitp)
2018-02-12 16:06:27 PST
Created attachment 333641 [details]
[JSC] cache TaggedTemplate arrays by callsite rather than by contents
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. 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 Created attachment 333671 [details]
Patch
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. Created attachment 333693 [details]
Patch
Remove unneeded member from UnlinkedCodeBlock and update test262.yaml
Comment on attachment 333693 [details]
Patch
r=me
Comment on attachment 333693 [details] Patch Clearing flags on attachment: 333693 Committed r228422: <https://trac.webkit.org/changeset/228422> All reviewed patches have been landed. Closing bug. |