From 42cc481091d1739983794d8f98b8141e2a19fd2b Mon Sep 17 00:00:00 2001
From: Andreas Kling <andreas@ladybird.org>
Date: Sat, 5 Apr 2025 21:46:37 +0200
Subject: [PATCH] LibJS: Make Optional<StringTableIndex> use less space

We can use the index's invalid state to signal an empty optional.
This makes Optional<StringTableIndex> 4 bytes instead of 8,
shrinking every bytecode instruction that uses these.
---
 Libraries/LibJS/Bytecode/Interpreter.cpp |   7 +-
 Libraries/LibJS/Bytecode/StringTable.cpp |   4 +-
 Libraries/LibJS/Bytecode/StringTable.h   | 105 ++++++++++++++++++++++-
 3 files changed, 110 insertions(+), 6 deletions(-)

diff --git a/Libraries/LibJS/Bytecode/Interpreter.cpp b/Libraries/LibJS/Bytecode/Interpreter.cpp
index 987e5ec8d3b..c64f9faba4d 100644
--- a/Libraries/LibJS/Bytecode/Interpreter.cpp
+++ b/Libraries/LibJS/Bytecode/Interpreter.cpp
@@ -1280,7 +1280,7 @@ static inline Completion throw_type_error_for_callee(Bytecode::Interpreter& inte
     auto& vm = interpreter.vm();
 
     if (expression_string.has_value())
-        return vm.throw_completion<TypeError>(ErrorType::IsNotAEvaluatedFrom, callee.to_string_without_side_effects(), callee_type, interpreter.current_executable().get_string(expression_string->value()));
+        return vm.throw_completion<TypeError>(ErrorType::IsNotAEvaluatedFrom, callee.to_string_without_side_effects(), callee_type, interpreter.current_executable().get_string(*expression_string));
 
     return vm.throw_completion<TypeError>(ErrorType::IsNotA, callee.to_string_without_side_effects(), callee_type);
 }
@@ -3078,9 +3078,10 @@ ByteString NewObject::to_byte_string_impl(Bytecode::Executable const& executable
 
 ByteString NewRegExp::to_byte_string_impl(Bytecode::Executable const& executable) const
 {
-    return ByteString::formatted("NewRegExp {}, source:{} (\"{}\") flags:{} (\"{}\")",
+    return ByteString::formatted("NewRegExp {}, source:\"{}\" flags:\"{}\"",
         format_operand("dst"sv, dst(), executable),
-        m_source_index, executable.get_string(m_source_index), m_flags_index, executable.get_string(m_flags_index));
+        executable.get_string(m_source_index),
+        executable.get_string(m_flags_index));
 }
 
 ByteString CopyObjectExcludingProperties::to_byte_string_impl(Bytecode::Executable const& executable) const
diff --git a/Libraries/LibJS/Bytecode/StringTable.cpp b/Libraries/LibJS/Bytecode/StringTable.cpp
index c9ff83d4b77..1b9312f43e1 100644
--- a/Libraries/LibJS/Bytecode/StringTable.cpp
+++ b/Libraries/LibJS/Bytecode/StringTable.cpp
@@ -11,12 +11,12 @@ namespace JS::Bytecode {
 StringTableIndex StringTable::insert(String string)
 {
     m_strings.append(move(string));
-    return m_strings.size() - 1;
+    return { static_cast<u32>(m_strings.size() - 1) };
 }
 
 String const& StringTable::get(StringTableIndex index) const
 {
-    return m_strings[index.value()];
+    return m_strings[index.value];
 }
 
 void StringTable::dump() const
diff --git a/Libraries/LibJS/Bytecode/StringTable.h b/Libraries/LibJS/Bytecode/StringTable.h
index 67adf602816..e9ed4bef75c 100644
--- a/Libraries/LibJS/Bytecode/StringTable.h
+++ b/Libraries/LibJS/Bytecode/StringTable.h
@@ -12,7 +12,11 @@
 
 namespace JS::Bytecode {
 
-AK_TYPEDEF_DISTINCT_NUMERIC_GENERAL(u32, StringTableIndex, Comparison);
+struct StringTableIndex {
+    static constexpr u32 invalid = 0xffffffffu;
+    bool is_valid() const { return value != invalid; }
+    u32 value { 0 };
+};
 
 class StringTable {
     AK_MAKE_NONMOVABLE(StringTable);
@@ -31,3 +35,102 @@ private:
 };
 
 }
+
+namespace AK {
+template<>
+class Optional<JS::Bytecode::StringTableIndex> : public OptionalBase<JS::Bytecode::StringTableIndex> {
+    template<typename U>
+    friend class Optional;
+
+public:
+    using ValueType = JS::Bytecode::StringTableIndex;
+
+    Optional() = default;
+
+    template<SameAs<OptionalNone> V>
+    Optional(V) { }
+
+    Optional(Optional<JS::Bytecode::StringTableIndex> const& other)
+    {
+        if (other.has_value())
+            m_value = other.m_value;
+    }
+
+    Optional(Optional&& other)
+        : m_value(other.m_value)
+    {
+    }
+
+    template<typename U = JS::Bytecode::StringTableIndex>
+    requires(!IsSame<OptionalNone, RemoveCVReference<U>>)
+    explicit(!IsConvertible<U&&, JS::Bytecode::StringTableIndex>) Optional(U&& value)
+    requires(!IsSame<RemoveCVReference<U>, Optional<JS::Bytecode::StringTableIndex>> && IsConstructible<JS::Bytecode::StringTableIndex, U &&>)
+        : m_value(forward<U>(value))
+    {
+    }
+
+    template<SameAs<OptionalNone> V>
+    Optional& operator=(V)
+    {
+        clear();
+        return *this;
+    }
+
+    Optional& operator=(Optional const& other)
+    {
+        if (this != &other) {
+            clear();
+            m_value = other.m_value;
+        }
+        return *this;
+    }
+
+    Optional& operator=(Optional&& other)
+    {
+        if (this != &other) {
+            clear();
+            m_value = other.m_value;
+        }
+        return *this;
+    }
+
+    void clear()
+    {
+        m_value.value = JS::Bytecode::StringTableIndex::invalid;
+    }
+
+    [[nodiscard]] bool has_value() const
+    {
+        return m_value.is_valid();
+    }
+
+    [[nodiscard]] JS::Bytecode::StringTableIndex& value() &
+    {
+        VERIFY(has_value());
+        return m_value;
+    }
+
+    [[nodiscard]] JS::Bytecode::StringTableIndex const& value() const&
+    {
+        VERIFY(has_value());
+        return m_value;
+    }
+
+    [[nodiscard]] JS::Bytecode::StringTableIndex value() &&
+    {
+        return release_value();
+    }
+
+    [[nodiscard]] JS::Bytecode::StringTableIndex release_value()
+    {
+        VERIFY(has_value());
+        JS::Bytecode::StringTableIndex released_value = m_value;
+        clear();
+        return released_value;
+    }
+
+private:
+    JS::Bytecode::StringTableIndex m_value { JS::Bytecode::StringTableIndex::invalid };
+};
+
+}