Closed Bug 721420 Opened 13 years ago Closed 13 years ago

WantAllTraces should disable Skippable CC optimizations

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla12

People

(Reporter: mccr8, Assigned: mccr8)

References

Details

Attachments

(1 file, 2 obsolete files)

The following should be disabled when we're under WantAllTraces in the CC: - CanSkipThis in NoteXPComChild, NoteRoot - CanSkipInCC in AddPurpleRoot - mBeforeUnlinkCB We could cache WantAllTraces in the graph builder to avoid virtual calls everywhere, but we do those calls all over the place anyways. In the future, when the async purple buffer clearing phase lands, we may have to do something more complex to disable that, like you do a normal CC, then turn off the async purple buffer until the next CC, which is then a WantAllTraces CC. That could be more complex and would probably mess up the scheduling.
Summary: WantAllTraces should disable Skippable CC optimizations → WantAllTraces should disable synchronous Skippable CC optimizations
Attached patch skip skipping with wantalltraces (obsolete) — Splinter Review
untested.
Assignee: nobody → continuation
Attached patch null checks are your friend (obsolete) — Splinter Review
I think the way to deal with the asynchronous cleanup phase is to just force a global GC on a WantAllTraces CC. This will also avoid issues with losing things due to the compartmental GC. Just have to make sure this doesn't trigger a FORCE_GC telemetry ping.
Attachment #592310 - Attachment is obsolete: true
I'm not sure how that will work with incremental GC, though.
You should be able to force non-incremental GC.
Attachment #592315 - Attachment is obsolete: true
Summary: WantAllTraces should disable synchronous Skippable CC optimizations → WantAllTraces should disable Skippable CC optimizations
Comment on attachment 592509 [details] [diff] [review] disable CanSkips, force GC on WantAllTraces Seems to work okay with regular CC dumps and WantAllTraces dumps. I don't disable the unlink callback as that seems to just be used for cleanups, not actually doing anything. https://tbpl.mozilla.org/?tree=Try&rev=610c52805848
Attachment #592509 - Flags: review?(bugs)
Comment on attachment 592509 [details] [diff] [review] disable CanSkips, force GC on WantAllTraces >+ if (!cp->CanSkipInCC(root) || builder.WantAllTraces()) { You should check WantAllTraces() first. CanSkipInCC can remove stuff from purple buffer.
Attachment #592509 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #7) > You should check WantAllTraces() first. CanSkipInCC can remove stuff from > purple buffer. Good point. https://hg.mozilla.org/integration/mozilla-inbound/rev/1725d83e893a
Target Milestone: --- → mozilla12
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: