This should be a big win in terms of removing barriers to tier up. More of my implementation plan to follow as I familiarize myself with the code.
Created attachment 258380 [details] Super rough WIP Super rough and crazy, but it's a start. This program works: function assert(b) { if (!b) throw new Error("Bad value"); } noInline(assert); function bar(a, b, c) { try { assert(a + b === c); } catch(e) { print("caught exception in bar " + e); } } function foo(a, b, c) { try { bar(a, b, c); assert(a + b === c); } catch(e) { print("caught exception in foo " + e); } } var start = preciseTime(); noInline(foo); for (var i = 0; i < 100000; i++) { foo(10, 20, 30); } foo(10, 20, 40); var end = preciseTime(); print(end - start);
Created attachment 258729 [details] WIP still rough but more programs are working. About ~50 test failures currently, but we need more test coverage, which I'm adding as I go. This patch still doesn't include renaming/refactoring of things.
Created attachment 259110 [details] WIP still rough. The patch for bug #147988 is subsumed into this patch until #147988 lands.
Created attachment 261041 [details] WIP getting much closer to being done.
Attachment 261041 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGJITCompiler.h:266: The parameter name "codeOrigin" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGJITCompiler.h:277: The parameter name "linkBuffer" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompiler.cpp:135: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/dfg/DFGPlan.cpp:264: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/dfg/DFGFlushLiveCatchVariablesInsertionPhase.cpp:67: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/dfg/DFGFlushLiveCatchVariablesInsertionPhase.cpp:113: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/dfg/DFGFlushLiveCatchVariablesInsertionPhase.cpp:138: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/dfg/DFGFlushLiveCatchVariablesInsertionPhase.cpp:153: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/interpreter/StackVisitor.cpp:74: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/interpreter/StackVisitor.cpp:78: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/bytecode/CodeBlock.h:927: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h:1912: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h:1913: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h:1914: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h:1915: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp:273: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp:278: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:3555: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/dfg/DFGJITCompiler.cpp:599: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 19 in 41 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 261041 [details] WIP Attachment 261041 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/163316 New failing tests: fast/workers/worker-close.html
Created attachment 261052 [details] Archive of layout-test-results from ews101 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-mavericks Platform: Mac OS X 10.9.5
Created attachment 261141 [details] patch I still want to write a few more tests but I think it's ready for comments/suggestions.
Comment on attachment 261141 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=261141&action=review > Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj:422 > + <ClInclude Include="..\dfg\DFGFlushLiveCatchVariablesInsertionPhase.cpp" /> better: "LiveCatchVariableFlushInsertionPhase" "performLiveCatchVariableFlushInsertion" reads better than "performFlushLiveCatchVariablesInsertion"
Comment on attachment 261141 [details] patch As part of this patch, I'd like to see one of raytrace, richards, deltablue, or earley-boyer converted to have try{}catch{} everyhwere. I think that this is important to prove the speed-up and also to introduce test coverage.
Attachment 261141 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGJITCompiler.h:282: The parameter name "codeOrigin" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompiler.cpp:132: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/dfg/DFGPlan.cpp:264: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/dfg/DFGFlushLiveCatchVariablesInsertionPhase.cpp:71: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/dfg/DFGFlushLiveCatchVariablesInsertionPhase.cpp:143: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/CMakeLists.txt:187: Alphabetical sorting problem. "dfg/DFGFlushLiveCatchVariablesInsertionPhase.cpp" should be before "dfg/DFGFlushedAt.cpp". [list/order] [5] ERROR: Source/JavaScriptCore/dfg/DFGJITCompiler.cpp:94: Should have a space between // and comment [whitespace/comments] [4] Total errors found: 7 in 45 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 261141 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=261141&action=review > Source/JavaScriptCore/dfg/DFGJITCompiler.cpp:94 > + //restoreCalleeSavesFromVMCalleeSavesBuffer(); I think you need to uncomment this line. > Source/JavaScriptCore/dfg/DFGJITCompiler.cpp:104 > + Delete this line. > Source/JavaScriptCore/dfg/DFGPlan.cpp:264 > + //dfg.dump(); Debug code?
Created attachment 261236 [details] patch
Comment on attachment 261236 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=261236&action=review > Source/JavaScriptCore/dfg/DFGLiveCatchVariablePreservationPhase.cpp:157 > + for (InlineCallFrame* inlineCallFrame : seenInlineCallFrames) This seems a little hacky to me. What do you guys think?
Attachment 261236 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGJITCompiler.h:282: The parameter name "codeOrigin" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompiler.cpp:132: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/dfg/DFGPlan.cpp:52: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGLiveCatchVariablePreservationPhase.cpp:70: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/dfg/DFGLiveCatchVariablePreservationPhase.cpp:141: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 5 in 58 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 261236 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=261236&action=review >> Source/JavaScriptCore/dfg/DFGLiveCatchVariablePreservationPhase.cpp:157 >> + for (InlineCallFrame* inlineCallFrame : seenInlineCallFrames) > > This seems a little hacky to me. What do you guys think? No, this seems fine! > Source/JavaScriptCore/dfg/DFGLiveCatchVariablePreservationPhase.h:35 > +// FIXME: comment here. :-) You should add a comment here!
landed in: http://trac.webkit.org/changeset/189938
Looks like this patch broke LLint Cloop build: https://build.webkit.org/builders/Apple%20Yosemite%20LLINT%20CLoop%20%28BuildAndTest%29?numbuilds=50
Fixed the build in http://trac.webkit.org/changeset/189952. Please verify the correctness.
This broke workers: https://build.webkit.org/results/Apple%20Yosemite%20Debug%20WK1%20(Tests)/r189954%20(7554)/results.html
Re-opened since this is blocked by bug 149329
There may have been memory corruption on non-worker tests too (but you can't now with memory corruption): https://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK1%20(Tests)/r189957%20(15679)/js/regress/richards-try-catch-crash-log.txt
Created attachment 261531 [details] patch for landing I want to let EWS take a look.
Attachment 261531 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompiler.cpp:132: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/dfg/DFGLiveCatchVariablePreservationPhase.cpp:137: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 2 in 62 files If any of these errors are false positives, please file a bug against check-webkit-style.
landed take 2 fix in: http://trac.webkit.org/changeset/189995