Changeset 244811 in webkit for trunk/Source/JavaScriptCore/bytecode/CodeBlock.cpp

Timestamp:

Apr 30, 2019, 4:37:27 PM (6 years ago)

Author:

[email protected]

Message:

CodeBlock::m_instructionCount is wrong
​https://bugs.webkit.org/show_bug.cgi?id=197304

Reviewed by Yusuke Suzuki.

What we were calling instructionCount() was wrong, as evidenced by
us using it incorrectly both in the sampling profiler and when we
dumped bytecode for a given CodeBlock. Prior to the bytecode rewrite,
instructionCount() was probably valid to do bounds checks against.
However, this is no longer the case. This patch renames what we called
instructionCount() to bytecodeCost(). It is now only used to make decisions
about inlining and tier up heuristics. I've also named options related to
this appropriately.

This patch also introduces instructionsSize(). The result of this method
is valid to do bounds checks against.

• bytecode/CodeBlock.cpp:

(JSC::CodeBlock::dumpAssumingJITType const):
(JSC::CodeBlock::CodeBlock):
(JSC::CodeBlock::finishCreation):
(JSC::CodeBlock::optimizationThresholdScalingFactor):
(JSC::CodeBlock::predictedMachineCodeSize):

• bytecode/CodeBlock.h:

(JSC::CodeBlock::instructionsSize const):
(JSC::CodeBlock::bytecodeCost const):
(JSC::CodeBlock::instructionCount const): Deleted.

• dfg/DFGByteCodeParser.cpp:

(JSC::DFG::ByteCodeParser::inliningCost):
(JSC::DFG::ByteCodeParser::getInliningBalance):

• dfg/DFGCapabilities.cpp:

(JSC::DFG::mightCompileEval):
(JSC::DFG::mightCompileProgram):
(JSC::DFG::mightCompileFunctionForCall):
(JSC::DFG::mightCompileFunctionForConstruct):
(JSC::DFG::mightInlineFunctionForCall):
(JSC::DFG::mightInlineFunctionForClosureCall):
(JSC::DFG::mightInlineFunctionForConstruct):

• dfg/DFGCapabilities.h:

(JSC::DFG::isSmallEnoughToInlineCodeInto):

• dfg/DFGDisassembler.cpp:

(JSC::DFG::Disassembler::dumpHeader):

• dfg/DFGDriver.cpp:

(JSC::DFG::compileImpl):

• dfg/DFGPlan.cpp:

(JSC::DFG::Plan::compileInThread):

• dfg/DFGTierUpCheckInjectionPhase.cpp:

(JSC::DFG::TierUpCheckInjectionPhase::run):

• ftl/FTLCapabilities.cpp:

(JSC::FTL::canCompile):

• ftl/FTLCompile.cpp:

(JSC::FTL::compile):

• ftl/FTLLink.cpp:

(JSC::FTL::link):

• jit/JIT.cpp:

(JSC::JIT::link):

• jit/JITDisassembler.cpp:

(JSC::JITDisassembler::dumpHeader):

• llint/LLIntSlowPaths.cpp:

(JSC::LLInt::shouldJIT):

• profiler/ProfilerBytecodes.cpp:

(JSC::Profiler::Bytecodes::Bytecodes):

• runtime/Options.h:
• runtime/SamplingProfiler.cpp:

(JSC::tryGetBytecodeIndex):
(JSC::SamplingProfiler::processUnverifiedStackTraces):

File:

• trunk/Source/JavaScriptCore/bytecode/CodeBlock.cpp (modified) (6 diffs)

trunk/Source/JavaScriptCore/bytecode/CodeBlock.cpp

@@ -191 +191 @@
     if (codeType() == FunctionCode)
         out.print(specializationKind());
-    out.print(", ", instructionCount());
+    out.print(", ", instructionsSize());
     if (this->jitType() == JITType::BaselineJIT && m_shouldAlwaysBeInlined)
         out.print(" (ShouldAlwaysBeInlined)");
@@ -299 +299 @@
     , m_steppingMode(SteppingModeDisabled)
     , m_numBreakpoints(0)
-    , m_instructionCount(other.m_instructionCount)
+    , m_bytecodeCost(other.m_bytecodeCost)
     , m_scopeRegister(other.m_scopeRegister)
     , m_hash(other.m_hash)
@@ -525 +525 @@
     for (const auto& instruction : instructionStream) {
         OpcodeID opcodeID = instruction->opcodeID();
-        m_instructionCount += opcodeLengths[opcodeID];
+        m_bytecodeCost += opcodeLengths[opcodeID];
         switch (opcodeID) {
         LINK(OpHasIndexedProperty, arrayProfile)
@@ -2336 +2336 @@
     const double d = 0.825914;
     
-    double instructionCount = this->instructionCount();
-    
-    ASSERT(instructionCount); // Make sure this is called only after we have an instruction stream; otherwise it'll just return the value of d, which makes no sense.
-    
-    double result = d + a * sqrt(instructionCount + b) + c * instructionCount;
+    double bytecodeCost = this->bytecodeCost();
+    
+    ASSERT(bytecodeCost); // Make sure this is called only after we have an instruction stream; otherwise it'll just return the value of d, which makes no sense.
+    
+    double result = d + a * sqrt(bytecodeCost + b) + c * bytecodeCost;
     
     result *= codeTypeThresholdMultiplier();
@@ -2346 +2346 @@
     if (Options::verboseOSR()) {
         dataLog(
-            *this, ": instruction count is ", instructionCount,
+            *this, ": bytecode cost is ", bytecodeCost,
             ", scaling execution counter by ", result, " * ", codeTypeThresholdMultiplier(),
             "\n");
@@ -2871 +2871 @@
         return 0;
     
-    double doubleResult = multiplier * instructionCount();
+    double doubleResult = multiplier * bytecodeCost();
     
     // Be even more paranoid: silently reject values that won't fit into a size_t. If