unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
From: ludo@chbouib.org (Ludovic Courtès)
To: guile-devel@gnu.org
Subject: Re: slowness in guile 1.8
Date: Fri, 25 May 2007 20:12:15 +0200	[thread overview]
Message-ID: <87tzu0pybk.fsf@chbouib.org> (raw)
In-Reply-To: 1180110804.4388.16.camel@localhost.localdomain

[-- Attachment #1: Type: text/plain, Size: 2019 bytes --]

Hello Guilers!

Andy Wingo <wingo@pobox.com> writes:

> 2004-12-22  Marius Vollmer  <marius.vollmer@uni-dortmund.de>
>
> 	* 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.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Reverting `module-make-local-var!' in HEAD --]
[-- Type: text/x-patch, Size: 1660 bytes --]

--- 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))))
 
 
 \f


[-- Attachment #3: Type: text/plain, Size: 143 bytes --]

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

  reply	other threads:[~2007-05-25 18:12 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-05-25 16:33 slowness in guile 1.8 Andy Wingo
2007-05-25 18:12 ` Ludovic Courtès [this message]
2007-05-26 10:49   ` Andy Wingo
2007-05-26 10:57     ` Andy Wingo
2007-05-26 13:15     ` Ludovic Courtès
2007-05-26 14:45       ` Ludovic Courtès
2007-05-26 15:39         ` Andy Wingo
2007-06-13 22:24         ` Ludovic Courtès

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/guile/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87tzu0pybk.fsf@chbouib.org \
    --to=ludo@chbouib.org \
    --cc=guile-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).