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+

Filip Pizlo
Reported 2014-02-05 22:25:38 PST
Patch forthcoming.
Attachments
getting close (19.63 KB, patch)
2014-02-07 13:17 PST, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (27.60 KB, patch)
2014-02-07 14:03 PST, Filip Pizlo 		oliver: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2014-02-07 13:17:38 PST
Created attachment 223488 [details] getting close
Filip Pizlo
Comment 2 2014-02-07 14:03:42 PST
Created attachment 223495 [details] the patch
Oliver Hunt
Comment 3 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?
Filip Pizlo
Comment 4 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.
Filip Pizlo
Comment 5 2014-02-07 21:02:54 PST
Landed in http://trac.webkit.org/changeset/163691
Simon Fraser (smfr)
Comment 6 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
Filip Pizlo
Comment 7 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
Note You need to log in before you can comment on or make changes to this bug.