pspp-cvs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Pspp-cvs] pspp/src data/ChangeLog data/automake.mk data/c...


From: Ben Pfaff
Subject: [Pspp-cvs] pspp/src data/ChangeLog data/automake.mk data/c...
Date: Wed, 19 Sep 2007 04:29:01 +0000

CVSROOT:        /cvsroot/pspp
Module name:    pspp
Changes by:     Ben Pfaff <blp> 07/09/19 04:29:00

Modified files:
        src/data       : ChangeLog automake.mk casereader-provider.h 
                         casereader.c procedure.c procedure.h 
        src/ui/gui     : ChangeLog helper.c psppire-data-store.c 
                         psppire-data-store.h psppire-dict.c 
                         psppire-dict.h 
Added files:
        src/data       : lazy-casereader.c lazy-casereader.h 

Log message:
        Fix bug #20910.
        
        * helper.c (create_casereader_from_data_store): New function.
        (execute_syntax): Only replace the active file data by a new
        casereader if syntax caused the active file to be read, to avoid
        exponential slowdown as an increasing number of snippets that do
        not read from the active file are consecutively executed.  Bug
        #20910.  Reviewed by and heavily influenced by John Darrington.
        
        * psppire-data-store.c (psppire_data_store_get_value_count): New
        function.
        
        * psppire-dict.c (psppire_dict_get_value_cnt): New function.
        
        * procedure.c (proc_extract_active_file_data): New function.
        
        * lazy-casereader.h: New file.
        
        * lazy-casereader.c: New file.
        
        * casereader.c (casereader_dynamic_cast): New function.

CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/pspp/src/data/ChangeLog?cvsroot=pspp&r1=1.158&r2=1.159
http://cvs.savannah.gnu.org/viewcvs/pspp/src/data/automake.mk?cvsroot=pspp&r1=1.28&r2=1.29
http://cvs.savannah.gnu.org/viewcvs/pspp/src/data/casereader-provider.h?cvsroot=pspp&r1=1.4&r2=1.5
http://cvs.savannah.gnu.org/viewcvs/pspp/src/data/casereader.c?cvsroot=pspp&r1=1.6&r2=1.7
http://cvs.savannah.gnu.org/viewcvs/pspp/src/data/procedure.c?cvsroot=pspp&r1=1.35&r2=1.36
http://cvs.savannah.gnu.org/viewcvs/pspp/src/data/procedure.h?cvsroot=pspp&r1=1.17&r2=1.18
http://cvs.savannah.gnu.org/viewcvs/pspp/src/data/lazy-casereader.c?cvsroot=pspp&rev=1.1
http://cvs.savannah.gnu.org/viewcvs/pspp/src/data/lazy-casereader.h?cvsroot=pspp&rev=1.1
http://cvs.savannah.gnu.org/viewcvs/pspp/src/ui/gui/ChangeLog?cvsroot=pspp&r1=1.86&r2=1.87
http://cvs.savannah.gnu.org/viewcvs/pspp/src/ui/gui/helper.c?cvsroot=pspp&r1=1.27&r2=1.28
http://cvs.savannah.gnu.org/viewcvs/pspp/src/ui/gui/psppire-data-store.c?cvsroot=pspp&r1=1.52&r2=1.53
http://cvs.savannah.gnu.org/viewcvs/pspp/src/ui/gui/psppire-data-store.h?cvsroot=pspp&r1=1.20&r2=1.21
http://cvs.savannah.gnu.org/viewcvs/pspp/src/ui/gui/psppire-dict.c?cvsroot=pspp&r1=1.32&r2=1.33
http://cvs.savannah.gnu.org/viewcvs/pspp/src/ui/gui/psppire-dict.h?cvsroot=pspp&r1=1.19&r2=1.20

