Closed Bug 1687973 Opened 1 year ago Closed 2 months ago

Move stencil instantiation of decode task to main thread

Categories

(Core :: JavaScript Engine, task, P3)

task

Tracking

()

RESOLVED FIXED
96 Branch
Tracking Status
firefox96 --- fixed

People

(Reporter: arai, Assigned: arai)

References

(Depends on 2 open bugs, Blocks 2 open bugs)

Details

Attachments

(8 files)

To revert the change from bug 1674305.

in short, currently single script decode task uses parse global and instantiate off-main-thread, because of the following reasons:

  • there are some case that the time taken by on-main-thread instantiation regresses benchmark score
  • if decode task finishes early, it affects scheduling and some benchmark score regresses, because the benchmark itself is bimodal and bad case happens more (see bug 1674305 comments)

bug 1667903 will help the latter, and we can look into this bug after that.

Depends on: 1667903
No longer depends on: 1655768

(fixed the wrong bug number in above comment also)

Depends on: 1721120

bug 1721413 patch can solve the issue partially, that the number of atomization is reduced.

Depends on: 1721413
Blocks: 1724236
Blocks: 1734098

I'll do the following at the same time, for dependency reasons:

  • remove parse global
  • remove some helper thread handling from zone and runtime
  • remove OffThreadPlaceholderObject
  • remove mergeRealm
  • simplify atom zone handling
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
See Also: → 1655768, 1587847, 538450
See Also: → 1468422

just flipping CompileOptions.useOffThreadParseGlobal results in 2-4% regression in browsertime tp6.

I'm going to land some more cleanup and optimization at the same time:
bug 538450 patch should improve the on-main-thread atomization

WIP: https://treeherder.mozilla.org/jobs?repo=try&group_state=expanded&revision=a2dd1f684de9dee70c3e2bfc8f08a2f92781dcf3

interestingly, the notable regression happens mostly in cold load, that means the regression for the decode case is solved by the patch stack,
but the non-decode case (== parse case) gets affected by it.

I'll investigate how the patch affects non-decode case.

reproduced the regression for buzfeed page loadtime locally.
it was slightly bi-modal, and the frequency of the good case is reduced by the patch stack.

will check microsoft page.

microsoft page also has multimodal behavior for loadtime, but I don't observe any notable regression or difference locally on macOS.
it might be more environment dependent/specific issue.

Changed all off-thread tasks not to use parse global.
Removed bug-1138390.js because off-thread script decode no longer wait for GC.

  • ParseTask::parseGlobal is removed in Part 2
  • OffThreadParsingMustWaitForGC is removed in Part 3
  • GlobalHelperThreadState::parseWaitingOnGC is removed in Part 3
  • GlobalHelperThreadState::generateLCovSources is removed in Part 5
  • ParseTask::scripts is removed in Part 6
  • ModuleObject::fixEnvironmentsAfterRealmMerge is removed in Part 7
  • Zone::{setCreatedForHelperThread,clearUsedByHelperThread} are removed in
    bug 538450
  • RealmCreationOptions::setMergeable is removed in bug 1655768
  • MergeRealms is removed in bug 1655768
  • Zone::{setHelperThreadOwnerContext,clearUsedByHelperThread} are removed in
    bug 538450
    • JSRuntime::{incOffThreadParsesRunning,decOffThreadParsesRunning} are removed
      in bug 538450

Depends on D131354

Instead of storing scripts in vector, directly use CompilationGCOutput.

Depends on D131359

so there are 2 issues, and both are not specific to this patch:

  • m-c push and try push aren't comparable, that's the reason why we were seeing "regression" above
  • browsertime tp6 warm on automation doesn't cover bytecode cache

then, given I don't see much notable regression in local runs (except the minor change in buzzfeed score),
I'd like to go with the current stack

Pushed by arai_a@mac.com:
https://hg.mozilla.org/integration/autoland/rev/cc1447c123a6
Part 1: Remove CompileOptions.useOffThreadParseGlobal. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/c2b3f1fbe182
Part 2: Remove ParseTask::parseGlobal. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/961d4175ecd0
Part 3: Remove the pending off-thread parse tasks waiting on GC. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/018beff38705
Part 4: Replace LeaveParseTaskZone with ParseTask::deactivate. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/607a5cf2b64f
Part 5: Remove GlobalHelperThreadState::generateLCovSources. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/84e011cd9c89
Part 6: Remove ParseTask::scripts. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/3ca43499f72f
Part 7: Remove ModuleObject::fixEnvironmentsAfterRealmMerge. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/3d7de0571335
Part 8: Cleanup ParseTask::parse code flow. r=tcampbell
You need to log in before you can comment on or make changes to this bug.