Update Firefox to utilize zstd for Translations models and WASM
Categories
(Firefox :: Translations, task)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox145 | --- | fixed |
People
(Reporter: nordzilla, Assigned: isaacbriandt10, Mentored)
References
Details
(Keywords: perf-alert)
Attachments
(5 files, 2 obsolete files)
Description
In Bug 1947431 we implemented zstd for DecompressionStream and allowed it for internal use within privileged Firefox contexts.
We now have v2 Remote Settings collections that are designed to host and serve zstd compressed models and WASM.
We need to hook everything together to allow Firefox to pull from the compressed collections, and decompress the models as-needed.
|
Assignee | |
Comment 1•4 months ago
|
||
|
|
Assignee | |
Comment 2•4 months ago
|
||
|
Reporter | |
Comment 3•4 months ago
|
||
|
||
Updated•4 months ago
|
|
|
||
Updated•4 months ago
|
|
|
||
Updated•4 months ago
|
|
|
Reporter | |
Comment 4•4 months ago
|
||
This patch modifies the RemoteSettings client's download()
method to be able to return a Blob directly, in addition to
the ArrayBuffer that it already returns.
This prevents the Translations code from having to manually
construct a Blob from the buffer, prevening a jank in the parent
process.
|
|
Reporter | |
Comment 5•4 months ago
|
||
The build should be exactly the same, though I technically
built it from a different hash on GitHub, so we should
update that.
|
|
Reporter | |
Comment 6•3 months ago
|
||
This patch is a condensed version of my initial WIP patch stack
for Bug 1982594 where I initially encountered this behavior.
The behavior reproduces by running the following command:
./mach test browser_translations_e2e
Rather than executing successfully, the WASM instantiation
will hang indefinitely until the test case times out.
This code is very likely to bit rot, though I hope it proves
to be useful in exploring this behavior. I haven't been able
to reproduce in a different way at the time of writing.
|
|
||
Comment 7•3 months ago
|
||
Comment on attachment 9512975 [details]
WIP: Bug 1982594 - (Temporary) Attempt to reproduce behavior
Revision D264662 was moved to bug 1988289. Setting attachment 9512975 [details] to obsolete.
|
|
Reporter | |
Comment 8•3 months ago
|
||
This patch modifies the WASM function for instantiating
to use synchronous functions instead of WebAssembly.instantiate().
See Bug 1988289 for more information.
|
|
||
Updated•3 months ago
|
|
|
||
Updated•3 months ago
|
|
|
||
Updated•3 months ago
|
|
|
||
Updated•3 months ago
|
|
|
||
Updated•3 months ago
|
|
|
||
Updated•3 months ago
|
|
||
Comment 10•3 months ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/eb6fd21e3556
https://hg.mozilla.org/mozilla-central/rev/c7cbbf7cf116
https://hg.mozilla.org/mozilla-central/rev/915cfd067f0e
https://hg.mozilla.org/mozilla-central/rev/8a41bb5bb9fc
https://hg.mozilla.org/mozilla-central/rev/e0d24a0fe50f
|
||
Comment 11•3 months ago
|
||
(In reply to Pulsebot from comment #9)
Pushed by enordin@mozilla.com:
https://github.com/mozilla-firefox/firefox/commit/a30981d711c7
https://hg.mozilla.org/integration/autoland/rev/eb6fd21e3556
Update Translations RemoteSettings schemas r=translations-reviewers,gregtatum
https://github.com/mozilla-firefox/firefox/commit/03f01898fa06
https://hg.mozilla.org/integration/autoland/rev/c7cbbf7cf116
Rework TranslationsEnginePaylod IPC Transfer
r=translations-reviewers,gregtatum
https://github.com/mozilla-firefox/firefox/commit/c6b8c023e69c
https://hg.mozilla.org/integration/autoland/rev/915cfd067f0e
Fix Translations Tests r=translations-reviewers,gregtatum
https://github.com/mozilla-firefox/firefox/commit/02fefb1e1bae
https://hg.mozilla.org/integration/autoland/rev/8a41bb5bb9fc
Update Bergamot build hash r=translations-reviewers,gregtatum
https://github.com/mozilla-firefox/firefox/commit/9140dac1eab8
https://hg.mozilla.org/integration/autoland/rev/e0d24a0fe50f
Patch Translations WASM Instantiation r=translations-reviewers,gregtatum
Perfherder has detected a browsertime performance change from push e0d24a0fe50f7056a03b8c4b7e66f2c7b5d5d401.
If you have any questions, please reach out to a performance sheriff. Alternatively, you can find help on Slack by joining #perf-help, and on Matrix you can find help by joining #perftest.
Improvements:
| Ratio | Test | Platform | Options | Absolute values (old vs new) | Performance Profiles |
|---|---|---|---|---|---|
| 9% | vpl-av1 estimatedFirstFrameLatency | linux1804-64-shippable-qr | cold fission webrender | 134.51 -> 122.44 | Before/After |
| 9% | vpl-av1 firstFrame | linux1804-64-shippable-qr | cold fission webrender | 134.51 -> 122.44 | Before/After |
| 8% | vpl-av1 estimatedAnyFrameLatency | linux1804-64-shippable-qr | cold fission webrender | 155.44 -> 143.35 | Before/After |
| 7% | vpl-vp9 estimatedAnyFrameLatency | linux1804-64-shippable-qr | cold fission webrender | 135.03 -> 125.11 | Before/After |
| 7% | vpl-av1 secondFrame | linux1804-64-shippable-qr | cold fission webrender | 278.02 -> 257.67 | Before/After |
| ... | ... | ... | ... | ... | ... |
| 3% | vpl-vp9 secondFrame | linux1804-64-shippable-qr | cold fission webrender | 248.31 -> 240.91 | Before/After |
Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.
If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a performance sheriff to do that for you.
You can run all of these tests on try with ./mach try perf --alert 46968
The following documentation link provides more information about this command.
|
|
Reporter | |
Comment 12•3 months ago
•
|
||
:aosmond
I genuinely have no idea why my patch stack would affect these metrics.
I'm happy they improved, but I'm wondering if there is a mistake.
|
||
Updated•2 months ago
|
|
||
Comment 13•2 months ago
|
||
The test can be a bit noisy sometimes where it jumps modes. I suspect that is it.
Description
•