From 8d37be29167049a5ed6de6e425d0bf0da367c50a Mon Sep 17 00:00:00 2001
From: ridiculousfish <rf@fishshell.com>
Date: Sun, 12 Jul 2020 14:42:18 -0700
Subject: [PATCH] ast lists to use new[] instead of vector

Because the list is not changed after construction, we do not need
the vector's capacity field. This reduces the size of lists from 48
to 32 bytes.
---
 src/ast.cpp | 22 +++++++++++++++++++++-
 src/ast.h   | 26 ++++++++++++++++++--------
 2 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/src/ast.cpp b/src/ast.cpp
index a73b45b5d..0d1685e6a 100644
--- a/src/ast.cpp
+++ b/src/ast.cpp
@@ -861,6 +861,8 @@ class ast_t::populator_t {
     // If exhaust_stream is set, then keep going until we get parse_token_type_t::terminate.
     template <type_t ListType, typename ContentsNode>
     void populate_list(list_t<ListType, ContentsNode> &list, bool exhaust_stream = false) {
+        assert(list.contents == nullptr && "List is not initially empty");
+
         // Do not attempt to parse a list if we are unwinding.
         if (is_unwinding()) {
             assert(!exhaust_stream &&
@@ -873,6 +875,10 @@ class ast_t::populator_t {
             return;
         }
 
+        // We're going to populate a vector with our nodes.
+        // Later on we will copy this to the heap with a single allocation.
+        std::vector<std::unique_ptr<ContentsNode>> contents;
+
         for (;;) {
             // If we are unwinding, then either we recover or we break the loop, dependent on the
             // loop type.
@@ -900,7 +906,7 @@ class ast_t::populator_t {
 
             // Now try parsing a node.
             if (auto node = this->try_parse<ContentsNode>()) {
-                list.contents.push_back(std::move(node));
+                contents.push_back(std::move(node));
             } else if (exhaust_stream && peek_type() != parse_token_type_t::terminate) {
                 // We aren't allowed to stop. Produce an error and keep going.
                 consume_excess_token_generating_error();
@@ -911,6 +917,20 @@ class ast_t::populator_t {
             }
         }
 
+        // Populate our list from our contents.
+        if (!contents.empty()) {
+            assert(contents.size() <= UINT32_MAX && "Contents size out of bounds");
+            assert(list.contents == nullptr && "List should still be empty");
+
+            // We're going to heap-allocate our array.
+            using contents_ptr_t = typename list_t<ListType, ContentsNode>::contents_ptr_t;
+            contents_ptr_t *array = new contents_ptr_t[contents.size()];
+            std::move(contents.begin(), contents.end(), array);
+
+            list.length = static_cast<uint32_t>(contents.size());
+            list.contents = array;
+        }
+
         FLOGF(ast_construction, L"%*s%ls size: %lu", spaces(), "", ast_type_to_string(ListType),
               (unsigned long)list.count());
     }
diff --git a/src/ast.h b/src/ast.h
index 9d92fc569..f5eba9395 100644
--- a/src/ast.h
+++ b/src/ast.h
@@ -352,8 +352,9 @@ struct list_t : public node_t {
     // This enables more natural iteration:
     //    for (const argument_t &arg : argument_list) ...
     struct contents_ptr_t {
-        std::unique_ptr<ContentsNode> ptr;
-        /* implicit */ contents_ptr_t(std::unique_ptr<ContentsNode> v) : ptr(std::move(v)) {}
+        std::unique_ptr<ContentsNode> ptr{};
+
+        void operator=(std::unique_ptr<ContentsNode> p) { ptr = std::move(p); }
 
         const ContentsNode *get() const {
             assert(ptr && "Null pointer");
@@ -362,7 +363,11 @@ struct list_t : public node_t {
 
         /* implicit */ operator const ContentsNode &() const { return *get(); }
     };
-    std::vector<contents_ptr_t> contents{};
+
+    // We use a new[]-allocated array to store our contents pointers, to reduce size.
+    // This would be a nice use case for std::dynarray.
+    uint32_t length{0};
+    const contents_ptr_t *contents{};
 
     /// \return a node at a given index, or nullptr if out of range.
     const ContentsNode *at(size_t idx, bool reverse = false) const {
@@ -371,15 +376,15 @@ struct list_t : public node_t {
     }
 
     /// \return our count.
-    size_t count() const { return contents.size(); }
+    size_t count() const { return length; }
 
     /// \return whether we are empty.
-    bool empty() const { return contents.size() == 0; }
+    bool empty() const { return length == 0; }
 
     /// Iteration support.
-    using iterator = typename decltype(contents)::const_iterator;
-    iterator begin() const { return contents.begin(); }
-    iterator end() const { return contents.end(); }
+    using iterator = const contents_ptr_t *;
+    iterator begin() const { return contents; }
+    iterator end() const { return contents + length; }
 
     // list types pretend their child nodes are direct embeddings.
     // This isn't used during AST construction because we need to construct the list.
@@ -392,6 +397,11 @@ struct list_t : public node_t {
     }
 
     list_t() : node_t(ListType, Category) {}
+    ~list_t() override { delete[] contents; }
+
+    // Disallow moving as we own a raw pointer.
+    list_t(list_t &&) = delete;
+    void operator=(list_t &&) = delete;
 };
 
 // Fully define all list types, as they are very uniform.