ValueProfile::computeUpdatedPrediction doesn't merge statistics correctly
https://bugs.webkit.org/show_bug.cgi?id=69906
Reviewed by Gavin Barraclough.
It turns out that the simplest fix is to switch computeUpdatedPredictions()
to using predictionFromValue() combined with mergePrediction(). Doing so
allowed me to kill off weakBuckets and visitWeakReferences(). Hence this
not only fixes a performance bug but kills off a lot of code that I never
liked to begin with.
This appears to be a 1% win on V8.
* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::visitAggregate):
* bytecode/CodeBlock.h:
* bytecode/PredictedType.cpp:
(JSC::predictionFromValue):
* bytecode/ValueProfile.cpp:
(JSC::ValueProfile::computeStatistics):
(JSC::ValueProfile::computeUpdatedPrediction):
* bytecode/ValueProfile.h:
(JSC::ValueProfile::classInfo):
(JSC::ValueProfile::numberOfSamples):
(JSC::ValueProfile::isLive):
(JSC::ValueProfile::dump):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@97294 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.cpp b/Source/JavaScriptCore/bytecode/CodeBlock.cpp
index 214430d..05505f1 100644
--- a/Source/JavaScriptCore/bytecode/CodeBlock.cpp
+++ b/Source/JavaScriptCore/bytecode/CodeBlock.cpp
@@ -1527,8 +1527,6 @@
void CodeBlock::visitAggregate(SlotVisitor& visitor)
{
- bool handleWeakReferences = false;
-
if (!!m_alternative)
m_alternative->visitAggregate(visitor);
visitor.append(&m_globalObject);
@@ -1580,61 +1578,8 @@
#endif
#if ENABLE(VALUE_PROFILER)
- for (unsigned profileIndex = 0; profileIndex < numberOfValueProfiles(); ++profileIndex) {
- ValueProfile* profile = valueProfile(profileIndex);
-
- for (unsigned index = 0; index < ValueProfile::numberOfBuckets; ++index) {
- if (!profile->m_buckets[index]) {
- if (!!profile->m_weakBuckets[index])
- handleWeakReferences = true;
- continue;
- }
-
- if (!JSValue::decode(profile->m_buckets[index]).isCell()) {
- profile->m_weakBuckets[index] = ValueProfile::WeakBucket();
- continue;
- }
-
- handleWeakReferences = true;
- }
- }
-#endif
-
- if (handleWeakReferences)
- visitor.addWeakReferenceHarvester(this);
-}
-
-void CodeBlock::visitWeakReferences(SlotVisitor&)
-{
-#if ENABLE(VALUE_PROFILER)
- for (unsigned profileIndex = 0; profileIndex < numberOfValueProfiles(); ++profileIndex) {
- ValueProfile* profile = valueProfile(profileIndex);
-
- for (unsigned index = 0; index < ValueProfile::numberOfBuckets; ++index) {
- if (!!profile->m_buckets[index]) {
- JSValue value = JSValue::decode(profile->m_buckets[index]);
- if (!value.isCell())
- continue;
-
- JSCell* cell = value.asCell();
- if (Heap::isMarked(cell))
- continue;
-
- profile->m_buckets[index] = JSValue::encode(JSValue());
- profile->m_weakBuckets[index] = cell->structure();
- }
-
- ValueProfile::WeakBucket weak = profile->m_weakBuckets[index];
- if (!weak || weak.isClassInfo())
- continue;
-
- ASSERT(weak.isStructure());
- if (Heap::isMarked(weak.asStructure()))
- continue;
-
- profile->m_weakBuckets[index] = weak.asStructure()->classInfo();
- }
- }
+ for (unsigned profileIndex = 0; profileIndex < numberOfValueProfiles(); ++profileIndex)
+ valueProfile(profileIndex)->computeUpdatedPrediction();
#endif
}
diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.h b/Source/JavaScriptCore/bytecode/CodeBlock.h
index cd9cd1a..52793ad 100644
--- a/Source/JavaScriptCore/bytecode/CodeBlock.h
+++ b/Source/JavaScriptCore/bytecode/CodeBlock.h
@@ -43,7 +43,6 @@
#include "PredictionTracker.h"
#include "RegExpObject.h"
#include "UString.h"
-#include "WeakReferenceHarvester.h"
#include "ValueProfile.h"
#include <wtf/FastAllocBase.h>
#include <wtf/PassOwnPtr.h>
@@ -232,7 +231,7 @@
}
#endif
- class CodeBlock: public WeakReferenceHarvester {
+ class CodeBlock {
WTF_MAKE_FAST_ALLOCATED;
friend class JIT;
protected:
@@ -248,7 +247,6 @@
PassOwnPtr<CodeBlock> releaseAlternative() { return m_alternative.release(); }
void visitAggregate(SlotVisitor&);
- void visitWeakReferences(SlotVisitor&);
static void dumpStatistics();
diff --git a/Source/JavaScriptCore/bytecode/PredictedType.cpp b/Source/JavaScriptCore/bytecode/PredictedType.cpp
index ed3d1e9..79d1aa3 100644
--- a/Source/JavaScriptCore/bytecode/PredictedType.cpp
+++ b/Source/JavaScriptCore/bytecode/PredictedType.cpp
@@ -137,6 +137,7 @@
return predictionFromCell(value.asCell());
if (value.isBoolean())
return PredictBoolean;
+ ASSERT(value.isUndefinedOrNull());
return PredictOther;
}
diff --git a/Source/JavaScriptCore/bytecode/ValueProfile.cpp b/Source/JavaScriptCore/bytecode/ValueProfile.cpp
index 3bd7b83..5527d2d 100644
--- a/Source/JavaScriptCore/bytecode/ValueProfile.cpp
+++ b/Source/JavaScriptCore/bytecode/ValueProfile.cpp
@@ -61,15 +61,8 @@
{
for (unsigned i = 0; i < numberOfBuckets; ++i) {
JSValue value = JSValue::decode(m_buckets[i]);
- if (!value) {
- WeakBucket weakBucket = m_weakBuckets[i];
- if (!!weakBucket) {
- statistics.samples++;
- computeStatistics(weakBucket.getClassInfo(), statistics);
- }
-
+ if (!value)
continue;
- }
statistics.samples++;
@@ -86,40 +79,17 @@
PredictedType ValueProfile::computeUpdatedPrediction()
{
- ValueProfile::Statistics statistics;
- computeStatistics(statistics);
-
- PredictedType prediction;
-
- if (!statistics.samples)
- prediction = PredictNone;
- else if (statistics.int32s == statistics.samples)
- prediction = PredictInt32;
- else if (statistics.doubles == statistics.samples)
- prediction = PredictDouble;
- else if (statistics.int32s + statistics.doubles == statistics.samples)
- prediction = PredictNumber;
- else if (statistics.arrays == statistics.samples)
- prediction = PredictArray;
- else if (statistics.finalObjects == statistics.samples)
- prediction = PredictFinalObject;
- else if (statistics.strings == statistics.samples)
- prediction = PredictString;
- else if (statistics.objects == statistics.samples)
- prediction = PredictObjectOther;
- else if (statistics.cells == statistics.samples)
- prediction = PredictCellOther;
- else if (statistics.booleans == statistics.samples)
- prediction = PredictBoolean;
- else
- prediction = PredictOther;
-
- m_numberOfSamplesInPrediction += statistics.samples;
- mergePrediction(m_prediction, prediction);
for (unsigned i = 0; i < numberOfBuckets; ++i) {
+ JSValue value = JSValue::decode(m_buckets[i]);
+ if (!value)
+ continue;
+
+ m_numberOfSamplesInPrediction++;
+ mergePrediction(m_prediction, predictionFromValue(value));
+
m_buckets[i] = JSValue::encode(JSValue());
- m_weakBuckets[i] = WeakBucket();
}
+
return m_prediction;
}
#endif // ENABLE(VALUE_PROFILER)
diff --git a/Source/JavaScriptCore/bytecode/ValueProfile.h b/Source/JavaScriptCore/bytecode/ValueProfile.h
index a1e85ee..58a7fd7 100644
--- a/Source/JavaScriptCore/bytecode/ValueProfile.h
+++ b/Source/JavaScriptCore/bytecode/ValueProfile.h
@@ -61,14 +61,14 @@
return 0;
return value.asCell()->structure()->classInfo();
}
- return m_weakBuckets[bucket].getClassInfo();
+ return 0;
}
unsigned numberOfSamples() const
{
unsigned result = 0;
for (unsigned i = 0; i < numberOfBuckets; ++i) {
- if (!!JSValue::decode(m_buckets[i]) || !!m_weakBuckets[i])
+ if (!!JSValue::decode(m_buckets[i]))
result++;
}
return result;
@@ -82,7 +82,7 @@
bool isLive() const
{
for (unsigned i = 0; i < numberOfBuckets; ++i) {
- if (!!JSValue::decode(m_buckets[i]) || !!m_weakBuckets[i])
+ if (!!JSValue::decode(m_buckets[i]))
return true;
}
return false;
@@ -239,19 +239,14 @@
bool first = true;
for (unsigned i = 0; i < numberOfBuckets; ++i) {
JSValue value = JSValue::decode(m_buckets[i]);
- if (!!value || !!m_weakBuckets[i]) {
+ if (!!value) {
if (first) {
fprintf(out, ": ");
first = false;
} else
fprintf(out, ", ");
- }
-
- if (!!value)
fprintf(out, "%s", value.description());
-
- if (!!m_weakBuckets[i])
- fprintf(out, "DeadCell");
+ }
}
}
#endif
@@ -290,70 +285,6 @@
unsigned m_numberOfSamplesInPrediction;
EncodedJSValue m_buckets[numberOfBuckets];
-
- class WeakBucket {
- public:
- WeakBucket()
- : m_value(0)
- {
- }
-
- WeakBucket(Structure* structure)
- : m_value(reinterpret_cast<uintptr_t>(structure))
- {
- }
-
- WeakBucket(const ClassInfo* classInfo)
- : m_value(reinterpret_cast<uintptr_t>(classInfo) | 1)
- {
- }
-
- bool operator!() const
- {
- return !m_value;
- }
-
- bool isEmpty() const
- {
- return !m_value;
- }
-
- bool isClassInfo() const
- {
- return !!(m_value & 1);
- }
-
- bool isStructure() const
- {
- return !isEmpty() && !isClassInfo();
- }
-
- Structure* asStructure() const
- {
- ASSERT(isStructure());
- return reinterpret_cast<Structure*>(m_value);
- }
-
- const ClassInfo* asClassInfo() const
- {
- ASSERT(isClassInfo());
- return reinterpret_cast<ClassInfo*>(m_value & ~static_cast<uintptr_t>(1));
- }
-
- const ClassInfo* getClassInfo() const
- {
- if (isEmpty())
- return 0;
- if (isClassInfo())
- return asClassInfo();
- return asStructure()->classInfo();
- }
-
- private:
- uintptr_t m_value;
- };
-
- WeakBucket m_weakBuckets[numberOfBuckets]; // this is not covered by a write barrier because it is only set from GC
};
inline int getValueProfileBytecodeOffset(ValueProfile* valueProfile)
