Changeset 178984 in webkit for trunk/Source/JavaScriptCore

Timestamp:

Jan 22, 2015, 11:04:05 PM (10 years ago)

Author:

[email protected]

Message:

EdenCollections unnecessarily visit SmallStrings
​https://bugs.webkit.org/show_bug.cgi?id=140762

Reviewed by Geoffrey Garen.

• heap/Heap.cpp:

(JSC::Heap::copyBackingStores): Also added a GCPhase for copying
backing stores, which is a significant portion of garbage collection.
(JSC::Heap::visitSmallStrings): Check to see if we need to visit
SmallStrings based on the collection type.

• runtime/SmallStrings.cpp:

(JSC::SmallStrings::SmallStrings):
(JSC::SmallStrings::visitStrongReferences): Set the fact that we have
visited the SmallStrings since the last modification.

• runtime/SmallStrings.h:

(JSC::SmallStrings::needsToBeVisited): If we're doing a
FullCollection, we need to visit. Otherwise, it depends on whether
we've been visited since the last modification/allocation.

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• heap/Heap.cpp (modified) (2 diffs)
• runtime/SmallStrings.cpp (modified) (5 diffs)
• runtime/SmallStrings.h (modified) (3 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2015-01-22  Mark Hahnenberg  <[email protected]>
+
+        EdenCollections unnecessarily visit SmallStrings
+        https://bugs.webkit.org/show_bug.cgi?id=140762
+
+        Reviewed by Geoffrey Garen.
+
+        * heap/Heap.cpp:
+        (JSC::Heap::copyBackingStores): Also added a GCPhase for copying
+        backing stores, which is a significant portion of garbage collection.
+        (JSC::Heap::visitSmallStrings): Check to see if we need to visit
+        SmallStrings based on the collection type.
+        * runtime/SmallStrings.cpp:
+        (JSC::SmallStrings::SmallStrings):
+        (JSC::SmallStrings::visitStrongReferences): Set the fact that we have
+        visited the SmallStrings since the last modification.
+        * runtime/SmallStrings.h:
+        (JSC::SmallStrings::needsToBeVisited): If we're doing a
+        FullCollection, we need to visit. Otherwise, it depends on whether
+        we've been visited since the last modification/allocation.
+
 2015-01-22  Ryosuke Niwa  <[email protected]>
 

trunk/Source/JavaScriptCore/heap/Heap.cpp

@@ -548 +548 @@
 void Heap::copyBackingStores()
 {
+    GCPHASE(CopyBackingStores);
     if (m_operationInProgress == EdenCollection)
         m_storageSpace.startedCopying<EdenCollection>();
@@ -612 +613 @@
 {
     GCPHASE(VisitSmallStrings);
+    if (!m_vm->smallStrings.needsToBeVisited(m_operationInProgress))
+        return;
+
     m_vm->smallStrings.visitStrongReferences(m_slotVisitor);
-
     if (Options::logGC() == GCLogging::Verbose)
         dataLog("Small strings:\n", m_slotVisitor);
-
     m_slotVisitor.donateAndDrain();
 }

trunk/Source/JavaScriptCore/runtime/SmallStrings.cpp

@@ -69 +69 @@
     , m_nullObjectString(nullptr)
     , m_undefinedObjectString(nullptr)
+    , m_needsToBeVisited(true)
 {
     COMPILE_ASSERT(singleCharacterStringCount == sizeof(m_singleCharacterStrings) / sizeof(m_singleCharacterStrings[0]), IsNumCharactersConstInSyncWithClassUsage);
@@ -90 +91 @@
 void SmallStrings::visitStrongReferences(SlotVisitor& visitor)
 {
+    m_needsToBeVisited = false;
     visitor.appendUnbarrieredPointer(&m_emptyString);
     for (unsigned i = 0; i <= maxSingleCharacterString; ++i)
@@ -108 +110 @@
     ASSERT(!m_emptyString);
     m_emptyString = JSString::createHasOtherOwner(*vm, StringImpl::empty());
+    m_needsToBeVisited = true;
 }
 
@@ -116 +119 @@
     ASSERT(!m_singleCharacterStrings[character]);
     m_singleCharacterStrings[character] = JSString::createHasOtherOwner(*vm, PassRefPtr<StringImpl>(m_storage->rep(character)));
+    m_needsToBeVisited = true;
 }
 
@@ -125 +129 @@
 }
 
-void SmallStrings::initialize(VM* vm, JSString*& string, const char* value) const
+void SmallStrings::initialize(VM* vm, JSString*& string, const char* value)
 {
     string = JSString::create(*vm, StringImpl::create(value));
+    m_needsToBeVisited = true;
 }
 

trunk/Source/JavaScriptCore/runtime/SmallStrings.h

@@ -89 +89 @@
     JSString* undefinedObjectString() const { return m_undefinedObjectString; }
 
+    bool needsToBeVisited(HeapOperation collectionType) const
+    {
+        if (collectionType == FullCollection)
+            return true;
+        return m_needsToBeVisited;
+    }
+
 private:
     static const unsigned singleCharacterStringCount = maxSingleCharacterString + 1;
@@ -95 +102 @@
     JS_EXPORT_PRIVATE void createSingleCharacterString(VM*, unsigned char);
 
-    void initialize(VM*, JSString*&, const char* value) const;
+    void initialize(VM*, JSString*&, const char* value);
 
     JSString* m_emptyString;
@@ -105 +112 @@
     JSString* m_singleCharacterStrings[singleCharacterStringCount];
     std::unique_ptr<SmallStringsStorage> m_storage;
+    bool m_needsToBeVisited;
 };