Patches:
Index: data/ChangeLog
===================================================================
RCS file: /cvsroot/pspp/pspp/src/data/ChangeLog,v
retrieving revision 1.158
retrieving revision 1.159
diff -u -b -r1.158 -r1.159
--- data/ChangeLog      14 Sep 2007 13:45:02 -0000      1.158
+++ data/ChangeLog      19 Sep 2007 04:28:59 -0000      1.159
@@ -1,3 +1,13 @@
+2007-09-18  Ben Pfaff  <address@hidden>
+
+       * procedure.c (proc_extract_active_file_data): New function.
+
+       * lazy-casereader.h: New file.
+
+       * lazy-casereader.c: New file.
+
+       * casereader.c (casereader_dynamic_cast): New function.
+
 2007-09-14  Ben Pfaff  <address@hidden>
 
        * dictionary.c (dict_clone): Copy case indexes from cloned

Index: data/automake.mk
===================================================================
RCS file: /cvsroot/pspp/pspp/src/data/automake.mk,v
retrieving revision 1.28
retrieving revision 1.29
diff -u -b -r1.28 -r1.29
--- data/automake.mk    13 Aug 2007 04:13:48 -0000      1.28
+++ data/automake.mk    19 Sep 2007 04:28:59 -0000      1.29
@@ -50,6 +50,8 @@
        src/data/format.def \
        src/data/identifier.c \
        src/data/identifier.h \
+       src/data/lazy-casereader.c \
+       src/data/lazy-casereader.h \
        src/data/missing-values.c \
        src/data/missing-values.h \
        src/data/make-file.c \

Index: data/casereader-provider.h
===================================================================
RCS file: /cvsroot/pspp/pspp/src/data/casereader-provider.h,v
retrieving revision 1.4
retrieving revision 1.5
diff -u -b -r1.4 -r1.5
--- data/casereader-provider.h  7 Jul 2007 06:14:07 -0000       1.4
+++ data/casereader-provider.h  19 Sep 2007 04:28:59 -0000      1.5
@@ -111,6 +111,8 @@
                               size_t value_cnt, casenumber case_cnt,
                               const struct casereader_class *, void *);
 
+void *casereader_dynamic_cast (struct casereader *, struct casereader_class *);
+
 /* Casereader class for random-access data sources. */
 struct casereader_random_class
   {

Index: data/casereader.c
===================================================================
RCS file: /cvsroot/pspp/pspp/src/data/casereader.c,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -b -r1.6 -r1.7
--- data/casereader.c   13 Aug 2007 00:43:48 -0000      1.6
+++ data/casereader.c   19 Sep 2007 04:29:00 -0000      1.7
@@ -312,6 +312,26 @@
   return reader;
 }
 
+/* If READER is a casereader of the given CLASS, returns its
+   associated auxiliary data; otherwise, returns a null pointer.
+
+   This function is intended for use from casereader
+   implementations, not by casereader users.  Even within
+   casereader implementations, its usefulness is quite limited,
+   for at least two reasons.  First, every casereader member
+   function already receives a pointer to the casereader's
+   auxiliary data.  Second, a casereader's class can change
+   (through a call to casereader_swap) and this is in practice
+   quite common (e.g. any call to casereader_clone on a
+   casereader that does not directly support clone will cause the
+   casereader to be replaced by a shim caseader). */
+void *
+casereader_dynamic_cast (struct casereader *reader,
+                         struct casereader_class *class)
+{
+  return reader->class == class ? reader->aux : NULL;
+}
+
 /* Random-access casereader implementation.
 
    This is a set of wrappers around casereader_create_sequential

Index: data/procedure.c
===================================================================
RCS file: /cvsroot/pspp/pspp/src/data/procedure.c,v
retrieving revision 1.35
retrieving revision 1.36
diff -u -b -r1.35 -r1.36
--- data/procedure.c    13 Aug 2007 04:23:29 -0000      1.35
+++ data/procedure.c    19 Sep 2007 04:29:00 -0000      1.36
@@ -636,6 +636,18 @@
   return ds->source != NULL;
 }
 
+/* Returns the active file data source from DS, or a null pointer
+   if DS has no data source, and removes it from DS. */
+struct casereader *
+proc_extract_active_file_data (struct dataset *ds)
+{
+  struct casereader *reader = ds->source;
+  ds->source = NULL;
+  if (ds->replace_source) ds->replace_source (reader);
+
+  return reader;
+}
+
 /* Checks whether DS has a corrupted active file.  If so,
    discards it and returns false.  If not, returns true without
    doing anything. */

