Bug 111497

Created attachment 191603 [details] Patch

Created attachment 191604 [details] Patch

Comment on attachment 191604 [details] Patch Attachment 191604 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/17079093 New failing tests: fast/js/dfg-int32-to-double-on-known-number.html fast/workers/shared-worker-frame-lifecycle.html fast/js/dfg-cse-cfa-discrepancy.html fast/frames/take-focus-from-iframe.html fast/workers/dedicated-worker-lifecycle.html fast/js/dfg-obvious-constant-cfa.html fast/js/regress/inline-get-scoped-var.html jquery/attributes.html

(In reply to comment #3) > (From update of attachment 191604 [details]) > Attachment 191604 [details] did not pass mac-wk2-ews (mac-wk2): > Output: http://webkit-commit-queue.appspot.com/results/17079093 > > New failing tests: > fast/js/dfg-int32-to-double-on-known-number.html > fast/workers/shared-worker-frame-lifecycle.html > fast/js/dfg-cse-cfa-discrepancy.html > fast/frames/take-focus-from-iframe.html > fast/workers/dedicated-worker-lifecycle.html > fast/js/dfg-obvious-constant-cfa.html > fast/js/regress/inline-get-scoped-var.html > jquery/attributes.html wk2 is a 64bit bot right?

Comment on attachment 191604 [details] Patch Attachment 191604 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/17066104 New failing tests: fast/js/dfg-int32-to-double-on-known-number.html fast/workers/shared-worker-frame-lifecycle.html fast/js/dfg-cse-cfa-discrepancy.html jquery/attributes.html fast/js/dfg-obvious-constant-cfa.html fast/js/regress/inline-get-scoped-var.html http/tests/workers/terminate-during-sync-operation.html fast/workers/dedicated-worker-lifecycle.html

Comment on attachment 191604 [details] Patch r-'ing until i work out wtf is happening to the mac bots

Created attachment 191809 [details] Patch

Comment on attachment 191809 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191809&action=review What's the performance result here? > Source/JavaScriptCore/bytecode/CodeBlock.cpp:1944 > +#if ENABLE(DFG_JIT) I believe this should be ENABLE(VALUE_PROFILING). > Source/JavaScriptCore/bytecode/CodeBlock.h:1158 > + bool m_needsFullScopeChain; Let's call this m_needsActivation. > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:241 > + , m_globalObjectRegister(0) Can we use -1 as the default value in the bytecode generator, to match the code block? > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1280 > + size_t depthOfFirstScopeWithDynamicChecks = 0; I believe this value is wrong if the current scope needsActivation(), since there isn't any code that adds 1 in that case. Actually, let's just remove this, since it's unused. > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1294 > + depth += m_codeBlock->needsFullScopeChain(); I think it would be clearer to initialize 'depth' to 0 or 1 based on needsActivation(), rather than adding 1 in at the end. That way, depth is always consistent with the depth you'll see at runtime. > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1296 > + // Global scope This comment would be better if it explained why global scope forces a dynamic resolve. I'm not sure, from reading this, why it should. > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1307 > + break; I think this would be clearer as an early return, like you used in other places in the function. > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1310 > + if (!(flags & ResolveResult::DynamicFlag)) { > + if (scopeRequiresDynamicChecks) > + flags |= ResolveResult::DynamicFlag; Can this |= just be unconditional? I think that would be clearer. > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1375 > + // Global object is the base I don't think this comment adds anything over the code below it.

Committed r145000: <http://trac.webkit.org/changeset/145000>