1332357 - DumpHeap always evicts the nursery regardless of nurseryBehaviour argument

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Jon Coppeard (:jonco)]

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.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: bug1332357-dump-heap

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?

Comment 2

[Steve Fink [:sfink] [:s:]]

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.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/44c956b966c7
Fix DumpHeap to only evict the nursery if requested by caller r=sfink

Comment 4

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/44c956b966c7