Title: [87448] trunk/Source/_javascript_Core
- Revision
- 87448
- Author
- [email protected]
- Date
- 2011-05-26 16:39:22 -0700 (Thu, 26 May 2011)
Log Message
2011-05-26 Geoffrey Garen <[email protected]>
Reviewed by Oliver Hunt.
Removed some interdependency between Heap and SmallStrings by simplifying
the SmallStrings lifetime model
https://bugs.webkit.org/show_bug.cgi?id=61579
SunSpider reports no change.
Using Weak<T> could accomplish this too, but we're not sure it will give
us the performance we need. This is a first step, and it accomplishes
most of the value of using Weak<T>.
* heap/Heap.cpp:
(JSC::Heap::destroy):
(JSC::Heap::markRoots):
(JSC::Heap::reset): Finalize small strings just like other weak handles.
* runtime/SmallStrings.cpp:
(JSC::finalize):
(JSC::SmallStrings::finalizeSmallStrings):
* runtime/SmallStrings.h: Make all small strings trivially weak, instead
of having an "all for one, one for all" memory model.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (87447 => 87448)
--- trunk/Source/_javascript_Core/ChangeLog 2011-05-26 23:17:05 UTC (rev 87447)
+++ trunk/Source/_javascript_Core/ChangeLog 2011-05-26 23:39:22 UTC (rev 87448)
@@ -1,3 +1,28 @@
+2011-05-26 Geoffrey Garen <[email protected]>
+
+ Reviewed by Oliver Hunt.
+
+ Removed some interdependency between Heap and SmallStrings by simplifying
+ the SmallStrings lifetime model
+ https://bugs.webkit.org/show_bug.cgi?id=61579
+
+ SunSpider reports no change.
+
+ Using Weak<T> could accomplish this too, but we're not sure it will give
+ us the performance we need. This is a first step, and it accomplishes
+ most of the value of using Weak<T>.
+
+ * heap/Heap.cpp:
+ (JSC::Heap::destroy):
+ (JSC::Heap::markRoots):
+ (JSC::Heap::reset): Finalize small strings just like other weak handles.
+
+ * runtime/SmallStrings.cpp:
+ (JSC::finalize):
+ (JSC::SmallStrings::finalizeSmallStrings):
+ * runtime/SmallStrings.h: Make all small strings trivially weak, instead
+ of having an "all for one, one for all" memory model.
+
2011-05-26 Oliver Hunt <[email protected]>
Reviewed by Geoffrey Garen.
Modified: trunk/Source/_javascript_Core/heap/Heap.cpp (87447 => 87448)
--- trunk/Source/_javascript_Core/heap/Heap.cpp 2011-05-26 23:17:05 UTC (rev 87447)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp 2011-05-26 23:39:22 UTC (rev 87448)
@@ -106,6 +106,7 @@
m_markListSet = 0;
m_markedSpace.clearMarks();
m_handleHeap.finalizeWeakHandles();
+ m_globalData->smallStrings.finalizeSmallStrings();
m_markedSpace.destroy();
m_globalData = 0;
@@ -259,12 +260,6 @@
m_handleStack.mark(heapRootMarker);
visitor.drain();
- // Mark the small strings cache as late as possible, since it will clear
- // itself if nothing else has marked it.
- // FIXME: Change the small strings cache to use Weak<T>.
- m_globalData->smallStrings.visitChildren(heapRootMarker);
- visitor.drain();
-
// Weak handles must be marked last, because their owners use the set of
// opaque roots to determine reachability.
int lastOpaqueRootCount;
@@ -408,6 +403,7 @@
markRoots();
m_handleHeap.finalizeWeakHandles();
+ m_globalData->smallStrings.finalizeSmallStrings();
_javascript_CORE_GC_MARKED();
Modified: trunk/Source/_javascript_Core/runtime/SmallStrings.cpp (87447 => 87448)
--- trunk/Source/_javascript_Core/runtime/SmallStrings.cpp 2011-05-26 23:17:05 UTC (rev 87447)
+++ trunk/Source/_javascript_Core/runtime/SmallStrings.cpp 2011-05-26 23:39:22 UTC (rev 87448)
@@ -34,9 +34,11 @@
namespace JSC {
-static inline bool isMarked(JSCell* string)
+static inline void finalize(JSString*& string)
{
- return string && Heap::isMarked(string);
+ if (!string || Heap::isMarked(string))
+ return;
+ string = 0;
}
class SmallStringsStorage {
@@ -75,33 +77,11 @@
{
}
-void SmallStrings::visitChildren(HeapRootVisitor& heapRootMarker)
+void SmallStrings::finalizeSmallStrings()
{
- /*
- Our hypothesis is that small strings are very common. So, we cache them
- to avoid GC churn. However, in cases where this hypothesis turns out to
- be false -- including the degenerate case where all _javascript_ execution
- has terminated -- we don't want to waste memory.
-
- To test our hypothesis, we check if any small string has been marked. If
- so, it's probably reasonable to mark the rest. If not, we clear the cache.
- */
-
- bool isAnyStringMarked = isMarked(m_emptyString);
- for (unsigned i = 0; i < singleCharacterStringCount && !isAnyStringMarked; ++i)
- isAnyStringMarked = isMarked(m_singleCharacterStrings[i]);
-
- if (!isAnyStringMarked) {
- clear();
- return;
- }
-
- if (m_emptyString)
- heapRootMarker.mark(&m_emptyString);
- for (unsigned i = 0; i < singleCharacterStringCount; ++i) {
- if (m_singleCharacterStrings[i])
- heapRootMarker.mark(&m_singleCharacterStrings[i]);
- }
+ finalize(m_emptyString);
+ for (unsigned i = 0; i < singleCharacterStringCount; ++i)
+ finalize(m_singleCharacterStrings[i]);
}
void SmallStrings::clear()
Modified: trunk/Source/_javascript_Core/runtime/SmallStrings.h (87447 => 87448)
--- trunk/Source/_javascript_Core/runtime/SmallStrings.h 2011-05-26 23:17:05 UTC (rev 87447)
+++ trunk/Source/_javascript_Core/runtime/SmallStrings.h 2011-05-26 23:39:22 UTC (rev 87448)
@@ -63,7 +63,7 @@
StringImpl* singleCharacterStringRep(unsigned char character);
- void visitChildren(HeapRootVisitor&);
+ void finalizeSmallStrings();
void clear();
unsigned count() const;
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
