[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: slowness in guile 1.8
From: |
Ludovic Courtès |
Subject: |
Re: slowness in guile 1.8 |
Date: |
Thu, 14 Jun 2007 00:24:13 +0200 |
User-agent: |
Gnus/5.110006 (No Gnus v0.6) Emacs/21.4 (gnu/linux) |
Hi,
address@hidden (Ludovic Courtès) writes:
> For 1.8, I'm pretty much inclined to commit a similar patch, i.e., where
> `module-make-local-var!' and `scm_m_define' are copied from HEAD. This
> would break code that does things like:
>
> (define foo (begin (set! foo 1) (+ foo 1)))
>
> but I think it's reasonable to break such code (which relies on
> non-R5RS-compliant behavior anyway), especially given the performance
> gain we get in return. What do you think?
I committed the attached patch to 1.8.
I added an "Incompatible Changes" category in `NEWS'. This sounds a bit
frightening, but the description hopefully emphasizes the
conservativeness of the change. Please do improve the wording or
presentation if you find it inappropriate.
Thanks,
Ludovic.
--- orig/ChangeLog
+++ mod/ChangeLog
@@ -1,3 +1,7 @@
+2007-06-13 Ludovic Courtès <address@hidden>
+
+ * NEWS: Mention top-level define incompatible change.
+
2007-06-12 Ludovic Courtès <address@hidden>
* NEWS: Mention `inet-ntop' bug fix.
--- orig/NEWS
+++ mod/NEWS
@@ -12,6 +12,16 @@ Changes in 1.8.2 (since 1.8.1):
** set-program-arguments
** make-vtable
+* Incompatible changes
+
+** The body of a top-level `define' no longer sees the binding being created
+
+In a top-level `define', the binding being created is no longer visible
+from the `define' body. This breaks code like
+"(define foo (begin (set! foo 1) (+ foo 1)))", where `foo' is now
+unbound in the body. However, such code was not R5RS-compliant anyway,
+per Section 5.2.1.
+
* Bugs fixed
** Fractions were not `equal?' if stored in unreduced form.
--- orig/ice-9/ChangeLog
+++ mod/ice-9/ChangeLog
@@ -1,3 +1,9 @@
+2007-06-13 Ludovic Courtès <address@hidden>
+
+ * boot-9.scm (module-make-local-var!): Simplified. No need to
+ check for the value of a same-named imported binding since the
+ newly created variable is systematically assigned afterwards.
+
2007-01-04 Kevin Ryde <address@hidden>
* boot-9.scm (top-repl): Check (defined? 'SIGBUS) before using that
--- orig/ice-9/boot-9.scm
+++ mod/ice-9/boot-9.scm
@@ -1515,19 +1515,10 @@
(module-modified m)
b)))
- ;; 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.
- (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)))
+ ;; Create a new local variable.
+ (let ((local-var (make-undefined-variable)))
+ (module-add! m v local-var)
+ local-var)))
;; module-ensure-local-variable! module symbol
;;
--- orig/libguile/ChangeLog
+++ mod/libguile/ChangeLog
@@ -1,3 +1,9 @@
+2007-06-13 Ludovic Courtès <address@hidden>
+
+ * eval.c (scm_m_define): Updated comment. Changed order for value
+ evaluation and `scm_sym2var ()' call, which is perfectly valid per
+ R5RS. This reverts the change dated 2004-04-22 by Dirk Herrmann.
+
2007-06-12 Ludovic Courtès <address@hidden>
* socket.c (scm_inet_ntop): In the `AF_INET' case, declare `addr4'
--- orig/libguile/eval.c
+++ mod/libguile/eval.c
@@ -1213,10 +1213,11 @@ canonicalize_define (const SCM expr)
return expr;
}
-/* According to section 5.2.1 of R5RS we first have to make sure that the
- * variable is bound, and then perform the (set! variable expression)
- * operation. This means, that within the expression we may already assign
- * values to variable: (define foo (begin (set! foo 1) (+ foo 1))) */
+/* According to Section 5.2.1 of R5RS we first have to make sure that the
+ variable is bound, and then perform the `(set! variable expression)'
+ operation. However, EXPRESSION _can_ be evaluated before VARIABLE is
+ bound. This means that EXPRESSION won't necessarily be able to assign
+ values to VARIABLE as in `(define foo (begin (set! foo 1) (+ foo 1)))'. */
SCM
scm_m_define (SCM expr, SCM env)
{
@@ -1226,9 +1227,9 @@ scm_m_define (SCM expr, SCM env)
const SCM canonical_definition = canonicalize_define (expr);
const SCM cdr_canonical_definition = SCM_CDR (canonical_definition);
const SCM variable = SCM_CAR (cdr_canonical_definition);
+ const SCM value = scm_eval_car (SCM_CDR (cdr_canonical_definition), env);
const SCM location
= scm_sym2var (variable, scm_env_top_level (env), SCM_BOOL_T);
- const SCM value = scm_eval_car (SCM_CDR (cdr_canonical_definition), env);
if (SCM_REC_PROCNAMES_P)
{
--- orig/test-suite/ChangeLog
+++ mod/test-suite/ChangeLog
@@ -1,3 +1,11 @@
+2007-06-13 Ludovic Courtès <address@hidden>
+
+ * tests/syntax.test (top-level define)[binding is created before
+ expression is evaluated]: Moved to "internal define", using `let'
+ instead of `begin'. The test was not necessarily valid for
+ top-level defines, according to Section 5.2.1 or R5RS.
+ [redefinition]: New.
+
2007-06-12 Ludovic Courtès <address@hidden>
* tests/socket.test: Renamed module to `(test-suite test-socket)'.
--- orig/test-suite/tests/syntax.test
+++ mod/test-suite/tests/syntax.test
@@ -725,15 +725,16 @@
(with-test-prefix "top-level define"
- (pass-if "binding is created before expression is evaluated"
- (= (eval '(begin
- (define foo
- (begin
- (set! foo 1)
- (+ foo 1)))
- foo)
- (interaction-environment))
- 2))
+ (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. According to R5RS (Section 5.2.1), `define'
+ ;; should behave like `set!' in this case (except that in the case of
+ ;; Guile, we respect module boundaries).
+ (eval '(define round round) m)
+ (eq? (module-ref m 'round) round)))
(with-test-prefix "currying"
@@ -780,6 +781,17 @@
(eq? 'c (a 2) (a 5))))
(interaction-environment)))
+ (pass-if "binding is created before expression is evaluated"
+ ;; Internal defines are equivalent to `letrec' (R5RS, Section 5.2.2).
+ (= (eval '(let ()
+ (define foo
+ (begin
+ (set! foo 1)
+ (+ foo 1)))
+ foo)
+ (interaction-environment))
+ 2))
+
(pass-if "internal defines with begin"
(false-if-exception
(eval '(let ((a identity) (b identity) (c identity))
* added files
--- /dev/null
+++
mod/{arch}/guile-core/guile-core--cvs-head/guile-core--cvs-head--0/address@hidden/patch-log/patch-53
@@ -0,0 +1,28 @@
+Revision: guile-core--cvs-head--0--patch-53
+Archive: address@hidden
+Creator: Ludovic Court`es <address@hidden>
+Date: Sat May 26 16:22:22 CEST 2007
+Standard-date: 2007-05-26 14:22:22 GMT
+Modified-files: libguile/eval.c
+ test-suite/tests/syntax.test
+New-patches: address@hidden/guile-core--devo--1.7--patch-27
+ address@hidden/guile-core--devo--1.7--patch-28
+ address@hidden/guile-core--devo--1.7--patch-29
+ address@hidden/guile-core--cvs-head--0--patch-53
+ address@hidden/guile-core--devo--0--patch-19
+ address@hidden/guile-core--devo--0--patch-20
+Summary: Allow for `(define round round)'.
+Keywords: module-make-local-var! scm_m_define
+
+Patches applied:
+
+ * address@hidden/guile-core--devo--1.7 (patch 27-29)
+
+ - variable lookup: Allow for `(define round round)'.
+ - Reverted the earlier patch and properly fixed `(define round round)'.
+ - Merge from address@hidden/guile-core--devo--0
+
+ * guile-core--devo--0 (patch 19-20)
+
+ - variable lookup: Allow for `(define round round)'.
+ - Reverted the earlier patch and properly fixed `(define round round)'.
--- /dev/null
+++
mod/{arch}/guile-core/guile-core--cvs-head/guile-core--cvs-head--0/address@hidden/patch-log/patch-54
@@ -0,0 +1,15 @@
+Revision: guile-core--cvs-head--0--patch-54
+Archive: address@hidden
+Creator: Ludovic Court`es <address@hidden>
+Date: Sat May 26 16:29:54 CEST 2007
+Standard-date: 2007-05-26 14:29:54 GMT
+Modified-files: libguile/ChangeLog test-suite/ChangeLog
+New-patches: address@hidden/guile-core--cvs-head--0--patch-54
+Summary: Updated ChangeLogs wrt. previous patch.
+Keywords:
+
+Patches applied:
+
+ * address@hidden/guile-core--cvs--1.7--patch-40
+ Avoid C++-style casts in `numbers.c' (Mike Gran).
+
--- /dev/null
+++
mod/{arch}/guile-core/guile-core--devo/guile-core--devo--0/address@hidden/patch-log/patch-19
@@ -0,0 +1,17 @@
+Revision: guile-core--devo--0--patch-19
+Archive: address@hidden
+Creator: Ludovic Courtes <address@hidden>
+Date: Sat May 26 16:12:14 CEST 2007
+Standard-date: 2007-05-26 14:12:14 GMT
+Modified-files: ice-9/boot-9.scm
+ test-suite/tests/modules.test
+New-patches: address@hidden/guile-core--devo--1.7--patch-27
+ address@hidden/guile-core--devo--0--patch-19
+Summary: variable lookup: Allow for `(define round round)'.
+Keywords: redefinition scm_m_define
+
+* ice-9/boot-9.scm (module-make-local-var!): Reverted to its previous
+ definition, i.e., look for a same-named imported var and use its value.
+
+* test-suite/tests/modules.test (redefinition): Added a test case that
+ previously failed.
--- /dev/null
+++
mod/{arch}/guile-core/guile-core--devo/guile-core--devo--0/address@hidden/patch-log/patch-20
@@ -0,0 +1,26 @@
+Revision: guile-core--devo--0--patch-20
+Archive: address@hidden
+Creator: Ludovic Courtes <address@hidden>
+Date: Sat May 26 16:13:57 CEST 2007
+Standard-date: 2007-05-26 14:13:57 GMT
+Modified-files: ice-9/boot-9.scm libguile/eval.c
+ test-suite/tests/modules.test
+ test-suite/tests/syntax.test
+New-patches: address@hidden/guile-core--devo--1.7--patch-28
+ address@hidden/guile-core--devo--1.7--patch-29
+ address@hidden/guile-core--devo--0--patch-20
+Summary: Reverted the earlier patch and properly fixed `(define round round)'.
+Keywords: module-make-local-var!
+
+* ice-9/boot-9.scm (module-make-local-var!): Reverted to the previous
+ value, i.e., to not check for same-named imported variables.
+
+* libguile/eval.c (scm_m_define): Updated comment. Changed order for
+ value evaluation and `scm_sym2var ()' call, which is perfectly valid
+ per R5RS.
+
+* test-suite/tests/modules.test (foundations)[redefinition]: Removed.
+
+* test-suite/tests/syntax.test (top-level define)[binding is created
+ before expression is evaluated]: Moved to "internal define".
+ [redefinition]: New.
--- /dev/null
+++
mod/{arch}/guile-core/guile-core--devo/guile-core--devo--1.7/address@hidden/patch-log/patch-27
@@ -0,0 +1,16 @@
+Revision: guile-core--devo--1.7--patch-27
+Archive: address@hidden
+Creator: Ludovic Courtes <address@hidden>
+Date: Fri May 25 22:47:29 CEST 2007
+Standard-date: 2007-05-25 20:47:29 GMT
+Modified-files: ice-9/boot-9.scm
+ test-suite/tests/modules.test
+New-patches: address@hidden/guile-core--devo--1.7--patch-27
+Summary: variable lookup: Allow for `(define round round)'.
+Keywords: redefinition scm_m_define
+
+* ice-9/boot-9.scm (module-make-local-var!): Reverted to its previous
+ definition, i.e., look for a same-named imported var and use its value.
+
+* test-suite/tests/modules.test (redefinition): Added a test case that
+ previously failed.
--- /dev/null
+++
mod/{arch}/guile-core/guile-core--devo/guile-core--devo--1.7/address@hidden/patch-log/patch-28
@@ -0,0 +1,24 @@
+Revision: guile-core--devo--1.7--patch-28
+Archive: address@hidden
+Creator: Ludovic Courtes <address@hidden>
+Date: Sat May 26 16:07:17 CEST 2007
+Standard-date: 2007-05-26 14:07:17 GMT
+Modified-files: ice-9/boot-9.scm libguile/eval.c
+ test-suite/tests/modules.test
+ test-suite/tests/syntax.test
+New-patches: address@hidden/guile-core--devo--1.7--patch-28
+Summary: Reverted the earlier patch and properly fixed `(define round round)'.
+Keywords: module-make-local-var!
+
+* ice-9/boot-9.scm (module-make-local-var!): Reverted to the previous
+ value, i.e., to not check for same-named imported variables.
+
+* libguile/eval.c (scm_m_define): Updated comment. Changed order for
+ value evaluation and `scm_sym2var ()' call, which is perfectly valid
+ per R5RS.
+
+* test-suite/tests/modules.test (foundations)[redefinition]: Removed.
+
+* test-suite/tests/syntax.test (top-level define)[binding is created
+ before expression is evaluated]: Moved to "internal define".
+ [redefinition]: New.
--- /dev/null
+++
mod/{arch}/guile-core/guile-core--devo/guile-core--devo--1.7/address@hidden/patch-log/patch-29
@@ -0,0 +1,14 @@
+Revision: guile-core--devo--1.7--patch-29
+Archive: address@hidden
+Creator: Ludovic Courtes <address@hidden>
+Date: Sat May 26 16:10:20 CEST 2007
+Standard-date: 2007-05-26 14:10:20 GMT
+New-patches: address@hidden/guile-core--devo--1.7--patch-29
+ address@hidden/guile-core--devo--0--patch-18
+Summary: Merge from address@hidden/guile-core--devo--0
+
+Patches applied:
+
+ * address@hidden/guile-core--devo--0 (patch 18)
+
+ - Fixed SRFI-19's `time-process'. Reported by Scott Shedden.
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: slowness in guile 1.8,
Ludovic Courtès <=