Structure should be more methodical about the relationship between m_offset and m_propertyTable
https://bugs.webkit.org/show_bug.cgi?id=109978
Reviewed by Mark Hahnenberg.
Allegedly, the previous relationship was that either m_propertyTable or m_offset
would be set, and if m_propertyTable was not set you could rebuild it. In reality,
we would sometimes "reset" both: some transitions wouldn't set m_offset, and other
transitions would clear the previous structure's m_propertyTable. So, in a
structure transition chain of A->B->C you could have:
A transitions to B: B doesn't copy m_offset but does copy m_propertyTable, because
that seemed like a good idea at the time (this was a common idiom in the code).
B transitions to C: C steals B's m_propertyTable, leaving B with neither a
m_propertyTable nor a m_offset.
Then we would ask for the size of the property storage of B and get the answer
"none". That's not good.
Now, there is a new relationship, which, hopefully, should fix things: m_offset is
always set and always refers to the maximum offset ever used by the property table.
From this, you can infer both the inline and out-of-line property size, and
capacity. This is accomplished by having PropertyTable::add() take a
PropertyOffset reference, which must be Structure::m_offset. It will update this
offset. As well, all transitions now copy m_offset. And we frequently assert
(using RELEASE_ASSERT) that the m_offset matches what m_propertyTable would tell
you. Hence if you ever modify the m_propertyTable, you'll also update the offset.
If you ever copy the property table, you'll also copy the offset. Life should be
good, I think.
* runtime/PropertyMapHashTable.h:
(JSC::PropertyTable::add):
* runtime/Structure.cpp:
(JSC::Structure::materializePropertyMap):
(JSC::Structure::addPropertyTransition):
(JSC::Structure::removePropertyTransition):
(JSC::Structure::changePrototypeTransition):
(JSC::Structure::despecifyFunctionTransition):
(JSC::Structure::attributeChangeTransition):
(JSC::Structure::toDictionaryTransition):
(JSC::Structure::sealTransition):
(JSC::Structure::freezeTransition):
(JSC::Structure::preventExtensionsTransition):
(JSC::Structure::nonPropertyTransition):
(JSC::Structure::flattenDictionaryStructure):
(JSC::Structure::checkConsistency):
(JSC::Structure::putSpecificValue):
(JSC::Structure::createPropertyMap):
(JSC::PropertyTable::checkConsistency):
* runtime/Structure.h:
(JSC):
(JSC::Structure::putWillGrowOutOfLineStorage):
(JSC::Structure::outOfLineCapacity):
(JSC::Structure::outOfLineSize):
(JSC::Structure::isEmpty):
(JSC::Structure::materializePropertyMapIfNecessary):
(JSC::Structure::materializePropertyMapIfNecessaryForPinning):
(Structure):
(JSC::Structure::checkOffsetConsistency):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@143097 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/runtime/Structure.h b/Source/JavaScriptCore/runtime/Structure.h
index 8f42078..628461f 100644
--- a/Source/JavaScriptCore/runtime/Structure.h
+++ b/Source/JavaScriptCore/runtime/Structure.h
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2008, 2009, 2012 Apple Inc. All rights reserved.
+ * Copyright (C) 2008, 2009, 2012, 2013 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -110,6 +110,8 @@
bool didTransition() const { return m_didTransition; }
bool putWillGrowOutOfLineStorage()
{
+ checkOffsetConsistency();
+
ASSERT(outOfLineCapacity() >= outOfLineSize());
if (!m_propertyTable) {
@@ -187,6 +189,8 @@
unsigned outOfLineCapacity() const
{
+ ASSERT(checkOffsetConsistency());
+
unsigned outOfLineSize = this->outOfLineSize();
if (!outOfLineSize)
@@ -201,14 +205,9 @@
}
unsigned outOfLineSize() const
{
+ ASSERT(checkOffsetConsistency());
ASSERT(structure()->classInfo() == &s_info);
- if (m_propertyTable) {
- unsigned totalSize = m_propertyTable->propertyStorageSize();
- unsigned inlineCapacity = this->inlineCapacity();
- if (totalSize < inlineCapacity)
- return 0;
- return totalSize - inlineCapacity;
- }
+
return numberOfOutOfLineSlotsForLastOffset(m_offset);
}
bool hasInlineStorage() const
@@ -221,17 +220,10 @@
}
unsigned inlineSize() const
{
- unsigned result;
- if (m_propertyTable)
- result = m_propertyTable->propertyStorageSize();
- else
- result = m_offset + 1;
- return std::min<unsigned>(result, m_inlineCapacity);
+ return std::min<unsigned>(m_offset + 1, m_inlineCapacity);
}
unsigned totalStorageSize() const
{
- if (m_propertyTable)
- return m_propertyTable->propertyStorageSize();
return numberOfSlotsForLastOffset(m_offset, m_inlineCapacity);
}
unsigned totalStorageCapacity() const
@@ -248,8 +240,6 @@
}
PropertyOffset lastValidOffset() const
{
- if (m_propertyTable)
- return offsetForPropertyNumber(m_propertyTable->propertyStorageSize() - 1, m_inlineCapacity);
return m_offset;
}
bool isValidOffset(PropertyOffset offset) const
@@ -281,8 +271,7 @@
bool isEmpty() const
{
- if (m_propertyTable)
- return m_propertyTable->isEmpty();
+ ASSERT(checkOffsetConsistency());
return !JSC::isValidOffset(m_offset);
}
@@ -405,12 +394,14 @@
void materializePropertyMapIfNecessary(JSGlobalData& globalData)
{
ASSERT(structure()->classInfo() == &s_info);
+ ASSERT(checkOffsetConsistency());
if (!m_propertyTable && previousID())
materializePropertyMap(globalData);
}
void materializePropertyMapIfNecessaryForPinning(JSGlobalData& globalData)
{
ASSERT(structure()->classInfo() == &s_info);
+ checkOffsetConsistency();
if (!m_propertyTable)
materializePropertyMap(globalData);
}
@@ -453,6 +444,20 @@
ASSERT(typeInfo().structureHasRareData());
return static_cast<StructureRareData*>(m_previousOrRareData.get());
}
+
+ ALWAYS_INLINE bool checkOffsetConsistency() const
+ {
+ if (!m_propertyTable) {
+ ASSERT(!m_isPinnedPropertyTable);
+ return true;
+ }
+
+ RELEASE_ASSERT(numberOfSlotsForLastOffset(m_offset, m_inlineCapacity) == m_propertyTable->propertyStorageSize());
+ unsigned totalSize = m_propertyTable->propertyStorageSize();
+ RELEASE_ASSERT((totalSize < inlineCapacity() ? 0 : totalSize - inlineCapacity()) == numberOfOutOfLineSlotsForLastOffset(m_offset));
+
+ return true;
+ }
void allocateRareData(JSGlobalData&);
void cloneRareDataFrom(JSGlobalData&, const Structure*);