Bug 130040

Summary: Remove ConditionalStore barrier
Product: WebKit Reporter: Mark Hahnenberg <mhahnenberg>
Component: JavaScriptCoreAssignee: Mark Hahnenberg <mhahnenberg>
Status: RESOLVED FIXED    
Severity: Normal CC: rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 121074, 130066    
Attachments:
Description Flags
Patch ggaren: review+

Mark Hahnenberg
Reported 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.
Attachments
Patch (18.60 KB, patch)
2014-03-10 14:30 PDT, Mark Hahnenberg 		ggaren: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Mark Hahnenberg
Comment 1 2014-03-10 14:30:52 PDT
Created attachment 226334 [details] Patch
Geoffrey Garen
Comment 2 2014-03-10 14:36:21 PDT
Comment on attachment 226334 [details] Patch r=me
Mark Hahnenberg
Comment 3 2014-03-10 15:15:55 PDT
Committed r165407: <http://trac.webkit.org/changeset/165407>
Ryosuke Niwa
Comment 4 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.
Ryosuke Niwa
Comment 5 2014-03-10 22:03:13 PDT
Filed the bug 130066 to track the crash.
Note You need to log in before you can comment on or make changes to this bug.