More work on LRU cache and adopting it in function and completion autoloading

autoload.cpp

@@ -13,8 +13,6 @@ The classes responsible for autoloading functions and completions.
 #include "exec.h"
 #include <assert.h>
 
-static const size_t kLRULimit = 16;
-
 file_access_attempt_t access_file(const wcstring &path, int mode) {
     file_access_attempt_t result = {0};
     struct stat statbuf;
@@ -35,7 +33,7 @@ file_access_attempt_t access_file(const wcstring &path, int mode) {
     return result;
 }
 
-lru_cache_impl_t::lru_cache_impl_t() : node_count(0), mouth(L"") {
+lru_cache_impl_t::lru_cache_impl_t(size_t size) : max_node_count(size), node_count(0), mouth(L"") {
     /* Hook up the mouth to itself: a one node circularly linked list */
     mouth.prev = mouth.next = &mouth;
 }
@@ -87,6 +85,7 @@ void lru_cache_impl_t::promote_node(lru_node_t *node) {
     
     /* Put us after the mouth */
     node->next = mouth.next;
+    node->next->prev = node;
     node->prev = &mouth;
     mouth.next = node;
 }
@@ -100,6 +99,7 @@ bool lru_cache_impl_t::add_node(lru_node_t *node) {
     
     /* Add the node after the mouth */
     node->next = mouth.next;
+    node->next->prev = node;
     node->prev = &mouth;
     mouth.next = node;
     
@@ -107,7 +107,7 @@ bool lru_cache_impl_t::add_node(lru_node_t *node) {
     node_count++;
     
     /* Evict */
-    while (node_count > kLRULimit)
+    while (node_count > max_node_count)
         evict_last_node();
     
     /* Success */
@@ -147,17 +147,12 @@ autoload_t::autoload_t(const wcstring &env_var_name_var, const builtin_script_t
 }
 
 void autoload_t::node_was_evicted(autoload_function_t *node) {
-    // Tell ourselves that the command was removed, unless it was a placeholder
+    // Tell ourselves that the command was removed if it was loaded
-    if (! node->is_placeholder)
+    if (! node->is_loaded)
         this->command_removed(node->key);
     delete node;
 }
 
-void autoload_t::reset( )
-{
-    this->evict_all_nodes();
-}
-
 int autoload_t::unload( const wcstring &cmd )
 {
     return this->evict_node(cmd);
@@ -166,19 +161,16 @@ int autoload_t::unload( const wcstring &cmd )
 int autoload_t::load( const wcstring &cmd, bool reload )
 {
 	int res;
-	int c, c2;
 	
 	CHECK_BLOCK( 0 );
 
-	const env_var_t path_var = env_get_string( env_var_name.c_str() );	
+	const env_var_t path_var = env_get_string( env_var_name.c_str() );
 	
 	/*
 	  Do we know where to look?
 	*/
 	if( path_var.empty() )
-	{
 		return 0;
-	}
     
     /*
       Check if the lookup path has changed. If so, drop all loaded
@@ -187,7 +179,7 @@ int autoload_t::load( const wcstring &cmd, bool reload )
     if( path_var != this->path )
     {
         this->path = path_var;
-        this->reset();
+        this->evict_all_nodes();
     }
 
     /**
@@ -203,70 +195,67 @@ int autoload_t::load( const wcstring &cmd, bool reload )
     }
 
 
-
     std::vector<wcstring> path_list;
 	tokenize_variable_array2( path_var, path_list );
 	
-	c = path_list.size();
-	
     is_loading_set.insert(cmd);
 
 	/*
 	  Do the actual work in the internal helper function
 	*/
-	res = this->load_internal( cmd, reload, path_list );
+	res = this->locate_file_and_maybe_load_it( cmd, true, reload, path_list );
     
     int erased = is_loading_set.erase(cmd);
     assert(erased);
-	
+		
-	c2 = path_list.size();
-
-	/**
-	   Make sure we didn't 'drop' something
-	*/
-	
-	assert( c == c2 );
-	
 	return res;
 }
 
+bool autoload_t::can_load( const wcstring &cmd )
+{
+    const env_var_t path_var = env_get_string( env_var_name.c_str() );
+    std::vector<wcstring> path_list;
+	tokenize_variable_array2( path_var, path_list );
+    return this->locate_file_and_maybe_load_it( cmd, false, false, path_list );
+}
+
 static bool script_name_precedes_script_name(const builtin_script_t &script1, const builtin_script_t &script2)
 {
     return wcscmp(script1.name, script2.name) < 0;
 }
 
+void autoload_t::unload_all(void) {
+    this->evict_all_nodes();
+}
+
 /**
    This internal helper function does all the real work. By using two
    functions, the internal function can return on various places in
    the code, and the caller can take care of various cleanup work.
 */
 
-int autoload_t::load_internal( const wcstring &cmd,
+bool autoload_t::locate_file_and_maybe_load_it( const wcstring &cmd, bool really_load, bool reload, const wcstring_list_t &path_list )
-                               int reload,
-                               const wcstring_list_t &path_list )
 {
 
 	size_t i;
-	int reloaded = 0;
+	bool reloaded = 0;
 
     /* Get the function */
-    autoload_function_t * func = this->get_function_with_name(cmd);
+    autoload_function_t * func = this->get_node(cmd);
 
     /* Return if already loaded and we are skipping reloading */
-	if( !reload && func )
+	if( !reload && func && func->is_loaded )
 		return 0;
 
     /* Nothing to do if we just checked it */
-    if (func && time(NULL) - func->access.last_checked <= 1)
+    if (func && func->is_loaded && time(NULL) - func->access.last_checked <= 1)
         return 0;
     
     /* The source of the script will end up here */
     wcstring script_source;
     bool has_script_source = false;
     
-    /*
+    /* Look for built-in scripts via a binary search */
-     Look for built-in scripts via a binary search
-    */
     const builtin_script_t *matching_builtin_script = NULL;
     if (builtin_script_count > 0)
     {
@@ -286,9 +275,7 @@ int autoload_t::load_internal( const wcstring &cmd,
     
     if (! has_script_source)
     {
-        /*
+        /* Iterate over path searching for suitable completion files */
-          Iterate over path searching for suitable completion files
-        */
         for( i=0; i<path_list.size(); i++ )
         {
             wcstring next = path_list.at(i);
@@ -301,14 +288,23 @@ int autoload_t::load_internal( const wcstring &cmd,
                     script_source = L". " + esc;
                     has_script_source = true;
                     
-                    if( !func )
+                    /* Remove this command because we are going to reload it */
+                    if (func && func->is_loaded) {
+                        command_removed(cmd);
+                        func->is_loaded = false;
+                        func->is_placeholder = false;
+                    }
+
+                    if (!func) {
                         func = new autoload_function_t(cmd);
-                    func->access = access;
+                        this->add_node(func);
+                        
+                    }
                     
-                    // Remove this command because we are going to reload it
+                    /* Record our access time */
-                    command_removed(cmd);
+                    func->access = access;
-                                        
+                                                            
-                    reloaded = 1;
+                    reloaded = true;
                 }
                 else if( func )
                 {
@@ -333,12 +329,14 @@ int autoload_t::load_internal( const wcstring &cmd,
             func = new autoload_function_t(cmd);
             func->access.last_checked = time(NULL);
             func->is_placeholder = true;
+            this->add_node(func);
         }
     }
     
     /* If we have a script, either built-in or a file source, then run it */
-    if (has_script_source)
+    if (really_load && has_script_source)
     {
+        if (func) func->is_loaded = true;
         if( exec_subshell( script_source.c_str(), 0 ) == -1 )
         {
             /*

autoload.h

@@ -49,8 +49,11 @@ private:
     void evict_node(lru_node_t *node);
     void evict_last_node(void);
     
+    /** Max node count */
+    const size_t max_node_count;
+    
     /** Count of nodes */
-    unsigned int node_count;
+    size_t node_count;
     
     /** The set of nodes */
     typedef std::set<lru_node_t *, dereference_less_t> node_set_t;
@@ -65,7 +68,7 @@ protected:
     
 public:
     /** Constructor */
-    lru_cache_impl_t();
+    lru_cache_impl_t(size_t max_size = 1024 );
     
     /** Returns the node for a given key, or NULL */
     lru_node_t *get_node(const wcstring &key);
@@ -76,6 +79,9 @@ public:
     /** Adds a node under the given key. Returns true if the node was added, false if the node was not because a node with that key is already in the set. */
     bool add_node(lru_node_t *node);
     
+    /** Counts nodes */
+    size_t size(void) { return node_count; }
+    
     /** Evicts all nodes */
     void evict_all_nodes(void);
 };
@@ -84,19 +90,17 @@ public:
 template<class node_type_t>
 class lru_cache_t : public lru_cache_impl_t {
 public:
-    node_type_t *get_node(const wcstring &key) { return static_cast<node_type_t>(lru_cache_impl_t::get_node(key)); }
+    node_type_t *get_node(const wcstring &key) { return static_cast<node_type_t *>(lru_cache_impl_t::get_node(key)); }
     bool add_node(node_type_t *node) { return lru_cache_impl_t::add_node(node); }
-protected:
+    lru_cache_t(size_t max_size = 1024 ) : lru_cache_impl_t(max_size) { }
-    virtual void node_was_evicted(lru_node_t *node) { this->node_was_evicted(static_cast<node_type_t *>(node)); }
-    virtual void node_was_evicted(node_type_t *node) { }
 };
 
-
 struct autoload_function_t : public lru_node_t
 {   
-    autoload_function_t(const wcstring &key) : lru_node_t(key), is_placeholder(false) { bzero(&access, sizeof access); }
+    autoload_function_t(const wcstring &key) : lru_node_t(key), is_loaded(false), is_placeholder(false) { bzero(&access, sizeof access); }
     file_access_attempt_t access; /** The last access attempt */
-    bool is_placeholder; /** Whether we are a placeholder that stands in for "no such function" */
+    bool is_loaded; /** Whether we have actually loaded this function */
+    bool is_placeholder; /** Whether we are a placeholder that stands in for "no such function". If this is true, then is_loaded must be false. */
 };
 
 
@@ -119,10 +123,6 @@ private:
 
     /** The path from which we most recently autoloaded */
     wcstring path;
-    
-    /** The map from function name to the function. */
-    typedef std::map<wcstring, autoload_function_t> autoload_functions_map_t;
-    autoload_functions_map_t autoload_functions;
 
 	/**
 	   A table containing all the files that are currently being
@@ -133,29 +133,12 @@ private:
     bool is_loading(const wcstring &name) const {
         return is_loading_set.find(name) != is_loading_set.end();
     }
-
-    bool remove_function_with_name(const wcstring &name) {
-        return autoload_functions.erase(name) > 0;
-    }
-    
-    autoload_function_t *get_function_with_name(const wcstring &name)
-    {
-        autoload_function_t *result = NULL;
-        autoload_functions_map_t::iterator iter = autoload_functions.find(name);
-        if (iter != autoload_functions.end())
-            result = &iter->second;
-        return result;
-    }
     
     void remove_all_functions(void) {
-        autoload_functions.clear();
+        this->evict_all_nodes();
-    }
-    
-    size_t function_count(void) const {
-        return autoload_functions.size();
     }
         
-    int load_internal( const wcstring &cmd, int reload, const wcstring_list_t &path_list );
+    bool locate_file_and_maybe_load_it( const wcstring &cmd, bool really_load, bool reload, const wcstring_list_t &path_list );
     
     virtual void node_was_evicted(autoload_function_t *node);
 
@@ -181,12 +164,6 @@ private:
        \param reload wheter to recheck file timestamps on already loaded files
     */
     int load( const wcstring &cmd, bool reload );
-    /**
-       Reset the loader for the specified path variable. This will cause
-       all information on loaded files in the specified directory to be
-       reset.
-    */
-    void reset();
 
     /**
        Tell the autoloader that the specified file, in the specified path,
@@ -197,6 +174,14 @@ private:
        \return non-zero if the file was removed, zero if the file had not yet been loaded
     */
     int unload( const wcstring &cmd );
+    
+    /**
+       Unloads all files.
+    */
+    void unload_all( );
+    
+    /** Check whether the given command could be loaded, but do not load it. */
+    bool can_load( const wcstring &cmd );
 
 };
 

complete.cpp

@@ -249,7 +249,7 @@ static void complete_destroy()
 	}
 	first_entry = 0;
 	
-	completion_autoloader.reset();
+	completion_autoloader.unload_all();
 	
 }
 

reader.cpp

@@ -422,7 +422,7 @@ int reader_exit_forced()
 static void reader_repaint()
 {
     //PCA INSTANCED_PARSER what is this call for?
-	//parser_test( data->buff, data->indent, 0, 0 );
+	parser_t::principal_parser().test( data->buff, data->indent, 0, 0 );
 	
 	s_write( &data->screen,
 		 data->prompt_buff.c_str(),