Closed Bug 730144 Opened 10 years ago Closed 9 years ago

Label objects with their mark colors when dumping the heap

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla13

People

(Reporter: sfink, Assigned: sfink)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

I have been tracking down places where we get black pointing to gray, and I found dumping the mark colors with the heap dump very useful. However, I know it probably screws up your analysis scripts. So I leave it up to you.
Attachment #600273 - Flags: review?(continuation)
Sounds like a good idea!  Could you give me a short snippet of what the output looks like?
0x7fffe05f8c70 (BG) XULElement 7fffdd7276a0
> 0x7fffe0598fa0 (B) type
> 0x7fffde612240 (BG) shape
> 0x7fffe0543af0 (B) XPCWrappedNativeProto::mJSProtoObject
> 0x7fffe5c72560 (B) XPCWrappedNativeScope::mGlobalJSObject
> 0x7fffe5ced180 (B) XPCWrappedNativeScope::mPrototypeJSObject
Thanks.  Here's on preliminary comment.  It would probably be better to give a slightly higher level view of the mark colors.
black + non gray = B
black + gray = G
nonblack + non gray = W (for white)
and I don't know, maybe an X or something for nonblack + gray, as that shouldn't happen.  The parens aren't really needed, either, because there are no spaces or weirdness in there, just a character.
Attachment #600273 - Attachment is obsolete: true
Attachment #600273 - Flags: review?(continuation)
Ok, that simplifies things.

Oh, and I forgot I added a comment at the top of the dump, and made up a magic syntax for specifying the current version of the format. That was to allow tools to work with older and newer files without needing to do magic autodetection. Obviously, that's separate from this bug, though it might be the first use of it. Feel free to tell me that it's overkill. :)
I wrote an ultra-simple tool using these mark indicators to check whether black points to gray anywhere in the graph. (And it doesn't always come up empty!)
I think the versioning is overkill.  :) You are I are probably the only people who have written parsers for the GC log format.  It is really new.  Plus this change is similar enough that old scripts can probably process it without any problems.

This code annotates roots, too, right?

I wrote a script like that too, but I didn't get a chance to look into it much.  Maybe it was for CC logs, though.
(In reply to Andrew McCreight [:mccr8] from comment #8)
> This code annotates roots, too, right?

Yes (though I left that out by mistake at first)

> I wrote a script like that too, but I didn't get a chance to look into it
> much.  Maybe it was for CC logs, though.

How could you write a script when the logs didn't include the mark color?
I looked at my script now, and it was a CC log, as it turns out.  The CC logs indicate color.  You have to do a WANT_ALL_TRACES to get it to show black objects, though.
Comment on attachment 600306 [details] [diff] [review]
Label objects with their mark colors when dumping the heap

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

Looks good.  Thanks for doing this.  It is much less bad than I thought it would be.

::: js/src/jsfriendapi.cpp
@@ +490,5 @@
> +{
> +    gc::Cell *cell = static_cast<gc::Cell*>(thing);
> +    if (cell->isMarked(gc::BLACK)) {
> +        if (cell->isMarked(gc::GRAY))
> +            return 'G';

The inner ifs would be a little more compact with the trinary operator, but no big deal.

@@ +544,5 @@
>      if (!dtrc.visited.init(10000))
>          return;
>  
> +    fprintf(dtrc.output, "# JS Heap Dump\n");
> +    fprintf(dtrc.output, "#:format-version=1.1\n");

As I said elsewhere, just remove this for now.  I've done far more radical changes to the CC log without bothering to do anything like this, and it was okay.
Attachment #600306 - Flags: review?(continuation) → review+
Blocks: 723783
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/2d3328813a98 - something in that push burned.

(We're actually at target milestone 13, 12 is what's on aurora now.)
Target Milestone: mozilla12 → ---
re-landing. Previous problem appears to have been from bug 719294, which mysteriously broke 'make package' or something like that.

https://hg.mozilla.org/integration/mozilla-inbound/rev/974cfb3a5156
Target Milestone: --- → mozilla13
I'm not doing so well. The previous landing somehow omitted part of the patch, and was harmless other than triggering a warning about an unused function. So Waldo deleted it (thus backing this bug's patch out) in https://hg.mozilla.org/integration/mozilla-inbound/rev/ee60ef57fcc2

Re-landing:

https://hg.mozilla.org/integration/mozilla-inbound/rev/746bee776179
https://hg.mozilla.org/mozilla-central/rev/746bee776179
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.