- Revision
- 200405
- Author
- [email protected]
- Date
- 2016-05-03 19:23:28 -0700 (Tue, 03 May 2016)
Log Message
References from code to Structures should be stronger than weak
https://bugs.webkit.org/show_bug.cgi?id=157324
Reviewed by Mark Lam.
If code refers to a Structure and the Structure dies, then previously we'd kill the code.
This makes sense because the Structure could be the only thing left referring to some global
object or prototype.
But this also causes unnecessary churn. Sometimes there will be a structure that we just
haven't really done anything with recently and so it appears dead. The approach we use
elsewhere in our type inference is that the type that the code uses is general enough to
handle every past execution. Having the GC clear code when some Structure it uses dies means
that we forget that the code used that Structure. We'll either assume that the code is more
monomorphic than it really is (because after GC we patch in some other structure but not the
deleted one, so it looks like we only ever saw the new structure), or we'll assume that it's
crazier than it really is (because we'll remember that there had been some structure that
caused deletion, so we'll assume that deletions might happen in the future, so we'll use a
fully dynamic IC).
This change introduces a more nuanced policy: if it's cheap to mark a dead Structure then we
should mark it just so that all of the code that refers to it remembers that there had been
this exact Structure in the past. If the code often goes through different Structures then
we already have great mechanisms to realize that the code is nutty (namely, the
PolymorphicAccess size limit). But if the code just does this a handful of times then
remembering this old Structure is probably net good:
- It obeys the "handle all past executions" law.
- It preserves the history of the property access, allowing a precise measure of its past
polymorphism.
- It makes the code ready to run fast if the user decides to use that Structure again.
Marking the Structure means it will stay in whatever property transition tables it was in,
so if the program does the same thing it did in the past, it will get this old Structure.
It looks like this is a progression in gbemu and it makes gbemu perform more
deterministically. Also, it seems that this makes JetStream run faster.
Over five in-browser runs of JetStream, here's what we see before and after:
Geometric Mean:
Before After
229.23 +- 8.2523 230.70 +- 12.888
232.91 +- 15.638 239.04 +- 13.766
234.79 +- 12.760 236.32 +- 15.562
236.20 +- 23.125 242.02 +- 3.3865
237.22 +- 2.1929 237.23 +- 17.664
Just gbemu:
Before After
541.0 +- 135.8 481.7 +- 143.4
518.9 +- 15.65 508.1 +- 136.3
362.5 +- 0.8884 489.7 +- 101.4
470.7 +- 313.3 530.7 +- 11.49
418.7 +- 180.6 537.2 +- 6.514
Notice that there is plenty of noise before and after, but the noise is now far less severe.
After this change I did not see any runs like "470.7 +- 313.3" where the size of the
confidence interval (313.3 * 2) is greater than the score (470.7). Also, notice that the
least noisy run before the change also got a lower score than we ever observed after the
change (36.5 +- 0.8884). The noise, and these occasional very low scores, are due to a
pathology where the GC would reset some stubs at an unfortunate time during profiling,
causing the optimizing compiler to make many poor decisions. That pathology doesn't exist
anymore.
On the other hand, prior to this change it was possible for gbemu to sometimes run sooooper
fast because the GC would cause the profiler to forget gbemu's behavior on the first tick
and focus only on its behavior in subsequent ticks. So, in steady state, we'd optimize gbemu
for its later behavior rather than a combination of its early behavior and later behavior.
We rarely got lucky this way, so it's not fair to view this quirk as a feature.
* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::propagateTransitions):
* bytecode/PolymorphicAccess.cpp:
(JSC::AccessCase::visitWeak):
(JSC::AccessCase::propagateTransitions):
(JSC::AccessCase::generateWithGuard):
(JSC::PolymorphicAccess::visitWeak):
(JSC::PolymorphicAccess::propagateTransitions):
(JSC::PolymorphicAccess::dump):
* bytecode/PolymorphicAccess.h:
* bytecode/StructureStubInfo.cpp:
(JSC::StructureStubInfo::visitWeakReferences):
(JSC::StructureStubInfo::propagateTransitions):
(JSC::StructureStubInfo::containsPC):
* bytecode/StructureStubInfo.h:
(JSC::StructureStubInfo::considerCaching):
* runtime/Structure.cpp:
(JSC::Structure::visitChildren):
(JSC::Structure::isCheapDuringGC):
(JSC::Structure::markIfCheap):
(JSC::Structure::prototypeChainMayInterceptStoreTo):
* runtime/Structure.h:
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (200404 => 200405)
--- trunk/Source/_javascript_Core/ChangeLog 2016-05-04 01:54:39 UTC (rev 200404)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-05-04 02:23:28 UTC (rev 200405)
@@ -1,3 +1,98 @@
+2016-05-03 Filip Pizlo <[email protected]>
+
+ References from code to Structures should be stronger than weak
+ https://bugs.webkit.org/show_bug.cgi?id=157324
+
+ Reviewed by Mark Lam.
+
+ If code refers to a Structure and the Structure dies, then previously we'd kill the code.
+ This makes sense because the Structure could be the only thing left referring to some global
+ object or prototype.
+
+ But this also causes unnecessary churn. Sometimes there will be a structure that we just
+ haven't really done anything with recently and so it appears dead. The approach we use
+ elsewhere in our type inference is that the type that the code uses is general enough to
+ handle every past execution. Having the GC clear code when some Structure it uses dies means
+ that we forget that the code used that Structure. We'll either assume that the code is more
+ monomorphic than it really is (because after GC we patch in some other structure but not the
+ deleted one, so it looks like we only ever saw the new structure), or we'll assume that it's
+ crazier than it really is (because we'll remember that there had been some structure that
+ caused deletion, so we'll assume that deletions might happen in the future, so we'll use a
+ fully dynamic IC).
+
+ This change introduces a more nuanced policy: if it's cheap to mark a dead Structure then we
+ should mark it just so that all of the code that refers to it remembers that there had been
+ this exact Structure in the past. If the code often goes through different Structures then
+ we already have great mechanisms to realize that the code is nutty (namely, the
+ PolymorphicAccess size limit). But if the code just does this a handful of times then
+ remembering this old Structure is probably net good:
+
+ - It obeys the "handle all past executions" law.
+ - It preserves the history of the property access, allowing a precise measure of its past
+ polymorphism.
+ - It makes the code ready to run fast if the user decides to use that Structure again.
+ Marking the Structure means it will stay in whatever property transition tables it was in,
+ so if the program does the same thing it did in the past, it will get this old Structure.
+
+ It looks like this is a progression in gbemu and it makes gbemu perform more
+ deterministically. Also, it seems that this makes JetStream run faster.
+
+ Over five in-browser runs of JetStream, here's what we see before and after:
+
+ Geometric Mean:
+ Before After
+ 229.23 +- 8.2523 230.70 +- 12.888
+ 232.91 +- 15.638 239.04 +- 13.766
+ 234.79 +- 12.760 236.32 +- 15.562
+ 236.20 +- 23.125 242.02 +- 3.3865
+ 237.22 +- 2.1929 237.23 +- 17.664
+
+ Just gbemu:
+ Before After
+ 541.0 +- 135.8 481.7 +- 143.4
+ 518.9 +- 15.65 508.1 +- 136.3
+ 362.5 +- 0.8884 489.7 +- 101.4
+ 470.7 +- 313.3 530.7 +- 11.49
+ 418.7 +- 180.6 537.2 +- 6.514
+
+ Notice that there is plenty of noise before and after, but the noise is now far less severe.
+ After this change I did not see any runs like "470.7 +- 313.3" where the size of the
+ confidence interval (313.3 * 2) is greater than the score (470.7). Also, notice that the
+ least noisy run before the change also got a lower score than we ever observed after the
+ change (36.5 +- 0.8884). The noise, and these occasional very low scores, are due to a
+ pathology where the GC would reset some stubs at an unfortunate time during profiling,
+ causing the optimizing compiler to make many poor decisions. That pathology doesn't exist
+ anymore.
+
+ On the other hand, prior to this change it was possible for gbemu to sometimes run sooooper
+ fast because the GC would cause the profiler to forget gbemu's behavior on the first tick
+ and focus only on its behavior in subsequent ticks. So, in steady state, we'd optimize gbemu
+ for its later behavior rather than a combination of its early behavior and later behavior.
+ We rarely got lucky this way, so it's not fair to view this quirk as a feature.
+
+ * bytecode/CodeBlock.cpp:
+ (JSC::CodeBlock::propagateTransitions):
+ * bytecode/PolymorphicAccess.cpp:
+ (JSC::AccessCase::visitWeak):
+ (JSC::AccessCase::propagateTransitions):
+ (JSC::AccessCase::generateWithGuard):
+ (JSC::PolymorphicAccess::visitWeak):
+ (JSC::PolymorphicAccess::propagateTransitions):
+ (JSC::PolymorphicAccess::dump):
+ * bytecode/PolymorphicAccess.h:
+ * bytecode/StructureStubInfo.cpp:
+ (JSC::StructureStubInfo::visitWeakReferences):
+ (JSC::StructureStubInfo::propagateTransitions):
+ (JSC::StructureStubInfo::containsPC):
+ * bytecode/StructureStubInfo.h:
+ (JSC::StructureStubInfo::considerCaching):
+ * runtime/Structure.cpp:
+ (JSC::Structure::visitChildren):
+ (JSC::Structure::isCheapDuringGC):
+ (JSC::Structure::markIfCheap):
+ (JSC::Structure::prototypeChainMayInterceptStoreTo):
+ * runtime/Structure.h:
+
2016-05-03 Joseph Pecoraro <[email protected]>
Web Inspector: Simplify console.clear
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (200404 => 200405)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2016-05-04 01:54:39 UTC (rev 200404)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2016-05-04 02:23:28 UTC (rev 200405)
@@ -2616,32 +2616,16 @@
#if ENABLE(JIT)
if (JITCode::isJIT(jitType())) {
- for (Bag<StructureStubInfo>::iterator iter = m_stubInfos.begin(); !!iter; ++iter) {
- StructureStubInfo& stubInfo = **iter;
- if (stubInfo.cacheType != CacheType::Stub)
- continue;
- PolymorphicAccess* list = stubInfo.u.stub;
- JSCell* origin = stubInfo.codeOrigin.codeOriginOwner();
- if (origin && !Heap::isMarked(origin)) {
- allAreMarkedSoFar = false;
- continue;
- }
- for (unsigned j = list->size(); j--;) {
- const AccessCase& access = list->at(j);
- if (access.type() != AccessCase::Transition)
- continue;
- if (Heap::isMarked(access.structure()))
- visitor.appendUnbarrieredReadOnlyPointer(access.newStructure());
- else
- allAreMarkedSoFar = false;
- }
- }
+ for (Bag<StructureStubInfo>::iterator iter = m_stubInfos.begin(); !!iter; ++iter)
+ allAreMarkedSoFar &= (*iter)->propagateTransitions(visitor);
}
#endif // ENABLE(JIT)
#if ENABLE(DFG_JIT)
if (JITCode::isOptimizingJIT(jitType())) {
DFG::CommonData* dfgCommon = m_jitCode->dfgCommon();
+ for (auto& weakReference : dfgCommon->weakStructureReferences)
+ allAreMarkedSoFar &= weakReference->markIfCheap(visitor);
for (unsigned i = 0; i < dfgCommon->transitions.size(); ++i) {
if (shouldMarkTransition(dfgCommon->transitions[i])) {
Modified: trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.cpp (200404 => 200405)
--- trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.cpp 2016-05-04 01:54:39 UTC (rev 200404)
+++ trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.cpp 2016-05-04 02:23:28 UTC (rev 200405)
@@ -554,6 +554,27 @@
return true;
}
+bool AccessCase::propagateTransitions(SlotVisitor& visitor) const
+{
+ bool result = true;
+
+ if (m_structure)
+ result &= m_structure->markIfCheap(visitor);
+
+ switch (m_type) {
+ case Transition:
+ if (Heap::isMarked(m_structure->previousID()))
+ visitor.appendUnbarrieredReadOnlyPointer(m_structure.get());
+ else
+ result = false;
+ break;
+ default:
+ break;
+ }
+
+ return result;
+}
+
void AccessCase::generateWithGuard(
AccessGenerationState& state, CCallHelpers::JumpList& fallThrough)
{
@@ -1487,6 +1508,14 @@
return true;
}
+bool PolymorphicAccess::propagateTransitions(SlotVisitor& visitor) const
+{
+ bool result = true;
+ for (unsigned i = 0; i < size(); ++i)
+ result &= at(i).propagateTransitions(visitor);
+ return result;
+}
+
void PolymorphicAccess::dump(PrintStream& out) const
{
out.print(RawPointer(this), ":[");
Modified: trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.h (200404 => 200405)
--- trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.h 2016-05-04 01:54:39 UTC (rev 200404)
+++ trunk/Source/_javascript_Core/bytecode/PolymorphicAccess.h 2016-05-04 02:23:28 UTC (rev 200405)
@@ -248,6 +248,7 @@
AccessCase();
bool visitWeak(VM&) const;
+ bool propagateTransitions(SlotVisitor&) const;
// FIXME: This only exists because of how AccessCase puts post-generation things into itself.
// https://bugs.webkit.org/show_bug.cgi?id=156456
@@ -396,6 +397,10 @@
// If this returns false then we are requesting a reset of the owning StructureStubInfo.
bool visitWeak(VM&) const;
+
+ // This returns true if it has marked everything it will ever marked. This can be used as an
+ // optimization to then avoid calling this method again during the fixpoint.
+ bool propagateTransitions(SlotVisitor&) const;
void aboutToDie();
Modified: trunk/Source/_javascript_Core/bytecode/StructureStubInfo.cpp (200404 => 200405)
--- trunk/Source/_javascript_Core/bytecode/StructureStubInfo.cpp 2016-05-04 01:54:39 UTC (rev 200404)
+++ trunk/Source/_javascript_Core/bytecode/StructureStubInfo.cpp 2016-05-04 02:23:28 UTC (rev 200405)
@@ -230,7 +230,10 @@
{
VM& vm = *codeBlock->vm();
- bufferedStructures.clear();
+ bufferedStructures.genericFilter(
+ [&] (Structure* structure) -> bool {
+ return Heap::isMarked(structure);
+ });
switch (cacheType) {
case CacheType::GetByIdSelf:
@@ -250,6 +253,22 @@
resetByGC = true;
}
+bool StructureStubInfo::propagateTransitions(SlotVisitor& visitor)
+{
+ switch (cacheType) {
+ case CacheType::Unset:
+ return true;
+ case CacheType::GetByIdSelf:
+ case CacheType::PutByIdReplace:
+ return u.byIdSelf.baseObjectStructure->markIfCheap(visitor);
+ case CacheType::Stub:
+ return u.stub->propagateTransitions(visitor);
+ }
+
+ RELEASE_ASSERT_NOT_REACHED();
+ return true;
+}
+
bool StructureStubInfo::containsPC(void* pc) const
{
if (cacheType != CacheType::Stub)
Modified: trunk/Source/_javascript_Core/bytecode/StructureStubInfo.h (200404 => 200405)
--- trunk/Source/_javascript_Core/bytecode/StructureStubInfo.h 2016-05-04 01:54:39 UTC (rev 200404)
+++ trunk/Source/_javascript_Core/bytecode/StructureStubInfo.h 2016-05-04 02:23:28 UTC (rev 200405)
@@ -81,6 +81,9 @@
// Check if the stub has weak references that are dead. If it does, then it resets itself,
// either entirely or just enough to ensure that those dead pointers don't get used anymore.
void visitWeakReferences(CodeBlock*);
+
+ // This returns true if it has marked everything that it will ever mark.
+ bool propagateTransitions(SlotVisitor&);
ALWAYS_INLINE bool considerCaching(Structure* structure)
{
Modified: trunk/Source/_javascript_Core/runtime/Structure.cpp (200404 => 200405)
--- trunk/Source/_javascript_Core/runtime/Structure.cpp 2016-05-04 01:54:39 UTC (rev 200404)
+++ trunk/Source/_javascript_Core/runtime/Structure.cpp 2016-05-04 02:23:28 UTC (rev 200405)
@@ -1135,6 +1135,25 @@
visitor.append(&thisObject->m_inferredTypeTable);
}
+bool Structure::isCheapDuringGC()
+{
+ // FIXME: We could make this even safer by returning false if this structure's property table
+ // has any large property names.
+ // https://bugs.webkit.org/show_bug.cgi?id=157334
+
+ return (!m_globalObject || Heap::isMarked(m_globalObject.get()))
+ && (!storedPrototypeObject() || Heap::isMarked(storedPrototypeObject()));
+}
+
+bool Structure::markIfCheap(SlotVisitor& visitor)
+{
+ if (!isCheapDuringGC())
+ return Heap::isMarked(this);
+
+ visitor.appendUnbarrieredReadOnlyPointer(this);
+ return true;
+}
+
bool Structure::prototypeChainMayInterceptStoreTo(VM& vm, PropertyName propertyName)
{
if (parseIndex(propertyName))
Modified: trunk/Source/_javascript_Core/runtime/Structure.h (200404 => 200405)
--- trunk/Source/_javascript_Core/runtime/Structure.h 2016-05-04 01:54:39 UTC (rev 200404)
+++ trunk/Source/_javascript_Core/runtime/Structure.h 2016-05-04 02:23:28 UTC (rev 200405)
@@ -260,6 +260,17 @@
StructureChain* prototypeChain(VM&, JSGlobalObject*) const;
StructureChain* prototypeChain(ExecState*) const;
static void visitChildren(JSCell*, SlotVisitor&);
+
+ // A Structure is cheap to mark during GC if doing so would only add a small and bounded amount
+ // to our heap footprint. For example, if the structure refers to a global object that is not
+ // yet marked, then as far as we know, the decision to mark this Structure would lead to a large
+ // increase in footprint because no other object refers to that global object. This method
+ // returns true if all user-controlled (and hence unbounded in size) objects referenced from the
+ // Structure are already marked.
+ bool isCheapDuringGC();
+
+ // Returns true if this structure is now marked.
+ bool markIfCheap(SlotVisitor&);
// Will just the prototype chain intercept this property access?
JS_EXPORT_PRIVATE bool prototypeChainMayInterceptStoreTo(VM&, PropertyName);
