From e0493c509e3d0563b6524219942d70dfe024c4f3 Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Mon, 17 May 2021 21:25:57 +0200
Subject: [PATCH] LibJS: Make the forward transition chain weakly cached

Before this patch, every shape would permanently remember every other
shape it had ever transitioned to. This could lead to pathological
accumulation of unused shape objects in some cases.

Fix this by using WeakPtr instead of a strongly visited Shape* in the
the forward transition chain map. This means that we will now miss out
on some shape sharing opportunities, but since this is not required
for correctness it doesn't matter.

Note that the backward transition chain is still strongly cached,
as it's necessary for the reification of property tables.

An interesting future optimization could be to allow property tables
to get garbage collected (by detaching them from the shape object)
and then reconstituted from the backwards transition chain (if needed.)
---
 Userland/Libraries/LibJS/Runtime/Shape.cpp | 22 ++++++++++++++++------
 Userland/Libraries/LibJS/Runtime/Shape.h   | 11 ++++++++---
 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/Userland/Libraries/LibJS/Runtime/Shape.cpp b/Userland/Libraries/LibJS/Runtime/Shape.cpp
index da93fafd88..09c2f216dd 100644
--- a/Userland/Libraries/LibJS/Runtime/Shape.cpp
+++ b/Userland/Libraries/LibJS/Runtime/Shape.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2020-2021, Andreas Kling <kling@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -23,10 +23,23 @@ Shape* Shape::create_unique_clone() const
     return new_shape;
 }
 
+Shape* Shape::get_or_prune_cached_forward_transition(TransitionKey const& key)
+{
+    auto it = m_forward_transitions.find(key);
+    if (it == m_forward_transitions.end())
+        return nullptr;
+    if (!it->value) {
+        // The cached forward transition has gone stale (from garbage collection). Prune it.
+        m_forward_transitions.remove(it);
+        return nullptr;
+    }
+    return it->value;
+}
+
 Shape* Shape::create_put_transition(const StringOrSymbol& property_name, PropertyAttributes attributes)
 {
     TransitionKey key { property_name, attributes };
-    if (auto* existing_shape = m_forward_transitions.get(key).value_or(nullptr))
+    if (auto* existing_shape = get_or_prune_cached_forward_transition(key))
         return existing_shape;
     auto* new_shape = heap().allocate_without_global_object<Shape>(*this, property_name, attributes, TransitionType::Put);
     m_forward_transitions.set(key, new_shape);
@@ -36,7 +49,7 @@ Shape* Shape::create_put_transition(const StringOrSymbol& property_name, Propert
 Shape* Shape::create_configure_transition(const StringOrSymbol& property_name, PropertyAttributes attributes)
 {
     TransitionKey key { property_name, attributes };
-    if (auto* existing_shape = m_forward_transitions.get(key).value_or(nullptr))
+    if (auto* existing_shape = get_or_prune_cached_forward_transition(key))
         return existing_shape;
     auto* new_shape = heap().allocate_without_global_object<Shape>(*this, property_name, attributes, TransitionType::Configure);
     m_forward_transitions.set(key, new_shape);
@@ -88,9 +101,6 @@ void Shape::visit_edges(Cell::Visitor& visitor)
     visitor.visit(m_prototype);
     visitor.visit(m_previous);
     m_property_name.visit_edges(visitor);
-    for (auto& it : m_forward_transitions)
-        visitor.visit(it.value);
-
     if (m_property_table) {
         for (auto& it : *m_property_table)
             it.key.visit_edges(visitor);
diff --git a/Userland/Libraries/LibJS/Runtime/Shape.h b/Userland/Libraries/LibJS/Runtime/Shape.h
index bff033b42e..6f77ef6744 100644
--- a/Userland/Libraries/LibJS/Runtime/Shape.h
+++ b/Userland/Libraries/LibJS/Runtime/Shape.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2020-2021, Andreas Kling <kling@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -8,6 +8,8 @@
 
 #include <AK/HashMap.h>
 #include <AK/OwnPtr.h>
+#include <AK/WeakPtr.h>
+#include <AK/Weakable.h>
 #include <LibJS/Forward.h>
 #include <LibJS/Heap/Cell.h>
 #include <LibJS/Runtime/PropertyAttributes.h>
@@ -31,7 +33,9 @@ struct TransitionKey {
     }
 };
 
-class Shape final : public Cell {
+class Shape final
+    : public Cell
+    , public Weakable<Shape> {
 public:
     virtual ~Shape() override;
 
@@ -84,6 +88,7 @@ private:
     virtual const char* class_name() const override { return "Shape"; }
     virtual void visit_edges(Visitor&) override;
 
+    Shape* get_or_prune_cached_forward_transition(TransitionKey const&);
     void ensure_property_table() const;
 
     PropertyAttributes m_attributes { 0 };
@@ -94,7 +99,7 @@ private:
 
     mutable OwnPtr<HashMap<StringOrSymbol, PropertyMetadata>> m_property_table;
 
-    HashMap<TransitionKey, Shape*> m_forward_transitions;
+    HashMap<TransitionKey, WeakPtr<Shape>> m_forward_transitions;
     Shape* m_previous { nullptr };
     StringOrSymbol m_property_name;
     Object* m_prototype { nullptr };