[Webkit-unassigned] [Bug 69176] New: JIT OSR recompilation trigger should also reset all patched operations in the old JIT

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=69176

           Summary: JIT OSR recompilation trigger should also reset all
                    patched operations in the old JIT
           Product: WebKit
           Version: 528+ (Nightly build)
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: JavaScriptCore
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: fpizlo at apple.com


The JIT OSR logic will trigger a recompilation of DFG code if it observes that speculation failures are happening at a higher rate than we like.  This does wonders for some types of code.  For example, if a function starts out mostly dealing with integers but then starts dealing with doubles instead, we can correctly optimize both the integer case and the double case.  This occurs often in numerical code whose input is written using integer literals, but that then starts producing fractional or overflowed results using multiplies and divisions.

But we don't have the same kind of resilience in the case of heap accesses.  If a function starts out operating on a particular structure, but does so only for a short while before switching to using a different structure, then we have no way of catching this other than by assuming that the access is polymorphic.  But this is not particularly rugged, since the "is polymorphic" trigger needs to be tuned carefully to balance between optimizing mostly-monomorphic accesses and the not-using-the-same-structure-as-before case, which is nothing more than a kludge to capture both true polymorhism and phase changes in the program.  Right now it's tuned in such a way that we get a speed-up in the average, but we definitely miss opportunities to speculate more aggressively in the mostly-monomorphic case, and we definitely revert to treating accesses as if they were polymorphic when all but one of the structures in the polymorphic access are dead.

One clean way to fix this is to reset all patches (get_by_id, put_by_id, method_check, and even possibly call and construct) when we recompile due to systematic speculation failures.  This is likely wise because:

1) The recompilation trigger already involves enough work (i.e. recompiling the code block and deleting the old one) that also clearing the patch points (and then subsequently repatching them) is unlikely to have detrimental effects on performance.

2) Recompilation in code that does heap accesses or calls heavily is often triggered because of structure and function check failures.  Hence, this trigger is probably the most precise method available for informing the system that the patched structures are no longer live.

When we do this, we need to probably also do the following:

A) Make sure that any structures that the DFG speculates on are recorded so that they are not GC'd.  If recompilation happens, then the old DFG code block may stay alive because there is code on the stack still running in it.  So, we still need to keep those structures alive until that code block becomes completely dead.  Currently DFG code blocks rely on the structures that they're speculating on being pinned by the old JIT's StructureStubInfos and friends.

B) Reset all rare case statistics and maybe value profiles as well.  Previously recompilation involved the new DFG code block being optimized using the least upper bound of the predictions that were used the last time the DFG optimized the code block, and the new predictions that arise from new value profiles and new rare case statistics.  This sort of makes sense, since it reduces the likelihood that we'll have to recompile again later.  But recompilation already uses an exponential back-off strategy.  So on each recompile, we will have twice as many *new* opportunities to gather rare case statistics and value profiles, as we had on the previous compile.  Thus, forcing those new profiles to be merged with the old ones is likely to be superfluous and in the worst case, harmful to producing good speculations.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.