Bug 121374 - Deoptimize deoptimization: make DFGOSRExitCompiler64.cpp more hackable
Summary: Deoptimize deoptimization: make DFGOSRExitCompiler64.cpp more hackable
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks: 121064 122025
  Show dependency treegraph
 
Reported: 2013-09-14 18:21 PDT by Filip Pizlo
Modified: 2013-09-27 11:38 PDT (History)
10 users (show)

See Also:


Attachments
this is just the start (4.89 KB, patch)
2013-09-14 18:21 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (34.69 KB, patch)
2013-09-14 21:01 PDT, Filip Pizlo
ggaren: review+
eflews.bot: commit-queue-
Details | Formatted Diff | Diff
patch for landing (36.39 KB, patch)
2013-09-15 11:02 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2013-09-14 18:21:00 PDT
It used to be the case that OSR exits happened a lot, and we sometimes let them happen roughly in proportion to the number of times that a function completed successfully.  This was a long time ago - before we had good reoptimization heuristics and before we got our act together in other ways.  Now we don't OSR exit that much.  More than ~100 exits means that we reoptimize.  Hence while we want OSR exit to not be terribly slow and it still makes sense to emit JIT code for it, we don't really care if it makes great use of registers or anything like that.

But right now the OSRExitCompiler is still soooper optimized, and that makes the code hard to hack.  This makes adding new kinds of data formats, which the exit compiler must know about, rather difficult.  For example I couldn't figure out a good way of adding Int48 recoveries without either copy-pasting code or just adding a lot of confusing code paths.

So, the best thing to do is probably to rewrite the exit compiler to be a lot dumber.  The plan is to make it first dump all state into a scratch buffer (even if it's already in the stack in the right place!) and then dump back from the scratch buffer into the stack while performing the recoveries.  This will allow for example combining the code for reboxing an int that was in the stack in the right place, in the stack in the wrong place, or in a register - all of those cases will work the same now, which is pretty awesome.
Comment 1 Filip Pizlo 2013-09-14 18:21:36 PDT
Created attachment 211678 [details]
this is just the start
Comment 2 Filip Pizlo 2013-09-14 21:01:58 PDT
Created attachment 211681 [details]
the patch
Comment 3 EFL EWS Bot 2013-09-14 21:11:08 PDT
Comment on attachment 211681 [details]
the patch

Attachment 211681 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/1861597
Comment 4 Filip Pizlo 2013-09-14 21:14:15 PDT
(In reply to comment #3)
> (From update of attachment 211681 [details])
> Attachment 211681 [details] did not pass efl-wk2-ews (efl-wk2):
> Output: http://webkit-queues.appspot.com/results/1861597

That's easy to fix.  Just remove the variable.
Comment 5 EFL EWS Bot 2013-09-14 21:24:07 PDT
Comment on attachment 211681 [details]
the patch

Attachment 211681 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/1817575
Comment 6 Geoffrey Garen 2013-09-14 21:51:25 PDT
Comment on attachment 211681 [details]
the patch

r=me
Comment 7 Early Warning System Bot 2013-09-15 02:16:48 PDT
Comment on attachment 211681 [details]
the patch

Attachment 211681 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/1878611
Comment 8 Early Warning System Bot 2013-09-15 02:18:42 PDT
Comment on attachment 211681 [details]
the patch

Attachment 211681 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/1817643
Comment 9 Filip Pizlo 2013-09-15 11:02:23 PDT
Created attachment 211715 [details]
patch for landing

Just checking if I fixed the build.
Comment 10 Filip Pizlo 2013-09-15 11:54:39 PDT
Landed in http://trac.webkit.org/changeset/155820