fn = function() { var args = arguments; return () => args[0]; }(1) vs. fn = function() { return () => arguments[0]; }(1) The first runs much faster than the second
I think this issue is related to mistake in calculation of the variable needsArguments in BytecodeGenerator.cpp. Using arguments in arrow function leads to emitting create_lexical_environment and create_direct_arguments twice, at first for wrapper function second for arrow function. I'll try to prepare fix during weekend
(In reply to comment #1) > I think this issue is related to mistake in calculation of the variable > needsArguments in BytecodeGenerator.cpp. Using arguments in arrow function > leads to emitting create_lexical_environment and create_direct_arguments > twice, at first for wrapper function second for arrow function. > I'll try to prepare fix during weekend Sounds good.
I fixed issue with needsArguments and byte code looks the same except difference in type of variable and still difference in performance. Difference is type of variable during resolve scope in first example is ClosureVar and in second case is Dynamic. See bytecode for second cas: [ 0] enter [ 1] get_scope loc3 [ 3] mov loc4, loc3 [ 6] create_lexical_environment loc5, loc3, Cell: 0x1081871a0 (0x1081e8a20:[SymbolTable, {}, NonArray, Leaf]), ID: 20(const0), Undefined(const1) [ 11] mov loc3, loc5 [ 14] create_direct_arguments loc6 [ 16] put_to_scope loc5, arguments(@id0), loc6, 2052<ThrowIfNotFound|LocalClosureVar|NotInitialization>, <structure>, 0 [ 23] new_func_exp loc8, loc3, f0 [ 27] ret loc8 Identifiers: id0 = arguments Constants: k0 = Cell: 0x1081871a0 (0x1081e8a20:[SymbolTable, {}, NonArray, Leaf]), ID: 20 k1 = Undefined #A6UXLB:[0x108168520->0x1081b7020, NoneFunctionCall, 31 (NeverInline)]: 31 m_instructions; 248 bytes; 1 parameter(s); 10 callee register(s); 6 variable(s); scope at loc3 [ 0] enter [ 1] get_scope loc3 [ 3] mov loc4, loc3 [ 6] resolve_scope loc7, loc3, arguments(@id0), <DYNAMIC>, 0, 0x0 [ 13] get_from_scope loc8, loc7, arguments(@id0), 2060<ThrowIfNotFound| DYNAMIC |NotInitialization>, 0 predicting None [ 21] get_by_val loc6, loc8, Int32: 0(const0) Original; predicting None [ 27] ret loc6 [ 29] ret Undefined(const1) Is it possible fix this?
Created attachment 302789 [details] Patch Patch for review
Comment on attachment 302789 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=302789&action=review r=me. > Source/JavaScriptCore/ChangeLog:12 > + patch it can be ClosureVar, that increase performance of access to arguments variable > + inside of the arrow function. How much faster is it now? Can you write that here. > Source/JavaScriptCore/runtime/JSScope.cpp:-55 > - if (ident == exec->propertyNames().arguments) { Nice. This looks like old code to me that should no longer be needed. > JSTests/stress/arrowfunction-lexical-bind-arguments-non-strict-1.js:269 > +var fn = function() { > + return () => arguments[0]; > +}(1); > + > +for (var i = 0; i < 10000; i++) { > + testCase(fn(2), 1); > +} > + > +var fn = function() { > + var args = arguments; > + return () => args[0]; > +}(1); > + > +for (var i = 0; i < 10000; i++) { > + testCase(fn(2), 1); > +} Can you add these tests under the microbenchmarks directory?
Can you also run the layout tests locally, or wait for EWS to come back online before landing?
Created attachment 302959 [details] Patch Fix comments
Comment on attachment 302959 [details] Patch Clearing flags on attachment: 302959 Committed r213165: <http://trac.webkit.org/changeset/213165>
All reviewed patches have been landed. Closing bug.