Bug 116748 - fourthTier: cti_optimize shouldn't allow GCs to get in the way of it seeing the state of its CodeBlock
Summary: fourthTier: cti_optimize shouldn't allow GCs to get in the way of it seeing t...
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
Depends on:
Blocks: 112836
  Show dependency treegraph
Reported: 2013-05-24 16:49 PDT by Filip Pizlo
Modified: 2013-05-26 11:44 PDT (History)
7 users (show)

See Also:

work in progress (14.38 KB, patch)
2013-05-24 16:50 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (4.62 KB, patch)
2013-05-25 09:28 PDT, Filip Pizlo
ggaren: 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 2013-05-24 16:49:51 PDT
Instead, cti_optimize should discover the state of its code block in a more racy way, using other available data.

This should fix crashes I'm seeing in Octane.  Those crashes trip on an important RELEASE_ASSERT in the current code, and removing that assertion would not be safe.  That assertion is saying that if we think that we're compiling the code then we shouldn't have an optimized replacement.  Except that we might have *thought* that we were compiling code prior to a GC and then the GC synchronously completed the compilation.

That's why we need to have a way of sort of re-asking whether we ought to be:

- waiting for compilation to finish
- initiating a new compile
- OSRing into an old compile
- OSRing into a newly finished compile
- etc.
Comment 1 Filip Pizlo 2013-05-24 16:50:44 PDT
Created attachment 202866 [details]
work in progress
Comment 2 Filip Pizlo 2013-05-24 19:39:00 PDT
I think I have a better way of doing this.  We just need to defer GC some more.
Comment 3 Filip Pizlo 2013-05-25 09:28:17 PDT
Created attachment 202889 [details]
the patch
Comment 4 Geoffrey Garen 2013-05-26 11:42:13 PDT
Comment on attachment 202889 [details]
the patch

Comment 5 Filip Pizlo 2013-05-26 11:44:42 PDT
Landed in http://trac.webkit.org/changeset/150721