Closed Bug 1202865 Opened 4 years ago Closed 4 years ago

Split out Zone selection from stats collection and malloc bytes reset

Categories

(Core :: JavaScript: GC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: terrence, Assigned: terrence)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

I assume this was to save a few zone iterations, maybe? That doesn't quite explain it though: we have to select zones to GC before we init gcstats::AutoGCSlice so that it can count the GCing zones. On the other hand, we don't actually set up the zone barriers until much lower down in (anon)::AutoGCSlice. On the other hand, we *do* unschedule the zone's GC state at that level. Given that we schedule above the repeats and unschedule below, I'm not sure how we're ever able to shutdown without leaking the world, or trip every other assert in the GC for that matter.

Another "interesting" thing I noticed is that we resetMallocCounter on *every* zone when we clear malloc counters, not just the malloc counters for the zones that are collecting. This seems like it could lead to pathological behavior in some (admittedly unlikely) edge cases. I'll file another bug for that.

This passed some shell and browser tests locally, so let's try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d53051efdf3a
Attachment #8658394 - Flags: review?(jcoppeard)
Try is looking pretty green. This adds a couple |explict| that S found and moves the new Auto class to an anonymous namespace adjacent to other Auto classes we use in the same place.
Attachment #8658394 - Attachment is obsolete: true
Attachment #8658394 - Flags: review?(jcoppeard)
Attachment #8658431 - Flags: review?(jcoppeard)
Comment on attachment 8658394 [details] [diff] [review]
4_split_out_zone_scheduling_from_stats_collection_and_malloc_reset-v0.diff

Review of attachment 8658394 [details] [diff] [review]:
-----------------------------------------------------------------

This is great.  I never noticed that the zone scheduling was entangled with calculating the zone stats like that.
Attachment #8658394 - Flags: review+
Attachment #8658431 - Flags: review?(jcoppeard) → review+
https://hg.mozilla.org/mozilla-central/rev/9bbb4ad6efc5
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.