Closed Bug 583763 Opened 14 years ago Closed 14 years ago

better stats about missing conservative roots

Categories

(Core :: JavaScript Engine, defect)

x86
Windows Server 2003
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: igor, Assigned: igor)

References

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

In the bug 574313 I have added asserts and debug printouts to check that the GC things stored in autorooters are also scanned by the conservative GC stack scanner. But the printouts are not that useful to identify the source of the problems like the reports from the bug 583598. It would be nice at least to know if the scanner have seen the thing and if yes, then why it rejected it. But adding that requires some re-factoring of the conservative GC implementation so I will do that here and then revisit the bug 583598. This refactoring should also allow to simplify code implemented for bug 583084.
Igor, you've listed this as a beta3+ blocker, presumably because it blocks bug 583598; does that mean you expect both to be done by today's code freeze at 11pm PT?
blocking2.0: beta3+ → betaN+
(In reply to comment #1) > Igor, you've listed this as a beta3+ blocker, presumably because it blocks bug > 583598; does that mean you expect both to be done by today's code freeze at > 11pm PT? No, I do not have chance to that.
Attached patch v1 (deleted) — Splinter Review
The patch makes the hash of GC chunks persistent and move that in JSRuntime. That change made ConservativeGCStackMarker unnecessary as all the debug-related stats can live in GCMarker. So now there is just MarkWordConservatively(trc, w) function that can be called as needed and an utility helper IsGCThingWord(rt, w) that can be invoked from the debugger and which is used to print an extra debug info when a missing conservative root is detected.
Attachment #462462 - Flags: review?(anygregor)
Comment on attachment 462462 [details] [diff] [review] v1 > #ifdef DEBUG >@@ -726,34 +746,35 @@ FreeGCChunks(JSRuntime *rt) > { > #ifdef DEBUG > while (rt->gcEmptyArenaList) { > JSGCArena *next = rt->gcEmptyArenaList->getInfo()->prev; > memset(rt->gcEmptyArenaList, JS_FREE_PATTERN, GC_ARENA_SIZE); > rt->gcEmptyArenaList = next; > } > #endif > >- /* Remove unused chunks. */ >- size_t available = 0; >- for (JSGCChunkInfo **i = rt->gcChunks.begin(); i != rt->gcChunks.end(); ++i) { >- JSGCChunkInfo *ci = *i; >+ /* Remove unused chunks and rebuild gcFreeArenaChunks. */ >+ rt->gcFreeArenaChunks.clear(); >+ JS_ASSERT(rt->gcFreeArenaChunks.capacity() >= rt->gcChunkSet.count()); >+ for (GCChunkSet::Enum e(rt->gcChunkSet); !e.empty(); e.popFront()) { >+ GCChunkInfo *ci = GCChunkInfo::fromChunk(e.front()); > JS_ASSERT(ci->runtime == rt); > if (ci->numFreeArenas == GC_ARENAS_PER_CHUNK) { > if (ci->gcChunkAge > GC_MAX_CHUNK_AGE) { >- ReleaseGCChunk(rt, ci->getChunk()); >+ e.removeFront(); > continue; > } Where do we call ReleaseGCChunk now? > > JSBool > js_InitGC(JSRuntime *rt, uint32 maxbytes) > { > InitGCArenaLists(rt); > >+ if (!rt->gcChunkSet.init(32)) >+ return false; >+ 32 for any special reason? >+ >+inline ConservativeGCTest >+IsGCThingWord(JSRuntime *rt, jsuword w) >+{ >+ void *thing; >+ uint32 traceKind; >+ return IsGCThingWord(rt, w, thing, traceKind); >+} >+ >+ >+#if defined(JS_DUMP_CONSERVATIVE_GC_ROOTS) || defined(JS_GCMETER) linebreak nit > js_TraceRuntime(&gcmarker); > js_MarkScriptFilenames(rt); >- >+ > /* > * Mark children of things that caused too deep recursion during the above > * tracing. > */ whitespace nit >@@ -382,18 +393,42 @@ class BackgroundSweepTask : public JSBac > else > replenishAndFreeLater(ptr); > } > > virtual void run(); > }; > > #endif /* JS_THREADSAFE */ > >+ >+struct GCChunkInfo; >+ >+struct GCChunkHasher >+{ linebreak nit?
Attachment #462462 - Flags: review?(anygregor) → review+
Whiteboard: fixed-in-tracemonkey
http://hg.mozilla.org/tracemonkey/rev/bdd944b09a53 - followup to add even more info into printout
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: