Refactor RegionTree

third_party/WebKit/Source/platform/heap/PageMemory.cpp

Index: third_party/WebKit/Source/platform/heap/PageMemory.cpp
diff --git a/third_party/WebKit/Source/platform/heap/PageMemory.cpp b/third_party/WebKit/Source/platform/heap/PageMemory.cpp
index d5d4cdf26b0e238919640d10a2a5e7ef53fec043..7945c6983daf4ce258d49957c94efe265c182ce6 100644
--- a/third_party/WebKit/Source/platform/heap/PageMemory.cpp
+++ b/third_party/WebKit/Source/platform/heap/PageMemory.cpp
@@ -33,15 +33,16 @@ PageMemoryRegion::PageMemoryRegion(Address base, size_t size, unsigned numPages)
     : MemoryRegion(base, size)
     , m_isLargePage(numPages == 1)
     , m_numPages(numPages)
+    , m_regionTree(nullptr)
 {
-    Heap::addPageMemoryRegion(this);
     for (size_t i = 0; i < blinkPagesPerRegion; ++i)
         m_inUse[i] = false;
 }
 
 PageMemoryRegion::~PageMemoryRegion()
 {
-    Heap::removePageMemoryRegion(this);
+    if (m_regionTree)
+        m_regionTree->remove(this);
     release();
 }
 
@@ -72,7 +73,8 @@ PageMemoryRegion* PageMemoryRegion::allocate(size_t size, unsigned numPages)
 
 PageMemoryRegion* RegionTree::lookup(Address address)
 {
-    RegionTree* current = this;
+    MutexLocker locker(m_mutex);
+    RegionTreeNode* current = m_root;
     while (current) {
         Address base = current->m_region->base();
         if (address < base) {
@@ -89,23 +91,38 @@ PageMemoryRegion* RegionTree::lookup(Address address)
     return nullptr;
 }
 
-void RegionTree::add(RegionTree* newTree, RegionTree** context)
+void RegionTree::add(PageMemoryRegion* region)
 {
-    ASSERT(newTree);
-    Address base = newTree->m_region->base();
-    for (RegionTree* current = *context; current; current = *context) {
+    ASSERT(region);
+    RegionTreeNode* newTree = new RegionTreeNode(region);
+    region->setRegionTree(this);

[haraken, 2016/03/15 05:49:50]

I'd prefer removing setRegionTree. It looks nicer to introduce
PageMemoryRegion::add() and set m_regionTree in the add() method.

[keishi, 2016/03/15 07:48:43]

Done.

+    MutexLocker locker(m_mutex);
+    newTree->addTo(&m_root);
+}
+
+void RegionTreeNode::addTo(RegionTreeNode** context)
+{
+    Address base = m_region->base();
+    for (RegionTreeNode* current = *context; current; current = *context) {
         ASSERT(!current->m_region->contains(base));
         context = (base < current->m_region->base()) ? &current->m_left : &current->m_right;
     }
-    *context = newTree;
+    *context = this;
 }
 
-void RegionTree::remove(PageMemoryRegion* region, RegionTree** context)
+void RegionTree::remove(PageMemoryRegion* region)
 {
+    region->setRegionTree(nullptr);
+    // Deletion of large objects (and thus their regions) can happen
+    // concurrently on sweeper threads.  Removal can also happen during thread
+    // shutdown, but that case is safe.  Regardless, we make all removals
+    // mutually exclusive.
+    MutexLocker locker(m_mutex);
     ASSERT(region);
-    ASSERT(context);
+    ASSERT(m_root);
     Address base = region->base();
-    RegionTree* current = *context;
+    RegionTreeNode** context = &m_root;
+    RegionTreeNode* current = m_root;
     for (; current; current = *context) {
         if (region == current->m_region)
             break;
@@ -118,11 +135,11 @@ void RegionTree::remove(PageMemoryRegion* region, RegionTree** context)
 
     *context = nullptr;
     if (current->m_left) {
-        add(current->m_left, context);
+        current->m_left->addTo(context);
         current->m_left = nullptr;
     }
     if (current->m_right) {
-        add(current->m_right, context);
+        current->m_right->addTo(context);
         current->m_right = nullptr;
     }
     delete current;
@@ -165,6 +182,7 @@ PageMemory* PageMemory::allocate(size_t payloadSize)
     // guard page at the beginning and end of blink heap page.
     size_t allocationSize = payloadSize + 2 * blinkGuardPageSize;
     PageMemoryRegion* pageMemoryRegion = PageMemoryRegion::allocateLargePage(allocationSize);
+    Heap::addPageMemoryRegion(pageMemoryRegion);
     PageMemory* storage = setupPageMemoryInRegion(pageMemoryRegion, 0, payloadSize);
     RELEASE_ASSERT(storage->commit());
     return storage;