Closed Bug 1687973 Opened 4 years ago Closed 3 years ago

Move stencil instantiation of decode task to main thread


(Core :: JavaScript Engine, task, P3)




96 Branch
Tracking Status
firefox96 --- fixed


(Reporter: arai, Assigned: arai)


(Blocks 2 open bugs)



(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: 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
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


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
Part 1: Remove CompileOptions.useOffThreadParseGlobal. r=tcampbell
Part 2: Remove ParseTask::parseGlobal. r=tcampbell
Part 3: Remove the pending off-thread parse tasks waiting on GC. r=tcampbell
Part 4: Replace LeaveParseTaskZone with ParseTask::deactivate. r=tcampbell
Part 5: Remove GlobalHelperThreadState::generateLCovSources. r=tcampbell
Part 6: Remove ParseTask::scripts. r=tcampbell
Part 7: Remove ModuleObject::fixEnvironmentsAfterRealmMerge. r=tcampbell
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.