From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: ludo@chbouib.org (Ludovic =?iso-8859-1?Q?Court=E8s?=) Newsgroups: gmane.lisp.guile.devel Subject: Re: slowness in guile 1.8 Date: Thu, 14 Jun 2007 00:24:13 +0200 Message-ID: <87bqfj7asy.fsf@chbouib.org> References: <1180110804.4388.16.camel@localhost.localdomain> <87tzu0pybk.fsf@chbouib.org> <1180176594.4388.29.camel@localhost.localdomain> <87sl9jn2tl.fsf@chbouib.org> <87k5uviqyq.fsf@chbouib.org> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: sea.gmane.org 1181773509 821 80.91.229.12 (13 Jun 2007 22:25:09 GMT) X-Complaints-To: usenet@sea.gmane.org NNTP-Posting-Date: Wed, 13 Jun 2007 22:25:09 +0000 (UTC) To: guile-devel@gnu.org Original-X-From: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Thu Jun 14 00:25:07 2007 Return-path: Envelope-to: guile-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.50) id 1HybH0-0004vx-Eo for guile-devel@m.gmane.org; Thu, 14 Jun 2007 00:25:06 +0200 Original-Received: from localhost ([127.0.0.1] helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1HybGz-0004bW-IL for guile-devel@m.gmane.org; Wed, 13 Jun 2007 18:25:05 -0400 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1HybGw-0004bM-L7 for guile-devel@gnu.org; Wed, 13 Jun 2007 18:25:02 -0400 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1HybGs-0004aE-8Z for guile-devel@gnu.org; Wed, 13 Jun 2007 18:25:00 -0400 Original-Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1HybGr-0004a5-UK for guile-devel@gnu.org; Wed, 13 Jun 2007 18:24:57 -0400 Original-Received: from main.gmane.org ([80.91.229.2] helo=ciao.gmane.org) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1HybGq-0007lB-Bj for guile-devel@gnu.org; Wed, 13 Jun 2007 18:24:57 -0400 Original-Received: from list by ciao.gmane.org with local (Exim 4.43) id 1HybGd-0001nN-Mp for guile-devel@gnu.org; Thu, 14 Jun 2007 00:24:43 +0200 Original-Received: from adh419.fdn.fr ([80.67.176.9]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Thu, 14 Jun 2007 00:24:43 +0200 Original-Received: from ludo by adh419.fdn.fr with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Thu, 14 Jun 2007 00:24:43 +0200 X-Injected-Via-Gmane: http://gmane.org/ Original-Lines: 371 Original-X-Complaints-To: usenet@sea.gmane.org X-Gmane-NNTP-Posting-Host: adh419.fdn.fr X-URL: http://www.laas.fr/~lcourtes/ X-PGP-Key-ID: 0xEB1F5364 X-PGP-Key: http://www.laas.fr/~lcourtes/ludovic.asc X-PGP-Fingerprint: 821D 815D 902A 7EAB 5CEE D120 7FBA 3D4F EB1F 5364 X-OS: i486-pc-linux-gnu User-Agent: Gnus/5.110006 (No Gnus v0.6) Emacs/21.4 (gnu/linux) Cancel-Lock: sha1:inU1oQuJ12ap4BiD2f9zVojMFG0= X-detected-kernel: Linux 2.6, seldom 2.4 (older, 4) X-BeenThere: guile-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Developers list for Guile, the GNU extensibility library" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Errors-To: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.lisp.guile.devel:6638 Archived-At: --=-=-= Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Hi, ludo@chbouib.org (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. --=-=-= Content-Type: text/x-patch; charset=iso-8859-1 Content-Disposition: inline; filename*=us-ascii''%2c%2cmodule-make-local-var.diff Content-Transfer-Encoding: 8bit Content-Description: The patch against `module-make-local-var!' in 1.8 --- orig/ChangeLog +++ mod/ChangeLog @@ -1,3 +1,7 @@ +2007-06-13 Ludovic Courtès + + * NEWS: Mention top-level define incompatible change. + 2007-06-12 Ludovic Courtès * 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 + + * 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 * 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 + + * 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 * 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 + + * 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 * 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/lcourtes@laas.fr--2006-libre/patch-log/patch-53 @@ -0,0 +1,28 @@ +Revision: guile-core--cvs-head--0--patch-53 +Archive: lcourtes@laas.fr--2006-libre +Creator: Ludovic Court`es +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: lcourtes@laas.fr--2005-mobile/guile-core--devo--1.7--patch-27 + lcourtes@laas.fr--2005-mobile/guile-core--devo--1.7--patch-28 + lcourtes@laas.fr--2005-mobile/guile-core--devo--1.7--patch-29 + lcourtes@laas.fr--2006-libre/guile-core--cvs-head--0--patch-53 + lcourtes@laas.fr--2006-libre/guile-core--devo--0--patch-19 + lcourtes@laas.fr--2006-libre/guile-core--devo--0--patch-20 +Summary: Allow for `(define round round)'. +Keywords: module-make-local-var! scm_m_define + +Patches applied: + + * lcourtes@laas.fr--2005-mobile/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 lcourtes@laas.fr--2006-libre/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/lcourtes@laas.fr--2006-libre/patch-log/patch-54 @@ -0,0 +1,15 @@ +Revision: guile-core--cvs-head--0--patch-54 +Archive: lcourtes@laas.fr--2006-libre +Creator: Ludovic Court`es +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: lcourtes@laas.fr--2006-libre/guile-core--cvs-head--0--patch-54 +Summary: Updated ChangeLogs wrt. previous patch. +Keywords: + +Patches applied: + + * lcourtes@laas.fr--2005-libre/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/lcourtes@laas.fr--2006-libre/patch-log/patch-19 @@ -0,0 +1,17 @@ +Revision: guile-core--devo--0--patch-19 +Archive: lcourtes@laas.fr--2006-libre +Creator: Ludovic Courtes +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: lcourtes@laas.fr--2005-mobile/guile-core--devo--1.7--patch-27 + lcourtes@laas.fr--2006-libre/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/lcourtes@laas.fr--2006-libre/patch-log/patch-20 @@ -0,0 +1,26 @@ +Revision: guile-core--devo--0--patch-20 +Archive: lcourtes@laas.fr--2006-libre +Creator: Ludovic Courtes +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: lcourtes@laas.fr--2005-mobile/guile-core--devo--1.7--patch-28 + lcourtes@laas.fr--2005-mobile/guile-core--devo--1.7--patch-29 + lcourtes@laas.fr--2006-libre/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/lcourtes@laas.fr--2005-mobile/patch-log/patch-27 @@ -0,0 +1,16 @@ +Revision: guile-core--devo--1.7--patch-27 +Archive: lcourtes@laas.fr--2005-mobile +Creator: Ludovic Courtes +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: lcourtes@laas.fr--2005-mobile/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/lcourtes@laas.fr--2005-mobile/patch-log/patch-28 @@ -0,0 +1,24 @@ +Revision: guile-core--devo--1.7--patch-28 +Archive: lcourtes@laas.fr--2005-mobile +Creator: Ludovic Courtes +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: lcourtes@laas.fr--2005-mobile/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/lcourtes@laas.fr--2005-mobile/patch-log/patch-29 @@ -0,0 +1,14 @@ +Revision: guile-core--devo--1.7--patch-29 +Archive: lcourtes@laas.fr--2005-mobile +Creator: Ludovic Courtes +Date: Sat May 26 16:10:20 CEST 2007 +Standard-date: 2007-05-26 14:10:20 GMT +New-patches: lcourtes@laas.fr--2005-mobile/guile-core--devo--1.7--patch-29 + lcourtes@laas.fr--2006-libre/guile-core--devo--0--patch-18 +Summary: Merge from lcourtes@laas.fr--2006-libre/guile-core--devo--0 + +Patches applied: + + * lcourtes@laas.fr--2006-libre/guile-core--devo--0 (patch 18) + + - Fixed SRFI-19's `time-process'. Reported by Scott Shedden. --=-=-= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Guile-devel mailing list Guile-devel@gnu.org http://lists.gnu.org/mailman/listinfo/guile-devel --=-=-=--