Bug 128297

Summary: GC should safepoint the DFG worklist in a smarter way rather than just waiting for everything to complete
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, ggaren, mark.lam, mhahnenberg, mmirman, msaboff, nrotem, oliver, sam, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 128291    
Attachments:
Description Flags
getting close
none
the patch oliver: review+

Description Filip Pizlo 2014-02-05 22:25:38 PST 
Patch forthcoming.
Comment 1 Filip Pizlo 2014-02-07 13:17:38 PST 
Created attachment 223488 [details]
getting close
Comment 2 Filip Pizlo 2014-02-07 14:03:42 PST 
Created attachment 223495 [details]
the patch
Comment 3 Oliver Hunt 2014-02-07 14:11:38 PST 
Comment on attachment 223495 [details]
the patch

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

> Source/JavaScriptCore/bytecode/CodeBlock.h:897
> -
> +    

drop this change

> Source/JavaScriptCore/dfg/DFGDesiredStructureChains.cpp:48
> +    for (unsigned i = m_vector.size(); i--;)
> +        m_vector[i]->visitChildren(visitor);

Why reverse iteration?  Forward iteration is much nicer.

> Source/JavaScriptCore/dfg/DFGDesiredWeakReferences.cpp:63
> +        visitor.appendUnbarrieredPointer(&m_references[i]);

and again, what's the reasoning here?
Comment 4 Filip Pizlo 2014-02-07 14:14:27 PST 
(In reply to comment #3)
> (From update of attachment 223495 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=223495&action=review
> 
> > Source/JavaScriptCore/bytecode/CodeBlock.h:897
> > -
> > +    
> 
> drop this change
> 
> > Source/JavaScriptCore/dfg/DFGDesiredStructureChains.cpp:48
> > +    for (unsigned i = m_vector.size(); i--;)
> > +        m_vector[i]->visitChildren(visitor);
> 
> Why reverse iteration?  Forward iteration is much nicer.

It takes less code.  It makes no difference which direction you use.  There is a lot of code in JSC that uses reverse iteration.

> 
> > Source/JavaScriptCore/dfg/DFGDesiredWeakReferences.cpp:63
> > +        visitor.appendUnbarrieredPointer(&m_references[i]);
> 
> and again, what's the reasoning here?

Less code.
Comment 5 Filip Pizlo 2014-02-07 21:02:54 PST 
Landed in http://trac.webkit.org/changeset/163691
Comment 6 Simon Fraser (smfr) 2014-02-07 22:27:53 PST 
Some new test assertions after this http://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK1%20(Tests)/r163691%20(2720)/results.html
Comment 7 Filip Pizlo 2014-02-07 22:30:05 PST 
(In reply to comment #6)
> Some new test assertions after this http://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK1%20(Tests)/r163691%20(2720)/results.html

I claim this will fix it: https://bugs.webkit.org/show_bug.cgi?id=128443