Bug 159569 – Dumping the object graph doesn't work with verbose GC logging

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Michael Saboff]

When the logGC option is set to verbose (2), the logging code attempts to dump the object graph.  That code doesn't work.  It is built to visit the object graph again and that method it fraught with difficulties.  Therefore the object graph dumping part of verbose GC logging should be removed.

[Michael Saboff]

Created attachment 283184 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/27249686>

[Mark Lam]

Comment on attachment 283184 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=283184&action=review

> Source/JavaScriptCore/ChangeLog:11
> +        The current object graph logging code tries to revisits the graph.  This doesn't work
> +        correctly and asking around it isn't used.  The only way to dump the true object graph
> +        is to log while we GC and that has obvious performance implications.
> +        Therefore I eliminated GCLogging::dumpObjectGraph() and related code.  

Then, you should also remove the verbose option from Options::logGC.

[Michael Saboff]

(In reply to comment #3)
> Comment on attachment 283184 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=283184&action=review
> 
> > Source/JavaScriptCore/ChangeLog:11
> > +        The current object graph logging code tries to revisits the graph.  This doesn't work
> > +        correctly and asking around it isn't used.  The only way to dump the true object graph
> > +        is to log while we GC and that has obvious performance implications.
> > +        Therefore I eliminated GCLogging::dumpObjectGraph() and related code.  
> 
> Then, you should also remove the verbose option from Options::logGC.

We still need the verbose option, which will show all of the roots.  This patch only removes the dump object graph code.  I still think that logging the roots is useful.

[Mark Lam]

Comment on attachment 283184 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=283184&action=review

r=me

>> Source/JavaScriptCore/ChangeLog:11
>> +        Therefore I eliminated GCLogging::dumpObjectGraph() and related code.  
> 
> Then, you should also remove the verbose option from Options::logGC.

My mistake: the verbose option is still in use.

[WebKit Commit Bot]

Comment on attachment 283184 [details]
Patch

Clearing flags on attachment: 283184

Committed r203000: <http://trac.webkit.org/changeset/203000>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.