From 1745e503aa95a571a28809f53bb71f46d8c546ef Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Tue, 10 Nov 2020 20:24:08 +0100
Subject: [PATCH] LibJS: Use a HashTable to identify potential cell pointers in
 GC scan

Previously we would iterate over all the live HeapBlocks in order to
learn if an arbitrary pointer-sized value was a pointer into a live
HeapBlock. This was quite time-consuming.

Instead of that, just put all the live HeapBlock*'s in a HashTable
and identify pointers by doing a bit-masked lookup into the table.
---
 Libraries/LibJS/Heap/Heap.cpp | 37 ++++++++++++++---------------------
 Libraries/LibJS/Heap/Heap.h   |  2 --
 2 files changed, 15 insertions(+), 24 deletions(-)

diff --git a/Libraries/LibJS/Heap/Heap.cpp b/Libraries/LibJS/Heap/Heap.cpp
index 02b9347f12..1170f82030 100644
--- a/Libraries/LibJS/Heap/Heap.cpp
+++ b/Libraries/LibJS/Heap/Heap.cpp
@@ -151,43 +151,36 @@ void Heap::gather_conservative_roots(HashTable<Cell*>& roots)
         possible_pointers.set(data);
     }
 
+    HashTable<HeapBlock*> all_live_heap_blocks;
+    for_each_block([&](auto& block) {
+        all_live_heap_blocks.set(&block);
+        return IterationDecision::Continue;
+    });
+
     for (auto possible_pointer : possible_pointers) {
         if (!possible_pointer)
             continue;
 #ifdef HEAP_DEBUG
         dbg() << "  ? " << (const void*)possible_pointer;
 #endif
-        if (auto* cell = cell_from_possible_pointer(possible_pointer)) {
-            if (cell->is_live()) {
+        auto* possible_heap_block = HeapBlock::from_cell(reinterpret_cast<const Cell*>(possible_pointer));
+        if (all_live_heap_blocks.contains(possible_heap_block)) {
+            if (auto* cell = possible_heap_block->cell_from_possible_pointer(possible_pointer)) {
+                if (cell->is_live()) {
 #ifdef HEAP_DEBUG
-                dbg() << "  ?-> " << (const void*)cell;
+                    dbg() << "  ?-> " << (const void*)cell;
 #endif
-                roots.set(cell);
-            } else {
+                    roots.set(cell);
+                } else {
 #ifdef HEAP_DEBUG
-                dbg() << "  #-> " << (const void*)cell;
+                    dbg() << "  #-> " << (const void*)cell;
 #endif
+                }
             }
         }
     }
 }
 
-Cell* Heap::cell_from_possible_pointer(FlatPtr pointer)
-{
-    auto* possible_heap_block = HeapBlock::from_cell(reinterpret_cast<const Cell*>(pointer));
-    bool found = false;
-    for_each_block([&](auto& block) {
-        if (&block == possible_heap_block) {
-            found = true;
-            return IterationDecision::Break;
-        }
-        return IterationDecision::Continue;
-    });
-    if (!found)
-        return nullptr;
-    return possible_heap_block->cell_from_possible_pointer(pointer);
-}
-
 class MarkingVisitor final : public Cell::Visitor {
 public:
     MarkingVisitor() { }
diff --git a/Libraries/LibJS/Heap/Heap.h b/Libraries/LibJS/Heap/Heap.h
index 1658c8f155..8d7d32298f 100644
--- a/Libraries/LibJS/Heap/Heap.h
+++ b/Libraries/LibJS/Heap/Heap.h
@@ -100,8 +100,6 @@ private:
     void mark_live_cells(const HashTable<Cell*>& live_cells);
     void sweep_dead_cells(bool print_report, const Core::ElapsedTimer&);
 
-    Cell* cell_from_possible_pointer(FlatPtr);
-
     Allocator& allocator_for_size(size_t);
 
     template<typename Callback>