The insertion of eval in the following script causes a slowdown of ~4x: a = 123; (function() { var i; eval("1"); for (i = 0; i < 100000; ++i) { a; a; a; a; a; a; } })(); Similarly for the following script: a = 123; (function() { eval("1"); (function() { for (var i = 0; i < 100000; ++i) { a; a; a; a; a; a; a; a; } })(); })(); It would be nice if the eval didn't hurt performance so much. The optimization should of course not break scripts like the following: a = 2; (function(code) { eval(code); return a + a; })("var a = 4"); // must return 8, not 4 Or like the following: a = 2; print((function() { eval("var a = 4"); return function() { return a + a; }(); })()); // must return 8, not 4
Created attachment 55432 [details] Patch
Attachment 55432 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 JavaScriptCore/interpreter/Interpreter.cpp:2073: vm_throw is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Total errors found: 1 in 20 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 55432 [details] Patch I think you can share emit_op_resolve_global_dynamic. + requiresDynamicChecks = d()->functionExecutable->usesEval(); Maybe this should be |=, to avoid stomping other functions' results. "requiresDynamicChecks" sounds a bit too much like "isDynamicScope" to me. How about "mayBecomeDynamicScope"? r=me
Committed r58986: <http://trac.webkit.org/changeset/58986>
http://trac.webkit.org/changeset/58986 might have broken Qt Linux Release
The tree is still on fire from this change: http://build.webkit.org/console
Oliver confirmed over IRC that he's looking into the test failures now.
Comment on attachment 55432 [details] Patch Cool, thanks for implementing this. > + while (skip--) { > + JSObject* o = *iter; > + if (o->hasCustomProperties()) { > + Identifier& ident = codeBlock->identifier(property); > + do { > + PropertySlot slot(o); > + if (o->getPropertySlot(callFrame, ident, slot)) { > + JSValue result = slot.getValue(callFrame, ident); > + exceptionValue = callFrame->globalData().exception; > + if (exceptionValue) > + return false; > + callFrame->r(dst) = JSValue(result); > + return true; > + } > + if (iter == end) > + break; > + o = *iter; > + ++iter; > + } while (true); > + exceptionValue = createUndefinedVariableError(callFrame, ident, vPC - codeBlock->instructions().begin(), codeBlock); > + return false; > + } > + ++iter; > + } The property is looked up twice in the first scope object with custom properties now, since o is reassigned before ++iter. Why is the lookup done dynamically for the whole scope chain in case there is just one object with custom properties? Wouldn't it still be possible to cache the property from the outer-most (global) scope and use that one when the dynamic lookup for the inner-most scope fails? This should enable the optimization to do some good in the following program: a = 2; (function(code) { eval(code); (function() { for (var i = 0; i < 100000; ++i) { a; a; a; a; a; a; a; a; } })(); })(String.fromCharCode(0x76, 0x61, 0x72, 0x20, 0x63, 0x3D, 0x34)); // "var c=4" If I replace "eval(code)" by "eval('123')" the caching kicks in, but that's a less interesting use case. My naive patch: diff --git a/JavaScriptCore/interpreter/Interpreter.cpp b/JavaScriptCore/interpreter/Interpreter.cpp index 921dfdf..f777519 100644 --- a/JavaScriptCore/interpreter/Interpreter.cpp +++ b/JavaScriptCore/interpreter/Interpreter.cpp @@ -210,23 +210,15 @@ NEVER_INLINE bool Interpreter::resolveGlobalDynamic(CallFrame* callFrame, Instru JSObject* o = *iter; if (o->hasCustomProperties()) { Identifier& ident = codeBlock->identifier(property); - do { - PropertySlot slot(o); - if (o->getPropertySlot(callFrame, ident, slot)) { - JSValue result = slot.getValue(callFrame, ident); - exceptionValue = callFrame->globalData().exception; - if (exceptionValue) - return false; - callFrame->r(dst) = JSValue(result); - return true; - } - if (iter == end) - break; - o = *iter; - ++iter; - } while (true); - exceptionValue = createUndefinedVariableError(callFrame, ident, vPC - codeBlock->instructions().begin(), codeBlock); - return false; + PropertySlot slot(o); + if (o->getPropertySlot(callFrame, ident, slot)) { + JSValue result = slot.getValue(callFrame, ident); + exceptionValue = callFrame->globalData().exception; + if (exceptionValue) + return false; + callFrame->r(dst) = JSValue(result); + return true; + } } ++iter; }