Changeset 54798 in webkit for trunk/JavaScriptCore/runtime/Structure.cpp

Timestamp:

Feb 15, 2010, 2:37:43 PM (15 years ago)

Author:

[email protected]

Message:

​https://bugs.webkit.org/show_bug.cgi?id=33731
Many false leaks in release builds due to PtrAndFlags

Reviewed by Darin Adler.

JavaScriptCore:

StructureTransitionTable was effectively a smart pointer type,
one machine word in size and wholly contained as a member of
of Structure. It either pointed to an actual table, or could
be used to describe a single transtion entry without use of a
table.

This, however, worked by using a PtrAndFlags, which is not
compatible with the leaks tool. Since there is no clear way to
obtain another bit for 'free' here, and since there are bits
available up in Structure, merge this functionality back up into
Structure. Having this in a separate class was quite clean
from an enacapsulation perspective, but this solution doesn't
seem to bad - all table access is now intermediated through the
Structure::structureTransitionTableFoo methods, keeping the
optimization fairly well contained.

This was the last use of PtrAndFlags, so removing the file too.

• JavaScriptCore.xcodeproj/project.pbxproj:
• bytecode/CodeBlock.h:
• runtime/Structure.cpp:

(JSC::Structure::Structure):
(JSC::Structure::~Structure):
(JSC::Structure::addPropertyTransitionToExistingStructure):
(JSC::Structure::addPropertyTransition):
(JSC::Structure::hasTransition):

• runtime/Structure.h:

(JSC::Structure::):
(JSC::Structure::structureTransitionTableContains):
(JSC::Structure::structureTransitionTableGet):
(JSC::Structure::structureTransitionTableHasTransition):
(JSC::Structure::structureTransitionTableRemove):
(JSC::Structure::structureTransitionTableAdd):
(JSC::Structure::structureTransitionTable):
(JSC::Structure::setStructureTransitionTable):
(JSC::Structure::singleTransition):
(JSC::Structure::setSingleTransition):

• runtime/StructureTransitionTable.h:
• wtf/PtrAndFlags.h: Removed.

WebCore:

PtrAndFlags has now been removed; remove forwarding header.

• ForwardingHeaders/wtf/PtrAndFlags.h: Removed.

File:

• trunk/JavaScriptCore/runtime/Structure.cpp (modified) (7 diffs)

trunk/JavaScriptCore/runtime/Structure.cpp

@@ -80 +80 @@
 static int comparePropertyMapEntryIndices(const void* a, const void* b);
 
