Optimize some fast paths in autoload loading. Use an iterator to avoid

doing multiple set lookups, and cache the tokenized path to avoid
multiple memory allocations.

2 files changed, 17 insertions, 20 deletions

diff --git a/autoload.cpp b/autoload.cpp
index e5d2334d..e2e532de 100644
--- a/autoload.cpp
+++ b/autoload.cpp
@@ -48,9 +48,7 @@ autoload_t::autoload_t(const wcstring &env_var_name_var, const builtin_script_t
     lock(),
     env_var_name(env_var_name_var),
     builtin_scripts(scripts),
-    builtin_script_count(script_count),
-    last_path(),
-    is_loading_set()
+    builtin_script_count(script_count)
 {
     pthread_mutex_init(&lock, NULL);
 }
@@ -94,33 +92,34 @@ int autoload_t::load(const wcstring &cmd, bool reload)
     if (path_var != this->last_path)
     {
         this->last_path = path_var;
+        this->last_path_tokenized.clear();
+        tokenize_variable_array(this->last_path, this->last_path_tokenized);
+        
         scoped_lock locker(lock);
         this->evict_all_nodes();
     }
 
+    /* Mark that we're loading this. Hang onto the iterator for fast erasing later. Note that std::set has guarantees about not invalidating iterators, so this is safe to do across the callouts below.  */
+    typedef std::set<wcstring>::iterator set_iterator_t;
+    std::pair<set_iterator_t, bool> insert_result = is_loading_set.insert(cmd);
+    set_iterator_t where = insert_result.first;
+    bool inserted = insert_result.second;
+
     /** Warn and fail on infinite recursion. It's OK to do this because this function is only called on the main thread. */
-    if (this->is_loading(cmd))
+    if (! inserted)
     {
+        /* We failed to insert */
         debug(0,
               _(L"Could not autoload item '%ls', it is already being autoloaded. "
                 L"This is a circular dependency in the autoloading scripts, please remove it."),
               cmd.c_str());
         return 1;
     }
-
-    /* Mark that we're loading this */
-    is_loading_set.insert(cmd);
-
-    /* Get the list of paths from which we will try to load */
-    std::vector<wcstring> path_list;
-    tokenize_variable_array(path_var, path_list);
-
     /* Try loading it */
-    res = this->locate_file_and_maybe_load_it(cmd, true, reload, path_list);
+    res = this->locate_file_and_maybe_load_it(cmd, true, reload, this->last_path_tokenized);
 
     /* Clean up */
-    bool erased = !! is_loading_set.erase(cmd);
-    assert(erased);
+    is_loading_set.erase(where);
 
     return res;
 }
diff --git a/autoload.h b/autoload.h
index 6c17c0b7..2f1e00d3 100644
--- a/autoload.h
+++ b/autoload.h
@@ -64,6 +64,9 @@ private:
 
     /** The path from which we most recently autoloaded */
     wcstring last_path;
+    
+    /** That path, tokenized (split on separators) */
+    wcstring_list_t last_path_tokenized;
 
     /**
        A table containing all the files that are currently being
@@ -71,11 +74,6 @@ private:
     */
     std::set<wcstring> is_loading_set;
 
-    bool is_loading(const wcstring &name) const
-    {
-        return is_loading_set.find(name) != is_loading_set.end();
-    }
-
     void remove_all_functions(void)
     {
         this->evict_all_nodes();