Re: slowness in guile 1.8

[Ludovic Courtès]

Hello Guilers!

Andy Wingo <address@hidden> writes:

> 2004-12-22  Marius Vollmer  <address@hidden>
>
>       * boot-9.scm (module-make-local-var!): When creating a new
>       variable, initialize it to the value of any imported variable with
>       the given name.  This allows code like (define round round) to
>       work as expected.

(See also: 
http://cvs.savannah.gnu.org/viewvc/guile/guile-core/ice-9/boot-9.scm?root=guile&r1=1.341&r2=1.342)

Andy and I discussed this issue on IRC.

It turns out that my recent changes in the module system (allowing for
lazy duplicate binding checks) revert this change, for the reasons
mentioned by Andy ("superfluous" overhead).

However, in doing so it breaks `(define round round)' ("unbound
variable").  Inverting the order of the calls to `scm_sym2var ()' and
`scm_eval_car ()' in `scm_m_define ()' so that it first evaluates the
value, and then creates the variable and assigns it the just-evaluated
value fixes the problem.

Alas, it breaks the following test in `syntax.test':

  (pass-if "binding is created before expression is evaluated"
    (= (eval '(begin
                (define foo
                  (begin
                    (set! foo 1)
                    (+ foo 1)))
                foo)
             (interaction-environment))
       2))

This test case illustrates the fact that _internal_ defines are
equivalent to `letrec' (Section 5.2.2); top-level defines should behave
similarly for new variables (Section 5.2.1).

For top-level defines as in `(define round round)', the rule is that
`define' is equivalent to `set!' when the variable is already bound
(Section 5.2.1).  This justifies the change made by Marius to
`module-make-local-var!' (above).

Consequently:

  * `module-make-local-var!' cannot be changed in 1.8;

  * `module-make-local-var!' in HEAD must be reverted to its previous
    definition (attached patch).

This is _very_ unfortunate performance-wise, as Andy noted, but I don't
have any better idea ATM.   :-(

Thanks,
Ludovic.


--- orig/ice-9/boot-9.scm
+++ mod/ice-9/boot-9.scm
@@ -1511,10 +1511,22 @@
               (module-modified m)
               b)))
 
-      ;; Create a new local variable.
-      (let ((local-var (make-undefined-variable)))
-        (module-add! m v local-var)
-        local-var)))
+      ;; No local variable yet, so we need to create a new one.  That
+      ;; new variable is initialized with the old imported value of V,
+      ;; if there is one.  This is required by Section 5.2.1 of R5RS which
+      ;; says that `define' is equivalent to `set!' when V is already bound
+      ;; (of course here we respect module boundaries so this is not exactly
+      ;; a `set!').
+      (let ((imported-var (module-variable m v))
+           (local-var (or (and (module-binder m)
+                               ((module-binder m) m v #t))
+                          (begin
+                            (let ((answer (make-undefined-variable)))
+                              (module-add! m v answer)
+                              answer)))))
+       (if (and imported-var (not (variable-bound? local-var)))
+           (variable-set! local-var (variable-ref imported-var)))
+       local-var)))
 
 ;; module-ensure-local-variable! module symbol
 ;;

--- orig/test-suite/tests/modules.test
+++ mod/test-suite/tests/modules.test
@@ -118,7 +118,16 @@
               (map module-variable
                    (map resolve-interface mods)
                    syms)
-              locals))))
+              locals)))
+
+  (pass-if "redefinition"
+    (let ((m (make-module)))
+      (beautify-user-module! m)
+
+      ;; The previous value of `round' must still be visible at the time the
+      ;; new `round' is defined.
+      (eval '(define round round) m)
+      (eq? (module-ref m 'stat) stat))))