diff --git a/Userland/Libraries/LibJS/Heap/Heap.cpp b/Userland/Libraries/LibJS/Heap/Heap.cpp index 43a8d41b87..a3aef36e0e 100644 --- a/Userland/Libraries/LibJS/Heap/Heap.cpp +++ b/Userland/Libraries/LibJS/Heap/Heap.cpp @@ -216,22 +216,18 @@ void Heap::sweep_dead_cells(bool print_report, const Core::ElapsedTimer& measure dbgln_if(HEAP_DEBUG, "sweep_dead_cells:"); Vector empty_blocks; Vector full_blocks_that_became_usable; - Vector swept_cells; size_t collected_cells = 0; size_t live_cells = 0; size_t collected_cell_bytes = 0; size_t live_cell_bytes = 0; - auto should_store_swept_cells = !m_weak_containers.is_empty(); for_each_block([&](auto& block) { bool block_has_live_cells = false; bool block_was_full = block.is_full(); block.template for_each_cell_in_state([&](Cell* cell) { if (!cell->is_marked()) { dbgln_if(HEAP_DEBUG, " ~ {}", cell); - if (should_store_swept_cells) - swept_cells.append(cell); #ifdef JS_TRACK_ZOMBIE_CELLS if (m_zombify_dead_cells) { cell->set_state(Cell::State::Zombie); @@ -269,7 +265,7 @@ void Heap::sweep_dead_cells(bool print_report, const Core::ElapsedTimer& measure } for (auto& weak_container : m_weak_containers) - weak_container.remove_swept_cells({}, swept_cells.span()); + weak_container.remove_dead_cells({}); if constexpr (HEAP_DEBUG) { for_each_block([&](auto& block) { diff --git a/Userland/Libraries/LibJS/Runtime/FinalizationRegistry.cpp b/Userland/Libraries/LibJS/Runtime/FinalizationRegistry.cpp index 0a57afea56..25c6486033 100644 --- a/Userland/Libraries/LibJS/Runtime/FinalizationRegistry.cpp +++ b/Userland/Libraries/LibJS/Runtime/FinalizationRegistry.cpp @@ -42,19 +42,17 @@ bool FinalizationRegistry::remove_by_token(Object& unregister_token) return removed; } -void FinalizationRegistry::remove_swept_cells(Badge, Span cells) +void FinalizationRegistry::remove_dead_cells(Badge) { - auto any_cells_were_swept = false; - for (auto* cell : cells) { - for (auto& record : m_records) { - if (record.target != cell) - continue; - record.target = nullptr; - any_cells_were_swept = true; - break; - } + auto any_cells_were_removed = false; + for (auto& record : m_records) { + if (!record.target || record.target->state() == Cell::State::Live) + continue; + record.target = nullptr; + any_cells_were_removed = true; + break; } - if (any_cells_were_swept) + if (any_cells_were_removed) vm().enqueue_finalization_registry_cleanup_job(*this); } diff --git a/Userland/Libraries/LibJS/Runtime/FinalizationRegistry.h b/Userland/Libraries/LibJS/Runtime/FinalizationRegistry.h index 681df05c3b..9fca911f34 100644 --- a/Userland/Libraries/LibJS/Runtime/FinalizationRegistry.h +++ b/Userland/Libraries/LibJS/Runtime/FinalizationRegistry.h @@ -30,7 +30,7 @@ public: bool remove_by_token(Object& unregister_token); void cleanup(FunctionObject* callback = nullptr); - virtual void remove_swept_cells(Badge, Span) override; + virtual void remove_dead_cells(Badge) override; private: virtual void visit_edges(Visitor& visitor) override; diff --git a/Userland/Libraries/LibJS/Runtime/WeakContainer.h b/Userland/Libraries/LibJS/Runtime/WeakContainer.h index 3a5cc8825b..554e0976b6 100644 --- a/Userland/Libraries/LibJS/Runtime/WeakContainer.h +++ b/Userland/Libraries/LibJS/Runtime/WeakContainer.h @@ -15,7 +15,7 @@ public: explicit WeakContainer(Heap&); virtual ~WeakContainer(); - virtual void remove_swept_cells(Badge, Span) = 0; + virtual void remove_dead_cells(Badge) = 0; protected: void deregister(); diff --git a/Userland/Libraries/LibJS/Runtime/WeakMap.cpp b/Userland/Libraries/LibJS/Runtime/WeakMap.cpp index f9e232cd95..4a375df62f 100644 --- a/Userland/Libraries/LibJS/Runtime/WeakMap.cpp +++ b/Userland/Libraries/LibJS/Runtime/WeakMap.cpp @@ -23,9 +23,15 @@ WeakMap::~WeakMap() { } -void WeakMap::remove_swept_cells(Badge, Span cells) +void WeakMap::remove_dead_cells(Badge) { - for (auto* cell : cells) + // FIXME: Do this in a single pass. + Vector to_remove; + for (auto& it : m_values) { + if (it.key->state() != Cell::State::Live) + to_remove.append(it.key); + } + for (auto* cell : to_remove) m_values.remove(cell); } diff --git a/Userland/Libraries/LibJS/Runtime/WeakMap.h b/Userland/Libraries/LibJS/Runtime/WeakMap.h index 9e15860c68..0d96638157 100644 --- a/Userland/Libraries/LibJS/Runtime/WeakMap.h +++ b/Userland/Libraries/LibJS/Runtime/WeakMap.h @@ -27,7 +27,7 @@ public: HashMap const& values() const { return m_values; }; HashMap& values() { return m_values; }; - virtual void remove_swept_cells(Badge, Span) override; + virtual void remove_dead_cells(Badge) override; private: #ifdef JS_TRACK_ZOMBIE_CELLS diff --git a/Userland/Libraries/LibJS/Runtime/WeakRef.cpp b/Userland/Libraries/LibJS/Runtime/WeakRef.cpp index b729722fd0..43e852578b 100644 --- a/Userland/Libraries/LibJS/Runtime/WeakRef.cpp +++ b/Userland/Libraries/LibJS/Runtime/WeakRef.cpp @@ -25,18 +25,16 @@ WeakRef::~WeakRef() { } -void WeakRef::remove_swept_cells(Badge, Span cells) +void WeakRef::remove_dead_cells(Badge) { VERIFY(m_value); - for (auto* cell : cells) { - if (m_value != cell) - continue; - m_value = nullptr; - // This is an optimization, we deregister from the garbage collector early (even if we were not garbage collected ourself yet) - // to reduce the garbage collection overhead, which we can do because a cleared weak ref cannot be reused. - WeakContainer::deregister(); - break; - } + if (m_value->state() == Cell::State::Live) + return; + + m_value = nullptr; + // This is an optimization, we deregister from the garbage collector early (even if we were not garbage collected ourself yet) + // to reduce the garbage collection overhead, which we can do because a cleared weak ref cannot be reused. + WeakContainer::deregister(); } void WeakRef::visit_edges(Visitor& visitor) diff --git a/Userland/Libraries/LibJS/Runtime/WeakRef.h b/Userland/Libraries/LibJS/Runtime/WeakRef.h index 5aa3ed1ee7..8bc2b9ad30 100644 --- a/Userland/Libraries/LibJS/Runtime/WeakRef.h +++ b/Userland/Libraries/LibJS/Runtime/WeakRef.h @@ -27,7 +27,7 @@ public: void update_execution_generation() { m_last_execution_generation = vm().execution_generation(); }; - virtual void remove_swept_cells(Badge, Span) override; + virtual void remove_dead_cells(Badge) override; private: virtual void visit_edges(Visitor&) override; diff --git a/Userland/Libraries/LibJS/Runtime/WeakSet.cpp b/Userland/Libraries/LibJS/Runtime/WeakSet.cpp index 6d269ea38c..6a40632eb0 100644 --- a/Userland/Libraries/LibJS/Runtime/WeakSet.cpp +++ b/Userland/Libraries/LibJS/Runtime/WeakSet.cpp @@ -23,9 +23,15 @@ WeakSet::~WeakSet() { } -void WeakSet::remove_swept_cells(Badge, Span cells) +void WeakSet::remove_dead_cells(Badge) { - for (auto* cell : cells) + // FIXME: Do this in a single pass. + Vector to_remove; + for (auto* cell : m_values) { + if (cell->state() != Cell::State::Live) + to_remove.append(cell); + } + for (auto* cell : to_remove) m_values.remove(cell); } diff --git a/Userland/Libraries/LibJS/Runtime/WeakSet.h b/Userland/Libraries/LibJS/Runtime/WeakSet.h index 645dd476d9..6d777cb97c 100644 --- a/Userland/Libraries/LibJS/Runtime/WeakSet.h +++ b/Userland/Libraries/LibJS/Runtime/WeakSet.h @@ -27,7 +27,7 @@ public: HashTable const& values() const { return m_values; }; HashTable& values() { return m_values; }; - virtual void remove_swept_cells(Badge, Span) override; + virtual void remove_dead_cells(Badge) override; private: #ifdef JS_TRACK_ZOMBIE_CELLS