+inline void Structure::setTransitionTable(TransitionTable* table)
+{
+    ASSERT(m_isUsingSingleSlot);
+#ifndef NDEBUG
+    setSingleTransition(0);
+#endif
+    m_isUsingSingleSlot = false;
+    m_transitions.m_table = table;
+    // This implicitly clears the flag that indicates we're using a single transition
+    ASSERT(!m_isUsingSingleSlot);
+}
+
+// The contains and get methods accept imprecise matches, so if an unspecialised transition exists
+// for the given key they will consider that transition to be a match.  If a specialised transition
+// exists and it matches the provided specificValue, get will return the specific transition.
+inline bool Structure::transitionTableContains(const StructureTransitionTableHash::Key& key, JSCell* specificValue)
+{
+    if (m_isUsingSingleSlot) {
+        Structure* existingTransition = singleTransition();
+        return existingTransition && existingTransition->m_nameInPrevious.get() == key.first
+               && existingTransition->m_attributesInPrevious == key.second
+               && (existingTransition->m_specificValueInPrevious == specificValue || existingTransition->m_specificValueInPrevious == 0);
+    }
+    TransitionTable::iterator find = transitionTable()->find(key);
+    if (find == transitionTable()->end())
+        return false;
+
+    return find->second.first || find->second.second->transitionedFor(specificValue);
+}
+
+inline Structure* Structure::transitionTableGet(const StructureTransitionTableHash::Key& key, JSCell* specificValue) const
+{
+    if (m_isUsingSingleSlot) {
+        Structure* existingTransition = singleTransition();
+        if (existingTransition && existingTransition->m_nameInPrevious.get() == key.first
+            && existingTransition->m_attributesInPrevious == key.second
+            && (existingTransition->m_specificValueInPrevious == specificValue || existingTransition->m_specificValueInPrevious == 0))
+            return existingTransition;
+        return 0;
+    }
+
+    Transition transition = transitionTable()->get(key);
+    if (transition.second && transition.second->transitionedFor(specificValue))
+        return transition.second;
+    return transition.first;
+}
+
+inline bool Structure::transitionTableHasTransition(const StructureTransitionTableHash::Key& key) const
+{
+    if (m_isUsingSingleSlot) {
+        Structure* transition = singleTransition();
+        return transition && transition->m_nameInPrevious == key.first
+        && transition->m_attributesInPrevious == key.second;
+    }
+    return transitionTable()->contains(key);
+}
+
+inline void Structure::transitionTableRemove(const StructureTransitionTableHash::Key& key, JSCell* specificValue)
+{
+    if (m_isUsingSingleSlot) {
+        ASSERT(transitionTableContains(key, specificValue));
+        setSingleTransition(0);
+        return;
+    }
+    TransitionTable::iterator find = transitionTable()->find(key);
+    if (!specificValue)
+        find->second.first = 0;
+    else
+        find->second.second = 0;
+    if (!find->second.first && !find->second.second)
+        transitionTable()->remove(find);
+}
+
+inline void Structure::transitionTableAdd(const StructureTransitionTableHash::Key& key, Structure* structure, JSCell* specificValue)
+{
+    if (m_isUsingSingleSlot) {
+        if (!singleTransition()) {
+            setSingleTransition(structure);
+            return;
+        }
+        Structure* existingTransition = singleTransition();
+        TransitionTable* transitionTable = new TransitionTable;
+        setTransitionTable(transitionTable);
+        if (existingTransition)
+            transitionTableAdd(std::make_pair(existingTransition->m_nameInPrevious.get(), existingTransition->m_attributesInPrevious), existingTransition, existingTransition->m_specificValueInPrevious);
+    }
+    if (!specificValue) {
+        TransitionTable::iterator find = transitionTable()->find(key);
+        if (find == transitionTable()->end())
+            transitionTable()->add(key, Transition(structure, 0));
+        else
+            find->second.first = structure;
+    } else {
+        // If we're adding a transition to a specific value, then there cannot be
+        // an existing transition
+        ASSERT(!transitionTable()->contains(key));
+        transitionTable()->add(key, Transition(0, structure));
+    }
+}
+
 void Structure::dumpStatistics()
 {
@@ -137 +237 @@
     , m_specificFunctionThrashCount(0)
     , m_anonymousSlotCount(anonymousSlotCount)
-{
+    , m_isUsingSingleSlot(true)
+{
+    m_transitions.m_singleTransition = 0;
+
     ASSERT(m_prototype);
     ASSERT(m_prototype.isObject() || m_prototype.isNull());
@@ -160 +263 @@
     if (m_previous) {
         ASSERT(m_nameInPrevious);
-        m_previous->table.remove(make_pair(m_nameInPrevious.get(), m_attributesInPrevious), m_specificValueInPrevious);
+        m_previous->transitionTableRemove(make_pair(m_nameInPrevious.get(), m_attributesInPrevious), m_specificValueInPrevious);
 
     }
@@ -177 +280 @@
         fastFree(m_propertyTable);
     }
+
+    if (!m_isUsingSingleSlot)
+        delete transitionTable();
 
 #ifndef NDEBUG
@@ -341 +447 @@
     ASSERT(structure->typeInfo().type() == ObjectType);
 
-    if (Structure* existingTransition = structure->table.get(make_pair(propertyName.ustring().rep(), attributes), specificValue)) {
+    if (Structure* existingTransition = structure->transitionTableGet(make_pair(propertyName.ustring().rep(), attributes), specificValue)) {
         ASSERT(existingTransition->m_offset != noOffset);
         offset = existingTransition->m_offset + existingTransition->m_anonymousSlotCount;
@@ -406 +512 @@
     transition->m_offset = offset - structure->m_anonymousSlotCount;
     ASSERT(structure->anonymousSlotCount() == transition->anonymousSlotCount());
-    structure->table.add(make_pair(propertyName.ustring().rep(), attributes), transition.get(), specificValue);
+    structure->transitionTableAdd(make_pair(propertyName.ustring().rep(), attributes), transition.get(), specificValue);
     return transition.release();
 }
@@ -853 +959 @@
 bool Structure::hasTransition(UString::Rep* rep, unsigned attributes)
 {
-    return table.hasTransition(make_pair(rep, attributes));
+    return transitionTableHasTransition(make_pair(rep, attributes));
 }