changeset 32131:fbadf4ce94c7

function cache to improve performance of function resolution * load-path.h, load-path.cc (load_path::signal_clear_fcn_cache, load_path::get_weak_n_updated): New static functions. (load_path::s_n_updated): New static variable. * variables.cc (Fclear): Call signal_clear_fcn_cache. * pt-eval.cc (tree_evaluator::remove_autoload): Call signal_clear_fcn_cache. * ov-base.h, ov-base.cc (octave_base_value::is_function_cache, octave_base_value::is_maybe_function, octave_base_value::has_function_cache, octave_base_value::get_cached_function, octave_base_value::fcn_cache_value): New virtual functions. * ov-fcn-handle.h, ov-fcn-handle.cc (simple_fcn_handle::get_cached_fcn, simple_fcn_handle::has_function_cache): New functions. (simple_fcn_handle::m_cache): New member variable. (octave_fcn_handle::octave_fcn_handle (const std::string&, octave_value)): New constructor. (octave_fcn_handle::get_cached_fcn, octave_fcn_handle::has_fcn_cache): New functions. * ov-fcn.h, ov-fcn.cc (class octave_fcn_cache): New class. (octave_function::is_compiled): New function. * ov.h, ov.cc (octave_value::is_function_cache, octave_value::has_function_cache, octave_value::get_cached_fcn, octave_value::is_maybe_function, octave_value::fcn_cache_value): New functions.
author Petter T. <petter.vilhelm@gmail.com>
date Sat, 17 Jun 2023 10:19:33 -0400
parents dedc746ecd58
children 020dd00fa64f
files libinterp/corefcn/load-path.cc libinterp/corefcn/load-path.h libinterp/corefcn/variables.cc libinterp/octave-value/ov-base-int.h libinterp/octave-value/ov-base-mat.h libinterp/octave-value/ov-base-scalar.h libinterp/octave-value/ov-base.h libinterp/octave-value/ov-fcn-handle.cc libinterp/octave-value/ov-fcn-handle.h libinterp/octave-value/ov-fcn.cc libinterp/octave-value/ov-fcn.h libinterp/octave-value/ov.h libinterp/parse-tree/pt-eval.cc
diffstat 13 files changed, 503 insertions(+), 0 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/corefcn/load-path.cc	Fri Jun 16 15:04:39 2023 -0400
+++ b/libinterp/corefcn/load-path.cc	Sat Jun 17 10:19:33 2023 -0400
@@ -251,6 +251,8 @@
 m_dir_info_list (), m_init_dirs (), m_command_line_path ()
 { }
 