Index: data/procedure.h
===================================================================
RCS file: /cvsroot/pspp/pspp/src/data/procedure.h,v
retrieving revision 1.17
retrieving revision 1.18
diff -u -b -r1.17 -r1.18
--- data/procedure.h    26 Jul 2007 02:02:23 -0000      1.17
+++ data/procedure.h    19 Sep 2007 04:29:00 -0000      1.18
@@ -64,6 +64,7 @@
                            struct casereader *, struct dictionary *);
 bool proc_set_active_file_data (struct dataset *, struct casereader *);
 bool proc_has_active_file (const struct dataset *ds);
+struct casereader *proc_extract_active_file_data (struct dataset *);
 
 void proc_discard_output (struct dataset *ds);
 

Index: ui/gui/ChangeLog
===================================================================
RCS file: /cvsroot/pspp/pspp/src/ui/gui/ChangeLog,v
retrieving revision 1.86
retrieving revision 1.87
diff -u -b -r1.86 -r1.87
--- ui/gui/ChangeLog    13 Sep 2007 12:37:10 -0000      1.86
+++ ui/gui/ChangeLog    19 Sep 2007 04:29:00 -0000      1.87
@@ -1,3 +1,17 @@
+2007-09-18  Ben Pfaff  <address@hidden>
+
+       * helper.c (create_casereader_from_data_store): New function.
+       (execute_syntax): Only replace the active file data by a new
+       casereader if syntax caused the active file to be read, to avoid
+       exponential slowdown as an increasing number of snippets that do
+       not read from the active file are consecutively executed.  Bug
+       #20910.  Reviewed by and heavily influenced by John Darrington.
+
+       * psppire-data-store.c (psppire_data_store_get_value_count): New
+       function.
+
+       * psppire-dict.c (psppire_dict_get_value_cnt): New function.
+
 2007-09-13  John Darrington <address@hidden>
 
        * find-dialog.c find-dialog.h: New files.

Index: ui/gui/helper.c
===================================================================
RCS file: /cvsroot/pspp/pspp/src/ui/gui/helper.c,v
retrieving revision 1.27
retrieving revision 1.28
diff -u -b -r1.27 -r1.28
--- ui/gui/helper.c     6 Sep 2007 07:43:02 -0000       1.27
+++ ui/gui/helper.c     19 Sep 2007 04:29:00 -0000      1.28
@@ -28,21 +28,26 @@
 #include <data/data-in.h>
 #include <data/data-out.h>
 #include <data/dictionary.h>
+#include <data/casereader-provider.h>
 #include <libpspp/message.h>
 
 #include <libpspp/i18n.h>
 
 #include <ctype.h>
 #include <string.h>
+#include <stdlib.h>
 #include <data/settings.h>
 
 #include <language/command.h>
+#include <data/lazy-casereader.h>
 #include <data/procedure.h>
 #include <language/lexer/lexer.h>
 #include "psppire-data-store.h"
 #include <output/manager.h>
 #include "output-viewer.h"
 
