Bug 130040 - Remove ConditionalStore barrier
Summary: Remove ConditionalStore barrier
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Hahnenberg
URL:
Keywords:
Depends on:
Blocks: 121074 130066
  Show dependency treegraph
 
Reported: 2014-03-10 14:26 PDT by Mark Hahnenberg
Modified: 2014-04-29 15:33 PDT (History)
1 user (show)

See Also:


Attachments
Patch (18.60 KB, patch)
2014-03-10 14:30 PDT, Mark Hahnenberg
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Hahnenberg 2014-03-10 14:26:55 PDT
ConditionalStoreBarrier was created when barriers were much more expensive. Now that they're cheap(er), we can get rid of them. This also allows us to get rid of the write barrier logic in emitPutTransitionStub because we always will have executed a write barrier on the base object in the case where we are allocating and storing a new Butterfly into it. Previously, a ConditionalStoreBarrier might or might not have barrier-ed the base object, so we'd have to emit a write barrier in the transition case.
Comment 1 Mark Hahnenberg 2014-03-10 14:30:52 PDT
Created attachment 226334 [details]
Patch
Comment 2 Geoffrey Garen 2014-03-10 14:36:21 PDT
Comment on attachment 226334 [details]
Patch

r=me
Comment 3 Mark Hahnenberg 2014-03-10 15:15:55 PDT
Committed r165407: <http://trac.webkit.org/changeset/165407>
Comment 4 Ryosuke Niwa 2014-03-10 20:49:00 PDT
Looks like this caused a crash on DYEBench.

http://build.webkit.org/builders/Apple%20MountainLion%20Release%20%28Perf%29/builds/8249
http://build.webkit.org/builders/Apple%20Mavericks%20Release%20%28Perf%29/builds/843

I've built r165406 and 165407 locally and only r165407 reproduces the crash quite reliably.  It appears, however, that this crash doesn't always occur.

While

Tools/Scripts/run-perf-tests PerformanceTests/DoYouEvenBench/Full.html --test-runner-count=4 --reset-results

reproduces the crash quite reliably,

Tools/Scripts/run-perf-tests PerformanceTests/DoYouEvenBench/Full.html --test-runner-count=1 --reset-results

doesn't.
Comment 5 Ryosuke Niwa 2014-03-10 22:03:13 PDT
Filed the bug 130066 to track the crash.