Closed Bug 1332357 Opened 3 years ago Closed 3 years ago

DumpHeap always evicts the nursery regardless of nurseryBehaviour argument

Categories

(Core :: JavaScript: GC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: jonco, Assigned: jonco)

Details

Attachments

(1 file)

DumpHeap takes an enum of type DumpHeapNurseryBehaviour which has the possible values CollectNurseryBeforeDump or IgnoreNurseryObjects.  This controls whether the nursery is evicted before the dump happens.  However, it goes on to call js::TraceRuntime which always evicts the nursery anyway.
Patch to fix DumpHeap to do as it's told.

I thought about just removing the evict call from js::TraceRunime as it's only used from here and by ubi::Node, but that seems kind of risky.  Do you know why we have this eviction in there anyway?
Assignee: nobody → jcoppeard
Attachment #8828407 - Flags: review?(sphink)
Comment on attachment 8828407 [details] [diff] [review]
bug1332357-dump-heap

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

I suspect it's just that callers expect to trace the whole runtime, not just the easily-traceable portions. It would be surprising to be missing objects.

But given the limited number of callers, perhaps we should just remove it? It seems to be a convenience API, but it bundles several things together, and not all callers want that particular set.
Attachment #8828407 - Flags: review?(sphink) → review+
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/44c956b966c7
Fix DumpHeap to only evict the nursery if requested by caller r=sfink
https://hg.mozilla.org/mozilla-central/rev/44c956b966c7
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.