583763 - better stats about missing conservative roots

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Igor Bukanov]

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.

Comment 1

[Mike Beltzner [:beltzner, not reading bugmail]]

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?

Comment 2

[Igor Bukanov]

(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.

Comment 3

[Igor Bukanov]

Attachment: v1

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.

Comment 4

[Gregor Wagner [:gwagner]]

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?

Comment 5

[Igor Bukanov]

http://hg.mozilla.org/tracemonkey/rev/564a6574d520

Comment 6

[Igor Bukanov]

http://hg.mozilla.org/tracemonkey/rev/bdd944b09a53 - followup to add even more info into printout

Comment 7

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/564a6574d520