+#include "xalloc.h"
+
 #include <gettext.h>
 
 /* Formats a value according to FORMAT
@@ -165,14 +170,43 @@
 extern struct source_stream *the_source_stream;
 extern PsppireDataStore *the_data_store;
 
+/* Lazy casereader callback function used by execute_syntax. */
+static struct casereader *
+create_casereader_from_data_store (void *data_store_)
+{
+  PsppireDataStore *data_store = data_store_;
+  return psppire_data_store_get_reader (data_store);
+}
+
 gboolean
 execute_syntax (struct getl_interface *sss)
 {
   struct lexer *lexer;
   gboolean retval = TRUE;
 
-  struct casereader *reader = psppire_data_store_get_reader (the_data_store);
-
+  struct casereader *reader;
+  size_t value_cnt;
+  casenumber case_cnt;
+  unsigned long int lazy_serial;
+
+  /* When the user executes a number of snippets of syntax in a
+     row, none of which read from the active file, the GUI becomes
+     progressively less responsive.  The reason is that each syntax
+     execution encapsulates the active file data in another
+     datasheet layer.  The cumulative effect of having a number of
+     layers of datasheets wastes time and space.
+
+     To solve the problem, we use a "lazy casereader", a wrapper
+     around the casereader obtained from the data store, that
+     only actually instantiates that casereader when it is
+     needed.  If the data store casereader is never needed, then
+     it is reused the next time syntax is run, without wrapping
+     it in another layer. */
+  value_cnt = psppire_data_store_get_value_count (the_data_store);
+  case_cnt = psppire_data_store_get_case_count (the_data_store);
+  reader = lazy_casereader_create (value_cnt, case_cnt,
+                                   create_casereader_from_data_store,
+                                   the_data_store, &lazy_serial);
   proc_set_active_file_data (the_dataset, reader);
 
   g_return_val_if_fail (proc_has_active_file (the_dataset), FALSE);
@@ -204,13 +238,10 @@
   psppire_dict_replace_dictionary (the_data_store->dict,
                                   dataset_dict (the_dataset));
 
-  {
-    PsppireCaseFile *pcf = psppire_case_file_new (dataset_source 
(the_dataset));
-
-    psppire_data_store_set_case_file (the_data_store, pcf);
-  }
-
-  proc_set_active_file_data (the_dataset, NULL);
+  reader = proc_extract_active_file_data (the_dataset);
+  if (!lazy_casereader_destroy (reader, lazy_serial))
+    psppire_data_store_set_case_file (the_data_store,
+                                      psppire_case_file_new (reader));
 
   som_flush ();
 

Index: ui/gui/psppire-data-store.c
===================================================================
RCS file: /cvsroot/pspp/pspp/src/ui/gui/psppire-data-store.c,v
retrieving revision 1.52
retrieving revision 1.53
diff -u -b -r1.52 -r1.53
--- ui/gui/psppire-data-store.c 7 Aug 2007 00:56:59 -0000       1.52
+++ ui/gui/psppire-data-store.c 19 Sep 2007 04:29:00 -0000      1.53
@@ -165,6 +165,12 @@
   return psppire_case_file_get_case_count (store->case_file);
 }
 
+size_t
+psppire_data_store_get_value_count (const PsppireDataStore *store)
+{
+  return psppire_dict_get_value_cnt (store->dict);
+}
+
 inline casenumber
 psppire_data_store_get_case_count_wrapper (const GSheetModel *model)
 {

Index: ui/gui/psppire-data-store.h
===================================================================
RCS file: /cvsroot/pspp/pspp/src/ui/gui/psppire-data-store.h,v
retrieving revision 1.20
retrieving revision 1.21
diff -u -b -r1.20 -r1.21
--- ui/gui/psppire-data-store.h 7 Aug 2007 00:56:59 -0000       1.20
+++ ui/gui/psppire-data-store.h 19 Sep 2007 04:29:00 -0000      1.21
@@ -142,6 +142,7 @@
                                        glong row, glong column);
 
 casenumber psppire_data_store_get_case_count (const PsppireDataStore *ds);
+size_t psppire_data_store_get_value_count (const PsppireDataStore *ds);
 
 #ifdef __cplusplus
 }

Index: ui/gui/psppire-dict.c
===================================================================
RCS file: /cvsroot/pspp/pspp/src/ui/gui/psppire-dict.c,v
retrieving revision 1.32
retrieving revision 1.33
diff -u -b -r1.32 -r1.33
--- ui/gui/psppire-dict.c       13 Aug 2007 03:44:46 -0000      1.32
+++ ui/gui/psppire-dict.c       19 Sep 2007 04:29:00 -0000      1.33
@@ -411,6 +411,17 @@
 }
 
 
