It would allow us to 1. Compile parts of a function in the baseline JIT more quickly. We could have super low tier up counts for a tracelet. 2. allow those parts to get the benefits the baseline JIT gives us: ICs, no dispatch overhead If we find this is a successful avenue, extending this to the DFG could be a next step. tracelet-DFG could be optimized to be a single basic block, with "jumps" that end up going outside of it. We'd probably want to modify what kind of optimizations we did in such a compiler to allow us to compile very quickly.
<rdar://problem/49920297>
Created attachment 367712 [details] WIP it begins
Created attachment 367768 [details] WIP
Created attachment 367960 [details] WIP
Created attachment 367998 [details] WIP It can now (at least one program) with traces, although things are quite dumb at the moment and there are probably many bugs. Gonna start to work on the mechanics of the data structures that represent traces, the JITCode we will use, etc. Once that's rock solid and makes sense, I will work on tuning the perf and intelligently placing down trace_profile. Currently, the heuristic I dropped into bytecode generator generates way too many trace_profile ops.
Created attachment 368007 [details] WIP
Created attachment 368292 [details] WIP
Created attachment 368299 [details] WIP Got some fun crashers going on.
Comment on attachment 368299 [details] WIP View in context: https://bugs.webkit.org/attachment.cgi?id=368299&action=review > Source/JavaScriptCore/jit/JITCode.h:240 > + CodeRef<JSEntryPtrTag> ref() { return m_ref; } this was my issue! Oops. Naming functions ref() when something is RefCounted is a bad idea :-)
Created attachment 368355 [details] WIP
Created attachment 368357 [details] WIP
Created attachment 368369 [details] WIP Things seem to be working (with DFG disabled)
Created attachment 368379 [details] WIP
Created attachment 368505 [details] WIP
Created attachment 368525 [details] WIP seems to be starting to pass (most/all?) tests.
Created attachment 368527 [details] WIP Tests are passing. Gonna do some cleanup and start working on perf.
Created attachment 368640 [details] WIP
Attachment 368640 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/jit/ExecutableAllocator.cpp:163: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/ExecutableAllocator.cpp:165: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JITCode.cpp:39: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JITCode.cpp:40: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JITCode.cpp:41: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JITCode.cpp:42: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JITCode.cpp:47: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JITCode.cpp:48: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JITCode.cpp:49: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JITCode.cpp:50: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JITCode.h:276: The parameter name "entry" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/jit/JITCode.h:287: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/bytecode/CodeBlock.cpp:1090: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JIT.h:926: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:44: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:60: Bad include order. Mixing system and custom headers. [build/include_order] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:201: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:300: Non-label code inside switch statements should be indented. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:308: Almost always, snprintf is better than strcat. [security/printf] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:842: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:851: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:857: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:863: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:869: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:870: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:879: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:896: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:912: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:978: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:981: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:1073: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:1121: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:1122: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:1124: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:1125: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:1126: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:1154: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:1167: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:1171: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JIT.cpp:1307: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/bytecode/CodeBlock.h:312: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/bytecode/CodeBlock.h:313: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/bytecode/CodeBlock.h:316: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/bytecode/CodeBlock.h:317: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/bytecode/CodeBlock.h:318: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/bytecode/CodeBlock.h:321: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/runtime/ScriptExecutable.cpp:187: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/runtime/ScriptExecutable.cpp:188: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/runtime/ScriptExecutable.cpp:189: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/runtime/ScriptExecutable.cpp:190: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:522: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:2000: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:2014: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:2019: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JITInlines.h:250: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JITWorklist.cpp:240: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/jit/JITCodeMap.h:61: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/JavaScriptCore/jit/JITCodeMap.h:96: Should have a space between // and comment [whitespace/comments] [4] Total errors found: 58 in 35 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 368640 [details] WIP Attachment 368640 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/12050413 New failing tests: js/dfg-double-vote-fuzz.html workers/bomb.html js/slow-stress/emscripten-memops.html
Created attachment 368658 [details] Archive of layout-test-results from ews102 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 368640 [details] WIP Attachment 368640 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/12050420 New failing tests: js/dfg-double-vote-fuzz.html js/slow-stress/emscripten-memops.html
Created attachment 368660 [details] Archive of layout-test-results from ews105 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 368640 [details] WIP Attachment 368640 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/12050486 New failing tests: js/dfg-double-vote-fuzz.html js/slow-stress/emscripten-memops.html
Created attachment 368662 [details] Archive of layout-test-results from ews126 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.14.4
Comment on attachment 368640 [details] WIP Attachment 368640 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/12050499 New failing tests: js/dfg-double-vote-fuzz.html js/slow-stress/marsaglia.html fast/canvas/webgl/uninitialized-test.html webgl/2.0.0/conformance2/textures/image_data/tex-3d-rgb8-rgb-unsigned_byte.html webgl/1.0.2/conformance/misc/uninitialized-test.html webgl/2.0.0/conformance/misc/uninitialized-test.html
Created attachment 368664 [details] Archive of layout-test-results from ews114 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 368640 [details] WIP Attachment 368640 [details] did not pass jsc-ews (mac): Output: https://webkit-queues.webkit.org/results/12050522 New failing tests: wasm.yaml/wasm/lowExecutableMemory/exports-oom.js.default-wasm wasm.yaml/wasm/lowExecutableMemory/executable-memory-oom.js.default-wasm jsc-layout-tests.yaml/js/script-tests/dfg-double-vote-fuzz.js.layout-no-ftl jsc-layout-tests.yaml/js/script-tests/dfg-ensure-contiguous-on-string.js.layout-ftl-eager-no-cjit wasm.yaml/wasm/lowExecutableMemory/imports-oom.js.default-wasm jsc-layout-tests.yaml/js/script-tests/dfg-ensure-contiguous-on-string.js.layout-dfg-eager-no-cjit
Created attachment 368754 [details] WIP
Created attachment 368767 [details] WIP
Created attachment 369170 [details] WIP
Created attachment 369453 [details] WIP
Created attachment 369455 [details] patch
Created attachment 369515 [details] WIP
Created attachment 369535 [details] WIP
Created attachment 369581 [details] WIP
Created attachment 369607 [details] WIP
Created attachment 369770 [details] WIP rebased
Created attachment 369792 [details] WIP
Perf not looking great so far. Continuing to investigate why it's slower. - It seems neutral on JS2 when baseline JIT is the final tier. - It's a slow down when enabling DFG/FTL. Seems like it shouldn't be too hard to find out why. - Initial testing shows it's a PLT slow down too
(In reply to Saam Barati from comment #39) > Perf not looking great so far. Continuing to investigate why it's slower. > > - It seems neutral on JS2 when baseline JIT is the final tier. > - It's a slow down when enabling DFG/FTL. Seems like it shouldn't be too > hard to find out why. > - Initial testing shows it's a PLT slow down too It seems my PLT regression is same as JS2 being regressed with DFG turned on. I need to look into why I made the DFG slower.
(In reply to Saam Barati from comment #40) > (In reply to Saam Barati from comment #39) > > Perf not looking great so far. Continuing to investigate why it's slower. > > > > - It seems neutral on JS2 when baseline JIT is the final tier. > > - It's a slow down when enabling DFG/FTL. Seems like it shouldn't be too > > hard to find out why. > > - Initial testing shows it's a PLT slow down too > > It seems my PLT regression is same as JS2 being regressed with DFG turned > on. I need to look into why I made the DFG slower. I think that if I was you, I’d stop running dog or FTL at all until you got the traveler-baseline to outperform baseline in some way and get to completely neutral versus baseline. The progression could even just be memory usage. I think it’s just useful to know from a strategy standpoint if we should be pursuing this at all, and if so, towards what end. It could become a different beast if the only benefit was memory.
(In reply to Filip Pizlo from comment #41) > (In reply to Saam Barati from comment #40) > > (In reply to Saam Barati from comment #39) > > > Perf not looking great so far. Continuing to investigate why it's slower. > > > > > > - It seems neutral on JS2 when baseline JIT is the final tier. > > > - It's a slow down when enabling DFG/FTL. Seems like it shouldn't be too > > > hard to find out why. > > > - Initial testing shows it's a PLT slow down too > > > > It seems my PLT regression is same as JS2 being regressed with DFG turned > > on. I need to look into why I made the DFG slower. > > I think that if I was you, I’d stop running dog or FTL at all until you got Lol I meant DFG. > the traveler-baseline to outperform baseline in some way and get to > completely neutral versus baseline. The progression could even just be > memory usage. > > I think it’s just useful to know from a strategy standpoint if we should be > pursuing this at all, and if so, towards what end. It could become a > different beast if the only benefit was memory.
(In reply to Filip Pizlo from comment #42) > (In reply to Filip Pizlo from comment #41) > > (In reply to Saam Barati from comment #40) > > > (In reply to Saam Barati from comment #39) > > > > Perf not looking great so far. Continuing to investigate why it's slower. > > > > > > > > - It seems neutral on JS2 when baseline JIT is the final tier. > > > > - It's a slow down when enabling DFG/FTL. Seems like it shouldn't be too > > > > hard to find out why. > > > > - Initial testing shows it's a PLT slow down too > > > > > > It seems my PLT regression is same as JS2 being regressed with DFG turned > > > on. I need to look into why I made the DFG slower. > > > > I think that if I was you, I’d stop running dog or FTL at all until you got > > Lol I meant DFG. > > > the traveler-baseline to outperform baseline in some way and get to Omg autocorrect. I meant tracelet-baseline. > > completely neutral versus baseline. The progression could even just be > > memory usage. > > > > I think it’s just useful to know from a strategy standpoint if we should be > > pursuing this at all, and if so, towards what end. It could become a > > different beast if the only benefit was memory.
I would expect tracelet-baseline to beat baseline on JS2-latency, PLT, or JSBench. Would also be interesting to try: (a) Significantly reducing the tier-up threshold for tracelet-baseline; (b) Writing some cooked benchmarks with big functions that try to prove the value of tracelet-baseline.
(In reply to Geoffrey Garen from comment #44) > I would expect tracelet-baseline to beat baseline on JS2-latency, PLT, or > JSBench. > > Would also be interesting to try: > > (a) Significantly reducing the tier-up threshold for tracelet-baseline; > > (b) Writing some cooked benchmarks with big functions that try to prove the > value of tracelet-baseline. That’s what we expected, too. But it’s been in this state - no progression - for a week or two now. Saam could tell more details. One part that stood out to me is that this gets less benefit from concurrent JITing because the concurrent JIT seems to work better at larger granularity.
Maybe try compiling small / all tracelets synchronously?
(In reply to Geoffrey Garen from comment #46) > Maybe try compiling small / all tracelets synchronously? I think Saam did that. :-) Problem is, then the cost of compiling that code is paid by the main thread. That could explain the lack of a progression.
(In reply to Filip Pizlo from comment #41) > (In reply to Saam Barati from comment #40) > > (In reply to Saam Barati from comment #39) > > > Perf not looking great so far. Continuing to investigate why it's slower. > > > > > > - It seems neutral on JS2 when baseline JIT is the final tier. > > > - It's a slow down when enabling DFG/FTL. Seems like it shouldn't be too > > > hard to find out why. > > > - Initial testing shows it's a PLT slow down too > > > > It seems my PLT regression is same as JS2 being regressed with DFG turned > > on. I need to look into why I made the DFG slower. > > I think that if I was you, I’d stop running dog or FTL at all until you got > the traveler-baseline to outperform baseline in some way and get to > completely neutral versus baseline. The progression could even just be > memory usage. > > I think it’s just useful to know from a strategy standpoint if we should be > pursuing this at all, and if so, towards what end. It could become a > different beast if the only benefit was memory. Yeah I've spent most of my time measuring with the DFG disabled both on baseline and tracelet. However, I enabled the DFG just to make sure I didn't regress stuff there. Which it's quite apparent that I have. It's almost certainly something that's fixable. And I'll focus on making baseline-only faster for now. With DFG disabled, my testing already shows that it's neutral. I haven't measure memory usage. I'm still trying to get it to be faster, so I'm working on that now for PLT5. If we decide to tune it for memory instead of perf, I'd change our tier up thresholds to be more conservative than I have now.
(In reply to Filip Pizlo from comment #47) > (In reply to Geoffrey Garen from comment #46) > > Maybe try compiling small / all tracelets synchronously? > > I think Saam did that. :-) > > Problem is, then the cost of compiling that code is paid by the main thread. > That could explain the lack of a progression. I only tried compiling *everything* synchronously instead of just small tracelets. It might be worth experimenting with this more.
Created attachment 370097 [details] WIP stashing away patch here. Probably has some fun logging in it atm.
> I only tried compiling *everything* synchronously instead of just small > tracelets. It might be worth experimenting with this more. That's an interesting detail. I'll leave it to y'all to decide how much experimentation we want to do in this area -- but I think this would be a really interesting thing to try. My theory: If you know you're about to execute a snippet of code, JIT compiling the snippet and then executing it doesn't have much overhead. The interpreter would have iterated and decoded all those opcodes anyway, and the amount of JIT code emitted isn't enough to trigger memory pathologies. So, a simple strategy that said "this is my second trip through this basic block / extended basic block / loop / whatever, I'll compile just what I'm about to run" seems like it might be a win.
Created attachment 370364 [details] WIP Leaving this here as I'm reverting these changes away locally