From 84b53b4cae30ad2078a980a995f28f648051b6b0 Mon Sep 17 00:00:00 2001
From: Mahmoud Al-Qudsi <mqudsi@neosmart.net>
Date: Mon, 24 Oct 2022 21:04:17 -0500
Subject: [PATCH] Significantly reduce size of `block_t`

A `block_t` instance is allocated for each live block type in memory when
executing a script or snippet of fish code. While many of the items in a
`block_t` class are specific to a particular type of block, the overhead of
`maybe_t<event_t>` that's unused except in the relatively extremely rare case of
an event block is more significant than the rest, given that 88 out of the 216
bytes of a `block_t` are set aside for this field that is rarely used.

This patch reorders the `block_t` members by order of decreasing alignment,
bringing down the size to 208 bytes, then changes `maybe_t<event_t>` to
`shared_ptr<event_t>` instead of allocating room for the event on the stack.
This brings down the runtime memory size of a `block_t` to 136 bytes for a 37%
reduction in size.

I would like to investigate using inheritance and virtual methods to have a
`block_t` only include the values that actually make sense for the block rather
than always allocating some sort of storage for them and then only sometimes
using it. In addition to further reducing the memory, I think this could also be
a safer and saner approach overall, as it would make it very clear when and
where we can expect each block_type_type_t-dependent member to be present and
hold a value.
---
 src/parser.cpp |  2 +-
 src/parser.h   | 41 ++++++++++++++++++++++++-----------------
 2 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/src/parser.cpp b/src/parser.cpp
index 5d382d3fd..419f7357d 100644
--- a/src/parser.cpp
+++ b/src/parser.cpp
@@ -740,7 +740,7 @@ block_t block_t::if_block() { return block_t(block_type_t::if_block); }
 
 block_t block_t::event_block(event_t evt) {
     block_t b{block_type_t::event};
-    b.event = std::move(evt);
+    b.event.reset(new event_t(std::move(evt)));
     return b;
 }
 
diff --git a/src/parser.h b/src/parser.h
index a6cad3b48..de2e11ef9 100644
--- a/src/parser.h
+++ b/src/parser.h
@@ -38,7 +38,7 @@ inline bool event_block_list_blocks_type(const event_blockage_list_t &ebls) {
 }
 
 /// Types of blocks.
-enum class block_type_t {
+enum class block_type_t : uint16_t {
     while_block,              /// While loop block
     for_block,                /// For loop block
     if_block,                 /// If block
@@ -63,40 +63,47 @@ enum class loop_status_t {
 
 /// block_t represents a block of commands.
 class block_t {
+   private:
     /// Construct from a block type.
     explicit block_t(block_type_t t);
 
-    /// Type of block.
-    const block_type_t block_type;
-
    public:
-    /// Name of file that created this block.
-    filename_ref_t src_filename{};
-    /// Line number where this block was created.
-    int src_lineno{0};
-    /// Whether we should pop the environment variable stack when we're popped off of the block
-    /// stack.
-    bool wants_pop_env{false};
+    // If this is a function block, the function name. Otherwise empty.
+    wcstring function_name{};
+
     /// List of event blocks.
     event_blockage_list_t event_blocks{};
 
-    // If this is a function block, the function name and arguments.
-    // Otherwise empty.
-    wcstring function_name{};
+    // If this is a function block, the function args. Otherwise empty.
     wcstring_list_t function_args{};
 
+    /// Name of file that created this block.
+    filename_ref_t src_filename{};
+
+    // If this is an event block, the event. Otherwise ignored.
+    std::shared_ptr<event_t> event;
+
     // If this is a source block, the source'd file, interned.
     // Otherwise nothing.
     filename_ref_t sourced_file{};
 
-    // If this is an event block, the event. Otherwise ignored.
-    maybe_t<event_t> event;
+    /// Line number where this block was created.
+    int src_lineno{0};
 
-    block_type_t type() const { return this->block_type; }
+   private:
+    /// Type of block.
+    const block_type_t block_type;
+
+   public:
+    /// Whether we should pop the environment variable stack when we're popped off of the block
+    /// stack.
+    bool wants_pop_env{false};
 
     /// Description of the block, for debugging.
     wcstring description() const;
 
+    block_type_t type() const { return this->block_type; }
+
     /// \return if we are a function call (with or without shadowing).
     bool is_function_call() const {
         return type() == block_type_t::function_call ||