Title: [191003] trunk/Source/_javascript_Core
Revision
191003
Author
[email protected]
Date
2015-10-13 13:08:54 -0700 (Tue, 13 Oct 2015)

Log Message

CodeBlock write barriers should be precise
https://bugs.webkit.org/show_bug.cgi?id=150042

Reviewed by Saam Barati.

CodeBlock performs lots of unnecessary write barriers. This wastes
performance and makes the code a bit harder to follow, and it might mask
important bugs. Now is a good time to unmask important bugs.

* bytecode/CodeBlock.h:
(JSC::CodeBlockSet::mark): Don't write barrier all CodeBlocks on the
stack. Only CodeBlocks that do value profiling need write barriers, and
they do those themselves.

In steady state, when most of our CodeBlocks are old and FTL-compiled,
and we're doing eden GC's, we should almost never visit a CodeBlock.

* dfg/DFGOSRExitCompilerCommon.cpp:
(JSC::DFG::osrWriteBarrier):
(JSC::DFG::adjustAndJumpToTarget): Don't write barrier all inlined
CodeBlocks on exit. That's not necessary. Instead, write barrier the 
CodeBlock(s) we will exit to, along with the one we will write a value
profile to.

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (191002 => 191003)


--- trunk/Source/_javascript_Core/ChangeLog	2015-10-13 19:15:55 UTC (rev 191002)
+++ trunk/Source/_javascript_Core/ChangeLog	2015-10-13 20:08:54 UTC (rev 191003)
@@ -1,3 +1,29 @@
+2015-10-12  Geoffrey Garen  <[email protected]>
+
+        CodeBlock write barriers should be precise
+        https://bugs.webkit.org/show_bug.cgi?id=150042
+
+        Reviewed by Saam Barati.
+
+        CodeBlock performs lots of unnecessary write barriers. This wastes
+        performance and makes the code a bit harder to follow, and it might mask
+        important bugs. Now is a good time to unmask important bugs.
+
+        * bytecode/CodeBlock.h:
+        (JSC::CodeBlockSet::mark): Don't write barrier all CodeBlocks on the
+        stack. Only CodeBlocks that do value profiling need write barriers, and
+        they do those themselves.
+
+        In steady state, when most of our CodeBlocks are old and FTL-compiled,
+        and we're doing eden GC's, we should almost never visit a CodeBlock.
+
+        * dfg/DFGOSRExitCompilerCommon.cpp:
+        (JSC::DFG::osrWriteBarrier):
+        (JSC::DFG::adjustAndJumpToTarget): Don't write barrier all inlined
+        CodeBlocks on exit. That's not necessary. Instead, write barrier the 
+        CodeBlock(s) we will exit to, along with the one we will write a value
+        profile to.
+
 2015-10-13  Yusuke Suzuki  <[email protected]>
 
         REGRESSION: ASSERT (impl->isAtomic()) @ facebook.com

Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.h (191002 => 191003)


--- trunk/Source/_javascript_Core/bytecode/CodeBlock.h	2015-10-13 19:15:55 UTC (rev 191002)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.h	2015-10-13 20:08:54 UTC (rev 191003)
@@ -1376,11 +1376,6 @@
     if (!codeBlock)
         return;
 
-    // Try to recover gracefully if we forget to execute a barrier for a
-    // CodeBlock that does value profiling. This is probably overkill, but we
-    // have always done it.
-    Heap::heap(codeBlock)->writeBarrier(codeBlock);
-
     m_currentlyExecuting.add(codeBlock);
 }
 

Modified: trunk/Source/_javascript_Core/dfg/DFGOSRExitCompilerCommon.cpp (191002 => 191003)


--- trunk/Source/_javascript_Core/dfg/DFGOSRExitCompilerCommon.cpp	2015-10-13 19:15:55 UTC (rev 191002)
+++ trunk/Source/_javascript_Core/dfg/DFGOSRExitCompilerCommon.cpp	2015-10-13 20:08:54 UTC (rev 191003)
@@ -266,30 +266,31 @@
     ownerIsRememberedOrInEden.link(&jit);
 }
 
-void adjustAndJumpToTarget(CCallHelpers& jit, const OSRExitBase& exit, bool isExitingToOpCatch)
+static void osrWriteBarrier(CCallHelpers& jit, const OSRExitBase& exit)
 {
-    jit.move(
-        AssemblyHelpers::TrustedImmPtr(
-            jit.codeBlock()->baselineAlternative()), GPRInfo::argumentGPR1);
-    osrWriteBarrier(jit, GPRInfo::argumentGPR1, GPRInfo::nonArgGPR0);
+    HashSet<CodeBlock*> codeBlocksToWriteBarrier;
 
-    // We barrier all inlined frames -- and not just the current inline stack --
-    // because we don't know which inlined function owns the value profile that
-    // we'll update when we exit. In the case of "f() { a(); b(); }", if both
-    // a and b are inlined, we might exit inside b due to a bad value loaded
-    // from a.
-    // FIXME: MethodOfGettingAValueProfile should remember which CodeBlock owns
-    // the value profile.
-    InlineCallFrameSet* inlineCallFrames = jit.codeBlock()->jitCode()->dfgCommon()->inlineCallFrames.get();
-    if (inlineCallFrames) {
-        for (InlineCallFrame* inlineCallFrame : *inlineCallFrames) {
-            jit.move(
-                AssemblyHelpers::TrustedImmPtr(
-                    inlineCallFrame->baselineCodeBlock.get()), GPRInfo::argumentGPR1);
-            osrWriteBarrier(jit, GPRInfo::argumentGPR1, GPRInfo::nonArgGPR0);
-        }
+    // Note that the value profiling CodeBlock and the baseline CodeBlock might
+    // not be equal. In "f() { a(); b(); }", if both 'a' and 'b' are inlined,
+    // we might exit to 'b' due to a bad value loaded from 'a'.
+    codeBlocksToWriteBarrier.add(jit.baselineCodeBlockFor(exit.m_codeOriginForExitProfile));
+
+    codeBlocksToWriteBarrier.add(jit.codeBlock()->baselineAlternative());
+
+    for (InlineCallFrame* inlineCallFrame = exit.m_codeOrigin.inlineCallFrame; inlineCallFrame; inlineCallFrame = inlineCallFrame->directCaller.inlineCallFrame)
+        codeBlocksToWriteBarrier.add(inlineCallFrame->baselineCodeBlock.get());
+
+    for (CodeBlock* codeBlock : codeBlocksToWriteBarrier) {
+        jit.move(
+            AssemblyHelpers::TrustedImmPtr(codeBlock), GPRInfo::argumentGPR1);
+        osrWriteBarrier(jit, GPRInfo::argumentGPR1, GPRInfo::nonArgGPR0);
     }
+}
 
+void adjustAndJumpToTarget(CCallHelpers& jit, const OSRExitBase& exit, bool isExitingToOpCatch)
+{
+    osrWriteBarrier(jit, exit);
+
     if (exit.m_codeOrigin.inlineCallFrame)
         jit.addPtr(AssemblyHelpers::TrustedImm32(exit.m_codeOrigin.inlineCallFrame->stackOffset * sizeof(EncodedJSValue)), GPRInfo::callFrameRegister);
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to