Bug 196943 – Add a baseline tracelet JIT

• Component: JavaScriptCore
• Status: NEW

[Saam Barati]

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.

[Radar WebKit Bug Importer]

<rdar://problem/49920297>

[Saam Barati]

Created attachment 367712 [details]
WIP

it begins

[Saam Barati]

Created attachment 367768 [details]
WIP

[Saam Barati]

Created attachment 367960 [details]
WIP

[Saam Barati]

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.

[Saam Barati]

Created attachment 368007 [details]
WIP

[Saam Barati]

Created attachment 368292 [details]
WIP

[Saam Barati]

Created attachment 368299 [details]
WIP

Got some fun crashers going on.

[Saam Barati]

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 :-)

[Saam Barati]

Created attachment 368355 [details]
WIP

[Saam Barati]

Created attachment 368357 [details]
WIP

[Saam Barati]

Created attachment 368369 [details]
WIP

Things seem to be working (with DFG disabled)

[Saam Barati]

Created attachment 368379 [details]
WIP

[Saam Barati]

Created attachment 368505 [details]
WIP

[Saam Barati]

Created attachment 368525 [details]
WIP

seems to be starting to pass (most/all?) tests.

[Saam Barati]

Created attachment 368527 [details]
WIP

Tests are passing. Gonna do some cleanup and start working on perf.

[Saam Barati]

Created attachment 368640 [details]
WIP

[EWS Watchlist]

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.

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[Saam Barati]

Created attachment 368754 [details]
WIP

[Saam Barati]

Created attachment 368767 [details]
WIP

[Saam Barati]

Created attachment 369170 [details]
WIP

[Saam Barati]

Created attachment 369453 [details]
WIP

[Saam Barati]

Created attachment 369455 [details]
patch

[Saam Barati]

Created attachment 369515 [details]
WIP

[Saam Barati]

Created attachment 369535 [details]
WIP

[Saam Barati]

Created attachment 369581 [details]
WIP

[Saam Barati]

Created attachment 369607 [details]
WIP

[Saam Barati]

Created attachment 369770 [details]
WIP

rebased

[Saam Barati]

Created attachment 369792 [details]
WIP

[Saam Barati]

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

[Saam Barati]

(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.

[Filip Pizlo]

(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.

[Filip Pizlo]

(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.

[Filip Pizlo]

(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.

[Geoffrey Garen]

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.

[Filip Pizlo]

(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.

[Geoffrey Garen]

Maybe try compiling small / all tracelets synchronously?

[Filip Pizlo]

(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.

[Saam Barati]

(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.

[Saam Barati]

(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.

[Saam Barati]

Created attachment 370097 [details]
WIP

stashing away patch here. Probably has some fun logging in it atm.

[Geoffrey Garen]

> 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.

[Saam Barati]

Created attachment 370364 [details]
WIP

Leaving this here as I'm reverting these changes away locally