Bug 128297 - GC should safepoint the DFG worklist in a smarter way rather than just waiting for everything to complete
Summary: GC should safepoint the DFG worklist in a smarter way rather than just waitin...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks: 128291
  Show dependency tree / graph
 
Reported: 2014-02-05 22:25 PST by Filip Pizlo
Modified: 2014-02-07 22:30 PST (History)
10 users (show)

See Also:

Attachments
getting close (19.63 KB, patch)
2014-02-07 13:17 PST, Filip Pizlo 		no flags Details | Formatted Diff | Diff
the patch (27.60 KB, patch)
2014-02-07 14:03 PST, Filip Pizlo 		oliver: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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