- Revision
- 221018
- Author
- [email protected]
- Date
- 2017-08-22 09:28:13 -0700 (Tue, 22 Aug 2017)
Log Message
We are using valueProfileForBytecodeOffset when there may not be a value profile
https://bugs.webkit.org/show_bug.cgi?id=175812
Reviewed by Michael Saboff.
This patch uses the type system to aid the code around CodeBlock's ValueProfile
accessor methods. valueProfileForBytecodeOffset used to return ValueProfile*,
so there were callers of this that thought it could return nullptr when there
was no such ValueProfile. This was not the case, it always returned a non-null
pointer. This patch changes valueProfileForBytecodeOffset to return ValueProfile&
and adds a new tryGetValueProfileForBytecodeOffset method that returns ValueProfile*
and does the right thing if there is no such ValueProfile.
This patch also changes the other ValueProfile accessors on CodeBlock to
return ValueProfile& instead of ValueProfile*. Some callers handled the null
case unnecessarily, and using the type system to specify the result can't be
null removes these useless branches.
* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::updateAllPredictionsAndCountLiveness):
(JSC::CodeBlock::dumpValueProfiles):
(JSC::CodeBlock::tryGetValueProfileForBytecodeOffset):
(JSC::CodeBlock::valueProfileForBytecodeOffset):
(JSC::CodeBlock::validate):
* bytecode/CodeBlock.h:
(JSC::CodeBlock::valueProfileForArgument):
(JSC::CodeBlock::valueProfile):
(JSC::CodeBlock::valueProfilePredictionForBytecodeOffset):
(JSC::CodeBlock::getFromAllValueProfiles):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::handleInlining):
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::methodOfGettingAValueProfileFor):
* dfg/DFGPredictionInjectionPhase.cpp:
(JSC::DFG::PredictionInjectionPhase::run):
* jit/JIT.h:
* jit/JITInlines.h:
(JSC::JIT::emitValueProfilingSite):
* profiler/ProfilerBytecodeSequence.cpp:
(JSC::Profiler::BytecodeSequence::BytecodeSequence):
* tools/HeapVerifier.cpp:
(JSC::HeapVerifier::validateJSCell):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (221017 => 221018)
--- trunk/Source/_javascript_Core/ChangeLog 2017-08-22 16:22:22 UTC (rev 221017)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-08-22 16:28:13 UTC (rev 221018)
@@ -1,3 +1,48 @@
+2017-08-22 Saam Barati <[email protected]>
+
+ We are using valueProfileForBytecodeOffset when there may not be a value profile
+ https://bugs.webkit.org/show_bug.cgi?id=175812
+
+ Reviewed by Michael Saboff.
+
+ This patch uses the type system to aid the code around CodeBlock's ValueProfile
+ accessor methods. valueProfileForBytecodeOffset used to return ValueProfile*,
+ so there were callers of this that thought it could return nullptr when there
+ was no such ValueProfile. This was not the case, it always returned a non-null
+ pointer. This patch changes valueProfileForBytecodeOffset to return ValueProfile&
+ and adds a new tryGetValueProfileForBytecodeOffset method that returns ValueProfile*
+ and does the right thing if there is no such ValueProfile.
+
+ This patch also changes the other ValueProfile accessors on CodeBlock to
+ return ValueProfile& instead of ValueProfile*. Some callers handled the null
+ case unnecessarily, and using the type system to specify the result can't be
+ null removes these useless branches.
+
+ * bytecode/CodeBlock.cpp:
+ (JSC::CodeBlock::updateAllPredictionsAndCountLiveness):
+ (JSC::CodeBlock::dumpValueProfiles):
+ (JSC::CodeBlock::tryGetValueProfileForBytecodeOffset):
+ (JSC::CodeBlock::valueProfileForBytecodeOffset):
+ (JSC::CodeBlock::validate):
+ * bytecode/CodeBlock.h:
+ (JSC::CodeBlock::valueProfileForArgument):
+ (JSC::CodeBlock::valueProfile):
+ (JSC::CodeBlock::valueProfilePredictionForBytecodeOffset):
+ (JSC::CodeBlock::getFromAllValueProfiles):
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::handleInlining):
+ * dfg/DFGGraph.cpp:
+ (JSC::DFG::Graph::methodOfGettingAValueProfileFor):
+ * dfg/DFGPredictionInjectionPhase.cpp:
+ (JSC::DFG::PredictionInjectionPhase::run):
+ * jit/JIT.h:
+ * jit/JITInlines.h:
+ (JSC::JIT::emitValueProfilingSite):
+ * profiler/ProfilerBytecodeSequence.cpp:
+ (JSC::Profiler::BytecodeSequence::BytecodeSequence):
+ * tools/HeapVerifier.cpp:
+ (JSC::HeapVerifier::validateJSCell):
+
2017-08-22 Keith Miller <[email protected]>
Unreviewed, fix windows build... maybe.
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (221017 => 221018)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2017-08-22 16:22:22 UTC (rev 221017)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2017-08-22 16:28:13 UTC (rev 221018)
@@ -2490,18 +2490,18 @@
numberOfLiveNonArgumentValueProfiles = 0;
numberOfSamplesInProfiles = 0; // If this divided by ValueProfile::numberOfBuckets equals numberOfValueProfiles() then value profiles are full.
for (unsigned i = 0; i < totalNumberOfValueProfiles(); ++i) {
- ValueProfile* profile = ""
- unsigned numSamples = profile->totalNumberOfSamples();
+ ValueProfile& profile = ""
+ unsigned numSamples = profile.totalNumberOfSamples();
if (numSamples > ValueProfile::numberOfBuckets)
numSamples = ValueProfile::numberOfBuckets; // We don't want profiles that are extremely hot to be given more weight.
numberOfSamplesInProfiles += numSamples;
- if (profile->m_bytecodeOffset < 0) {
- profile->computeUpdatedPrediction(locker);
+ if (profile.m_bytecodeOffset < 0) {
+ profile.computeUpdatedPrediction(locker);
continue;
}
- if (profile->numberOfSamples() || profile->m_prediction != SpecNone)
+ if (profile.numberOfSamples() || profile.m_prediction != SpecNone)
numberOfLiveNonArgumentValueProfiles++;
- profile->computeUpdatedPrediction(locker);
+ profile.computeUpdatedPrediction(locker);
}
#if ENABLE(DFG_JIT)
@@ -2609,17 +2609,17 @@
{
dataLog("ValueProfile for ", *this, ":\n");
for (unsigned i = 0; i < totalNumberOfValueProfiles(); ++i) {
- ValueProfile* profile = ""
- if (profile->m_bytecodeOffset < 0) {
- ASSERT(profile->m_bytecodeOffset == -1);
+ ValueProfile& profile = ""
+ if (profile.m_bytecodeOffset < 0) {
+ ASSERT(profile.m_bytecodeOffset == -1);
dataLogF(" arg = %u: ", i);
} else
- dataLogF(" bc = %d: ", profile->m_bytecodeOffset);
- if (!profile->numberOfSamples() && profile->m_prediction == SpecNone) {
+ dataLogF(" bc = %d: ", profile.m_bytecodeOffset);
+ if (!profile.numberOfSamples() && profile.m_prediction == SpecNone) {
dataLogF("<empty>\n");
continue;
}
- profile->dump(WTF::dataFile());
+ profile.dump(WTF::dataFile());
dataLogF("\n");
}
dataLog("RareCaseProfile for ", *this, ":\n");
@@ -2739,11 +2739,19 @@
return "";
}
-ValueProfile* CodeBlock::valueProfileForBytecodeOffset(int bytecodeOffset)
+ValueProfile* CodeBlock::tryGetValueProfileForBytecodeOffset(int bytecodeOffset)
{
+ return tryBinarySearch<ValueProfile, int>(
+ m_valueProfiles, m_valueProfiles.size(), bytecodeOffset,
+ getValueProfileBytecodeOffset<ValueProfile>);
+}
+
+ValueProfile& CodeBlock::valueProfileForBytecodeOffset(int bytecodeOffset)
+{
OpcodeID opcodeID = Interpreter::getOpcodeID(instructions()[bytecodeOffset]);
unsigned length = opcodeLength(opcodeID);
- return instructions()[bytecodeOffset + length - 1].u.profile;
+ ASSERT(!!tryGetValueProfileForBytecodeOffset(bytecodeOffset));
+ return *instructions()[bytecodeOffset + length - 1].u.profile;
}
void CodeBlock::validate()
@@ -2770,6 +2778,14 @@
endValidationDidFail();
}
}
+
+ for (unsigned i = 0; i + 1 < numberOfValueProfiles(); ++i) {
+ if (valueProfile(i).m_bytecodeOffset > valueProfile(i + 1).m_bytecodeOffset) {
+ beginValidationDidFail();
+ dataLog(" Value profiles are not sorted.\n");
+ endValidationDidFail();
+ }
+ }
}
void CodeBlock::beginValidationDidFail()
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.h (221017 => 221018)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2017-08-22 16:22:22 UTC (rev 221017)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2017-08-22 16:28:13 UTC (rev 221018)
@@ -401,19 +401,20 @@
ASSERT(m_argumentValueProfiles.size() == static_cast<unsigned>(m_numParameters));
return m_argumentValueProfiles.size();
}
- ValueProfile* valueProfileForArgument(unsigned argumentIndex)
+ ValueProfile& valueProfileForArgument(unsigned argumentIndex)
{
- ValueProfile* result = &m_argumentValueProfiles[argumentIndex];
- ASSERT(result->m_bytecodeOffset == -1);
+ ValueProfile& result = m_argumentValueProfiles[argumentIndex];
+ ASSERT(result.m_bytecodeOffset == -1);
return result;
}
unsigned numberOfValueProfiles() { return m_valueProfiles.size(); }
- ValueProfile* valueProfile(int index) { return &m_valueProfiles[index]; }
- ValueProfile* valueProfileForBytecodeOffset(int bytecodeOffset);
+ ValueProfile& valueProfile(int index) { return m_valueProfiles[index]; }
+ ValueProfile& valueProfileForBytecodeOffset(int bytecodeOffset);
+ ValueProfile* tryGetValueProfileForBytecodeOffset(int bytecodeOffset);
SpeculatedType valueProfilePredictionForBytecodeOffset(const ConcurrentJSLocker& locker, int bytecodeOffset)
{
- if (ValueProfile* valueProfile = valueProfileForBytecodeOffset(bytecodeOffset))
+ if (ValueProfile* valueProfile = tryGetValueProfileForBytecodeOffset(bytecodeOffset))
return valueProfile->computeUpdatedPrediction(locker);
return SpecNone;
}
@@ -422,7 +423,7 @@
{
return numberOfArgumentValueProfiles() + numberOfValueProfiles();
}
- ValueProfile* getFromAllValueProfiles(unsigned index)
+ ValueProfile& getFromAllValueProfiles(unsigned index)
{
if (index < numberOfArgumentValueProfiles())
return valueProfileForArgument(index);
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (221017 => 221018)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2017-08-22 16:22:22 UTC (rev 221017)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2017-08-22 16:28:13 UTC (rev 221018)
@@ -1908,8 +1908,8 @@
// calls.
if (codeBlock && argument < static_cast<unsigned>(codeBlock->numParameters())) {
ConcurrentJSLocker locker(codeBlock->m_lock);
- if (ValueProfile* profile = ""
- variable->predict(profile->computeUpdatedPrediction(locker));
+ ValueProfile& profile = ""
+ variable->predict(profile.computeUpdatedPrediction(locker));
}
Node* setArgument = addToGraph(SetArgument, OpInfo(variable));
Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.cpp (221017 => 221018)
--- trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2017-08-22 16:22:22 UTC (rev 221017)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2017-08-22 16:28:13 UTC (rev 221018)
@@ -1532,7 +1532,7 @@
return nullptr;
if (node->variableAccessData() != argumentNode->variableAccessData())
return nullptr;
- return profiledBlock->valueProfileForArgument(argument);
+ return &profiledBlock->valueProfileForArgument(argument);
}();
if (result)
return result;
@@ -1546,7 +1546,7 @@
}
if (node->hasHeapPrediction())
- return profiledBlock->valueProfileForBytecodeOffset(node->origin.semantic.bytecodeIndex);
+ return &profiledBlock->valueProfileForBytecodeOffset(node->origin.semantic.bytecodeIndex);
if (profiledBlock->hasBaselineJITProfiling()) {
if (ArithProfile* result = profiledBlock->arithProfileForBytecodeOffset(node->origin.semantic.bytecodeIndex))
Modified: trunk/Source/_javascript_Core/dfg/DFGPredictionInjectionPhase.cpp (221017 => 221018)
--- trunk/Source/_javascript_Core/dfg/DFGPredictionInjectionPhase.cpp 2017-08-22 16:22:22 UTC (rev 221017)
+++ trunk/Source/_javascript_Core/dfg/DFGPredictionInjectionPhase.cpp 2017-08-22 16:28:13 UTC (rev 221018)
@@ -52,12 +52,9 @@
ConcurrentJSLocker locker(profiledBlock()->m_lock);
for (size_t arg = 0; arg < static_cast<size_t>(codeBlock()->numParameters()); ++arg) {
- ValueProfile* profile = ""
- if (!profile)
- continue;
-
+ ValueProfile& profile = ""
m_graph.m_arguments[arg]->variableAccessData()->predict(
- profile->computeUpdatedPrediction(locker));
+ profile.computeUpdatedPrediction(locker));
}
}
Modified: trunk/Source/_javascript_Core/jit/JIT.h (221017 => 221018)
--- trunk/Source/_javascript_Core/jit/JIT.h 2017-08-22 16:22:22 UTC (rev 221017)
+++ trunk/Source/_javascript_Core/jit/JIT.h 2017-08-22 16:28:13 UTC (rev 221018)
@@ -334,7 +334,7 @@
// This assumes that the value to profile is in regT0 and that regT3 is available for
// scratch.
- void emitValueProfilingSite(ValueProfile*);
+ void emitValueProfilingSite(ValueProfile&);
void emitValueProfilingSite(unsigned bytecodeOffset);
void emitValueProfilingSite();
void emitArrayProfilingSiteWithCell(RegisterID cell, RegisterID indexingType, ArrayProfile*);
Modified: trunk/Source/_javascript_Core/jit/JITInlines.h (221017 => 221018)
--- trunk/Source/_javascript_Core/jit/JITInlines.h 2017-08-22 16:22:22 UTC (rev 221017)
+++ trunk/Source/_javascript_Core/jit/JITInlines.h 2017-08-22 16:28:13 UTC (rev 221018)
@@ -968,10 +968,9 @@
return m_codeBlock->isConstantRegisterIndex(src) && getConstantOperand(src).isString() && asString(getConstantOperand(src).asCell())->length() == 1;
}
-inline void JIT::emitValueProfilingSite(ValueProfile* valueProfile)
+inline void JIT::emitValueProfilingSite(ValueProfile& valueProfile)
{
ASSERT(shouldEmitProfiling());
- ASSERT(valueProfile);
const RegisterID value = regT0;
#if USE(JSVALUE32_64)
@@ -981,9 +980,9 @@
// We're in a simple configuration: only one bucket, so we can just do a direct
// store.
#if USE(JSVALUE64)
- store64(value, valueProfile->m_buckets);
+ store64(value, valueProfile.m_buckets);
#else
- EncodedValueDescriptor* descriptor = bitwise_cast<EncodedValueDescriptor*>(valueProfile->m_buckets);
+ EncodedValueDescriptor* descriptor = bitwise_cast<EncodedValueDescriptor*>(valueProfile.m_buckets);
store32(value, &descriptor->asBits.payload);
store32(valueTag, &descriptor->asBits.tag);
#endif
Modified: trunk/Source/_javascript_Core/profiler/ProfilerBytecodeSequence.cpp (221017 => 221018)
--- trunk/Source/_javascript_Core/profiler/ProfilerBytecodeSequence.cpp 2017-08-22 16:22:22 UTC (rev 221017)
+++ trunk/Source/_javascript_Core/profiler/ProfilerBytecodeSequence.cpp 2017-08-22 16:28:13 UTC (rev 221018)
@@ -41,7 +41,7 @@
for (unsigned i = 0; i < codeBlock->numberOfArgumentValueProfiles(); ++i) {
ConcurrentJSLocker locker(codeBlock->m_lock);
- CString description = codeBlock->valueProfileForArgument(i)->briefDescription(locker);
+ CString description = codeBlock->valueProfileForArgument(i).briefDescription(locker);
if (!description.length())
continue;
out.reset();
Modified: trunk/Source/_javascript_Core/tools/HeapVerifier.cpp (221017 => 221018)
--- trunk/Source/_javascript_Core/tools/HeapVerifier.cpp 2017-08-22 16:22:22 UTC (rev 221017)
+++ trunk/Source/_javascript_Core/tools/HeapVerifier.cpp 2017-08-22 16:28:13 UTC (rev 221018)
@@ -331,9 +331,9 @@
if (UNLIKELY(codeBlock)) {
bool success = true;
for (unsigned i = 0; i < codeBlock->totalNumberOfValueProfiles(); ++i) {
- ValueProfile* valueProfile = codeBlock->getFromAllValueProfiles(i);
+ ValueProfile& valueProfile = codeBlock->getFromAllValueProfiles(i);
for (unsigned i = 0; i < ValueProfile::totalNumberOfBuckets; ++i) {
- JSValue value = JSValue::decode(valueProfile->m_buckets[i]);
+ JSValue value = JSValue::decode(valueProfile.m_buckets[i]);
if (!value)
continue;
if (!value.isCell())
