Changeset 185161 in webkit for trunk/Source/JavaScriptCore/bytecode/CallLinkStatus.cpp

Timestamp:

Jun 3, 2015, 1:08:01 PM (10 years ago)

Author:

[email protected]

Message:

CallLinkStatus should return takesSlowPath if the GC often cleared the IC
​https://bugs.webkit.org/show_bug.cgi?id=145502

Reviewed by Geoffrey Garen.

CallLinkInfo now remembers when it has been cleared by GC. This has some safeguards for when
a call gets cleared by GC only because we hadn't converted it into a closure call; in that
case the GC will just tell us that it should be a closure call. The DFG will not optimize
a call that was cleared by GC, and the DFG will always prefer a closure call if the GC told
us that the specific callee was dead but the executable wasn't.

This guards us from some scenarios that came up in Speedometer. It's neutral on the pure JS
benchmarks, most likely just because those benchmarks aren't real enough to have interesting
GC of code.

• bytecode/CallLinkInfo.cpp:

(JSC::CallLinkInfo::visitWeak):
(JSC::CallLinkInfo::dummy):

• bytecode/CallLinkInfo.h:

(JSC::CallLinkInfo::CallLinkInfo):

• bytecode/CallLinkStatus.cpp:

(JSC::CallLinkStatus::computeFromCallLinkInfo):

File:

• trunk/Source/JavaScriptCore/bytecode/CallLinkStatus.cpp (modified) (2 diffs)

trunk/Source/JavaScriptCore/bytecode/CallLinkStatus.cpp

@@ -136 +136 @@
     const ConcurrentJITLocker&, CallLinkInfo& callLinkInfo)
 {
+    if (callLinkInfo.clearedByGC)
+        return takesSlowPath();
+    
     // Note that despite requiring that the locker is held, this code is racy with respect
     // to the CallLinkInfo: it may get cleared while this code runs! This is because
@@ -148 +151 @@
     // that is still marginally valid (i.e. the pointers ain't stale). This kind of raciness
     // is probably OK for now.
-    
-    // FIXME: If the GC often clears this call, we should probably treat it like it always takes the
-    // slow path. We could be smart about this; for example if we cleared a specific callee but the
-    // despecified executable was alive then we could note that separately.
-    // https://bugs.webkit.org/show_bug.cgi?id=145502
     
     // PolymorphicCallStubRoutine is a GCAwareJITStubRoutine, so if non-null, it will stay alive