LibJS: Move source range cache from ExecutionContext to Executable

CachedSourceRange was a GC-allocated cell stored on the
ExecutionContext, only needed because ExecutionContext must be
trivially destructible.

Move the source range cache to a HashMap<u32, SourceRange> on the
Executable (keyed by program counter), where it belongs. This
eliminates the GC::Cell subclass entirely and removes the
cached_source_range field from ExecutionContext.

StackTraceElement and TracebackFrame now store Optional<SourceRange>
directly instead of GC::Ptr<CachedSourceRange>.

Shrinks ExecutionContext from 144 to 136 bytes.

Libraries/LibJS/Bytecode/Executable.cpp

@@ -246,6 +246,17 @@ UnrealizedSourceRange Executable::source_range_at(size_t offset) const
     };
 }
 
+SourceRange const& Executable::get_source_range(u32 program_counter)
+{
+    return m_source_range_cache.ensure(program_counter, [&] {
+        auto unrealized = source_range_at(program_counter);
+        if (unrealized.source_code)
+            return unrealized.realize();
+        static SourceRange dummy { SourceCode::create({}, {}), {}, {} };
+        return dummy;
+    });
+}
+
 Operand Executable::original_operand_from_raw(u32 raw) const
 {
     // NB: Layout is [registers | locals | constants | arguments]

Libraries/LibJS/Bytecode/Executable.h

@@ -183,6 +183,8 @@ public:
 
     [[nodiscard]] UnrealizedSourceRange source_range_at(size_t offset) const;
 
+    [[nodiscard]] SourceRange const& get_source_range(u32 program_counter);
+
     void fixup_cache_pointers();
 
     void dump() const;
@@ -194,6 +196,8 @@ public:
 
 private:
     virtual void visit_edges(Visitor&) override;
+
+    HashMap<u32, SourceRange> m_source_range_cache;
 };
 
 }

Libraries/LibJS/Console.cpp

@@ -358,8 +358,8 @@ ThrowCompletionOr<Value> Console::trace()
         auto function_name = (context && context->function) ? context->function->name_for_call_stack() : ""_utf16;
         frame.function_name = function_name.is_empty() ? "<anonymous>"_string : function_name.to_utf8();
 