+std::atomic<octave_idx_type> load_path::s_n_updated;
+
 void
 load_path::initialize (bool set_initial_path)
 {
@@ -292,6 +294,8 @@
 void
 load_path::clear ()
 {
+  signal_clear_fcn_cache ();
+
   m_dir_info_list.clear ();
 
   m_top_level_package.clear ();
@@ -415,6 +419,8 @@
   // preserve the correct directory ordering for new files that
   // have appeared.
 
+  signal_clear_fcn_cache ();
+
   m_top_level_package.clear ();
 
   m_package_map.clear ();
--- a/libinterp/corefcn/load-path.h	Fri Jun 16 15:04:39 2023 -0400
+++ b/libinterp/corefcn/load-path.h	Sat Jun 17 10:19:33 2023 -0400
@@ -215,7 +215,15 @@
   static const int OCT_FILE = 2;
   static const int MEX_FILE = 4;
 
+  static octave_idx_type get_weak_n_updated () { return s_n_updated; }
+
+  static void signal_clear_fcn_cache ()
+  {
+    s_n_updated++;
+  }
+
 private:
+  static std::atomic<octave_idx_type> s_n_updated;
 
   class dir_info
   {
--- a/libinterp/corefcn/variables.cc	Fri Jun 16 15:04:39 2023 -0400
+++ b/libinterp/corefcn/variables.cc	Sat Jun 17 10:19:33 2023 -0400
@@ -1235,6 +1235,11 @@
 
   string_vector argv = args.make_argv ("clear");
 
+  // FIXME: This action should probably happen in the functions that
+  // are called below, not here.
+  // Mark any function cache in use by the VM as invalid
+  octave::load_path::signal_clear_fcn_cache ();
+
   if (argc == 1)
     {
       do_clear_variables (interp, argv, argc, true);
--- a/libinterp/octave-value/ov-base-int.h	Fri Jun 16 15:04:39 2023 -0400
+++ b/libinterp/octave-value/ov-base-int.h	Sat Jun 17 10:19:33 2023 -0400
@@ -136,6 +136,8 @@
 
   octave_base_value * try_narrowing_conversion () { return nullptr; }
 
+  bool is_maybe_function (void) const { return false; }
+
   bool isreal () const { return true; }
 
   bool is_real_scalar () const { return true; }
--- a/libinterp/octave-value/ov-base-mat.h	Fri Jun 16 15:04:39 2023 -0400
+++ b/libinterp/octave-value/ov-base-mat.h	Sat Jun 17 10:19:33 2023 -0400
@@ -80,6 +80,7 @@
   octave_value full_value () const { return m_matrix; }
 
   void maybe_economize () { m_matrix.maybe_economize (); }
+  bool is_maybe_function (void) const { return false; }
 
   // We don't need to override all three forms of subsref.  The using
   // declaration will avoid warnings about partially-overloaded virtual
--- a/libinterp/octave-value/ov-base-scalar.h	Fri Jun 16 15:04:39 2023 -0400
+++ b/libinterp/octave-value/ov-base-scalar.h	Sat Jun 17 10:19:33 2023 -0400
@@ -135,6 +135,8 @@
   MatrixType matrix_type (const MatrixType&) const
   { return matrix_type (); }
 
+  bool is_maybe_function (void) const { return false; }
+
   bool is_scalar_type () const { return true; }
 
   bool isnumeric () const { return true; }
--- a/libinterp/octave-value/ov-base.h	Fri Jun 16 15:04:39 2023 -0400
+++ b/libinterp/octave-value/ov-base.h	Sat Jun 17 10:19:33 2023 -0400
@@ -78,6 +78,7 @@
 class octave_user_code;
 class octave_fcn_handle;
 class octave_value_list;
+class octave_fcn_cache;
 
 enum builtin_type_t
 {
@@ -529,6 +530,19 @@
 
   virtual bool is_mex_function () const { return false; }
 
+  virtual bool is_function_cache (void) const { return false; }
+
+  // Checks if the ov could be a function. If it is undefined,
+  // the name associated with the ov could be a function to call.
+  virtual bool is_maybe_function (void) const
+  { return !is_defined () || is_function (); }
+
+  virtual bool has_function_cache (void) const { return false; }
+
+  virtual octave_function * get_cached_fcn (const octave_value_list&) { return nullptr; }
+
+  virtual octave_fcn_cache * fcn_cache_value (void) { return nullptr; }
+
   virtual void erase_subfunctions () { }
 
   virtual short int short_value (bool = false, bool = false) const;
--- a/libinterp/octave-value/ov-fcn-handle.cc	Fri Jun 16 15:04:39 2023 -0400
+++ b/libinterp/octave-value/ov-fcn-handle.cc	Sat Jun 17 10:19:33 2023 -0400
@@ -229,9 +229,17 @@
   friend bool is_equal_to (const simple_fcn_handle& fh1,
                            const simple_fcn_handle& fh2);
 
+  octave_function *
+  get_cached_fcn (const octave_value_list &args);
+
+  bool has_function_cache (void) const;
+
 private:
 
   octave_value m_fcn;
+
+  // Only used by the VM via get_cached_fcn() and has_function_cache()
+  octave_fcn_cache m_cache;
 };
 
 class scoped_fcn_handle : public base_fcn_handle
@@ -674,6 +682,15 @@
 
   bool parse (const std::string& fcn_text);
 
+  octave_function *
+  get_cached_fcn (const octave_value_list&) { return m_fcn.function_value (); }
+  // TODO: This is a hack to get uncompiled anonymous functions to be subsrefed in the VM
+  bool has_function_cache (void) const 
+  { 
+    octave_function *fn = m_fcn.function_value ();
+    return fn ? fn->is_compiled () : false; 
+  }
+
 protected:
 
   // The function we are handling.
@@ -942,6 +959,205 @@
     return false;
 }
 
+// FIXME: Find a way to avoid duplication of code in
+// simple_fcn_handle::call
+octave_function *
+simple_fcn_handle::
+get_cached_fcn (const octave_value_list &args) 
+{
+  if (m_cache.has_cached_function (args))
+    return m_cache.get_cached_fcn ();
+
+  {
+    // The lookup is done like in call()
+    interpreter& interp = __get_interpreter__ ();
+    symbol_table& symtab = interp.get_symbol_table ();
+
+    octave_value fcn_to_call;
+    octave_value ov_fcn = symtab.find_function (m_name, args);
+
+    if (m_fcn.is_defined ())
+      {
+        // A simple function was found when the handle was created.
+        // Use that unless we find a class method to override it.
+
+        fcn_to_call = m_fcn;
+
+        if (ov_fcn.is_defined ())
+          {
+            octave_function *fcn = ov_fcn.function_value ();
+
+            std::string dispatch_class = fcn->dispatch_class ();
+
+            if (fcn->is_class_method ())
+              {
+                // Function found through lookup is a class method
+                // so use it instead of the simple one found when
+                // the handle was created.
+
+                fcn_to_call = ov_fcn;
+              }
+          }
+      }
+    else
+      {
+        // There was no simple function found when the handle was
+        // created so use the one found here (if any).
+
+        fcn_to_call = ov_fcn;
+      }
+
+
+    if (! fcn_to_call.is_defined ())
+      err_invalid_fcn_handle (m_name);
+
+    m_cache.set_cached_function (fcn_to_call, args, 0);
+
+    return fcn_to_call.function_value ();
+  }
+}
+
+// FIXME: Find a way to avoid duplication of code in
+// simple_fcn_handle::call
+// Like call(), but instead returns true if the call() would end
+// up with another call(), or false if there would be a subsref()
+// or an error on the path to subsref() call.
+bool
+simple_fcn_handle::has_function_cache () const
+{
+  // FIXME: if m_name has a '.' in the name, lookup first component.  If
+  // it is a classdef meta object, then build TYPE and IDX arguments and
+  // make a subsref call using them.
+
+  interpreter& interp = __get_interpreter__ ();
+
+  octave_value fcn_to_call;
+
+  // The following code is similar to part of
+  // tree_evaluator::visit_index_expression but simpler because it
+  // handles a more restricted case.
+
+  symbol_table& symtab = interp.get_symbol_table ();
+
+  std::size_t pos = m_name.find ('.');
+
+  if (pos != std::string::npos)
+    {
+      // FIXME: check to see which of these cases actually work in
+      // Octave and Matlab.  For the last two, assume handle is
+      // created before object is defined as an object.
+      //
+      // We can have one of
+      //
+      //   pkg-list . fcn  (args)
+      //   pkg-list . cls . meth (args)
+      //   class-name . method  (args)
+      //   class-name . static-method  (args)
+      //   object . method  (args)
+      //   object . static-method  (args)
+
+      // Evaluate package elements until we find a function,
+      // classdef object, or classdef_meta object that is not a
+      // package.  An object may only appear as the first element,
+      // then it must be followed directly by a function name.
+
+      std::size_t beg = 0;
+      std::size_t end = pos;
+
+      std::vector<std::string> idx_elts;
+
+      while (true)
+        {
+          end = m_name.find ('.', beg);
+
+          idx_elts.push_back (m_name.substr (beg, end-beg));
+
+          if (end == std::string::npos)
+            break;
+
+          beg = end+1;
+        }
+
+      std::size_t n_elts = idx_elts.size ();
+
+      bool have_object = false;
+      octave_value partial_expr_val;
+
+      // Lazy evaluation.  The first element was not known to be defined
+      // as an object in the scope where the handle was created.  See if
+      // there is a definition in the current scope.
+
+      partial_expr_val = interp.varval (idx_elts[0]);
+
+      if (partial_expr_val.is_defined ())
+        {
+          if (! partial_expr_val.is_classdef_object () || n_elts != 2)
+            return false;
+
+          have_object = true;
+        }
+      else
+        partial_expr_val = symtab.find_function (idx_elts[0], ovl ());
+
+      std::string type;
+      std::list<octave_value_list> arg_list;
+
+      for (std::size_t i = 1; i < n_elts; i++)
+        {
+          if (partial_expr_val.is_package ())
+            {
+              if (have_object)
+                return false;
+
+              type = ".";
+              arg_list.push_back (ovl (idx_elts[i]));
+
+              try
+                {
+                  // Silently ignore extra output values.
+
+                  octave_value_list tmp_list
+                    = partial_expr_val.subsref (type, arg_list, 0);
+
+                  partial_expr_val
+                    = tmp_list.length () ? tmp_list(0) : octave_value ();
+
+                  if (partial_expr_val.is_cs_list ())
+                    return false;
+
+                  arg_list.clear ();
+                }
+              catch (const index_exception&)
+                {
+                  return false;
+                }
+            }
+          else if (have_object || partial_expr_val.is_classdef_meta ())
+            {
+              // Object or class name must be the next to the last
+              // element (it was the previous one, so if this is the
+              // final element, it should be a classdef method,
+              // but we'll let the classdef or classdef_meta subsref
+              // function sort that out.
+              return false;
+            }
+          else
+            return false;
+        }
+
+      // If we get here, we must have a function to call.
+
+      if (! partial_expr_val.is_function ())
+        return false;
+
+      return true;
+    }
+  else
+    {
+      return true;
+    }
+}
+
 octave_value_list
 simple_fcn_handle::call (int nargout, const octave_value_list& args)
 {
--- a/libinterp/octave-value/ov-fcn-handle.h	Fri Jun 16 15:04:39 2023 -0400
+++ b/libinterp/octave-value/ov-fcn-handle.h	Sat Jun 17 10:19:33 2023 -0400
@@ -154,6 +154,11 @@
     return false;
   }
 
+  virtual octave_function *
+  get_cached_fcn (const octave_value_list&) { return nullptr; }
+  virtual bool 
+  has_function_cache (void) const { return false; }
+
 protected:
 
   void warn_load (const char *file_type) const;
@@ -221,6 +226,11 @@
                      const std::shared_ptr<octave::stack_frame>& closure_frames
                      = std::shared_ptr<octave::stack_frame> ());
 
+  // Create a simple function handle that is not bound to a function.
+  // Lookup happens when a function call is attempted and the function
+  // lookup is cached in a octave_fcn_cache.
+  octave_fcn_handle (const std::string& name, octave_value cache);
+
   octave_fcn_handle (octave::base_fcn_handle *rep);
 
   octave_fcn_handle (const octave_fcn_handle& fh);
@@ -361,6 +371,10 @@
   friend bool
   is_equal_to (const octave_fcn_handle& fh1, const octave_fcn_handle& fh2);
 
+  octave_function *
+  get_cached_fcn (const octave_value_list& args) { return m_rep->get_cached_fcn (args); }
+  bool has_function_cache (void) const { return m_rep->has_function_cache (); }
+
 private:
 
   std::shared_ptr<octave::base_fcn_handle> m_rep;
--- a/libinterp/octave-value/ov-fcn.cc	Fri Jun 16 15:04:39 2023 -0400
+++ b/libinterp/octave-value/ov-fcn.cc	Sat Jun 17 10:19:33 2023 -0400
@@ -34,6 +34,9 @@
 #include "ov-fcn.h"
 #include "pt-eval.h"
 
+#include "interpreter-private.h"
+#include "symtab.h"
+#include "interpreter.h"
 octave_base_value *
 octave_function::clone () const
 {
@@ -56,3 +59,133 @@
 
   return execute (tw, nargout, args);
 }
+
+
+void
+octave_fcn_cache::set_cached_function (octave_value ov,
+                                       const octave_value_list &args,
+                                       octave_idx_type current_n_updated)
+{
+  clear_cached_function ();
+
+  // We need to keep a reference to the metaobject for as long as the function is alive
+  if (ov.is_classdef_meta ())
+    m_cached_object = ov;
+
+  std::vector<int> v_types;
+
+  for (int i = 0; i < args.length (); i++)
+    {
+      // FIXME: We don't cache methods or functions with class object
+      // arguments. Classes need some kind of unique simple key for this
+      // simple approach.
+      if (args(i).isobject())
+        return;
+
+      v_types.push_back (args (i).type_id ());
+    }
+
+  m_cached_args = v_types;
+  m_cached_function = ov;
+
+  m_n_updated = current_n_updated;
+}
+
+octave_value
+octave_fcn_cache::
+get_cached_obj (const octave_value_list& args)
+{
+  octave_function *fcn = nullptr;
+
+  octave_idx_type current_n_updated = octave::load_path::get_weak_n_updated ();
+  if (has_cached_function (args))
+    {
+      if (m_n_updated == current_n_updated)
+        return m_cached_function;
+      else
+        clear_cached_function ();
+    }
+
+  if (! fcn)
+    {
+      octave::interpreter& interp =
+        octave::__get_interpreter__ ();
+
+      octave::symbol_table& symtab = interp.get_symbol_table ();
+      octave_value val = symtab.find_function (m_fcn_name, args);
+
+      if (val.is_function ())
+        {
+          fcn = val.function_value (true);
+          set_cached_function (val, args, current_n_updated);
+          return val;
+        }
+      
+      val = symtab.find_function (m_fcn_name);
+      if (val.is_function ())
+        {
+          return val;
+        }
+    }
+
+  return {};
+}
+
+octave_function *
+octave_fcn_cache::
+get_cached_fcn (const octave_value_list& args)
+{
+  octave_function *fcn = nullptr;
+
+  octave_idx_type current_n_updated = octave::load_path::get_weak_n_updated ();
+  if (has_cached_function (args))
+    {
+      if (m_n_updated == current_n_updated)
+        return m_cached_function.function_value (true);
+      else
+        clear_cached_function ();
+    }
+
+  if (! fcn)
+    {
+      octave::interpreter& interp =
+        octave::__get_interpreter__ ();
+
+      octave::symbol_table& symtab = interp.get_symbol_table ();
+      octave_value val = symtab.find_function (m_fcn_name, args);
+
+      if (val.is_function ())
+        {
+          fcn = val.function_value (true);
+          set_cached_function (val, args, current_n_updated);
+          return fcn;
+        }
+      
+      val = symtab.find_function (m_fcn_name);
+      if (val.is_function ())
+        {
+          return val.function_value (true);
+        }
+    }
+
+  return fcn;
+}
+
+octave_value_list
+octave_fcn_cache::
+call (octave::tree_evaluator& tw,
+      octave_function *fcn,
+      const octave_value_list& args,
+      int nargout)
+{
+  try
+    {
+      return fcn->call (tw, nargout, args);
+    }
+  catch (octave::index_exception& ie)
+    {
+      error ("Proper error message here for function calls");
+      // Maybe return the octave_function pointer?
+      //tw.final_index_error (ie, m_expr);
+    }
+}
--- a/libinterp/octave-value/ov-fcn.h	Fri Jun 16 15:04:39 2023 -0400
+++ b/libinterp/octave-value/ov-fcn.h	Sat Jun 17 10:19:33 2023 -0400
@@ -48,6 +48,78 @@
 
 // Functions.
 
+// Class that holds a cached reference to a octave function
+// for use in the bytecode VM.
+class
+OCTINTERP_API
+octave_fcn_cache : public octave_base_value
+{
+public:
+  octave_fcn_cache (const std::string &name) :m_fcn_name (name) { }
+  octave_fcn_cache () {}
+
+  bool is_function_cache (void) const { return true; }
+
+  bool has_function_cache (void) const { return true; }
+
+  octave_function *
+  get_cached_fcn (const octave_value_list& args);
+
+  octave_function *
+  get_cached_fcn () { return m_cached_function.function_value (); }
+
+  octave_value
+  get_cached_obj (const octave_value_list& args);
+
+  octave_fcn_cache * fcn_cache_value (void)
+  {
+    return this;
+  }
+
+  octave_value_list
+  call (octave::tree_evaluator& tw,
+        octave_function *fcn,
+        const octave_value_list& args,
+        int nargout);
+
+  void set_cached_function (octave_value ov, const octave_value_list &args, octave_idx_type current_n_updated);
+
+  bool has_cached_function (const octave_value_list &args) const
+  {
+    auto vec_n = static_cast <octave_idx_type> (m_cached_args.size ());
+
+    if (args.length () != vec_n)
+      return false;
+
+    for (int i = 0; i < args.length (); i++)
+      {
+        if (args (i).type_id () != m_cached_args [i])
+          return false;
+      }
+
+    return m_cached_function.is_defined ();
+  }
+
+private:
+
+  void clear_cached_function ()
+  {
+    m_cached_object = octave_value {};
+    m_cached_function = octave_value {};
+    m_n_updated = 0;
+    m_cached_args.clear ();
+  }
+
+  //std::weak_ptr<int> get_lp_n_updated () const { return m_n_updated; }
+
+  octave_value m_cached_object;
+  octave_value m_cached_function;
+  std::vector<int> m_cached_args;
+  octave_idx_type m_n_updated = 0;
+  std::string m_fcn_name;
+};
+
+
 class
 OCTINTERP_API
 octave_function : public octave_base_value
@@ -105,6 +177,8 @@
 
   virtual bool is_subfunction () const { return false; }
 
+  virtual bool is_compiled () const { return false; }
+
   bool is_class_constructor (const std::string& cname = "") const
   {
     return (is_classdef_constructor (cname) || is_legacy_constructor (cname));
--- a/libinterp/octave-value/ov.h	Fri Jun 16 15:04:39 2023 -0400
+++ b/libinterp/octave-value/ov.h	Sat Jun 17 10:19:33 2023 -0400
@@ -60,6 +60,7 @@
 class octave_user_function;
 class octave_fcn_handle;
 class octave_value_list;
+class octave_fcn_cache;
 
 #include "mxtypes.h"
 
@@ -1529,6 +1530,27 @@
 
 protected:
 
+  bool is_function_cache (void) const
+  { return m_rep->is_function_cache (); }
+
+  // function handles might have a function cache embedded
+  bool has_function_cache (void) const
+  { return m_rep->has_function_cache (); }
+
+  octave_function * get_cached_fcn (const octave_value_list& args)
+  { return m_rep->get_cached_fcn (args); }
+
+  // Returns true if the octave_value is either undefined or
+  // or a function.
+  bool is_maybe_function (void) const
+  { return m_rep->is_maybe_function (); }
+
+  octave_fcn_cache *
+  fcn_cache_value () const
+  {
+    return m_rep->fcn_cache_value ();
+  }
+
   //! The real representation.
   octave_base_value *m_rep;
 
--- a/libinterp/parse-tree/pt-eval.cc	Fri Jun 16 15:04:39 2023 -0400
+++ b/libinterp/parse-tree/pt-eval.cc	Sat Jun 17 10:19:33 2023 -0400
@@ -4578,6 +4578,9 @@
 {
   std::string file_name = check_autoload_file (nm);
 
+  // Signal to load path that the function cache is invalid
+  octave::load_path::signal_clear_fcn_cache ();
+
   m_autoload_map[fcn] = file_name;
 }
 
@@ -4586,6 +4589,9 @@
 {
   check_autoload_file (nm);
 
+  // Signal to load path that the function cache is invalid
+  octave::load_path::signal_clear_fcn_cache ();
+
   // Remove function from symbol table and autoload map.
   symbol_table& symtab = m_interpreter.get_symbol_table ();