changeset 32047:e3de59065cf1

inputParser.m: Re-architect internal data structures for 60% speed improvement. Key idea is to replace Parameter and Switch structs with name field containing a nested struct (def, val fields) with a struct array that has fields "name", "def", and "val". This makes it possible to eliminate for loops. * NEWS.9.md: Announce performance improvement. * inputParser.m: Declare "Parameter" and "Switch" properties as struct arrays. Add new private property "last_idx" which caches the last Parameter or Switch lookup. * inputParser.m (addRequired): Adjust input validation to use numel() now rather than numfields(). * inputParser.m (addOptional): Adjust input validation to use numel() now rather than numfields(). * inputParser.m (addParameter): Change code to add new Parameter to a struct array. * inputParser.m (addSwitch): Change code to add new Switch to a struct array. * inputParser.m (parse): Adjust initialization of internal variables ParameterNames and SwitchNames to match new data structure. Use last_idx to simplify call to validate_arg() for Parameters. * inputParser.m (validate_arg): Move creation of error message to code path when error has been found---No need to do it for every function call. Don't capture "exception" variable in try/catch as it is no longer used. * inputParser.m (is_argname): Cache "last_idx" if match was found. * inputParser.m (add_missing): Replace call to setdiff() with in-place code which can take advantage of implicit knowledge to eliminate calls to unique and input validation. Replace for loop with cell2struct/struct2cell combination.
author Rik <rik@octave.org>
date Fri, 21 Apr 2023 11:01:40 -0700
parents 39700c1ea93e
children 61db3c9377fb
files etc/NEWS.9.md scripts/miscellaneous/inputParser.m
diffstat 2 files changed, 47 insertions(+), 25 deletions(-) [+]
line wrap: on
line diff
--- a/etc/NEWS.9.md	Thu Apr 20 18:59:26 2023 -0700
+++ b/etc/NEWS.9.md	Fri Apr 21 11:01:40 2023 -0700
@@ -22,6 +22,9 @@
 - `quiver` now honors the scaling factor input when there is only a single
 arrow, whereas the factor was previously ignored in that case.
 
+- The `inputParser` function has been re-architected for a 60% performance
+improvement.
+
 ### Graphical User Interface
 
 ### Graphics backend
--- a/scripts/miscellaneous/inputParser.m	Thu Apr 20 18:59:26 2023 -0700
+++ b/scripts/miscellaneous/inputParser.m	Fri Apr 21 11:01:40 2023 -0700
@@ -184,19 +184,19 @@
     ## structs with the fields "name", "def" (default), and "val" (validator).
     Required = cell ();
     Optional = cell ();
-    ## Parameter and Switch are unordered so we have a struct whose fieldnames
-    ## are the argname, and values are a struct with fields "def" and "val"
-    Parameter = struct ();
-    Switch    = struct ();
+    ## Parameter and Switch are unordered so we use a struct array with
+    ## fields "name", "def" (default", and "val" (validator).
+    Parameter = struct ([]);  # create 0x0 struct array, not scalar struct
+    Switch    = struct ([]);
 
     ## List of Parameter and Switch names to simplify searches
     ParameterNames = cell ();
     SwitchNames    = cell ();
 
-    ## When checking for fieldnames in a Case Insensitive way, this variable
-    ## holds the correct identifier for the last searched named using the
-    ## is_argname method.
+    ## Simplify searches by cacheing the last name and last index of a
+    ## match from is_argname() into the Parameter or Switch struct arrays.
     last_name = "";
+    last_idx = 1;
   endproperties
 
   properties (Access = protected, Constant = true)
@@ -232,8 +232,8 @@
 
       if (nargin < 2)
         print_usage ();
-      elseif (numel (this.Optional) || numfields (this.Parameter)
-              || numfields (this.Switch))
+      elseif (numel (this.Optional)
+              || numel (this.Parameter) || numel (this.Switch))
         error (["inputParser.addRequired: can't have a Required argument " ...
                 "after Optional, Parameter, or Switch"]);
       endif