+/* Return the number of `union value's in the dictionary */
+size_t
+psppire_dict_get_value_cnt (const PsppireDict *d)
+{
+  g_return_val_if_fail (d, -1);
+  g_return_val_if_fail (d->dict, -1);
+
+  return dict_get_next_value_idx (d->dict);
+}
+
+
 /* Return a variable by name.
    Return NULL if it doesn't exist
 */

Index: ui/gui/psppire-dict.h
===================================================================
RCS file: /cvsroot/pspp/pspp/src/ui/gui/psppire-dict.h,v
retrieving revision 1.19
retrieving revision 1.20
diff -u -b -r1.19 -r1.20
--- ui/gui/psppire-dict.h       18 Jul 2007 00:50:59 -0000      1.19
+++ ui/gui/psppire-dict.h       19 Sep 2007 04:29:00 -0000      1.20
@@ -69,6 +69,9 @@
 /* Return the number of variables in the dictionary */
 gint psppire_dict_get_var_cnt (const PsppireDict *d);
 
+/* Return the number of `union value's in the dictionary */
+size_t psppire_dict_get_value_cnt (const PsppireDict *d);
+
 /* Return a variable by name.
    Return NULL if it doesn't exist
 */

Index: data/lazy-casereader.c
===================================================================
RCS file: data/lazy-casereader.c
diff -N data/lazy-casereader.c
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ data/lazy-casereader.c      19 Sep 2007 04:29:00 -0000      1.1
@@ -0,0 +1,161 @@
+/* PSPP - a program for statistical analysis.
+   Copyright (C) 2007 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
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>. */
+
+#include <config.h>
+
+#include <data/lazy-casereader.h>
+
+#include <stdlib.h>
+
+#include <data/case.h>
+#include <data/casereader.h>
+#include <data/casereader-provider.h>
+#include <libpspp/assertion.h>
+
+#include "xalloc.h"
+
+/* A lazy casereader's auxiliary data. */
+struct lazy_casereader
+  {
+    unsigned long int serial;
+    struct casereader *(*callback) (void *aux);
+    void *aux;
+  };
+
+static struct casereader_class lazy_casereader_class;
+
+/* Creates and returns a new lazy casereader that will
+   instantiate its underlying casereader, if necessary, by
+   calling CALLBACK, passing AUX as its argument.  *SERIAL is set
+   to a "serial number" that uniquely identifies the new lazy
+   casereader, for use with lazy_casereader_destroy.
+
+   VALUE_CNT must be the number of struct values per case read
+   from the casereader.
+
+   CASE_CNT is an upper limit on the number of cases that
+   casereader_read will return from the casereader in successive
+   calls.  Ordinarily, this is the actual number of cases in the
+   data source or CASENUMBER_MAX if the number of cases cannot be
+   predicted in advance. */
+struct casereader *
+lazy_casereader_create (size_t value_cnt, casenumber case_cnt,
+                        struct casereader *(*callback) (void *aux), void *aux,
+                        unsigned long int *serial)
+{
+  static unsigned long int next_serial = 0;
+  struct lazy_casereader *lc;
+  assert (callback != NULL);
+  lc = xmalloc (sizeof *lc);
+  *serial = lc->serial = next_serial++;
+  lc->callback = callback;
+  lc->aux = aux;
+  return casereader_create_sequential (NULL, value_cnt, case_cnt,
+                                       &lazy_casereader_class, lc);
+}
+
+/* If READER is the lazy casereader that was returned by
+   lazy_casereader_create along with SERIAL, and READER was never
+   instantiated by any use of a casereader function, then this
+   function destroys READER without instantiating it, and returns
+   true.  Returns false in any other case; that is, if READER is
+   not a lazy casereader, or if READER is a lazy casereader with
+   a serial number different from SERIAL, or if READER is a lazy
+   casereader that was instantiated.
+
+   When this function returns false, it necessarily indicates
+   that the lazy casereader was never cloned and never
+   destroyed. */
+bool
+lazy_casereader_destroy (struct casereader *reader, unsigned long int serial)
+{
+  struct lazy_casereader *lc;
+
+  if (reader == NULL)
+    return false;
+
+  lc = casereader_dynamic_cast (reader, &lazy_casereader_class);
+  if (lc == NULL || lc->serial != serial)
+    return false;
+
+  lc->callback = NULL;
+  casereader_destroy (reader);
+  return true;
+}
+
+/* Instantiates lazy casereader READER, which is associated with
+   LC. */
+static void
+instantiate_lazy_casereader (struct casereader *reader,
+                             struct lazy_casereader *lc)
+{
+  struct casereader *subreader;
+
+  /* Call the client-provided callback to obtain the real
+     casereader, then swap READER with that casereader. */
+  subreader = lc->callback (lc->aux);
+  casereader_swap (reader, subreader);
+
+  /* Now destroy the lazy casereader, which is no longer needed
+     since we already swapped it out.  Set the callback to null
+     to prevent lazy_casereader_do_destroy from trying to
+     instantiate it again.  */
+  lc->callback = NULL;
+  casereader_destroy (subreader);
+}
+
+static bool
+lazy_casereader_read (struct casereader *reader, void *lc_,
+                      struct ccase *c)
+{
+  struct lazy_casereader *lc = lc_;
+  instantiate_lazy_casereader (reader, lc);
+  return casereader_read (reader, c);
+}
+
+static void
+lazy_casereader_do_destroy (struct casereader *reader UNUSED, void *lc_)
+{
+  struct lazy_casereader *lc = lc_;
+  if (lc->callback != NULL)
+    casereader_destroy (lc->callback (lc->aux));
+  free (lc);
+}
+
+static struct casereader *
+lazy_casereader_clone (struct casereader *reader, void *lc_)
+{
+  struct lazy_casereader *lc = lc_;
+  instantiate_lazy_casereader (reader, lc);
+  return casereader_clone (reader);
+}
+
+static bool
+lazy_casereader_peek (struct casereader *reader, void *lc_,
+                      casenumber idx, struct ccase *c)
+{
+  struct lazy_casereader *lc = lc_;
+  instantiate_lazy_casereader (reader, lc);
+  return casereader_peek (reader, idx, c);
+}
+
+static struct casereader_class lazy_casereader_class =
+  {
+    lazy_casereader_read,
+    lazy_casereader_do_destroy,
+    lazy_casereader_clone,
+    lazy_casereader_peek,
+  };

