Bug 166707

Created attachment 298062 [details] WIP It begins. Preliminary tests are it's 5-6% faster on pbkdf2. There is still a lot to do to make the code correct, and to make it spec compliant.

Cool!

(In reply to comment #2) > Cool! Yeah. If we're lucky we'll get 0.5-1% on Kraken overall. My initial 5-6% number will probably decrease slightly since I'll need to add a couple branches to verify we're doing the right thing.

Created attachment 298163 [details] WIP I think it's correct now w.r.t weird inputs to slice. I still need to figure out how to quickly detect if we're constructing a subclass of Array and bail out if so.

Created attachment 298220 [details] WIP Close to being done.

Created attachment 298246 [details] patch

Attachment 298246 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:3890: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1658: Missing space before ( in switch( [whitespace/parens] [5] Total errors found: 2 in 29 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 298247 [details] patch rebased

Attachment 298247 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:3890: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1658: Missing space before ( in switch( [whitespace/parens] [5] Total errors found: 2 in 29 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 298247 [details] patch Attachment 298247 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2846492 New failing tests: fast/workers/postMessage-missing-parameter.html webgl/1.0.2/conformance/context/context-release-with-workers.html fast/workers/worker-terminate-forever.html

Created attachment 298254 [details] Archive of layout-test-results from ews117 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-elcapitan Platform: Mac OS X 10.11.6

(In reply to comment #11) > Created attachment 298254 [details] > Archive of layout-test-results from ews117 for mac-elcapitan > > The attached test failures were seen while running run-webkit-tests on the > mac-debug-ews. > Bot: ews117 Port: mac-elcapitan Platform: Mac OS X 10.11.6 I'm building a debug webkit now to figure out why we're crashing during global object creation while setting up the proper watchpoints.

(In reply to comment #12) > (In reply to comment #11) > > Created attachment 298254 [details] > > Archive of layout-test-results from ews117 for mac-elcapitan > > > > The attached test failures were seen while running run-webkit-tests on the > > mac-debug-ews. > > Bot: ews117 Port: mac-elcapitan Platform: Mac OS X 10.11.6 > > I'm building a debug webkit now to figure out why we're crashing during > global object creation while setting up the proper watchpoints. It's very curious that the assertion being hit is that an exception is being thrown.

<rdar://problem/29913445>

Created attachment 298255 [details] patch (probably for landing)

Attachment 298255 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:3890: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1656: Missing space before ( in switch( [whitespace/parens] [5] Total errors found: 2 in 27 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 298256 [details] patch for landing (Fixed style issues). The previously crashing tests are now passing for me locally. Will wait until EWS finishes before landing.

Comment on attachment 298256 [details] patch for landing Clearing flags on attachment: 298256 Committed r210476: <http://trac.webkit.org/changeset/210476>

All reviewed patches have been landed. Closing bug.

So this is showing up as a 5% improvement on phidf2 on the bots. It's also showing a 5% slowdown on AES on the bots. This is really weird because I can't reproduce this slowdown on aes at all locally. I'll try to reproduce it on another device.

Re-opened since this is blocked by bug 166859

Created attachment 298441 [details] patch Just about done. I'm not showing a speedup on aes and pbkdf2 locally. I've made Slice be polymorphic over the input array's indexing type when it is Contiguous/Int32/Double. I just need to rebase and revise the changelog as needed.

Kraken results I'm seeing locally: VMs tested: "og" at /Volumes/Data/WK/c/OpenSource/WebKitBuild/Release/jsc (r210553) "change" at /Volumes/Data/WK/b/OpenSource/WebKitBuild/Release/jsc (r210553) Collected 40 samples per benchmark/VM, with 40 VM invocations per benchmark. Emitted a call to gc() between sample measurements. Used 1 benchmark iteration per VM invocation for warm-up. Used the jsc-specific preciseTime() function to get microsecond-level timing. Reporting benchmark execution times with 95% confidence intervals in milliseconds. og change ai-astar 91.657+-0.948 91.154+-0.939 audio-beat-detection 35.801+-0.227 35.673+-0.231 audio-dft 94.211+-1.592 92.901+-0.595 might be 1.0141x faster audio-fft 27.581+-0.133 ? 27.679+-0.221 ? audio-oscillator 43.191+-0.340 ? 43.325+-0.378 ? imaging-darkroom 56.904+-0.403 ? 57.297+-1.189 ? imaging-desaturate 44.541+-1.555 ? 44.976+-1.562 ? imaging-gaussian-blur 59.351+-0.925 ? 59.730+-1.127 ? json-parse-financial 31.741+-0.276 ? 32.184+-0.396 ? might be 1.0139x slower json-stringify-tinderbox 22.105+-0.452 21.763+-0.345 might be 1.0157x faster stanford-crypto-aes 35.369+-0.285 ^ 34.852+-0.170 ^ definitely 1.0148x faster stanford-crypto-ccm 33.375+-0.747 32.865+-1.719 might be 1.0155x faster stanford-crypto-pbkdf2 91.430+-0.695 ^ 86.424+-2.628 ^ definitely 1.0579x faster stanford-crypto-sha256-iterative 30.389+-0.255 29.630+-0.845 might be 1.0256x faster <arithmetic> 49.832+-0.215 ^ 49.318+-0.250 ^ definitely 1.0104x faster

Created attachment 298543 [details] patch Putting this up for review. My tests are revealing a bug that's already in ToT. (Probably concurrent GC related). I'll hold off on landing until that's resolved since my tests trigger crashes with high frequency.

Attachment 298543 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGCallArrayAllocatorSlowPathGenerator.h:132: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGCallArrayAllocatorSlowPathGenerator.h:133: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGCallArrayAllocatorSlowPathGenerator.h:134: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGCallArrayAllocatorSlowPathGenerator.h:135: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGCallArrayAllocatorSlowPathGenerator.h:136: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGCallArrayAllocatorSlowPathGenerator.h:137: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGCallArrayAllocatorSlowPathGenerator.h:138: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] Total errors found: 7 in 29 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 298543 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=298543&action=review > JSTests/stress/array-slice-intrinsic.js:14 > +let tests = [ It should be helpful if ```shallowEq``` helper function logs what you were expecting and what is the current result, because test cases are going to be iterated in further steps and makes debug this test a little painful. > JSTests/stress/array-slice-jettison-on-constructor-change.js:19 > +let tests = [ Ditto. > JSTests/stress/array-slice-osr-exit-2.js:19 > +let tests = [ Ditto. > JSTests/stress/array-slice-osr-exit.js:19 > +let tests = [ Ditto.

Comment on attachment 298543 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=298543&action=review >> JSTests/stress/array-slice-intrinsic.js:14 >> +let tests = [ > > It should be helpful if ```shallowEq``` helper function logs what you were expecting and what is the current result, because test cases are going to be iterated in further steps and makes debug this test a little painful. SGTM, I'll have the assert function take an optional message.

Created attachment 298748 [details] patch for landing

Comment on attachment 298748 [details] patch for landing Clearing flags on attachment: 298748 Committed r210695: <http://trac.webkit.org/changeset/210695>

All reviewed patches have been landed. Closing bug.