@@ -277,7 +277,7 @@
 
       if (nargin < 3)
         print_usage ();
-      elseif (numfields (this.Parameter) || numfields (this.Switch))
+      elseif (numel (this.Parameter) || numel (this.Switch))
         error (["inputParser.Optional: can't have Optional arguments " ...
                 "after Parameter or Switch"]);
       endif
@@ -358,8 +358,7 @@
       endif
 
       this.validate_name ("Parameter", name);
-      this.Parameter.(name).def = def;
-      this.Parameter.(name).val = val;
+      this.Parameter(end+1) = struct ("name", name, "def", def, "val", val);
 
     endfunction
 
@@ -391,7 +390,7 @@
         print_usage ();
       endif
       this.validate_name ("Switch", name);
-      this.Switch.(name).def = false;
+      this.Switch(end+1) = struct ("name", name, "def", false);
 
     endfunction
 
@@ -419,8 +418,14 @@
         endif
       endif
       pnargin = numel (varargin);
-      this.ParameterNames = fieldnames (this.Parameter);
-      this.SwitchNames    = fieldnames (this.Switch);
+      this.ParameterNames = {};
+      if (numel (this.Parameter))
+        this.ParameterNames = {this.Parameter.name};
+      endif
+      this.SwitchNames = {};
+      if (numel (this.Switch))
+        this.SwitchNames = {this.Switch.name};
+      endif
 
       ## Evaluate the Required arguments first
       nReq = numel (this.Required);
@@ -500,7 +505,7 @@
             this.error (sprintf ("no value for parameter '%s'", name));
           endif
           this.validate_arg (this.last_name,
-                             this.Parameter.(this.last_name).val,
+                             this.Parameter(this.last_idx).val,
                              varargin{vidx});
         elseif (this.is_argname ("Switch", name))
           this.Results.(this.last_name) = true;
@@ -568,20 +573,19 @@
       ## relies on side effect, but calls validation function only once
       ## which is a performance win and may also be necessary if validation
       ## function maintains state.  See bug #49793.
-      err = sprintf ('Checked with "%s"', func2str (val));
       ans = true;
       try
         val (in);  # call function with no arguments in case nargout == 0
         ok = ans;  # use side effect of assignment to 'ans' when nargout == 1
-      catch exception
+      catch
         ok = false;
-        err = exception.message;
       end_try_catch
 
       if (nargout > 0)
         r = ok;
       else
         if (! ok)
+          err = sprintf ('Checked with "%s"', func2str (val));
           this.error (sprintf ("failed validation of '%s'.  %s", name, err));
         endif
         this.Results.(name) = in;
@@ -639,19 +643,34 @@
       endif
 
       if (r)
+        ## Cache values to simplify and speed up later code.
         this.last_name = fnames{idx};
+        this.last_idx = find (idx);
       endif
 
     endfunction
 
     function add_missing (this, type)
 
-      unmatched = setdiff (fieldnames (this.(type)), fieldnames (this.Results));
-      for namec = unmatched(:)'
-        name = namec{1};
-        this.UsingDefaults{end+1} = name;
-        this.Results.(name) = this.(type).(name).def;
-      endfor
+      if (isempty (this.(type)))
+        return;
+      endif
+
+      ## Implement setdiff() without calling out to function for performance.
+      typenames = {this.(type).name}(:); 
+      Resultnames = fieldnames (this.Results);
+      [sorted, sidx] = sort ([typenames; Resultnames]);
+      dups = strcmp (sorted(1:end-1), sorted(2:end));
+      idx = true (size (typenames));
+      idx(sidx(dups)) = false;
+      if (any (idx))
+        unmatched_names = typenames(idx);
+        unmatched_def = {this.(type)(idx).def}(:);
+        this.Results = ...
+          cell2struct (vertcat (struct2cell (this.Results), unmatched_def),
+                       [Resultnames; unmatched_names]);
+        this.UsingDefaults = [this.UsingDefaults, unmatched_names];
+      endif
 
     endfunction