Index: data/lazy-casereader.h
===================================================================
RCS file: data/lazy-casereader.h
diff -N data/lazy-casereader.h
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ data/lazy-casereader.h      19 Sep 2007 04:29:00 -0000      1.1
@@ -0,0 +1,39 @@
+/* PSPP - a program for statistical analysis.
+   Copyright (C) 2007 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
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>. */
+
+/* Lazy casereader.
+
+   A "lazy casereader" is a casereader that saves an underlying
+   casereader from the need to be instantiated in the case where
+   it is never used.  If any casereader operation is ever
+   performed on a lazy casereader, it invokes a callback function
+   (provided by the lazy casereader's creator) to instantiate the
+   underlying reader. */
+
+#ifndef DATA_LAZY_CASEREADER_H
+#define DATA_LAZY_CASEREADER_H 1
+
+#include <stdbool.h>
+#include <data/case.h>
+
+struct casereader *lazy_casereader_create (size_t value_cnt,
+                                           casenumber case_cnt,
+                                           struct casereader *(*) (void *aux),
+                                           void *aux,
+                                           unsigned long int *serial);
+bool lazy_casereader_destroy (struct casereader *, unsigned long int serial);
+
+#endif /* data/lazy-casereader.h */




reply via email to

[Prev in Thread] Current Thread [Next in Thread]