1307399 - Investigate not waiting for off thread ion compilation during GC

Status: NEW

(Core :: JavaScript: GC, defect, P3)

Description

[Jon Coppeard (:jonco)]

Hannes says that we don't need to wait for off thread Ion compilations to finish after we cancel them during GC, since Ion never accesses GC pointers and cancelled compilations will not be linked.

Comment 1

[Jan de Mooij [:jandem]]

We do use (and dereference) template objects in codegen. Maybe we can cancel, then do some GC stuff, and then wait when we do sweeping? OTOH, we now do trace template objects we use off-thread, so maybe it is safe to just cancel without waiting.

Comment 2

[Ted Campbell [:tcampbell]]

Jan, do you know if this still is valid / makes sense?

Comment 3

[Jan de Mooij [:jandem]]

(In reply to Ted Campbell [:tcampbell] from comment #2)

Jan, do you know if this still is valid / makes sense?

The relevant code is here: https://searchfox.org/mozilla-central/rev/94c6b5f06d2464f6780a52f32e917d25ddc30d6b/js/src/vm/HelperThreads.cpp#321-335

The idea would be to just cancel() these builders without waiting for the thread to finish. A safer approach is to make it a two-step process, for example here in DiscardJITCodeForGC we could do something like this:

1. cancel() off-thread builders.
2. Discard all JIT code.
3. wait() for threads to finish.

So instead of waiting immediately we go do some other stuff first and hope that gives the threads enough time to finish. There could be a RAII class for steps 1 and 3.

The risk is the background thread accessing a script's BaselineScript, TypeScript, ICScript etc that we're discarding (but it really shouldn't).

Jon, do you know what the performance impact on GC is these days?

Comment 4

[Jon Coppeard (:jonco)]

(In reply to Jan de Mooij [:jandem] from comment #3)

Jon, do you know what the performance impact on GC is these days?

We don't have telemetry for cancelling Ion compilation specifically, so it's difficult to say. During marking this happens on the main thread during the PREPARE phase. This has a very low incidence in over budget slices, so I think it's likely overshadowed by the longer operations we do on helper threads concurrently. So I don't think it's a problem for us right now.

I'll file a bug to add telemetry, although we'll lose this again if we do your suggestion of combining cancelling Ion with discarding JIT code.