Bug 155479 - Remove the Baker barrier from JSC
Summary: Remove the Baker barrier from JSC
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks: 149432
  Show dependency treegraph
 
Reported: 2016-03-14 17:34 PDT by Filip Pizlo
Modified: 2016-03-15 18:32 PDT (History)
2 users (show)

See Also:


Attachments
work in progress (108.70 KB, patch)
2016-03-14 17:34 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (124.28 KB, patch)
2016-03-14 22:40 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (123.84 KB, patch)
2016-03-14 22:48 PDT, Filip Pizlo
sbarati: 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 2016-03-14 17:34:17 PDT
Patch forthcoming.
Comment 1 Filip Pizlo 2016-03-14 17:34:58 PDT
Created attachment 274059 [details]
work in progress
Comment 2 Filip Pizlo 2016-03-14 22:40:25 PDT
Created attachment 274076 [details]
the patch
Comment 3 WebKit Commit Bot 2016-03-14 22:41:35 PDT
Attachment 274076 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/JSObject.h:872:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Filip Pizlo 2016-03-14 22:48:31 PDT
Created attachment 274077 [details]
the patch
Comment 5 WebKit Commit Bot 2016-03-14 22:50:23 PDT
Attachment 274077 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/JSObject.h:872:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Saam Barati 2016-03-15 01:06:14 PDT
Comment on attachment 274077 [details]
the patch

r=me
Comment 7 Filip Pizlo 2016-03-15 08:26:52 PDT
Landed in http://trac.webkit.org/changeset/198212
Comment 8 Ryan Haddad 2016-03-15 17:44:30 PDT
This change may have introduced a new JSC test failure
<https://build.webkit.org/builders/Apple%20El%20Capitan%20Debug%20JSC%20%28Tests%29/builds/2125>

stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: Timed out after 339.000000 seconds!
stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: 1   0x10afd8a40 WTFCrash
stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: 2   0x109ede0aa timeoutThreadMain(void*)
stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: 3   0x10b043c39 WTF::createThread(void (*)(void*), void*, char const*)::$_0::operator()() const
stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: 4   0x10b043c0d void std::__1::__invoke_void_return_wrapper<void>::__call<WTF::createThread(void (*)(void*), void*, char const*)::$_0&>(WTF::createThread(void (*)(void*), void*, char const*)::$_0&&&)
stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: 5   0x10b043bac std::__1::__function::__func<WTF::createThread(void (*)(void*), void*, char const*)::$_0, std::__1::allocator<WTF::createThread(void (*)(void*), void*, char const*)::$_0>, void ()>::operator()()
stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: 6   0x10a8e1eda std::__1::function<void ()>::operator()() const
stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: 7   0x10b04290e WTF::threadEntryPoint(void*)
stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: 8   0x10b0441a1 WTF::wtfThreadEntryPoint(void*)
stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: 9   0x7fff9aefac13 _pthread_body
stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: 10  0x7fff9aefab90 _pthread_body
stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: 11  0x7fff9aef8375 thread_start
stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: test_script_15145: line 2: 39785 Segmentation fault: 11  ( "$@" ../../.vm/JavaScriptCore.framework/Resources/jsc --useFTLJIT\=false --useFunctionDotArguments\=true --jitMemoryReservationSize\=50000 --useFTLJIT\=true --useConcurrentJIT\=false --thresholdForJITAfterWarmUp\=100 materialize-past-butterfly-allocation.js )
stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: ERROR: Unexpected exit code: 139
FAIL: stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool
Comment 9 Filip Pizlo 2016-03-15 17:48:47 PDT
(In reply to comment #8)
> This change may have introduced a new JSC test failure
> <https://build.webkit.org/builders/
> Apple%20El%20Capitan%20Debug%20JSC%20%28Tests%29/builds/2125>
> 
> stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool:
> Timed out after 339.000000 seconds!
> stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: 1  
> 0x10afd8a40 WTFCrash
> stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: 2  
> 0x109ede0aa timeoutThreadMain(void*)
> stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: 3  
> 0x10b043c39 WTF::createThread(void (*)(void*), void*, char
> const*)::$_0::operator()() const
> stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: 4  
> 0x10b043c0d void
> std::__1::__invoke_void_return_wrapper<void>::__call<WTF::createThread(void
> (*)(void*), void*, char const*)::$_0&>(WTF::createThread(void (*)(void*),
> void*, char const*)::$_0&&&)
> stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: 5  
> 0x10b043bac std::__1::__function::__func<WTF::createThread(void (*)(void*),
> void*, char const*)::$_0, std::__1::allocator<WTF::createThread(void
> (*)(void*), void*, char const*)::$_0>, void ()>::operator()()
> stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: 6  
> 0x10a8e1eda std::__1::function<void ()>::operator()() const
> stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: 7  
> 0x10b04290e WTF::threadEntryPoint(void*)
> stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: 8  
> 0x10b0441a1 WTF::wtfThreadEntryPoint(void*)
> stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: 9  
> 0x7fff9aefac13 _pthread_body
> stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: 10 
> 0x7fff9aefab90 _pthread_body
> stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool: 11 
> 0x7fff9aef8375 thread_start
> stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool:
> test_script_15145: line 2: 39785 Segmentation fault: 11  ( "$@"
> ../../.vm/JavaScriptCore.framework/Resources/jsc --useFTLJIT\=false
> --useFunctionDotArguments\=true --jitMemoryReservationSize\=50000
> --useFTLJIT\=true --useConcurrentJIT\=false
> --thresholdForJITAfterWarmUp\=100 materialize-past-butterfly-allocation.js )
> stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool:
> ERROR: Unexpected exit code: 139
> FAIL: stress/materialize-past-butterfly-allocation.js.ftl-no-cjit-small-pool

Extremely unlikely. This pitch just kills dead code.
Comment 10 Ryan Haddad 2016-03-15 18:23:57 PDT
(In reply to comment #9)
> Extremely unlikely. This pitch just kills dead code.

In that case, apologies for the false alarm. This was the only JSC change in the failing run on Yosemite and El Capitan.

I filed https://bugs.webkit.org/show_bug.cgi?id=155526 to track the failure.
Comment 11 Filip Pizlo 2016-03-15 18:32:40 PDT
(In reply to comment #10)
> (In reply to comment #9)
> > Extremely unlikely. This pitch just kills dead code.
> 
> In that case, apologies for the false alarm. This was the only JSC change in
> the failing run on Yosemite and El Capitan.
> 
> I filed https://bugs.webkit.org/show_bug.cgi?id=155526 to track the failure.

Looks like it's actually likely that this change did it, given that it's been failing consistently (according to https://bugs.webkit.org/show_bug.cgi?id=155526#c1).

I think that since this times out in debug but not release, the most likely reason for the failure is that this is just a super inefficient test.  That means that even if it wasn't timing out, it would be taking up a disproportionate amount of time.  It's best to remove the test.