unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
* Patch for speeding up the debugging evaluator by a factor of 600
@ 2004-07-08 19:37 Matthias Koeppe
  2004-08-11 19:25 ` Marius Vollmer
  0 siblings, 1 reply; 4+ messages in thread
From: Matthias Koeppe @ 2004-07-08 19:37 UTC (permalink / raw)


I am using Guile 1.6.4 with GOOPS and about 4000 primitive procedures.
In this setting, when I try to TRACE a function, the debugging
evaluator becomes unusably slow.  I illustrate this on a call to
APROPOS:

        guile> (time (apropos "doesntexist"))
        ;; Evaluation took:
        ;;   0.05 seconds of real time
        ;;   0.04 seconds of user time
        ;;   0 seconds of system time
        guile> (trace load)
        $3 = (load-module)
        guile> (time (apropos "doesntexist"))
        ;; Evaluation took:
        ;;   32.02 seconds of real time
        ;;   32.02 seconds of user time
        ;;   0 seconds of system time

I found out that a one-line change by Mikael Djurfeldt in CVS HEAD
makes a major speed-up:

2003-03-06  Mikael Djurfeldt  <djurfeldt@nada.kth.se>

	* procprop.c (scm_stand_in_scm_proc): Use scm_assq instead of
	scm_assoc.

--- procprop.c.~1.37.2.2.~	2002-03-21 17:53:18.000000000 +0100
+++ procprop.c	2004-07-08 19:43:16.746126000 +0200
@@ -160,7 +160,7 @@
 scm_stand_in_scm_proc(SCM proc)
 {
   SCM answer;
-  answer = scm_assoc (proc, scm_stand_in_procs);
+  answer = scm_assq (proc, scm_stand_in_procs);
   if (SCM_FALSEP (answer))
     {
       answer = scm_closure (scm_list_2 (SCM_EOL, SCM_BOOL_F), SCM_EOL);

With this change applied to branch_release_1_6, I get:

        guile> (trace load)
        $3 = (load-module)
        guile> (time (apropos "doesntexist"))
        ;; Evaluation took:
        ;;   1.17 seconds of real time
        ;;   1.17 seconds of user time
        ;;   0 seconds of system time

A 30-fold speedup, but still much slower than the untraced version.
The scm_assq is still the bottleneck (90% of the run time).  The real
fix is to use a hash table rather than an alist.  Here is a patch
(against the stable branch) that does this:

Index: libguile/gc.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/gc.c,v
retrieving revision 1.208.2.11
diff -u -p -r1.208.2.11 gc.c
--- libguile/gc.c	1 Oct 2003 19:51:43 -0000	1.208.2.11
+++ libguile/gc.c	8 Jul 2004 19:28:45 -0000
@@ -2827,7 +2827,7 @@ scm_init_storage ()
 #endif
 #endif

-  scm_stand_in_procs = SCM_EOL;
+  scm_stand_in_procs = scm_c_make_hash_table (257);
   scm_permobjs = SCM_EOL;
   scm_protects = scm_c_make_hash_table (31);
   scm_gc_registered_roots = scm_c_make_hash_table (31);
Index: libguile/procprop.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/procprop.c,v
retrieving revision 1.37.2.2
diff -u -p -r1.37.2.2 procprop.c
--- libguile/procprop.c	14 Mar 2002 05:26:15 -0000	1.37.2.2
+++ libguile/procprop.c	8 Jul 2004 19:28:45 -0000
@@ -52,6 +52,7 @@
 #include "libguile/smob.h"
 #include "libguile/root.h"
 #include "libguile/vectors.h"
+#include "libguile/hashtab.h"

 #include "libguile/validate.h"
 #include "libguile/procprop.h"
@@ -159,15 +160,16 @@ scm_i_procedure_arity (SCM proc)
 static SCM
 scm_stand_in_scm_proc(SCM proc)
 {
-  SCM answer;
-  answer = scm_assoc (proc, scm_stand_in_procs);
-  if (SCM_FALSEP (answer))
+  SCM handle, answer;
+  handle = scm_hashq_get_handle (scm_stand_in_procs, proc);
+  if (SCM_FALSEP (handle))
     {
       answer = scm_closure (scm_list_2 (SCM_EOL, SCM_BOOL_F), SCM_EOL);
-      scm_stand_in_procs = scm_acons (proc, answer, scm_stand_in_procs);
+      scm_hashq_set_x (scm_stand_in_procs, proc, answer);
     }
-  else
-    answer = SCM_CDR (answer);
+  else {
+    answer = SCM_CDR(handle);
+  }
   return answer;
 }

With this patch, I get about the same timings as without tracing.

        guile> (trace load)
        $3 = (load-module)
        guile> (time (apropos "doesntexist"))
        ;; Evaluation took:
        ;;   0.05 seconds of real time
        ;;   0.05 seconds of user time
        ;;   0 seconds of system time 

Could someone please put the fix in CVS (stable branch and CVS HEAD)?
Thanks.

-- 
Matthias Koeppe -- http://www.math.uni-magdeburg.de/~mkoeppe


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Patch for speeding up the debugging evaluator by a factor of 600
  2004-07-08 19:37 Patch for speeding up the debugging evaluator by a factor of 600 Matthias Koeppe
@ 2004-08-11 19:25 ` Marius Vollmer
  2004-08-11 19:58   ` Matthias Koeppe
  0 siblings, 1 reply; 4+ messages in thread
From: Marius Vollmer @ 2004-08-11 19:25 UTC (permalink / raw)
  Cc: guile-devel

Matthias Koeppe <mkoeppe@mail.math.uni-magdeburg.de> writes:

> The real fix is to use a hash table rather than an alist.

Excellent detective work!  I dug a little further and noted that we
have SCM_SUBR_PROPS but don't use it at all!

So your patch is definitely good, but would you be willing to improve
it the situation further by making use of SCM_SUBR_PROPS for subrs,
and maybe other means for other kinds of objects that are
scm_procedure_p, and only use scm_stand_in_proc for the remaining
unsolved cases?

I will apply your patch to both 1.6 and HEAD.  Thanks!

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Patch for speeding up the debugging evaluator by a factor of 600
  2004-08-11 19:25 ` Marius Vollmer
