Bug 128443 - Don't throw away code if there is code on the worklists
Summary: Don't throw away code if there is code on the worklists
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 treegraph
 
Reported: 2014-02-07 22:26 PST by Filip Pizlo
Modified: 2014-02-07 23:03 PST (History)
10 users (show)

See Also:


Attachments
the patch (2.82 KB, patch)
2014-02-07 22:28 PST, Filip Pizlo
joepeck: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2014-02-07 22:26:36 PST
Patch forthcoming.
Comment 1 Filip Pizlo 2014-02-07 22:28:15 PST
Created attachment 223551 [details]
the patch
Comment 2 Joseph Pecoraro 2014-02-07 22:35:41 PST
Comment on attachment 223551 [details]
the patch

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

r=me

> Source/JavaScriptCore/dfg/DFGWorklist.h:73
> +    bool isActive() { return !!m_plans.size(); }

Nit: const
Comment 3 Filip Pizlo 2014-02-07 22:51:33 PST
Landed in http://trac.webkit.org/changeset/163695
Comment 4 Geoffrey Garen 2014-02-07 23:03:00 PST
Comment on attachment 223551 [details]
the patch

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

> Source/JavaScriptCore/heap/Heap.cpp:738
> +    // If we have things on any worklist, then don't delete code. This is kind of
> +    // a weird heuristic. It's definitely not safe to throw away code that is on
> +    // the worklist. But this change was made in a hurry so we just avoid throwing

What about when we throw away code to enable the debugger?
Comment 5 Filip Pizlo 2014-02-07 23:03:29 PST
(In reply to comment #4)
> (From update of attachment 223551 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=223551&action=review
> 
> > Source/JavaScriptCore/heap/Heap.cpp:738
> > +    // If we have things on any worklist, then don't delete code. This is kind of
> > +    // a weird heuristic. It's definitely not safe to throw away code that is on
> > +    // the worklist. But this change was made in a hurry so we just avoid throwing
> 
> What about when we throw away code to enable the debugger?

That's a different code path.  That code path first waits for the worklist to finish.