Closed
Bug 1099463
Opened 10 years ago
Closed 10 years ago
GC_SHRINK seems to clean up too much
Categories
(Core :: JavaScript: GC, defect)
Core
JavaScript: GC
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: sfink, Assigned: jonco)
References
Details
Attachments
(1 file)
1.96 KB,
patch
|
shu
:
review+
|
Details | Diff | Splinter Review |
I'm getting a strange assertion with my patches + cgc: TEST-UNEXPECTED-FAIL | js/src/jit-test/tests/gc/bug-975959.js | Assertion failure: activeParallelEntryScripts_ && activeParallelEntryScripts_->has(script), at /builds/slave/m-in_l64-d_sm-ggc-000000000000/src/js/src/jit/Ion.cpp:446 (code -11, args "--ion-eager --ion-offthread-compile=off") What's happening is that notifyOfActiveParallelEntryScript with a script that gets added to the parallelEntryScripts_ table. Then we do a GC with gc.cleanUpEverything set to true. Then notifyOfActiveParallelEntryScript gets called again with that script, and we assert because it's an entry script but it isn't in the table. gc.cleanUpEverything is set to true because GCRuntime::collect gets called with reason=DEBUG_GC, gckind=GC_SHRINK. This seems to be intermittent on tbpl, but I can reproduce it 100% locally with a debug build. The other reasons for setting cleanUpEverything seem pretty drastic (DESTROY_RUNTIME, SHUTDOWN_CC) and would happen when no scripts are running. But GC_SHRINK happens all the time with JS_GC_ZEAL=14, even with scripts active. So it looks to me like cleanUpEverything is too big of a hammer to use for GC_SHRINK. But I don't know this stuff well enough to know what should be cleaned up for a GC_SHRINK.
Reporter | ||
Comment 1•10 years ago
|
||
The patches are the ones for bug 1088831, btw, though I've had to fix up one of them a bit: - MOZ_ASSERT_IF(gcDepth == 1, phases[phase].parent == parent); + // Major and minor GCs can nest inside PHASE_GC_BEGIN/PHASE_GC_END. + MOZ_ASSERT_IF(gcDepth == 1 && phase != PHASE_MINOR_GC, phases[phase].parent == parent); but I *think* my patches only coincidentally trigger this (I still don't know why.)
Flags: needinfo?(jcoppeard)
Assignee | ||
Comment 2•10 years ago
|
||
Reproduced. I think it's reasonable to have cleanUpEverything set for a shrinking GC as a shrinking GC is supposed to reclaim as much memory as possible. I think the problem is that we remove scripts from this table without clearing the flag that indicates that it's in the table.
Assignee | ||
Comment 3•10 years ago
|
||
Patch to clear the parallel entry script flag when removing a script from the active parallel entry scripts table due to shrinking GC.
Attachment #8523809 -
Flags: review?(shu)
Comment 4•10 years ago
|
||
Comment on attachment 8523809 [details] [diff] [review] bug-1099463 Review of attachment 8523809 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for fixing.
Attachment #8523809 -
Flags: review?(shu) → review+
Assignee | ||
Comment 5•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/0b76fb98e53c
Comment 6•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0b76fb98e53c
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in
before you can comment on or make changes to this bug.
Description
•