It looks like arrow functions have slowed us down by 8-10%. I've ran Octane's code-load benchmarks from the revision before arrow functions vs ToT. Here is where we stand: ``` "og" at r185988 "new" at r199068 Collected 10 samples per benchmark/VM, with 10 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 new closure 0.48247+-0.00579 ! 0.51322+-0.00369 ! definitely 1.0637x slower jquery 6.06290+-0.04444 ! 6.65674+-0.06247 ! definitely 1.0979x slower <geometric> 1.71024+-0.01302 ! 1.84830+-0.01326 ! definitely 1.0807x slower ``` (Note that arrow functions were introduced in http://trac.webkit.org/changeset/185989) There were also other slow downs along the way such as block scoping. I've recovered performance since those slow downs and have actually made our parser slightly faster since the introduction of block scoping. We're about 0.5% faster since the revision before block scoping: ``` "og" at r186859 "new" at r199068 Collected 10 samples per benchmark/VM, with 10 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 new closure 0.50085+-0.00177 ? 0.50171+-0.00414 ? jquery 6.49236+-0.02348 ^ 6.39803+-0.05652 ^ definitely 1.0147x faster <geometric> 1.80322+-0.00305 1.79157+-0.00915 might be 1.0065x faster ``` (note that block scoping was introduced in http://trac.webkit.org/changeset/186860). The main slowdown with parsing arrow function is that we have to do a lookahead for the "=>" token. Arrow functions parse under the assignment expression grammar production: (https://tc39.github.io/ecma262/#prod-AssignmentExpression) Arrow functions can start in two ways: "IDENT =>" or "(<params>) =>" Because AssignmentExpressions are common to parse, any time we see an IDENT, we may have to do a lookahead for "=>". Anytime we see "(", we may have to do a lookahead for arrow function parameters. I've improved this a bit by doing this lazily by first assuming we're not parsing an arrow function and only falling back to parsing an arrow function if parsing a non-arrowfunction failed. This helped speed things up, but there is still a slow down because we need to save some state anytime we might be starting to parse an arrow function. If anybody has ideas on how to speed things up further, or how to speed up other parts of the parser, I'm all ears.
note that the interesting bits are inside: Parser::parseAssignmentExpression(.)
Created attachment 281773 [details] Patch Some approach to avoid one stepback during parsing arrow function parameters
Attachment 281773 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/parser/Parser.cpp:3063: An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] ERROR: Source/JavaScriptCore/parser/Parser.cpp:3717: One line control clauses should not use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/parser/Parser.cpp:3736: One line control clauses should not use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/parser/Parser.h:816: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/parser/Parser.h:820: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/parser/Parser.h:821: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/parser/Parser.h:822: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/parser/Parser.h:846: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/JavaScriptCore/parser/Parser.h:847: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] Total errors found: 9 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #0) > It looks like arrow functions have slowed us down by 8-10%. > I've ran Octane's code-load benchmarks from the revision > before arrow functions vs ToT. Here is where we stand: .... > If anybody has ideas on how to speed things up further, or how to speed up > other parts of the parser, I'm all ears. I think there is a way how we can avoid at least one step back when parsing arrow function parameters. We can create some variable that will answer if current expression can be arrow function, and will check it during parseConditionalExpression. This function cover both header for arrow function x=> and (x)=>. In this function we will check if symbol => follow after variable or (), if not it is definitely not arrow function, if yes it should be arrow function, and in case we can't parse it, it is mean we have syntax error. Some like Proof of concept you can find in attach. What do you think if it approach viable and can give some improvement is performance?
Comment on attachment 281773 [details] Patch Attachment 281773 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1543703 Number of test failures exceeded the failure limit.
Created attachment 281777 [details] Archive of layout-test-results from ews102 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 281773 [details] Patch Attachment 281773 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1543717 Number of test failures exceeded the failure limit.
Created attachment 281778 [details] Archive of layout-test-results from ews107 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 281773 [details] Patch Attachment 281773 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1543696 Number of test failures exceeded the failure limit.
Created attachment 281779 [details] Archive of layout-test-results from ews116 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 281773 [details] Patch Attachment 281773 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1543735 New failing tests: js/parser-syntax-check.html js/arrowfunction-syntax.html js/arrowfunction-syntax-errors.html
Created attachment 281782 [details] Archive of layout-test-results from ews121 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.4
There are some result of performance tests. Important to receive this result was used modified code-load.js file, where it possible were replace common function to arrow function in following way: unction () replace by (a0,b0,c0,d0,e0,f0,g0,h0,j0,k0)=>{ function (a) replace by (a,b1,c1,d1,e1,f1,g1,h1,j1,k1)=>{ function (a,b) replace by (a,b,c2,d2,e2,f2,g2,h2,j2,k2)=>{ --- Not sure if it is cheating MacBook-Pro-Skachkov-2:Webkit2 Developer$ Tools/Scripts/run-jsc-benchmarks before:/Users/Developer/Projects/Webkit2/WebKitBuild/before_patch/Release/jsc after:/Users/Developer/Projects/Webkit2/WebKitBuild/es6_functions/Release/jsc --octane --benchmark="jquery|closure" --outer 10 Warning: could not identify checkout location for before Warning: could not identify checkout location for after 92/92 Benchmark report for Octane on MacBook-Pro-Skachkov-2 (MacBookPro8,1). VMs tested: "before" at /Users/Developer/Projects/Webkit2/WebKitBuild/before_patch/Release/jsc "after" at /Users/Developer/Projects/Webkit2/WebKitBuild/es6_functions/Release/jsc Collected 10 samples per benchmark/VM, with 10 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. before after closure 1.04113+-0.01106 ^ 0.99330+-0.01229 ^ definitely 1.0482x faster jquery 12.17617+-0.15386 11.96693+-0.10068 might be 1.0175x faster <geometric> 3.56038+-0.03687 ^ 3.44761+-0.03026 ^ definitely 1.0327x faster MacBook-Pro-Skachkov-2:Webkit2 Developer$ Tools/Scripts/run-jsc-benchmarks before:/Users/Developer/Projects/Webkit2/WebKitBuild/before_patch/Release/jsc after:/Users/Developer/Projects/Webkit2/WebKitBuild/es6_functions/Release/jsc --octane --benchmark="jquery|closure" --outer 10 Warning: could not identify checkout location for before Warning: could not identify checkout location for after 92/92 Benchmark report for Octane on MacBook-Pro-Skachkov-2 (MacBookPro8,1). VMs tested: "before" at /Users/Developer/Projects/Webkit2/WebKitBuild/before_patch/Release/jsc "after" at /Users/Developer/Projects/Webkit2/WebKitBuild/es6_functions/Release/jsc Collected 10 samples per benchmark/VM, with 10 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. before after closure 1.05545+-0.00767 ^ 0.98806+-0.01082 ^ definitely 1.0682x faster jquery 12.21856+-0.13663 12.02093+-0.07467 might be 1.0164x faster <geometric> 3.59098+-0.02558 ^ 3.44619+-0.01687 ^ definitely 1.0420x faster MacBook-Pro-Skachkov-2:Webkit2 Developer$ Tools/Scripts/run-jsc-benchmarks before:/Users/Developer/Projects/Webkit2/WebKitBuild/before_patch/Release/jsc after:/Users/Developer/Projects/Webkit2/WebKitBuild/es6_functions/Release/jsc --octane --benchmark="jquery|closure" --outer 10 Warning: could not identify checkout location for before Warning: could not identify checkout location for after 92/92 Benchmark report for Octane on MacBook-Pro-Skachkov-2 (MacBookPro8,1). VMs tested: "before" at /Users/Developer/Projects/Webkit2/WebKitBuild/before_patch/Release/jsc "after" at /Users/Developer/Projects/Webkit2/WebKitBuild/es6_functions/Release/jsc Collected 10 samples per benchmark/VM, with 10 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. before after closure 1.05095+-0.00968 ^ 0.98810+-0.01133 ^ definitely 1.0636x faster jquery 12.31999+-0.13188 ^ 12.01412+-0.07400 ^ definitely 1.0255x faster <geometric> 3.59817+-0.02872 ^ 3.44529+-0.01942 ^ definitely 1.0444x faster MacBook-Pro-Skachkov-2:Webkit2 Developer$ Tools/Scripts/run-jsc-benchmarks before:/Users/Developer/Projects/Webkit2/WebKitBuild/before_patch/Release/jsc after:/Users/Developer/Projects/Webkit2/WebKitBuild/es6_functions/Release/jsc --octane --benchmark="jquery|closure" --outer 10 Warning: could not identify checkout location for before Warning: could not identify checkout location for after 92/92 Benchmark report for Octane on MacBook-Pro-Skachkov-2 (MacBookPro8,1). VMs tested: "before" at /Users/Developer/Projects/Webkit2/WebKitBuild/before_patch/Release/jsc "after" at /Users/Developer/Projects/Webkit2/WebKitBuild/es6_functions/Release/jsc Collected 10 samples per benchmark/VM, with 10 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. before after closure 1.05823+-0.01207 ^ 0.99516+-0.01640 ^ definitely 1.0634x faster jquery 12.26944+-0.11244 12.07709+-0.11698 might be 1.0159x faster <geometric> 3.60322+-0.03151 ^ 3.46645+-0.02885 ^ definitely 1.0395x faster MacBook-Pro-Skachkov-2:Webkit2 Developer$ Tools/Scripts/run-jsc-benchmarks before:/Users/Developer/Projects/Webkit2/WebKitBuild/before_patch/Release/jsc after:/Users/Developer/Projects/Webkit2/WebKitBuild/es6_functions/Release/jsc --octane --benchmark="jquery|closure" --outer 10 Warning: could not identify checkout location for before Warning: could not identify checkout location for after 92/92 Benchmark report for Octane on MacBook-Pro-Skachkov-2 (MacBookPro8,1). VMs tested: "before" at /Users/Developer/Projects/Webkit2/WebKitBuild/before_patch/Release/jsc "after" at /Users/Developer/Projects/Webkit2/WebKitBuild/es6_functions/Release/jsc Collected 10 samples per benchmark/VM, with 10 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. before after closure 1.05397+-0.01681 ^ 0.98210+-0.00726 ^ definitely 1.0732x faster jquery 12.44263+-0.21901 ^ 12.03256+-0.04026 ^ definitely 1.0341x faster <geometric> 3.62080+-0.03777 ^ 3.43754+-0.01005 ^ definitely 1.0533x faster
(In reply to comment #14) > There are some result of performance tests. > Important to receive this result was used modified code-load.js file, where > it possible were replace common function to arrow function in following way: > unction () replace by (a0,b0,c0,d0,e0,f0,g0,h0,j0,k0)=>{ > function (a) replace by (a,b1,c1,d1,e1,f1,g1,h1,j1,k1)=>{ > function (a,b) replace by (a,b,c2,d2,e2,f2,g2,h2,j2,k2)=>{ > --- Not sure if it is cheating I wouldn't call this cheating. It's important that we know that there are programs that we can parse faster. This is a particular program that you've created. So it's a helpful test. It would also be useful to create a version of code-load where the parameter count isn't artificially inflated because this will be more representative of real programs. Also, is your method sound? It appears that many tests are failing. > > MacBook-Pro-Skachkov-2:Webkit2 Developer$ Tools/Scripts/run-jsc-benchmarks > before:/Users/Developer/Projects/Webkit2/WebKitBuild/before_patch/Release/ > jsc > after:/Users/Developer/Projects/Webkit2/WebKitBuild/es6_functions/Release/ > jsc --octane --benchmark="jquery|closure" --outer 10 > Warning: could not identify checkout location for before > Warning: could not identify checkout location for after > 92/92 > > Benchmark report for Octane on MacBook-Pro-Skachkov-2 (MacBookPro8,1). > > VMs tested: > "before" at > /Users/Developer/Projects/Webkit2/WebKitBuild/before_patch/Release/jsc > "after" at > /Users/Developer/Projects/Webkit2/WebKitBuild/es6_functions/Release/jsc > > Collected 10 samples per benchmark/VM, with 10 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. > > before after > > > closure 1.04113+-0.01106 ^ 0.99330+-0.01229 ^ > definitely 1.0482x faster > jquery 12.17617+-0.15386 11.96693+-0.10068 > might be 1.0175x faster > > <geometric> 3.56038+-0.03687 ^ 3.44761+-0.03026 ^ > definitely 1.0327x faster > > MacBook-Pro-Skachkov-2:Webkit2 Developer$ Tools/Scripts/run-jsc-benchmarks > before:/Users/Developer/Projects/Webkit2/WebKitBuild/before_patch/Release/ > jsc > after:/Users/Developer/Projects/Webkit2/WebKitBuild/es6_functions/Release/ > jsc --octane --benchmark="jquery|closure" --outer 10 > Warning: could not identify checkout location for before > Warning: could not identify checkout location for after > 92/92 > > Benchmark report for Octane on MacBook-Pro-Skachkov-2 (MacBookPro8,1). > > VMs tested: > "before" at > /Users/Developer/Projects/Webkit2/WebKitBuild/before_patch/Release/jsc > "after" at > /Users/Developer/Projects/Webkit2/WebKitBuild/es6_functions/Release/jsc > > Collected 10 samples per benchmark/VM, with 10 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. > > before after > > > closure 1.05545+-0.00767 ^ 0.98806+-0.01082 ^ > definitely 1.0682x faster > jquery 12.21856+-0.13663 12.02093+-0.07467 > might be 1.0164x faster > > <geometric> 3.59098+-0.02558 ^ 3.44619+-0.01687 ^ > definitely 1.0420x faster > > MacBook-Pro-Skachkov-2:Webkit2 Developer$ Tools/Scripts/run-jsc-benchmarks > before:/Users/Developer/Projects/Webkit2/WebKitBuild/before_patch/Release/ > jsc > after:/Users/Developer/Projects/Webkit2/WebKitBuild/es6_functions/Release/ > jsc --octane --benchmark="jquery|closure" --outer 10 > Warning: could not identify checkout location for before > Warning: could not identify checkout location for after > 92/92 > > Benchmark report for Octane on MacBook-Pro-Skachkov-2 (MacBookPro8,1). > > VMs tested: > "before" at > /Users/Developer/Projects/Webkit2/WebKitBuild/before_patch/Release/jsc > "after" at > /Users/Developer/Projects/Webkit2/WebKitBuild/es6_functions/Release/jsc > > Collected 10 samples per benchmark/VM, with 10 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. > > before after > > > closure 1.05095+-0.00968 ^ 0.98810+-0.01133 ^ > definitely 1.0636x faster > jquery 12.31999+-0.13188 ^ 12.01412+-0.07400 ^ > definitely 1.0255x faster > > <geometric> 3.59817+-0.02872 ^ 3.44529+-0.01942 ^ > definitely 1.0444x faster > > MacBook-Pro-Skachkov-2:Webkit2 Developer$ Tools/Scripts/run-jsc-benchmarks > before:/Users/Developer/Projects/Webkit2/WebKitBuild/before_patch/Release/ > jsc > after:/Users/Developer/Projects/Webkit2/WebKitBuild/es6_functions/Release/ > jsc --octane --benchmark="jquery|closure" --outer 10 > Warning: could not identify checkout location for before > Warning: could not identify checkout location for after > 92/92 > > Benchmark report for Octane on MacBook-Pro-Skachkov-2 (MacBookPro8,1). > > VMs tested: > "before" at > /Users/Developer/Projects/Webkit2/WebKitBuild/before_patch/Release/jsc > "after" at > /Users/Developer/Projects/Webkit2/WebKitBuild/es6_functions/Release/jsc > > Collected 10 samples per benchmark/VM, with 10 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. > > before after > > > closure 1.05823+-0.01207 ^ 0.99516+-0.01640 ^ > definitely 1.0634x faster > jquery 12.26944+-0.11244 12.07709+-0.11698 > might be 1.0159x faster > > <geometric> 3.60322+-0.03151 ^ 3.46645+-0.02885 ^ > definitely 1.0395x faster > > MacBook-Pro-Skachkov-2:Webkit2 Developer$ Tools/Scripts/run-jsc-benchmarks > before:/Users/Developer/Projects/Webkit2/WebKitBuild/before_patch/Release/ > jsc > after:/Users/Developer/Projects/Webkit2/WebKitBuild/es6_functions/Release/ > jsc --octane --benchmark="jquery|closure" --outer 10 > Warning: could not identify checkout location for before > Warning: could not identify checkout location for after > 92/92 > > Benchmark report for Octane on MacBook-Pro-Skachkov-2 (MacBookPro8,1). > > VMs tested: > "before" at > /Users/Developer/Projects/Webkit2/WebKitBuild/before_patch/Release/jsc > "after" at > /Users/Developer/Projects/Webkit2/WebKitBuild/es6_functions/Release/jsc > > Collected 10 samples per benchmark/VM, with 10 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. > > before after > > > closure 1.05397+-0.01681 ^ 0.98210+-0.00726 ^ > definitely 1.0732x faster > jquery 12.44263+-0.21901 ^ 12.03256+-0.04026 ^ > definitely 1.0341x faster > > <geometric> 3.62080+-0.03777 ^ 3.43754+-0.01005 ^ > definitely 1.0533x faster
(In reply to comment #15) > (In reply to comment #14) > > There are some result of performance tests. > > Important to receive this result was used modified code-load.js file, where > > it possible were replace common function to arrow function in following way: > > unction () replace by (a0,b0,c0,d0,e0,f0,g0,h0,j0,k0)=>{ > > function (a) replace by (a,b1,c1,d1,e1,f1,g1,h1,j1,k1)=>{ > > function (a,b) replace by (a,b,c2,d2,e2,f2,g2,h2,j2,k2)=>{ > > --- Not sure if it is cheating > I wouldn't call this cheating. It's important that we know that there are > programs that we can parse faster. This is a particular program that you've > created. So it's a helpful test. It would also be useful to create a version > of code-load where the parameter count isn't artificially inflated because > this will be more representative of real programs. > > Also, is your method sound? It appears that many tests are failing. > OK. I see. Thanks fro review. I'll back with patch that cover your comments.
Comment on attachment 281773 [details] Patch r-, as this has been pending review for over a year now. It is near-impossible that this patch still applies to trunk and unlikely to still be relevant in its current form.