From cdce8511a14e0a84f3796cce6889c3ce005d40b1 Mon Sep 17 00:00:00 2001
From: Mahmoud Al-Qudsi <mqudsi@neosmart.net>
Date: Sat, 13 Apr 2019 11:26:10 -0500
Subject: [PATCH] Optimize function calls by reducing inherit vars heap
 allocations and copies

* Convert `function_get_inherit_vars()` to return a reference to the
  (possibly) existing map, rather than a copy;
* Preallocate and reuse a static (read-only) map for the (very) common
  case of no inherited vars;
* Pass references to the inherit vars map around thereafter, never
  triggering the map copy (or even move) constructor.

NB: If it turns out the reference is unsafe, we can switch the inherit vars
to be a shared_ptr and return that instead.
---
 src/builtin_functions.cpp | 2 +-
 src/exec.cpp              | 2 +-
 src/function.cpp          | 6 ++++--
 src/function.h            | 2 +-
 4 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/builtin_functions.cpp b/src/builtin_functions.cpp
index 91b334e6e..8ae6b8880 100644
--- a/src/builtin_functions.cpp
+++ b/src/builtin_functions.cpp
@@ -199,7 +199,7 @@ static wcstring functions_def(const wcstring &name) {
     }
 
     // Output any inherited variables as `set -l` lines.
-    std::map<wcstring, env_var_t> inherit_vars = function_get_inherit_vars(name);
+    const std::map<wcstring, env_var_t> &inherit_vars = function_get_inherit_vars(name);
     for (const auto &kv : inherit_vars) {
         wcstring_list_t lst;
         kv.second.to_list(lst);
diff --git a/src/exec.cpp b/src/exec.cpp
index 70f50c0f2..da1b5aa44 100644
--- a/src/exec.cpp
+++ b/src/exec.cpp
@@ -813,7 +813,7 @@ static bool exec_block_or_func_process(parser_t &parser, std::shared_ptr<job_t>
             return false;
         }
 
-        const std::map<wcstring, env_var_t> inherit_vars = function_get_inherit_vars(func_name);
+        const std::map<wcstring, env_var_t> &inherit_vars = function_get_inherit_vars(func_name);
 
         function_block_t *fb =
             parser.push_block<function_block_t>(p, func_name, props->shadow_scope);
diff --git a/src/function.cpp b/src/function.cpp
index 1f43efd54..6fb2041c5 100644
--- a/src/function.cpp
+++ b/src/function.cpp
@@ -257,10 +257,12 @@ bool function_get_definition(const wcstring &name, wcstring &out_definition) {
     return func != NULL;
 }
 
-std::map<wcstring, env_var_t> function_get_inherit_vars(const wcstring &name) {
+const std::map<wcstring, env_var_t> &function_get_inherit_vars(const wcstring &name) {
+    static const std::map<wcstring, env_var_t> empty_inherit_vars;
+
     scoped_rlock locker(functions_lock);
     const function_info_t *func = function_get(name);
-    return func ? func->inherit_vars : std::map<wcstring, env_var_t>();
+    return func ? func->inherit_vars : empty_inherit_vars;
 }
 
 bool function_get_desc(const wcstring &name, wcstring &out_desc) {
diff --git a/src/function.h b/src/function.h
index fc6529e72..5440a8bcf 100644
--- a/src/function.h
+++ b/src/function.h
@@ -102,7 +102,7 @@ int function_get_definition_lineno(const wcstring &name);
 
 /// Returns a mapping of all variables of the specified function that were inherited from the scope
 /// of the function definition to their values.
-std::map<wcstring, env_var_t> function_get_inherit_vars(const wcstring &name);
+const std::map<wcstring, env_var_t> &function_get_inherit_vars(const wcstring &name);
 
 /// Creates a new function using the same definition as the specified function. Returns true if copy
 /// is successful.