Stop using aligned allocation for WeakBlock
authorggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 28 Apr 2012 04:26:01 +0000 (04:26 +0000)
committerggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 28 Apr 2012 04:26:01 +0000 (04:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=85124

Reviewed by Anders Carlsson.

We don't actually use the alignment for anything.

* heap/WeakBlock.cpp:
(JSC::WeakBlock::create):
(JSC::WeakBlock::WeakBlock): Switched from aligned allocation to regular
allocation.

* heap/WeakBlock.h:
(WeakBlock): Don't use HeapBlock because HeapBlock requires aligned
allocation. This change required me to add some declarations that we used
to inherit from HeapBlock.

(WeakBlock::blockFor): Removed. This function relied on aligned allocation
but didn't do anything for us.

(WeakBlock::deallocate): Removed. WeakBlock doesn't own any of the deallocation
logic, so it shouldn't own the function.

* heap/WeakSet.cpp:
(JSC::WeakSet::~WeakSet):
(JSC::WeakSet::finalizeAll):
(JSC::WeakSet::visitLiveWeakImpls):
(JSC::WeakSet::visitDeadWeakImpls):
(JSC::WeakSet::sweep):
(JSC::WeakSet::shrink):
(JSC::WeakSet::resetAllocator):
(JSC::WeakSet::tryFindAllocator):
* heap/WeakSet.h:
(WeakSet): Updated declarations to reflect WeakBlock not inheriting from
HeapBlock. This allowed me to remove some casts, which was nice.

(JSC::WeakSet::deallocate): Directly set the deallocated flag instead of
asking WeakBlock to do it for us.  We don't need to have a WeakBlock
pointer to set the flag, so stop asking for one.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@115538 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/heap/WeakBlock.cpp
Source/JavaScriptCore/heap/WeakBlock.h
Source/JavaScriptCore/heap/WeakSet.cpp
Source/JavaScriptCore/heap/WeakSet.h

index cd2e9d4..18d0d92 100644 (file)
@@ -1,3 +1,45 @@
+2012-04-27  Geoffrey Garen  <ggaren@apple.com>
+
+        Stop using aligned allocation for WeakBlock
+        https://bugs.webkit.org/show_bug.cgi?id=85124
+
+        Reviewed by Anders Carlsson.
+
+        We don't actually use the alignment for anything.
+
+        * heap/WeakBlock.cpp:
+        (JSC::WeakBlock::create):
+        (JSC::WeakBlock::WeakBlock): Switched from aligned allocation to regular
+        allocation.
+
+        * heap/WeakBlock.h:
+        (WeakBlock): Don't use HeapBlock because HeapBlock requires aligned
+        allocation. This change required me to add some declarations that we used
+        to inherit from HeapBlock.
+
+        (WeakBlock::blockFor): Removed. This function relied on aligned allocation
+        but didn't do anything for us.
+
+        (WeakBlock::deallocate): Removed. WeakBlock doesn't own any of the deallocation
+        logic, so it shouldn't own the function.
+
+        * heap/WeakSet.cpp:
+        (JSC::WeakSet::~WeakSet):
+        (JSC::WeakSet::finalizeAll):
+        (JSC::WeakSet::visitLiveWeakImpls):
+        (JSC::WeakSet::visitDeadWeakImpls):
+        (JSC::WeakSet::sweep):
+        (JSC::WeakSet::shrink):
+        (JSC::WeakSet::resetAllocator):
+        (JSC::WeakSet::tryFindAllocator):
+        * heap/WeakSet.h:
+        (WeakSet): Updated declarations to reflect WeakBlock not inheriting from
+        HeapBlock. This allowed me to remove some casts, which was nice.
+
+        (JSC::WeakSet::deallocate): Directly set the deallocated flag instead of
+        asking WeakBlock to do it for us.  We don't need to have a WeakBlock
+        pointer to set the flag, so stop asking for one.
+
 2012-04-27  Kentaro Hara  <haraken@chromium.org>
 
         [JSC] Implement a helper method createNotEnoughArgumentsError()
index 13c20ae..f307e11 100644 (file)
@@ -36,7 +36,7 @@ namespace JSC {
 
 WeakBlock* WeakBlock::create()
 {
-    PageAllocationAligned allocation = PageAllocationAligned::allocate(blockSize, blockSize, OSAllocator::JSGCHeapPages);
+    PageAllocation allocation = PageAllocation::allocate(blockSize, OSAllocator::JSGCHeapPages);
     if (!static_cast<bool>(allocation))
         CRASH();
     return new (NotNull, allocation.base()) WeakBlock(allocation);
@@ -47,8 +47,8 @@ void WeakBlock::destroy(WeakBlock* block)
     block->m_allocation.deallocate();
 }
 
-WeakBlock::WeakBlock(PageAllocationAligned& allocation)
-    : HeapBlock(allocation)
+WeakBlock::WeakBlock(PageAllocation& allocation)
+    : m_allocation(allocation)
 {
     for (size_t i = 0; i < weakImplCount(); ++i) {
         WeakImpl* weakImpl = &weakImpls()[i];
index 7f10962..dab50b0 100644 (file)
@@ -30,7 +30,7 @@
 #include "WeakHandleOwner.h"
 #include "WeakImpl.h"
 #include <wtf/DoublyLinkedList.h>
-#include <wtf/PageAllocationAligned.h>
+#include <wtf/PageAllocation.h>
 #include <wtf/StdLibExtras.h>
 
 namespace JSC {
@@ -39,10 +39,10 @@ class HeapRootVisitor;
 class JSValue;
 class WeakHandleOwner;
 
-class WeakBlock : public HeapBlock {
+class WeakBlock : public DoublyLinkedListNode<WeakBlock> {
 public:
+    friend class DoublyLinkedListNode<WeakBlock>;
     static const size_t blockSize = 4 * KB;
-    static const size_t blockMask = ~(blockSize - 1);
 
     struct FreeCell {
         FreeCell* next;
@@ -59,15 +59,12 @@ public:
     static WeakBlock* create();
     static void destroy(WeakBlock*);
 
-    static WeakBlock* blockFor(WeakImpl*);
     static WeakImpl* asWeakImpl(FreeCell*);
 
     void sweep();
     const SweepResult& sweepResult();
     SweepResult takeSweepResult();
 
-    void deallocate(WeakImpl*);
-
     void visitLiveWeakImpls(HeapRootVisitor&);
     void visitDeadWeakImpls(HeapRootVisitor&);
 
@@ -76,13 +73,16 @@ public:
 private:
     static FreeCell* asFreeCell(WeakImpl*);
 
-    WeakBlock(PageAllocationAligned&);
+    WeakBlock(PageAllocation&);
     WeakImpl* firstWeakImpl();
     void finalize(WeakImpl*);
     WeakImpl* weakImpls();
     size_t weakImplCount();
     void addToFreeList(FreeCell**, WeakImpl*);
 
+    PageAllocation m_allocation;
+    WeakBlock* m_prev;
+    WeakBlock* m_next;
     SweepResult m_sweepResult;
 };
 
@@ -98,11 +98,6 @@ inline bool WeakBlock::SweepResult::isNull() const
     return blockIsFree && !freeList; // This state is impossible, so we can use it to mean null.
 }
 
-inline WeakBlock* WeakBlock::blockFor(WeakImpl* weakImpl)
-{
-    return reinterpret_cast<WeakBlock*>(reinterpret_cast<uintptr_t>(weakImpl) & blockMask);
-}
-
 inline WeakImpl* WeakBlock::asWeakImpl(FreeCell* freeCell)
 {
     return reinterpret_cast<WeakImpl*>(freeCell);
@@ -126,11 +121,6 @@ inline WeakBlock::FreeCell* WeakBlock::asFreeCell(WeakImpl* weakImpl)
     return reinterpret_cast<FreeCell*>(weakImpl);
 }
 
-inline void WeakBlock::deallocate(WeakImpl* weakImpl)
-{
-    weakImpl->setState(WeakImpl::Deallocated);
-}
-
 inline void WeakBlock::finalize(WeakImpl* weakImpl)
 {
     ASSERT(weakImpl->state() == WeakImpl::Dead);
index a27e0c2..d9c773c 100644 (file)
@@ -33,8 +33,8 @@ namespace JSC {
 WeakSet::~WeakSet()
 {
     WeakBlock* next = 0;
-    for (WeakBlock* block = static_cast<WeakBlock*>(m_blocks.head()); block; block = next) {
-        next = static_cast<WeakBlock*>(block->next());
+    for (WeakBlock* block = m_blocks.head(); block; block = next) {
+        next = block->next();
         WeakBlock::destroy(block);
     }
     m_blocks.clear();
@@ -42,27 +42,27 @@ WeakSet::~WeakSet()
 
 void WeakSet::finalizeAll()
 {
-    for (WeakBlock* block = static_cast<WeakBlock*>(m_blocks.head()); block; block = static_cast<WeakBlock*>(block->next()))
+    for (WeakBlock* block = m_blocks.head(); block; block = block->next())
         block->finalizeAll();
 }
 
 void WeakSet::visitLiveWeakImpls(HeapRootVisitor& visitor)
 {
-    for (WeakBlock* block = static_cast<WeakBlock*>(m_blocks.head()); block; block = static_cast<WeakBlock*>(block->next()))
+    for (WeakBlock* block = m_blocks.head(); block; block = block->next())
         block->visitLiveWeakImpls(visitor);
 }
 
 void WeakSet::visitDeadWeakImpls(HeapRootVisitor& visitor)
 {
-    for (WeakBlock* block = static_cast<WeakBlock*>(m_blocks.head()); block; block = static_cast<WeakBlock*>(block->next()))
+    for (WeakBlock* block = m_blocks.head(); block; block = block->next())
         block->visitDeadWeakImpls(visitor);
 }
 
 void WeakSet::sweep()
 {
     WeakBlock* next;
-    for (WeakBlock* block = static_cast<WeakBlock*>(m_blocks.head()); block; block = next) {
-        next = static_cast<WeakBlock*>(block->next());
+    for (WeakBlock* block = m_blocks.head(); block; block = next) {
+        next = block->next();
 
         // If a block is completely empty, a new sweep won't have any effect.
         if (!block->sweepResult().isNull() && block->sweepResult().blockIsFree)
@@ -76,8 +76,8 @@ void WeakSet::sweep()
 void WeakSet::shrink()
 {
     WeakBlock* next;
-    for (WeakBlock* block = static_cast<WeakBlock*>(m_blocks.head()); block; block = next) {
-        next = static_cast<WeakBlock*>(block->next());
+    for (WeakBlock* block = m_blocks.head(); block; block = next) {
+        next = block->next();
 
         if (!block->sweepResult().isNull() && block->sweepResult().blockIsFree)
             removeAllocator(block);
@@ -87,7 +87,7 @@ void WeakSet::shrink()
 void WeakSet::resetAllocator()
 {
     m_allocator = 0;
-    m_nextAllocator = static_cast<WeakBlock*>(m_blocks.head());
+    m_nextAllocator = m_blocks.head();
 }
 
 WeakBlock::FreeCell* WeakSet::findAllocator()
@@ -102,7 +102,7 @@ WeakBlock::FreeCell* WeakSet::tryFindAllocator()
 {
     while (m_nextAllocator) {
         WeakBlock* block = m_nextAllocator;
-        m_nextAllocator = static_cast<WeakBlock*>(m_nextAllocator->next());
+        m_nextAllocator = m_nextAllocator->next();
 
         block->sweep();
         WeakBlock::SweepResult sweepResult = block->takeSweepResult();
index b107fc5..a9caf4c 100644 (file)
@@ -58,7 +58,7 @@ private:
 
     WeakBlock::FreeCell* m_allocator;
     WeakBlock* m_nextAllocator;
-    DoublyLinkedList<HeapBlock> m_blocks;
+    DoublyLinkedList<WeakBlock> m_blocks;
     Heap* m_heap;
 };
 
@@ -82,7 +82,7 @@ inline WeakImpl* WeakSet::allocate(JSValue jsValue, WeakHandleOwner* weakHandleO
 
 inline void WeakSet::deallocate(WeakImpl* weakImpl)
 {
-    WeakBlock::blockFor(weakImpl)->deallocate(weakImpl);
+    weakImpl->setState(WeakImpl::Deallocated);
 }
 
 } // namespace JSC