@ 2004-08-11 19:58   ` Matthias Koeppe
  2004-08-15 22:36     ` Marius Vollmer
  0 siblings, 1 reply; 4+ messages in thread
From: Matthias Koeppe @ 2004-08-11 19:58 UTC (permalink / raw)
  Cc: guile-devel

Problem there: 

Marius Vollmer <mvo@zagadka.de> writes:

> So your patch is definitely good, but would you be willing to improve
> it the situation further by making use of SCM_SUBR_PROPS for subrs,
> and maybe other means for other kinds of objects that are
> scm_procedure_p, and only use scm_stand_in_proc for the remaining
> unsolved cases?

Sorry, I don't have time to work on that.

-- 
Matthias Koeppe -- http://www.math.uni-magdeburg.de/~mkoeppe


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Patch for speeding up the debugging evaluator by a factor of 600
  2004-08-11 19:58   ` Matthias Koeppe
@ 2004-08-15 22:36     ` Marius Vollmer
  0 siblings, 0 replies; 4+ messages in thread
From: Marius Vollmer @ 2004-08-15 22:36 UTC (permalink / raw)
  Cc: guile-devel

Matthias Koeppe <mkoeppe@merkur.math.uni-magdeburg.de> writes:

> Problem there: 
>
> Marius Vollmer <mvo@zagadka.de> writes:
>
>> So your patch is definitely good, but would you be willing to improve
>> it the situation further by making use of SCM_SUBR_PROPS for subrs,
>> and maybe other means for other kinds of objects that are
>> scm_procedure_p, and only use scm_stand_in_proc for the remaining
>> unsolved cases?
>
> Sorry, I don't have time to work on that.

OK, I'll put a comment in the code and into the TODO list.

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2004-08-15 22:36 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-07-08 19:37 Patch for speeding up the debugging evaluator by a factor of 600 Matthias Koeppe
2004-08-11 19:25 ` Marius Vollmer
2004-08-11 19:58   ` Matthias Koeppe
2004-08-15 22:36     ` Marius Vollmer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).