LibJS: Use a Vector<u8> for BasicBlock instruction storage

This reduces the minimum size of a basic block from 4 KiB to 0 bytes.
With this change, memory usage at the end of Speedometer is 1.2 GiB,
down from 1.8 GiB.

Userland/Libraries/LibJS/Bytecode/BasicBlock.cpp

@@ -10,19 +10,14 @@
 
 namespace JS::Bytecode {
 
-NonnullOwnPtr<BasicBlock> BasicBlock::create(DeprecatedString name, size_t size)
+NonnullOwnPtr<BasicBlock> BasicBlock::create(DeprecatedString name)
 {
-    return adopt_own(*new BasicBlock(move(name), max(size, static_cast<size_t>(4 * KiB))));
+    return adopt_own(*new BasicBlock(move(name)));
 }
 
-BasicBlock::BasicBlock(DeprecatedString name, size_t size)
+BasicBlock::BasicBlock(DeprecatedString name)
     : m_name(move(name))
 {
-    // FIXME: This is not the smartest solution ever. Find something cleverer!
-    // The main issue we're working around here is that we don't want pointers into the bytecode stream to become invalidated
-    // during code generation due to dynamic buffer resizing. Otherwise we could just use a Vector.
-    m_buffer_capacity = size;
-    m_buffer = new u8[m_buffer_capacity];
 }
 
 BasicBlock::~BasicBlock()
@@ -33,16 +28,6 @@ BasicBlock::~BasicBlock()
         ++it;
         Instruction::destroy(const_cast<Instruction&>(to_destroy));
     }
-
-    delete[] m_buffer;
-}
-
-void BasicBlock::seal()
-{
-    // FIXME: mprotect the instruction stream as PROT_READ
-    // This is currently not possible because instructions can have destructors (that clean up strings)
-    // Instructions should instead be destructor-less and refer to strings in a string table on the Bytecode::Block.
-    // It also doesn't work because instructions that have String members use RefPtr internally which must be in writable memory.
 }
 
 void BasicBlock::dump(Bytecode::Executable const& executable) const
@@ -58,8 +43,7 @@ void BasicBlock::dump(Bytecode::Executable const& executable) const
 
 void BasicBlock::grow(size_t additional_size)
 {
-    m_buffer_size += additional_size;
+    m_buffer.resize(m_buffer.size() + additional_size);
-    VERIFY(m_buffer_size <= m_buffer_capacity);
 }
 
 }

Userland/Libraries/LibJS/Bytecode/BasicBlock.h

@@ -27,33 +27,28 @@ class BasicBlock {
     AK_MAKE_NONCOPYABLE(BasicBlock);
 
 public:
-    static NonnullOwnPtr<BasicBlock> create(DeprecatedString name, size_t size = 4 * KiB);
+    static NonnullOwnPtr<BasicBlock> create(DeprecatedString name);
     ~BasicBlock();
 
-    void seal();
-
     void dump(Executable const&) const;
-    ReadonlyBytes instruction_stream() const { return ReadonlyBytes { m_buffer, m_buffer_size }; }
+    ReadonlyBytes instruction_stream() const { return m_buffer.span(); }
-    size_t size() const { return m_buffer_size; }
+    u8* data() { return m_buffer.data(); }
+    u8 const* data() const { return m_buffer.data(); }
+    size_t size() const { return m_buffer.size(); }
 
-    void* next_slot() { return m_buffer + m_buffer_size; }
-    bool can_grow(size_t additional_size) const { return m_buffer_size + additional_size <= m_buffer_capacity; }
     void grow(size_t additional_size);
 
-    void terminate(Badge<Generator>, Instruction const* terminator) { m_terminator = terminator; }
+    void terminate(Badge<Generator>) { m_terminated = true; }
-    bool is_terminated() const { return m_terminator != nullptr; }
+    bool is_terminated() const { return m_terminated; }
-    Instruction const* terminator() const { return m_terminator; }
 
     DeprecatedString const& name() const { return m_name; }
 
 private:
-    BasicBlock(DeprecatedString name, size_t size);
+    explicit BasicBlock(DeprecatedString name);
 
-    u8* m_buffer { nullptr };
+    Vector<u8> m_buffer;
-    Instruction const* m_terminator { nullptr };
-    size_t m_buffer_capacity { 0 };
-    size_t m_buffer_size { 0 };
     DeprecatedString m_name;
+    bool m_terminated { false };
 };
 
 }

Userland/Libraries/LibJS/Bytecode/Generator.cpp

@@ -82,12 +82,6 @@ void Generator::grow(size_t additional_size)
     m_current_basic_block->grow(additional_size);
 }
 
-void* Generator::next_slot()
-{
-    VERIFY(m_current_basic_block);
-    return m_current_basic_block->next_slot();
-}
-
 Register Generator::allocate_register()
 {
     VERIFY(m_next_register != NumericLimits<u32>::max());

Userland/Libraries/LibJS/Bytecode/Generator.h

@@ -34,18 +34,6 @@ public:
 
     Register allocate_register();
 
-    void ensure_enough_space(size_t size)
-    {
-        // Make sure there's always enough space for a single jump at the end.
-        if (!m_current_basic_block->can_grow(size + sizeof(Op::Jump))) {
-            auto& new_block = make_block();
-            emit<Op::Jump>().set_targets(
-                Label { new_block },
-                {});
-            switch_to_basic_block(new_block);
-        }
-    }
-
     class SourceLocationScope {
     public:
         SourceLocationScope(Generator&, ASTNode const& node);
@@ -60,15 +48,12 @@ public:
     OpType& emit(Args&&... args)
     {
         VERIFY(!is_current_block_terminated());
-        // If the block doesn't have enough space, switch to another block
+        size_t slot_offset = m_current_basic_block->size();
-        if constexpr (!OpType::IsTerminator)
-            ensure_enough_space(sizeof(OpType));
-
-        void* slot = next_slot();
         grow(sizeof(OpType));
+        void* slot = m_current_basic_block->data() + slot_offset;
         new (slot) OpType(forward<Args>(args)...);
         if constexpr (OpType::IsTerminator)
-            m_current_basic_block->terminate({}, static_cast<Instruction const*>(slot));
+            m_current_basic_block->terminate({});
         auto* op = static_cast<OpType*>(slot);
         op->set_source_record({ m_current_ast_node->start_offset(), m_current_ast_node->end_offset() });
         return *op;
@@ -80,16 +65,12 @@ public:
         VERIFY(!is_current_block_terminated());
 
         size_t size_to_allocate = round_up_to_power_of_two(sizeof(OpType) + extra_register_slots * sizeof(Register), alignof(void*));
-
+        size_t slot_offset = m_current_basic_block->size();
-        // If the block doesn't have enough space, switch to another block
-        if constexpr (!OpType::IsTerminator)
-            ensure_enough_space(size_to_allocate);
-
-        void* slot = next_slot();
         grow(size_to_allocate);
+        void* slot = m_current_basic_block->data() + slot_offset;
         new (slot) OpType(forward<Args>(args)...);
         if constexpr (OpType::IsTerminator)
-            m_current_basic_block->terminate({}, static_cast<Instruction const*>(slot));
+            m_current_basic_block->terminate({});
         auto* op = static_cast<OpType*>(slot);
         op->set_source_record({ m_current_ast_node->start_offset(), m_current_ast_node->end_offset() });
         return *op;
@@ -238,7 +219,6 @@ private:
     ~Generator() = default;
 
     void grow(size_t);
-    void* next_slot();
 
     struct LabelableScope {
         Label bytecode_target;