1814550 - Experiment 2 separate immutable script data table for main thread runtime and all other threads

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P1)

Description

[Tooru Fujisawa [:arai]]

The issue in bug 1810160 and bug 1807228 yahoo-mail is that we move the bytecode de-duplication from off-thread compilation to main-thread instantiation, which increases the instantiation time and regresses the benchmark score.

Especially when the same page is loaded again and again, the immutable script data
table contains the same bytecode from previous load, and all bytecode gets de-duplicated.
The de-duplication task here is time-consuming because it needs to compare the entire bytecode if the bytecode already exists in the table.

The bytecode de-duplication has 2 purposes:

• (a) de-duplicate bytecode for different functions/scripts within single compilation and across multiple compilations
• (b) de-duplicate bytecode for same function/script across multiple compilations

and the (b) is problematic here.

Then, the purpose of bytecode de-duplication is purely about reducing memory consumption, and not about correctness.
which means, having small amount of duplication is acceptable, as long as it doesn't regress AWSY too much.

We can have 2 separate SharedImmutableScriptDataTable:

• one for main thread compilation which has access to JSRuntime, and
• one for all other compilation, which doesn't have access to JSRuntime

assuming that, if a script is compiled on the main thread in the previous load, the same script will be compiled on the main thread in the next load, and if the script is compiled off main thread, it will be compiled off main thread in the next load,
so that, (b) can be achieved even with 2 separate tables.

I'll experiment this here, to see how much AWSY regression happens with the 2 separate table.

Comment 1

[Tooru Fujisawa [:arai]]

https://treeherder.mozilla.org/jobs?repo=try&revision=53ed0aa7634f8150ba37bb5ca5a9855779e5167a

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: Bug 1814550 - Part 1: Use 2 separate SharedImmutableScriptDataTable instances for main-thread and all-other threads. r?tcampbell!

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: Bug 1814550 - Part 2: Remove BytecodeEmitter::cx. r?tcampbell!

Depends on D168767

Comment 4

[Tooru Fujisawa [:arai]]

here's benchmark result:
before: https://treeherder.mozilla.org/jobs?repo=try&revision=803a1ca8c4c320c9aee1ddb658a0b2aafb50b2c2
after: https://treeherder.mozilla.org/jobs?repo=try&revision=53ed0aa7634f8150ba37bb5ca5a9855779e5167a

AWSY: https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=803a1ca8c4c320c9aee1ddb658a0b2aafb50b2c2&newProject=try&newRevision=53ed0aa7634f8150ba37bb5ca5a9855779e5167a&framework=4&page=1&showOnlyComparable=1
browsertime yahoo-mail: https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=803a1ca8c4c320c9aee1ddb658a0b2aafb50b2c2&newProject=try&newRevision=53ed0aa7634f8150ba37bb5ca5a9855779e5167a&framework=13&page=1&showOnlyComparable=1

I don't see notable regression.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b08446bfcd74
Part 1: Use 2 separate SharedImmutableScriptDataTable instances for main-thread and all-other threads. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/602d77ebb399
Part 2: Remove BytecodeEmitter::cx. r=tcampbell

Comment 6

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/b08446bfcd74
https://hg.mozilla.org/mozilla-central/rev/602d77ebb399

Comment 7

[Tooru Fujisawa [:arai]]

I was wrong about the behavior.
Worker runtime still uses their own table.