Closed Bug 1573931 Opened Last month Closed Last month

Disallow INCREMENTAL_ALLOC_TRIGGER GCs when recording/replaying

Categories

(Core :: Web Replay, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox70 --- fixed

People

(Reporter: bhackett, Assigned: bhackett)

References

Details

Crash Data

Attachments

(1 file)

Similar to ALLOC_TRIGGER and EAGER_ALLOC_TRIGGER, INCREMENTAL_ALLOC_TRIGGER GCs can occur at non-deterministic points when recording/replaying and should be disallowed.

The patch looks fine. I'm interested to know how GC does get triggered when recording though if we disallow all these triggers? (For replay we do it at the same point as in the recording, right?). We're trying to move to relying less on timers for triggering GC and more on thing like heap/malloc usage.

Flags: needinfo?(bhackett1024)

The challenge here is that we want to GC at the same time while replaying as we did while recording. The GC is allowed to collect different sets of things, but calling into the embedder to mark roots can require interacting with the recording (taking locks etc.). The strategy we've used up to now was to rely on the timer based GCs to get them to happen at consistent points, but if that's not how the browser triggers GCs anymore we need a new strategy.

One option, which I think would work but need to study things some more, is that we request a major GC and are record/replaying, we don't set majorGCTriggerReason but instead call a record/replay API which will schedule a call to JS_GC() on the current thread's runtime to happen soon, at a consistent point between recording and replaying (the API's implementation will probably ignore worker threads for now, handling them well isn't a priority). RecordReplayCheckCanGC would be removed. Does that sound reasonable?

Flags: needinfo?(bhackett1024)
Pushed by bhackett@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f9bebc85ddb9
Disallow INCREMENTAL_ALLOC_TRIGGER GCs when recording/replaying, r=jonco.

(In reply to Brian Hackett (:bhackett) from comment #3)
Thanks. Yes, that sounds good to me.

Status: NEW → RESOLVED
Closed: Last month
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
Duplicate of this bug: 1574165
Crash Signature: [@ RecordReplayInterface_InternalRecordReplayAssert]
You need to log in before you can comment on or make changes to this bug.