Bug 128297 – GC should safepoint the DFG worklist in a smarter way rather than just waiting for everything to complete

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

[Filip Pizlo]

Patch forthcoming.

[Filip Pizlo]

Created attachment 223488 [details]
getting close

[Filip Pizlo]

Created attachment 223495 [details]
the patch

[Oliver Hunt]

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?

[Filip Pizlo]

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

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/163691

[Simon Fraser (smfr)]

Some new test assertions after this http://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK1%20(Tests)/r163691%20(2720)/results.html

[Filip Pizlo]

(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