-        if (element.source_range) {
-            auto const& source_range = element.source_range->realize_source_range();
+        if (element.source_range.has_value()) {
+            auto const& source_range = *element.source_range;
             if (!source_range.filename().is_empty()) {
                 frame.source_file = MUST(String::from_byte_string(source_range.filename()));
                 frame.line = source_range.start.line;

Libraries/LibJS/Forward.h

@@ -170,7 +170,6 @@ struct AsyncGeneratorRequest;
 class BigInt;
 class BoundFunction;
 class BuiltinIterator;
-class CachedSourceRange;
 class Cell;
 class ClassExpression;
 struct ClassFieldDefinition;

Libraries/LibJS/Runtime/Error.cpp

@@ -21,9 +21,9 @@ static SourceRange dummy_source_range { SourceCode::create({}, {}), {}, {} };
 
 SourceRange const& TracebackFrame::source_range() const
 {
-    if (!cached_source_range)
+    if (!cached_source_range.has_value())
         return dummy_source_range;
-    return cached_source_range->realize_source_range();
+    return *cached_source_range;
 }
 
 GC::Ref<Error> Error::create(Realm& realm)
@@ -53,8 +53,6 @@ void Error::visit_edges(Visitor& visitor)
 {
     Base::visit_edges(visitor);
     visitor.visit(m_cached_string);
-    for (auto& frame : m_traceback)
-        visitor.visit(frame.cached_source_range);
 }
 
 // 20.5.8.1 InstallErrorCause ( O, options ), https://tc39.es/ecma262/#sec-installerrorcause
@@ -89,12 +87,10 @@ void Error::populate_stack()
     m_traceback.ensure_capacity(stack_trace.size());
     for (auto& element : stack_trace) {
         auto* context = element.execution_context;
-        TracebackFrame frame {
+        m_traceback.append({
             .function_name = context->function ? context->function->name_for_call_stack() : ""_utf16,
-            .cached_source_range = element.source_range,
-        };
-
-        m_traceback.append(move(frame));
+            .cached_source_range = move(element.source_range),
+        });
     }
 }
 

Libraries/LibJS/Runtime/Error.h

@@ -21,7 +21,7 @@ struct JS_API TracebackFrame {
     Utf16String function_name;
     [[nodiscard]] SourceRange const& source_range() const;
 
-    GC::Ptr<CachedSourceRange> cached_source_range;
+    Optional<SourceRange> cached_source_range;
 };
 
 enum CompactTraceback {

Libraries/LibJS/Runtime/ExecutionContext.cpp

@@ -15,8 +15,6 @@
 
 namespace JS {
 
-GC_DEFINE_ALLOCATOR(CachedSourceRange);
-
 class ExecutionContextAllocator {
 public:
     NonnullOwnPtr<ExecutionContext> allocate(u32 registers_and_locals_count, u32 constants_count, u32 arguments_count)
@@ -131,7 +129,6 @@ void ExecutionContext::visit_edges(Cell::Visitor& visitor)
     visitor.visit(variable_environment);
     visitor.visit(lexical_environment);
     visitor.visit(private_environment);
-    visitor.visit(cached_source_range);
     visitor.visit(this_value);
     visitor.visit(executable);
     visitor.visit(caller_executable);

Libraries/LibJS/Runtime/ExecutionContext.h

@@ -22,35 +22,6 @@ namespace JS {
 
 using ScriptOrModule = Variant<Empty, GC::Ref<Script>, GC::Ref<Module>>;
 
-class CachedSourceRange final : public GC::Cell {
-    GC_CELL(CachedSourceRange, GC::Cell);
-    GC_DECLARE_ALLOCATOR(CachedSourceRange);
-
-public:
-    CachedSourceRange(size_t program_counter, Variant<UnrealizedSourceRange, SourceRange> source_range)
-        : program_counter(program_counter)
-        , source_range(move(source_range))
-    {
-    }
-
-    SourceRange const& realize_source_range()
-    {
-        static SourceRange dummy_source_range { SourceCode::create({}, {}), {}, {} };
-
-        if (auto* unrealized = source_range.get_pointer<UnrealizedSourceRange>()) {
-            if (unrealized->source_code) {
-                source_range = unrealized->realize();
-            } else {
-                source_range = dummy_source_range;
-            }
-        }
-        return source_range.get<SourceRange>();
-    }
-
-    size_t program_counter { 0 };
-    Variant<UnrealizedSourceRange, SourceRange> source_range;
-};
-
 // 9.4 Execution Contexts, https://tc39.es/ecma262/#sec-execution-contexts
 struct JS_API ExecutionContext {
     static NonnullOwnPtr<ExecutionContext> create(u32 registers_and_locals_count, u32 constants_count, u32 arguments_count);
@@ -115,8 +86,6 @@ public:
 
     Span<Value> arguments;
 
-    mutable GC::Ptr<CachedSourceRange> cached_source_range;
-
     // Non-standard: Inline frame linkage for the bytecode interpreter.
     // When a JS-to-JS call is inlined in the dispatch loop, these fields
     // allow the Return handler to restore the caller's frame.
@@ -142,7 +111,7 @@ static_assert(IsTriviallyDestructible<ExecutionContext>);
 
 struct StackTraceElement {
     ExecutionContext* execution_context { nullptr };
-    GC::Ptr<CachedSourceRange> source_range;
+    Optional<SourceRange> source_range;
 };
 
 }

Libraries/LibJS/Runtime/VM.cpp

@@ -797,30 +797,17 @@ void VM::load_imported_module(ImportedModuleReferrer referrer, ModuleRequest con
     finish_loading_imported_module(referrer, module_request, payload, module);
 }
 
-static GC::Ptr<CachedSourceRange> get_source_range(ExecutionContext* context)
+Vector<StackTraceElement> VM::stack_trace() const
 {
-    // native function
-    if (!context->executable)
-        return {};
-
-    if (!context->cached_source_range
-        || context->cached_source_range->program_counter != context->program_counter) {
-        auto unrealized_source_range = context->executable->source_range_at(context->program_counter);
-        context->cached_source_range = context->executable->heap().allocate<CachedSourceRange>(
-            context->program_counter,
-            move(unrealized_source_range));
-    }
-    return context->cached_source_range;
-}
-
-GC::ConservativeVector<StackTraceElement> VM::stack_trace() const
-{
-    GC::ConservativeVector<StackTraceElement> stack_trace(heap());
+    Vector<StackTraceElement> stack_trace;
     stack_trace.ensure_capacity(m_execution_context_stack.size());
     for (auto* context : m_execution_context_stack.in_reverse()) {
+        Optional<SourceRange> source_range;
+        if (context->executable)
+            source_range = context->executable->get_source_range(context->program_counter);
         stack_trace.append({
             .execution_context = context,
-            .source_range = get_source_range(context),
+            .source_range = move(source_range),
         });
     }
 

Libraries/LibJS/Runtime/VM.h

@@ -300,7 +300,7 @@ public:
     Function<ThrowCompletionOr<void>(Realm&, NonnullOwnPtr<ExecutionContext>, ShadowRealm&)> host_initialize_shadow_realm;
     Function<Crypto::SignedBigInteger(Object const& global)> host_system_utc_epoch_nanoseconds;
 
-    [[nodiscard]] GC::ConservativeVector<StackTraceElement> stack_trace() const;
+    [[nodiscard]] Vector<StackTraceElement> stack_trace() const;
 
 private:
     using ErrorMessages = AK::Array<Utf16String, to_underlying(ErrorMessage::__Count)>;