dictionary: Make dict_delete_var() much faster in common case.
authorBen Pfaff <blp@cs.stanford.edu>
Sat, 25 Jul 2015 15:58:06 +0000 (08:58 -0700)
committerBen Pfaff <blp@cs.stanford.edu>
Sat, 25 Jul 2015 15:59:30 +0000 (08:59 -0700)
When a system file is opened, dict_delete_consecutive_vars() deletes lots
of variables if the system file has long string variables.  This was very
slow because of the O(n**2) behavior in dict_delete_var().  The biggest
cost of that O(n**2) behavior was in calling var_clone() on every variable
several times.  Fortunately, the var_clone() call is not necessary in this
case, because the cloned variable is only needed for callbacks, which
aren't set when a dictionary is being read.

This reduced the time to read the dictionary in a particular sample system
file from seconds to milliseconds.

src/data/dictionary.c

index 4eaaefae18ed9ae4889275bccd00b0e568c26e8d..dc748517921363e6483a6918ef8972d2fe8beda4 100644 (file)
@@ -1,5 +1,5 @@
 /* PSPP - a program for statistical analysis.
-   Copyright (C) 1997-9, 2000, 2006, 2007, 2009, 2010, 2011, 2012, 2013, 2014 Free Software Foundation, Inc.
+   Copyright (C) 1997-9, 2000, 2006, 2007, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Free Software Foundation, Inc.
 
    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -546,17 +546,21 @@ unindex_var (struct dictionary *d, struct vardict_info *vardict)
 static void
 reindex_var (struct dictionary *d, struct vardict_info *vardict)
 {
-  struct variable *var = vardict->var;
-  struct variable *old = var_clone (var);
+  struct variable *old = (d->callbacks && d->callbacks->var_changed
+                          ? var_clone (vardict->var)
+                          : NULL);
 
+  struct variable *var = vardict->var;
   var_set_vardict (var, vardict);
   hmap_insert_fast (&d->name_map, &vardict->name_node,
                     vardict->name_node.hash);
 
   if ( d->changed ) d->changed (d, d->changed_data);
-  if ( d->callbacks &&  d->callbacks->var_changed )
-    d->callbacks->var_changed (d, var_get_dict_index (var), VAR_TRAIT_POSITION, old, d->cb_data);
-  var_destroy (old);
+  if (old)
+    {
+      d->callbacks->var_changed (d, var_get_dict_index (var), VAR_TRAIT_POSITION, old, d->cb_data);
+      var_destroy (old);
+    }
 }
 
 /* Sets the case_index in V's vardict to CASE_INDEX. */