Take some lowhanging fruit to speed up R6RS fixnum operations

Take some lowhanging fruit to speed up R6RS fixnum operations

[Andreas Rottmann]

In porting dorodango[0], I have noticed that Guile's R6RS fixnum
operations are quite slow; here's a patch to remedy that a bit.  The
patch contains a benchmark for `fxxor', which experiences a performance
improvement of >2 with the patch.

[0] http://home.gna.org/dorodango/

I also used real code to validate the performance impact, namely Göran
Weinholt's ZIP implementation from his industria collection[1]. There
the patch makes for an ~1.7x improvement for uncompressing a 675KiB
(compressed) ZIP file containing text files that yield good compression.

[1] https://code.launchpad.net/~weinholt/scheme-libraries/industria

However, it must be noted that while this patch improves things,
uncompressing the aforementioned ZIP file takes still 42 seconds on my
machine (down from 74 before).  I have another patch in the queue to
improve this by making `fixnum?' a VM primitive, but even with that,
performance is not stellar.

[PATCH] Take some lowhanging fruit to speed up R6RS fixnum operations

[Andreas Rottmann]

* module/rnrs/arithmetic/fixnums.scm (assert-fixnum): Is now a
  macro.
  (assert-fixnums): New procedure checking a the elements of a list
  for fixnum-ness.  All callers applying `assert-fixnum' to a list
  now changed to use this procedure.

* module/rnrs/arithmetic/fixnums.scm (define-fxop*): New macro for
  defining n-ary procedures special-casing the binary case via
  case-lambda.  All applicable procedures redefined using this
  macro.

* benchmark-suite/benchmarks/r6rs-arithmetic.bm: New file containing
  some benchmarks for R6RS fixnum operations.
---
 benchmark-suite/benchmarks/r6rs-arithmetic.bm |   35 +++++++++++++
 module/rnrs/arithmetic/fixnums.scm            |   69 +++++++++++--------------
 2 files changed, 66 insertions(+), 38 deletions(-)
 create mode 100644 benchmark-suite/benchmarks/r6rs-arithmetic.bm

diff --git a/benchmark-suite/benchmarks/r6rs-arithmetic.bm b/benchmark-suite/benchmarks/r6rs-arithmetic.bm
new file mode 100644
index 0000000..4c9b8e6
--- /dev/null
+++ b/benchmark-suite/benchmarks/r6rs-arithmetic.bm
@@ -0,0 +1,35 @@
+;;; -*- mode: scheme; coding: utf-8; -*-
+;;; R6RS-specific arithmetic benchmarks
+;;;
+;;; Copyright (C) 2011 Free Software Foundation, Inc.
+;;;
+;;; This library is free software; you can redistribute it and/or
+;;; modify it under the terms of the GNU Lesser General Public
+;;; License as published by the Free Software Foundation; either
+;;; version 3 of the License, or (at your option) any later version.
+;;;
+;;; This library is distributed in the hope that it will be useful,
+;;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;;; Lesser General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU Lesser General Public
+;;; License along with this library. If not, see
+;;; <http://www.gnu.org/licenses/>.
+
+(define-module (benchmarks r6rs-arithmetic)
+  #:use-module (benchmark-suite lib)
+  #:use-module (rnrs arithmetic fixnums))
+
+\f
+(with-benchmark-prefix "fixnum"
+
+  (benchmark "fixnum? [yes]" 1e7
+    (fixnum? 10000))
+
+  (let ((n (+ most-positive-fixnum 100)))
+    (benchmark "fixnum? [no]" 1e7
+      (fixnum? n)))
+
+  (benchmark "fxxor [2]" 1e7
+    (fxxor 3 8)))
diff --git a/module/rnrs/arithmetic/fixnums.scm b/module/rnrs/arithmetic/fixnums.scm
index befbe9d..8c35dc6 100644
--- a/module/rnrs/arithmetic/fixnums.scm
+++ b/module/rnrs/arithmetic/fixnums.scm
@@ -87,6 +87,7 @@
 			most-negative-fixnum)
 	  (ice-9 optargs)
 	  (rnrs base (6))
+	  (rnrs control (6))
 	  (rnrs arithmetic bitwise (6))
 	  (rnrs conditions (6))
 	  (rnrs exceptions (6))
@@ -105,50 +106,42 @@
 	 (>= obj most-negative-fixnum) 
 	 (<= obj most-positive-fixnum)))
 
-  (define (assert-fixnum . args)
+  (define-syntax assert-fixnum
+    (syntax-rules ()
+      ((_ arg ...)
+       (or (and (fixnum? arg) ...)
+	   (raise (make-assertion-violation))))))
+
+  (define (assert-fixnums args)
     (or (for-all fixnum? args) (raise (make-assertion-violation))))
 
-  (define (fx=? fx1 fx2 . rst)
-    (let ((args (cons* fx1 fx2 rst)))
-      (apply assert-fixnum args) 
-      (apply = args)))
-
-  (define (fx>? fx1 fx2 . rst)
-    (let ((args (cons* fx1 fx2 rst))) 
-      (apply assert-fixnum args) 
-      (apply > args)))
-
-  (define (fx<? fx1 fx2 . rst)
-    (let ((args (cons* fx1 fx2 rst)))
-      (apply assert-fixnum rst)
-      (apply < args)))
-
-  (define (fx>=? fx1 fx2 . rst)
-    (let ((args (cons* fx1 fx2 rst)))
-      (apply assert-fixnum rst)
-      (apply >= args)))
-
-  (define (fx<=? fx1 fx2 . rst)
-    (let ((args (cons* fx1 fx2 rst)))
-      (apply assert-fixnum rst)
-      (apply <= args)))
-  
+  (define-syntax define-fxop*
+    (syntax-rules ()
+      ((_ name op)
+       (define name
+	 (case-lambda
+	   ((x y)
+	    (assert-fixnum x y)
+	    (op x y))
+	   (args
+	    (assert-fixnums args)
+	    (apply op args)))))))
+
+  (define-fxop* fx=? =)
+  (define-fxop* fx>? >)
+  (define-fxop* fx<? <)
+  (define-fxop* fx>=? >=)
+  (define-fxop* fx<=? <=)
+
   (define (fxzero? fx) (assert-fixnum fx) (zero? fx))
   (define (fxpositive? fx) (assert-fixnum fx) (positive? fx))
   (define (fxnegative? fx) (assert-fixnum fx) (negative? fx))
   (define (fxodd? fx) (assert-fixnum fx) (odd? fx))
   (define (fxeven? fx) (assert-fixnum fx) (even? fx))
 
-  (define (fxmax fx1 fx2 . rst)
-    (let ((args (cons* fx1 fx2 rst)))
-      (apply assert-fixnum args)
-      (apply max args)))
+  (define-fxop* fxmax max)
+  (define-fxop* fxmin min)
 
-  (define (fxmin fx1 fx2 . rst)
-    (let ((args (cons* fx1 fx2 rst)))
-      (apply assert-fixnum args)
-      (apply min args)))
- 
   (define (fx+ fx1 fx2)
     (assert-fixnum fx1 fx2) 
     (let ((r (+ fx1 fx2))) 
@@ -219,9 +212,9 @@
       (values s0 s1)))
 
   (define (fxnot fx) (assert-fixnum fx) (lognot fx))
-  (define (fxand . args) (apply assert-fixnum args) (apply logand args))
-  (define (fxior . args) (apply assert-fixnum args) (apply logior args))
-  (define (fxxor . args) (apply assert-fixnum args) (apply logxor args))
+  (define-fxop* fxand logand)
+  (define-fxop* fxior logior)
+  (define-fxop* fxxor logxor)
 
   (define (fxif fx1 fx2 fx3) 
     (assert-fixnum fx1 fx2 fx3) 
-- 
1.7.4.1

Re: [PATCH] Take some lowhanging fruit to speed up R6RS fixnum operations

[Ludovic Courtès]

Hi Andreas,

I’m all for your suggestion.

Andreas Rottmann <a.rottmann@gmx.at> writes:

> +  (define-syntax define-fxop*
> +    (syntax-rules ()
> +      ((_ name op)
> +       (define name
> +	 (case-lambda
> +	   ((x y)
> +	    (assert-fixnum x y)
> +	    (op x y))
> +	   (args
> +	    (assert-fixnums args)
> +	    (apply op args)))))))
> +
> +  (define-fxop* fx=? =)

How about making something like this (untested):

  (define-syntax define-fxop*
    (syntax-rules ()
      ((_ name op)
       (define-syntax name
         (lambda (s)
           (syntax-case s ()
             ((_ x y)
              #'(begin
                  (assert-fixnum x y)
                  (op x y)))
             ((_ args ...)
              #'(apply op args))
             (_ #'op)))))))

This way, there’d be no procedure call involved and ‘=’, ‘+’, etc. would
use the corresponding instruction in the base case.

Thanks,
Ludo’.

Re: [PATCH] Take some lowhanging fruit to speed up R6RS fixnum operations

[Andreas Rottmann]

ludo@gnu.org (Ludovic Courtès) writes:

> Hi Andreas,
>
> I’m all for your suggestion.
>
> Andreas Rottmann <a.rottmann@gmx.at> writes:
>
>> +  (define-syntax define-fxop*
>> +    (syntax-rules ()
>> +      ((_ name op)
>> +       (define name
>> +	 (case-lambda
>> +	   ((x y)
>> +	    (assert-fixnum x y)
>> +	    (op x y))
>> +	   (args
>> +	    (assert-fixnums args)
>> +	    (apply op args)))))))
>> +
>> +  (define-fxop* fx=? =)
>
> How about making something like this (untested):
>
>   (define-syntax define-fxop*
>     (syntax-rules ()
>       ((_ name op)
>        (define-syntax name
>          (lambda (s)
>            (syntax-case s ()
>              ((_ x y)
>               #'(begin
>                   (assert-fixnum x y)
>                   (op x y)))
>              ((_ args ...)
>               #'(apply op args))
>              (_ #'op)))))))
>
> This way, there’d be no procedure call involved and ‘=’, ‘+’, etc. would
> use the corresponding instruction in the base case.
>
That's an excellent idea, and would probably boost performance quite a
bit.  However, there's still the issue that the non-two-args cases don't
throw the exceptions required by R6RS.  Upon trying to interpret the
R6RS in a way that would lift that requirement, I still see no way to do
so in the general case, due to the following quite explicit language:

  Fixnum operations perform integer arithmetic on their fixnum
  arguments, but raise an exception with condition type
  &implementation-restriction if the result is not a fixnum.

So even if checking the arguments seems to be not explicitly required
(but might be implictly intended -- "fixnum arguments"), the result must
definitly be a fixnum, or the mentioned exception be thrown.

So for the sake of efficiency, while heeding what is explictly required,
I propose the following:

- Alias all fixnum operations that yield non-fixnums (e.g. fx<?, fx>?,
  ...) with the corresponding non-fixnum procedures.  IMHO, that seems
  to be within bounds wrt. R6RS; Section 5.4 "Argument checking"
  says:

  The implementation must check that the restrictions in the
  specification are indeed met, to the extent that it is reasonable,
  possible, and necessary to allow the specified operation to complete
  successfully.  

  One might argue that it is not reasonable (for efficiency reasons
  related to the way Guile is currently implemented) and certainly not
  necessary to check the arguments for fixnum-ness in these procedures.

- Otherwise, do the macro trick as above, but fall back on a procedure
  like in SRFI-9's `define-inlinable' for the non-binary case.

Thoughts?

Regards, Rotty
-- 
Andreas Rottmann -- <http://rotty.yi.org/>

Re: [PATCH] Take some lowhanging fruit to speed up R6RS fixnum operations

[Andreas Rottmann]

Andreas Rottmann <a.rottmann@gmx.at> writes:

> ludo@gnu.org (Ludovic Courtès) writes:
>
>> Hi Andreas,
>>
>> I’m all for your suggestion.
>>
>> Andreas Rottmann <a.rottmann@gmx.at> writes:
>>
>>> +  (define-syntax define-fxop*
>>> +    (syntax-rules ()
>>> +      ((_ name op)
>>> +       (define name
>>> +	 (case-lambda
>>> +	   ((x y)
>>> +	    (assert-fixnum x y)
>>> +	    (op x y))
>>> +	   (args
>>> +	    (assert-fixnums args)
>>> +	    (apply op args)))))))
>>> +
>>> +  (define-fxop* fx=? =)
>>
>> How about making something like this (untested):
>>
>>   (define-syntax define-fxop*
>>     (syntax-rules ()
>>       ((_ name op)
>>        (define-syntax name
>>          (lambda (s)
>>            (syntax-case s ()
>>              ((_ x y)
>>               #'(begin
>>                   (assert-fixnum x y)
>>                   (op x y)))
>>              ((_ args ...)
>>               #'(apply op args))
>>              (_ #'op)))))))
>>
>> This way, there’d be no procedure call involved and ‘=’, ‘+’, etc. would
>> use the corresponding instruction in the base case.
>>
> That's an excellent idea, and would probably boost performance quite a
> bit.
>
Sorry to reply to myself, but new evidence has been produced :-).

Using the following macro:

  (define-syntax define-fxop*
    (lambda (stx)
      (define prefix (string->symbol "% "))
      (define (make-procedure-name name)
	(datum->syntax name
		       (symbol-append prefix (syntax->datum name)
				      '-procedure)))
      (syntax-case stx ()
	((_ name op)
	 (with-syntax ((proc-name (make-procedure-name #'name)))
	   #'(begin
	       (define (proc-name . args)
		 (assert-fixnums args)
		 (apply op args))
	       (define-syntax name
		 (lambda (stx)
		   (syntax-case stx ()
		     ((_ x-expr y-expr)
		      #'(let ((x x-expr)
			      (y y-expr))
			  (assert-fixnum x y)
			  (op x y)))
		     ((_ . args)
		      #'(proc-name . args))
		     (_
		      (identifier? stx)
		      #'proc-name))))))))))

It turns out that this does not have a great effect on the efficiency;
here are the numbers:

    * stable-2.0 + rotty/wip-fixnum-speed (simple define-fxop*, clever fixnum?)

    #+begin_example
    % _build/meta/guile -e main -s benchmark-suite/guile-benchmark --benchmark-suite benchmarks r6rs-arithmetic.bm
    ;; running guile version 2.0.0.133-e47c9-dirty
    ;; calibrating the benchmarking framework...
    ;; framework time per iteration: 1.1444091796875e-7
    ("r6rs-arithmetic.bm: fixnum: fixnum? [yes]" 10000000 user 3.42 benchmark 2.2755908203125 bench/interp 2.2755908203125 gc 0.0)
    ("r6rs-arithmetic.bm: fixnum: fixnum? [no]" 10000000 user 3.42 benchmark 2.2755908203125 bench/interp 2.2755908203125 gc 0.0)
    ("r6rs-arithmetic.bm: fixnum: fxxor [2]" 10000000 user 6.96 benchmark 5.8155908203125 bench/interp 5.8155908203125 gc 0.0)

    scheme@(guile-user)> ,time (run-benchmark)
    clock utime stime cutime cstime gctime
    23.81 23.76  0.00   0.00   0.00   0.00
    #+end_example

    * stable-2.0 + rotty/wip-fixnum-speed (macro-based define-fxop*, clever fixnum?)

    #+begin_example
    % _build/meta/guile -e main -s benchmark-suite/guile-benchmark --benchmark-suite benchmarks r6rs-arithmetic.bm
    ;; running guile version 2.0.0.133-e47c9-dirty
    ;; calibrating the benchmarking framework...
    ;; framework time per iteration: 1.52587890625e-7
    ("r6rs-arithmetic.bm: fixnum: fixnum? [yes]" 10000000 user 3.4 benchmark 1.87412109375 bench/interp 1.87412109375 gc 0.0)
    ("r6rs-arithmetic.bm: fixnum: fixnum? [no]" 10000000 user 3.42 benchmark 1.89412109375 bench/interp 1.89412109375 gc 0.0)
    ("r6rs-arithmetic.bm: fixnum: fxxor [2]" 10000000 user 6.72 benchmark 5.19412109375 bench/interp 5.19412109375 gc 0.0)

    scheme@(guile-user)> ,time (run-benchmark)
    clock utime stime cutime cstime gctime
    22.76 22.73  0.00   0.00   0.00   0.00
    #+end_example

So, that's around 5% improvment (on the ZIP benchmark) for an IMHO
significantly more hackish implementation.  I'm not sure that's worth
it. WDYT?

Regards, Rotty
-- 
Andreas Rottmann -- <http://rotty.yi.org/>

Re: [PATCH] Take some lowhanging fruit to speed up R6RS fixnum operations

[Ludovic Courtès]

Hello,

Andreas Rottmann <a.rottmann@gmx.at> writes:

> So, that's around 5% improvment (on the ZIP benchmark) for an IMHO
> significantly more hackish implementation.  I'm not sure that's worth
> it. WDYT?

Was it with ‘fixnum?’ inline, or with the ‘fixnum?’ instruction?

It’s ironic that while R6RS fixnums are a performance hack, they end up
being less efficient than unbounded integers in Guile.

Do you know how other implementations deal with that?

Thanks,
Ludo’.

Re: [PATCH] Take some lowhanging fruit to speed up R6RS fixnum operations

[Andreas Rottmann]

ludo@gnu.org (Ludovic Courtès) writes:

> Hello,
>
> Andreas Rottmann <a.rottmann@gmx.at> writes:
>
>> So, that's around 5% improvment (on the ZIP benchmark) for an IMHO
>> significantly more hackish implementation.  I'm not sure that's worth
>> it. WDYT?
>
> Was it with ‘fixnum?’ inline, or with the ‘fixnum?’ instruction?
>
The numbers from my previous mail were with the "clever" `fixnum?'
(i.e. the one that uses object-address and bit-twiddling), defined with
`define-inline'.  I have yet to produce numbers with `fixnum?' as VM
primitive and the more complicated, partially-inlinable fixnum
operators.  I'll keep you posted.

> It’s ironic that while R6RS fixnums are a performance hack, they end up
> being less efficient than unbounded integers in Guile.
>
Indeed.

> Do you know how other implementations deal with that?
>
I will do some research on that topic, investigating Racket, Ikarus and
Ypsilon -- these are the R6RS implementations, which I'm most familiar
with (besides Guile, obviously).

Regards, Rotty
-- 
Andreas Rottmann -- <http://rotty.yi.org/>

Re: Take some lowhanging fruit to speed up R6RS fixnum operations

[Andy Wingo]

On Wed 23 Mar 2011 00:20, Andreas Rottmann <a.rottmann@gmx.at> writes:

> In porting dorodango[0], I have noticed that Guile's R6RS fixnum
> operations are quite slow; here's a patch to remedy that a bit.

What is the state of things here?  I'm a bit lost in the discussion :)

You said that decompressing the zip file takes 74 seconds with Guile as
it is now.  What if you change to use Guile's arithmetic instead of
fxops?  That would give is a good baseline so that we can know how much
overhead the fxops have.

Andy
-- 
http://wingolog.org/

Re: Take some lowhanging fruit to speed up R6RS fixnum operations

[Andreas Rottmann]

Andy Wingo <wingo@pobox.com> writes:

> On Wed 23 Mar 2011 00:20, Andreas Rottmann <a.rottmann@gmx.at> writes:
>
>> In porting dorodango[0], I have noticed that Guile's R6RS fixnum
>> operations are quite slow; here's a patch to remedy that a bit.
>
> What is the state of things here?  I'm a bit lost in the discussion :)
>
I'll post a summary mail regarding performace soonish; it's quite
tedious to set up branches for all the possibly interesting combinations
of optimizations, and then actually running the benchmarks.

> You said that decompressing the zip file takes 74 seconds with Guile as
> it is now.  What if you change to use Guile's arithmetic instead of
> fxops?  That would give is a good baseline so that we can know how much
> overhead the fxops have.
>
I'll put numbers for that into the benchmark; i.e. with a `(rnrs
arithmetic fixnum)' module just containing aliases.

Regards, Rotty
-- 
Andreas Rottmann -- <http://rotty.yi.org/>

Re: Take some lowhanging fruit to speed up R6RS fixnum operations

[Andreas Rottmann]

Andy Wingo <wingo@pobox.com> writes:

> On Wed 23 Mar 2011 00:20, Andreas Rottmann <a.rottmann@gmx.at> writes:
>
>> In porting dorodango[0], I have noticed that Guile's R6RS fixnum
>> operations are quite slow; here's a patch to remedy that a bit.
>
> What is the state of things here?  I'm a bit lost in the discussion :)
>
> You said that decompressing the zip file takes 74 seconds with Guile as
> it is now.  What if you change to use Guile's arithmetic instead of
> fxops?  That would give is a good baseline so that we can know how much
> overhead the fxops have.
>
OK, here are a bunch of numbers that will hopefully help to determine
the desired course of action:

| Codebase         | fixnum? | fxxor | unzip |
|------------------+---------+-------+-------|
| vanilla          |    5.66 | 24.69 |  65.9 |
| generic          |    5.62 |  1.92 |   7.7 |
| smart            |    3.42 | 21.29 |  56.3 |
| smart + opt        |    3.42 |  6.99 |  21.9 |
| smart + opt-macros |     3.4 |  5.56 |  21.0 |
| vmop             |    1.78 | 18.83 |  47.9 |
| vmop+opt         |    1.77 |  3.94 |  14.1 |
| vmop+opt-macros  |    1.78 |   3.7 |  13.3 |

As you can see, in the "vanilla" case (i.e. current stable-2.0), it's
65.9 seconds -- I do not know where I got the 74 second number from,
perhaps it was from a profiling run.

The "generic" case is the one where all fixnum checks have been
eliminated, thus allowing the compiler to emit VM primitives for most of
the operations.

Now, with these baselines established, I implemented the following
optimizations, and benchmarked combinations of them as can be seen from
the above table.

-- 
Andreas Rottmann -- <http://rotty.yi.org/>

Re: Take some lowhanging fruit to speed up R6RS fixnum operations

[Andreas Rottmann]

[ Sorry for the incomplete mail I just sent; hit the wrong key combo ]

Andy Wingo <wingo@pobox.com> writes:

> On Wed 23 Mar 2011 00:20, Andreas Rottmann <a.rottmann@gmx.at> writes:
>
>> In porting dorodango[0], I have noticed that Guile's R6RS fixnum
>> operations are quite slow; here's a patch to remedy that a bit.
>
> What is the state of things here?  I'm a bit lost in the discussion :)
>
> You said that decompressing the zip file takes 74 seconds with Guile as
> it is now.  What if you change to use Guile's arithmetic instead of
> fxops?  That would give is a good baseline so that we can know how much
> overhead the fxops have.
>
As promised, here are a bunch of numbers that will hopefully help to
determine the desired course of action:

| Codebase           | fixnum? | fxxor | unzip |
|--------------------+---------+-------+-------|
| vanilla            |    5.66 | 24.69 |  65.9 |
| generic            |    5.62 |  1.92 |   7.7 |
| smart              |    3.42 | 21.29 |  56.3 |
| smart + opt        |    3.42 |  6.99 |  21.9 |
| smart + opt-macros |     3.4 |  5.56 |  21.0 |
| vmop               |    1.78 | 18.83 |  47.9 |
| vmop + opt         |    1.77 |  3.94 |  14.1 |
| vmop + opt-macros  |    1.78 |   3.7 |  13.3 |

As you can see, in the "vanilla" case (i.e. current stable-2.0), it's
65.9 seconds -- I do not know where I got the 74 second number from,
perhaps it was from a profiling run.

The "generic" case is the one where all fixnum checks have been
eliminated (i.e. using the generic procedures, such as logxor), thus
allowing the compiler to emit VM primitives for most of the operations.

Now, with these baselines established, I implemented the following
optimizations, and benchmarked combinations of them as can be seen from
the above table:

- "smart" refers to having `fixnum?' implemented via `define-inline' and
  using `object-address'.

- "opt" refers to having many fixnum procedures (e.g. fx+, fxxor, ...)
  going through a case-lambda, thus optimizing the common, two-argument
  case not to cons.

- "opt-macros" refers to making the same procedures as in "opt" actually
  macros, expanding to a `(let ((x x-expr) (y y-expr)) (OP x y))' in the
  binary case.

- "vmop" means that the newly-introduced VM primitive `fixnum?' is used.

Regarding the columns, "fixnum?" is the time needed for 10 million
`fixnum?' checks, "fxxor" for 10 million binary `fxxor' operations, and
"unzip" times the unzipping of a not-so-large (675KiB) ZIP file using
Göran Weinholts ZIP implementation.

Now there are several questions:

- Can we put the `fixnum?' VM primitive into stable-2.0?  AFAIK, yes we
  can, but we will break things for users mixing 2.0.0 and 2.0.1 on the
  same machine (or sharing $HOME).  So do we want to?

- Should we complicate the implementation and take a potential code size
  hit by using the "opt-macros" hack, for a 5-6% gain in speed? Of
  course, that's just for the ZIP benchmark, but I think it gives a good
  estimate in what kind of ballpark we are in here.

  My preference would be to go with the simpler implementation ("opt"
  instead of "opt-macros").

- Should further improvements be considered?  I guess if we put the
  fixnum operations into the VM, we could get as fast as the "generic"
  case, and perhaps even a bit faster (i.e. about a 2x speedup on the
  ZIP benchmark compared to vmop + opt).  I'm not sure if that's worth
  it.

Regards, Rotty
-- 
Andreas Rottmann -- <http://rotty.yi.org/>

R6RS fixnum arithmetic optimizations

[Andreas Rottmann]

After getting no feedback on my last mail so far, here is a series of
proposed patches for the stable-2.0 branch.  In total, they speed up the
unzip benchmark by a factor of about 4.7, but it's still about 1.8 times
slower than what we'd get for using generic (non-fixnum) operations.

[PATCH 1/3] Add a few benchmarks for R6RS fixnum arithmetic

[Andreas Rottmann]

* benchmark-suite/benchmarks/r6rs-arithmetic.bm: New file containing
  some benchmarks for R6RS fixnum operations.
* benchmark-suite/Makefile.am (SCM_BENCHMARKS): Add
  benchmarks/r6rs-arithmetic.
---
 benchmark-suite/Makefile.am                   |    1 +
 benchmark-suite/benchmarks/r6rs-arithmetic.bm |   35 +++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 0 deletions(-)
 create mode 100644 benchmark-suite/benchmarks/r6rs-arithmetic.bm

diff --git a/benchmark-suite/Makefile.am b/benchmark-suite/Makefile.am
index bac1df3..f29743f 100644
--- a/benchmark-suite/Makefile.am
+++ b/benchmark-suite/Makefile.am
@@ -6,6 +6,7 @@ SCM_BENCHMARKS = benchmarks/0-reference.bm		\
                  benchmarks/if.bm			\
                  benchmarks/logand.bm			\
 		 benchmarks/ports.bm			\
+		 benchmarks/r6rs-arithmetic.bm		\
 		 benchmarks/read.bm			\
 		 benchmarks/srfi-1.bm			\
 		 benchmarks/srfi-13.bm			\
diff --git a/benchmark-suite/benchmarks/r6rs-arithmetic.bm b/benchmark-suite/benchmarks/r6rs-arithmetic.bm
new file mode 100644
index 0000000..4c9b8e6
--- /dev/null
+++ b/benchmark-suite/benchmarks/r6rs-arithmetic.bm
@@ -0,0 +1,35 @@
+;;; -*- mode: scheme; coding: utf-8; -*-
+;;; R6RS-specific arithmetic benchmarks
+;;;
+;;; Copyright (C) 2011 Free Software Foundation, Inc.
+;;;
+;;; This library is free software; you can redistribute it and/or
+;;; modify it under the terms of the GNU Lesser General Public
+;;; License as published by the Free Software Foundation; either
+;;; version 3 of the License, or (at your option) any later version.
+;;;
+;;; This library is distributed in the hope that it will be useful,
+;;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;;; Lesser General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU Lesser General Public
+;;; License along with this library. If not, see
+;;; <http://www.gnu.org/licenses/>.
+
+(define-module (benchmarks r6rs-arithmetic)
+  #:use-module (benchmark-suite lib)
+  #:use-module (rnrs arithmetic fixnums))
+
+\f
+(with-benchmark-prefix "fixnum"
+
+  (benchmark "fixnum? [yes]" 1e7
+    (fixnum? 10000))
+
+  (let ((n (+ most-positive-fixnum 100)))
+    (benchmark "fixnum? [no]" 1e7
+      (fixnum? n)))
+
+  (benchmark "fxxor [2]" 1e7
+    (fxxor 3 8)))
-- 
1.7.4.1

[PATCH 2/3] Several optimizations for R6RS fixnum arithmetic

[Andreas Rottmann]

* module/rnrs/arithmetic/fixnums.scm (assert-fixnum): Is now a
  macro.
  (assert-fixnums): New procedure checking a the elements of a list
  for fixnum-ness.  All callers applying `assert-fixnum' to a list
  now changed to use this procedure.

* module/rnrs/arithmetic/fixnums.scm (define-fxop*): New for defining
  n-ary inlinable special-casing the binary case using `case-lambda'.
  All applicable procedures redefined using this macro.

* module/rnrs/arithmetic/fixnums.scm: Alias all predicates to
  their non-fixnum counterparts.
---
 module/rnrs/arithmetic/fixnums.scm |   86 +++++++++++++++++-------------------
 1 files changed, 41 insertions(+), 45 deletions(-)

diff --git a/module/rnrs/arithmetic/fixnums.scm b/module/rnrs/arithmetic/fixnums.scm
index befbe9d..03511ed 100644
--- a/module/rnrs/arithmetic/fixnums.scm
+++ b/module/rnrs/arithmetic/fixnums.scm
@@ -87,6 +87,7 @@
 			most-negative-fixnum)
 	  (ice-9 optargs)
 	  (rnrs base (6))
+	  (rnrs control (6))
 	  (rnrs arithmetic bitwise (6))
 	  (rnrs conditions (6))
 	  (rnrs exceptions (6))
@@ -105,50 +106,45 @@
 	 (>= obj most-negative-fixnum) 
 	 (<= obj most-positive-fixnum)))
 
-  (define (assert-fixnum . args)
+  (define-syntax assert-fixnum
+    (syntax-rules ()
+      ((_ arg ...)
+       (or (and (fixnum? arg) ...)
+	   (raise (make-assertion-violation))))))
+
+  (define (assert-fixnums args)
     (or (for-all fixnum? args) (raise (make-assertion-violation))))
 
-  (define (fx=? fx1 fx2 . rst)
-    (let ((args (cons* fx1 fx2 rst)))
-      (apply assert-fixnum args) 
-      (apply = args)))
-
-  (define (fx>? fx1 fx2 . rst)
-    (let ((args (cons* fx1 fx2 rst))) 
-      (apply assert-fixnum args) 
-      (apply > args)))
-
-  (define (fx<? fx1 fx2 . rst)
-    (let ((args (cons* fx1 fx2 rst)))
-      (apply assert-fixnum rst)
-      (apply < args)))
-
-  (define (fx>=? fx1 fx2 . rst)
-    (let ((args (cons* fx1 fx2 rst)))
-      (apply assert-fixnum rst)
-      (apply >= args)))
-
-  (define (fx<=? fx1 fx2 . rst)
-    (let ((args (cons* fx1 fx2 rst)))
-      (apply assert-fixnum rst)
-      (apply <= args)))
-  
-  (define (fxzero? fx) (assert-fixnum fx) (zero? fx))
-  (define (fxpositive? fx) (assert-fixnum fx) (positive? fx))
-  (define (fxnegative? fx) (assert-fixnum fx) (negative? fx))
-  (define (fxodd? fx) (assert-fixnum fx) (odd? fx))
-  (define (fxeven? fx) (assert-fixnum fx) (even? fx))
-
-  (define (fxmax fx1 fx2 . rst)
-    (let ((args (cons* fx1 fx2 rst)))
-      (apply assert-fixnum args)
-      (apply max args)))
-
-  (define (fxmin fx1 fx2 . rst)
-    (let ((args (cons* fx1 fx2 rst)))
-      (apply assert-fixnum args)
-      (apply min args)))
- 
+  (define-syntax define-fxop*
+    (syntax-rules ()
+      ((_ name op)
+       (define name
+	(case-lambda
+	  ((x y)
+	   (assert-fixnum x y)
+	   (op x y))
+	  (args
+	   (assert-fixnums args)
+           (apply op args)))))))
+
+  ;; All these predicates don't check their arguments for fixnum-ness,
+  ;; as this doesn't seem to be strictly required by R6RS.
+
+  (define fx=? =)
+  (define fx>? >)
+  (define fx<? <)
+  (define fx>=? >=)
+  (define fx<=? <=)
+
+  (define fxzero? zero?)
+  (define fxpositive? positive?)
+  (define fxnegative? negative?)
+  (define fxodd? odd?)
+  (define fxeven? even?)
+
+  (define-fxop* fxmax max)
+  (define-fxop* fxmin min)
+
   (define (fx+ fx1 fx2)
     (assert-fixnum fx1 fx2) 
     (let ((r (+ fx1 fx2))) 
@@ -219,9 +215,9 @@
       (values s0 s1)))
 
   (define (fxnot fx) (assert-fixnum fx) (lognot fx))
-  (define (fxand . args) (apply assert-fixnum args) (apply logand args))
-  (define (fxior . args) (apply assert-fixnum args) (apply logior args))
-  (define (fxxor . args) (apply assert-fixnum args) (apply logxor args))
+  (define-fxop* fxand logand)
+  (define-fxop* fxior logior)
+  (define-fxop* fxxor logxor)
 
   (define (fxif fx1 fx2 fx3) 
     (assert-fixnum fx1 fx2 fx3) 
-- 
1.7.4.1

[PATCH 3/3] Add `fixnum?' VM primitive

[Andreas Rottmann]

This primitive can be used to significantly speed up the operations in
`(rnrs arithmetic fixnums)'.

* libguile/r6rs-arithmetic.c: New file containing `fixnum?' procedure
  implementation as a new extension.
* libguile/r6rs-arithmetic.h: New file with prototypes for the above.
* libguile/Makefile.am: Add above files in relevant places.

* libguile/init.c (scm_i_init_guile): Register R6RS arithmetic
  extension.

* libguile/vm-i-scheme.c (fixnump): New VM primitive.
* module/language/tree-il/compile-glil.scm (*primcall-ops*): Add
  `fixnum?'.
* module/language/tree-il/primitives.scm
  (*interesting-primitive-names*, *effect-free-primitives*)
  (*effect+exception-free-primitives*): Add `fixnum?'.
---
 libguile/Makefile.am                     |    4 ++
 libguile/init.c                          |    2 +
 libguile/numbers.c                       |    1 -
 libguile/numbers.h                       |    1 +
 libguile/r6rs-arithmetic.c               |   48 ++++++++++++++++++++++++++++++
 libguile/r6rs-arithmetic.h               |   30 ++++++++++++++++++
 libguile/vm-i-scheme.c                   |    8 ++++-
 module/language/tree-il/compile-glil.scm |    1 +
 module/language/tree-il/primitives.scm   |    9 +++--
 module/rnrs/arithmetic/fixnums.scm       |   11 +++----
 10 files changed, 103 insertions(+), 12 deletions(-)
 create mode 100644 libguile/r6rs-arithmetic.c
 create mode 100644 libguile/r6rs-arithmetic.h

diff --git a/libguile/Makefile.am b/libguile/Makefile.am
index ac27eb8..01a384d 100644
--- a/libguile/Makefile.am
+++ b/libguile/Makefile.am
@@ -179,6 +179,7 @@ libguile_@GUILE_EFFECTIVE_VERSION@_la_SOURCES =				\
 	procs.c					\
 	programs.c				\
 	promises.c				\
+	r6rs-arithmetic.c			\
 	r6rs-ports.c				\
 	random.c				\
 	rdelim.c				\
@@ -275,6 +276,7 @@ DOT_X_FILES =					\
 	procprop.x				\
 	procs.x					\
 	promises.x				\
+	r6rs-arithmetic.x			\
 	r6rs-ports.x				\
 	random.x				\
 	rdelim.x				\
@@ -375,6 +377,7 @@ DOT_DOC_FILES = 				\
 	procprop.doc				\
 	procs.doc				\
 	promises.doc				\
+	r6rs-arithmetic.doc			\
 	r6rs-ports.doc				\
 	random.doc				\
 	rdelim.doc				\
@@ -569,6 +572,7 @@ modinclude_HEADERS =				\
 	programs.h				\
 	promises.h				\
 	pthread-threads.h			\
+	r6rs-arithmetic.h			\
 	r6rs-ports.h				\
 	random.h				\
 	rdelim.h				\
diff --git a/libguile/init.c b/libguile/init.c
index 8b3b8cd..2c23b1e 100644
--- a/libguile/init.c
+++ b/libguile/init.c
@@ -100,6 +100,7 @@
 #include "libguile/programs.h"
 #include "libguile/promises.h"
 #include "libguile/array-map.h"
+#include "libguile/r6rs-arithmetic.h"
 #include "libguile/random.h"
 #include "libguile/rdelim.h"
 #include "libguile/read.h"
@@ -403,6 +404,7 @@ scm_i_init_guile (void *base)
   scm_bootstrap_programs ();
   scm_bootstrap_vm ();
   scm_register_r6rs_ports ();
+  scm_register_r6rs_arithmetic ();
   scm_register_foreign ();
   scm_register_srfi_1 ();
   scm_register_srfi_60 ();
diff --git a/libguile/numbers.c b/libguile/numbers.c
index 427e772..0a10030 100644
--- a/libguile/numbers.c
+++ b/libguile/numbers.c
@@ -6122,7 +6122,6 @@ SCM_DEFINE (scm_integer_p, "integer?", 1, 0, 0,
 }
 #undef FUNC_NAME
 
-
 SCM scm_i_num_eq_p (SCM, SCM, SCM);
 SCM_PRIMITIVE_GENERIC (scm_i_num_eq_p, "=", 0, 2, 1,
                        (SCM x, SCM y, SCM rest),
diff --git a/libguile/numbers.h b/libguile/numbers.h
index ab96981..fb97785 100644
--- a/libguile/numbers.h
+++ b/libguile/numbers.h
@@ -240,6 +240,7 @@ SCM_API SCM scm_complex_p (SCM x);
 SCM_API SCM scm_real_p (SCM x);
 SCM_API SCM scm_rational_p (SCM z);
 SCM_API SCM scm_integer_p (SCM x);
+SCM_API SCM scm_fixnum_p (SCM x);
 SCM_API SCM scm_inexact_p (SCM x);
 SCM_API SCM scm_num_eq_p (SCM x, SCM y);
 SCM_API SCM scm_less_p (SCM x, SCM y);
diff --git a/libguile/r6rs-arithmetic.c b/libguile/r6rs-arithmetic.c
new file mode 100644
index 0000000..b00f1f4
--- /dev/null
+++ b/libguile/r6rs-arithmetic.c
@@ -0,0 +1,48 @@
+/* Copyright (C) 2011 Free Software Foundation, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either version 3 of
+ * the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>.
+ */
+
+#ifdef HAVE_CONFIG_H
+# include <config.h>
+#endif
+
+#include "libguile/_scm.h"
+#include "libguile/numbers.h"
+#include "libguile/r6rs-arithmetic.h"
+
+SCM_DEFINE (scm_fixnum_p, "fixnum?", 1, 0, 0,
+            (SCM x),
+            "Return @code{#t} if @var{x} is a fixnum, @code{#f} otherwise.")
+#define FUNC_NAME s_scm_fixnum_p
+{
+  return scm_from_bool (SCM_I_INUMP (x));
+}
+#undef FUNC_NAME
+
+void
+scm_register_r6rs_arithmetic (void)
+{
+  scm_c_register_extension ("libguile-" SCM_EFFECTIVE_VERSION,
+                            "scm_init_r6rs_arithmetic",
+			    (scm_t_extension_init_func) scm_init_r6rs_arithmetic,
+			    NULL);
+}
+
+void
+scm_init_r6rs_arithmetic (void)
+{
+#include "libguile/r6rs-arithmetic.x"
+}
diff --git a/libguile/r6rs-arithmetic.h b/libguile/r6rs-arithmetic.h
new file mode 100644
index 0000000..833426a
--- /dev/null
+++ b/libguile/r6rs-arithmetic.h
@@ -0,0 +1,30 @@
+#ifndef SCM_R6RS_ARITHMETIC_H
+#define SCM_R6RS_ARITHMETIC_H
+
+/* Copyright (C) 2011 Free Software Foundation, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either version 3 of
+ * the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>.
+ */
+
+\f
+
+#include "libguile/__scm.h"
+
+/* R6RS Fixnum Arithmetic  */
+
+SCM_API void scm_init_r6rs_arithmetic (void);
+SCM_INTERNAL void scm_register_r6rs_arithmetic (void);
+
+#endif /* SCM_R6RS_ARITHMETIC_H */
diff --git a/libguile/vm-i-scheme.c b/libguile/vm-i-scheme.c
index 9e249bc..21255c6 100644
--- a/libguile/vm-i-scheme.c
+++ b/libguile/vm-i-scheme.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 2001, 2009, 2010 Free Software Foundation, Inc.
+/* Copyright (C) 2001, 2009, 2010, 2011 Free Software Foundation, Inc.
  * 
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public License
@@ -111,6 +111,12 @@ VM_DEFINE_FUNCTION (139, vectorp, "vector?", 1)
   RETURN (scm_from_bool (SCM_I_IS_VECTOR (x)));
 }
 
+VM_DEFINE_FUNCTION (210, fixnump, "fixnum?", 1)
+{
+  ARGS1 (x);
+  RETURN (scm_from_bool (SCM_I_INUMP (x)));
+}
+
 \f
 /*
  * Basic data
diff --git a/module/language/tree-il/compile-glil.scm b/module/language/tree-il/compile-glil.scm
index f193e9d..b4d860f 100644
--- a/module/language/tree-il/compile-glil.scm
+++ b/module/language/tree-il/compile-glil.scm
@@ -108,6 +108,7 @@
    ((list? . 1) . list?)
    ((symbol? . 1) . symbol?)
    ((vector? . 1) . vector?)
+   ((fixnum? . 1) . fixnum?)
    (list . list)
    (vector . vector)
    ((class-of . 1) . class-of)
diff --git a/module/language/tree-il/primitives.scm b/module/language/tree-il/primitives.scm
index 316a462..24e6021 100644
--- a/module/language/tree-il/primitives.scm
+++ b/module/language/tree-il/primitives.scm
@@ -1,6 +1,6 @@
 ;;; open-coding primitive procedures
 
-;; Copyright (C) 2009, 2010 Free Software Foundation, Inc.
+;; Copyright (C) 2009, 2010, 2011 Free Software Foundation, Inc.
 
 ;;;; This library is free software; you can redistribute it and/or
 ;;;; modify it under the terms of the GNU Lesser General Public
@@ -21,6 +21,7 @@
 (define-module (language tree-il primitives)
   #:use-module (system base pmatch)
   #:use-module (rnrs bytevectors)
+  #:use-module (rnrs arithmetic fixnums)
   #:use-module (system base syntax)
   #:use-module (language tree-il)
   #:use-module (srfi srfi-4)
@@ -43,7 +44,7 @@
     + * - / 1- 1+ quotient remainder modulo
     ash logand logior logxor
     not
-    pair? null? list? symbol? vector? acons cons cons*
+    fixnum? pair? null? list? symbol? vector? acons cons cons*
 
     list vector
 
@@ -112,7 +113,7 @@
     = < > <= >= zero?
     + * - / 1- 1+ quotient remainder modulo
     not
-    pair? null? list? symbol? vector? acons cons cons*
+    pair? null? list? symbol? vector? fixnum? acons cons cons*
     list vector
     car cdr
     caar cadr cdar cddr
@@ -137,7 +138,7 @@
   '(values
     eq? eqv? equal?
     not
-    pair? null? list? symbol? vector? acons cons cons*
+    pair? null? list? symbol? vector? fixnum? acons cons cons*
     list vector
     struct?))
 
diff --git a/module/rnrs/arithmetic/fixnums.scm b/module/rnrs/arithmetic/fixnums.scm
index 03511ed..b519920 100644
--- a/module/rnrs/arithmetic/fixnums.scm
+++ b/module/rnrs/arithmetic/fixnums.scm
@@ -76,7 +76,9 @@
 	  fxreverse-bit-field)
   (import (only (guile) ash
 		        cons*
+			effective-version
 			inexact->exact
+			load-extension
 			logand
 			logbit?
 			logcount
@@ -93,18 +95,15 @@
 	  (rnrs exceptions (6))
 	  (rnrs lists (6)))
 
+  (load-extension (string-append "libguile-" (effective-version))
+                  "scm_init_r6rs_arithmetic")
+
   (define fixnum-width 
     (let ((w (inexact->exact (round (/ (log (+ most-positive-fixnum 1)) (log 2))))))
       (lambda () w)))
 
   (define (greatest-fixnum) most-positive-fixnum)
   (define (least-fixnum) most-negative-fixnum)
-  
-  (define (fixnum? obj) 
-    (and (integer? obj) 
-	 (exact? obj) 
-	 (>= obj most-negative-fixnum) 
-	 (<= obj most-positive-fixnum)))
 
   (define-syntax assert-fixnum
     (syntax-rules ()
-- 
1.7.4.1

Re: Take some lowhanging fruit to speed up R6RS fixnum operations

[Andy Wingo]

On Wed 30 Mar 2011 12:58, Andreas Rottmann <a.rottmann@gmx.at> writes:

> "unzip" times the unzipping of a not-so-large (675KiB) ZIP file using
> Göran Weinholts ZIP implementation.

How long does the standard unix utility take, for the record?

Andy
-- 
http://wingolog.org/

Re: [PATCH 3/3] Add `fixnum?' VM primitive

[Andy Wingo]

Hi Andreas,

I applied the first two, thanks.

I am hesitant about this one for three reasons:

 1) I don't want the compiler to import (rnrs arithmetic fixnums).
    Rather, if we were to do this, I would have that module register its
    primitives, as GOOPS does.

 2) Something about this sounds wrong to me.  If fixnum? is important,
    why not have it in Guile's default environment directly?  Or some
    other non-R6RS namespace.  What about, for example,
    `exact-integer?'?  And why for fixnums and not flonums?

 3) Are there no alternatives, like exposing Guile's tags to Scheme
    directly?  (ice-9 tags) for example.  Then we could inline tc3?,
    tc8?, and tc16? ops.  Some other predicates would become
    unnecessary; char? for example.  Perhaps this is a bad idea though.

Apologies for being crabby here after you've done all this work :)  It
could well be that something close to this is the right thing.

Cheers,

Andy
-- 
http://wingolog.org/

Re: Take some lowhanging fruit to speed up R6RS fixnum operations

[Andreas Rottmann]

Andy Wingo <wingo@pobox.com> writes:

> On Wed 30 Mar 2011 12:58, Andreas Rottmann <a.rottmann@gmx.at> writes:
>
>> "unzip" times the unzipping of a not-so-large (675KiB) ZIP file using
>> Göran Weinholts ZIP implementation.
>
> How long does the standard unix utility take, for the record?
>
On a tmpfs, with the same ZIP file:

unzip ~/public_html/tmp/bundles/xitomatl-20110103.zip  0.07s user 0.03s system 77% cpu 0.135 total

So there's quite some room for improvement.  Other data points: Ikarus
runs the benchmark in 0.55 seconds, Racket v5.1.0.2 takes 2.39 seconds.

For Dorodango, I intend to refactor ZIP support to allow for different
backends, selectable at runtime, and implement a libzip binding.  This
will get reasonable performance on implementations that don't have a
native-code compiler as fancy as Ikarus's.

Regards, Rotty
-- 
Andreas Rottmann -- <http://rotty.yi.org/>

Re: Take some lowhanging fruit to speed up R6RS fixnum operations

[Andy Wingo]

On Tue 05 Apr 2011 00:00, Andreas Rottmann <a.rottmann@gmx.at> writes:

> Andy Wingo <wingo@pobox.com> writes:
>
>> How long does the standard unix utility take, for the record?
>>
> On a tmpfs, with the same ZIP file:
>
> unzip ~/public_html/tmp/bundles/xitomatl-20110103.zip  0.07s user 0.03s system 77% cpu 0.135 total

OK, so Guile (in its standard, "generic" configuration) is about 100
times slower (7.7s) than well-optimized native code.  I guess the
positive way to spin that is that it's great that we have so much room
for improvement ;-)

Andy
-- 
http://wingolog.org/

Re: [PATCH 3/3] Add `fixnum?' VM primitive

[Andreas Rottmann]

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

Andy Wingo <wingo@pobox.com> writes:

> Hi Andreas,
>
> I applied the first two, thanks.
>
> I am hesitant about this one for three reasons:
>
>  1) I don't want the compiler to import (rnrs arithmetic fixnums).
>     Rather, if we were to do this, I would have that module register its
>     primitives, as GOOPS does.
>
>  2) Something about this sounds wrong to me.  If fixnum? is important,
>
I don't hold the opinion that `fixnum?' per se is important; it's just
that to achieve better performance for R6RS' fixnum operations (while
still remaining within R6RS spec territory), it needs to be fast, due to
the abundance of calls to it.  If, for example, we'd make fixnum
operations VM primitives, we wouldn't need to care about `fixnum?'s
speed so much.

>  why not have it in Guile's default environment directly?
>
Well, this was what my original patch did [0], but Ludovic objected [1].

[0] http://lists.gnu.org/archive/html/guile-devel/2011-03/msg00223.html
[1] http://lists.gnu.org/archive/html/guile-devel/2011-03/msg00234.html

If you guys can reach an agreement that it should/could live in Guile's
default namespace, either named `fixnum?', or by some other name, I'll
happily re-do the patch accordingly.  My personal feeling is that having
an extension for `fixnum?' alone is clumsy, and I did this just due to
Ludovic's response.

>   Or some other non-R6RS namespace.
>
Suggestions?

>  What about, for example, `exact-integer?'?  And why for fixnums and
>  not flonums?
>
Well, the intention behind naming the files r6rs-arithmetic.{c,h} is
that similiar performance hacks for flonums could go in there as well,
but ATM I don't really care about flonum operations, so I didn't include
that in the patch.

>  3) Are there no alternatives, like exposing Guile's tags to Scheme
>     directly?  (ice-9 tags) for example.  Then we could inline tc3?,
>     tc8?, and tc16? ops.  Some other predicates would become
>     unnecessary; char? for example.  Perhaps this is a bad idea though.
>
I don't like this idea.  It sounds like exposing what is a _really_
low-level implementation detail to Scheme for not a very good reason.

Granted, `fixnum?' exposes also an implementation detail, but one that
is probably common to all non-toy implementations of Scheme, and if
hardware architectures do not change fundamentally, will probably remain
to be so.  This is also my response to Ludovic's response that "this
fixnum thing is a breach in the numerical tower": Yes, it is, but one
that can potentially provide performance gains by having the user
explicitly stating (commonly found, in some areas) invariants about the
numerical range of certain expressions.  A compiler can then leverage
this additional information to give better performance on such code.
IMO, it's a performance hack, but a potentially useful one.

> Apologies for being crabby here after you've done all this work :)  It
> could well be that something close to this is the right thing.
>
Well, I've attached my Plan-B solution.  We can come back to the
`fixnum?' VM primitive when it's clear where its corresponding Scheme
binding should go.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: refactor-define-inlinable.diff --]
[-- Type: text/x-diff, Size: 6037 bytes --]

From: Andreas Rottmann <a.rottmann@gmx.at>
Subject: Move `define-inlinable' into the default namespace

* module/ice-9/boot-9.scm (define-inlineable): Moved here from SRFI-9.
* module/srfi/srfi-9 (define-inlinable): Removed here.

* doc/ref/api-procedures.texi (Inlinable Procedures): Add subsection
  about `define-inlinable'.

---
 doc/ref/api-procedures.texi |   27 ++++++++++++++++++++++++++-
 module/ice-9/boot-9.scm     |   36 ++++++++++++++++++++++++++++++++++++
 module/srfi/srfi-9.scm      |   32 --------------------------------
 3 files changed, 62 insertions(+), 33 deletions(-)

diff --git a/doc/ref/api-procedures.texi b/doc/ref/api-procedures.texi
index 02889c4..4b4870d 100644
--- a/doc/ref/api-procedures.texi
+++ b/doc/ref/api-procedures.texi
@@ -1,6 +1,6 @@
 @c -*-texinfo-*-
 @c This is part of the GNU Guile Reference Manual.
-@c Copyright (C)  1996, 1997, 2000, 2001, 2002, 2003, 2004, 2009, 2010
+@c Copyright (C)  1996, 1997, 2000, 2001, 2002, 2003, 2004, 2009, 2010, 2011
 @c   Free Software Foundation, Inc.
 @c See the file guile.texi for copying conditions.
 
@@ -16,6 +16,7 @@
 * Higher-Order Functions::      Function that take or return functions.
 * Procedure Properties::        Procedure properties and meta-information.
 * Procedures with Setters::     Procedures with setters.
+* Inlinable Procedures::        Procedures that can be inlined.
 @end menu
 
 
@@ -797,6 +798,30 @@ Return the setter of @var{proc}, which must be either a procedure with
 setter or an operator struct.
 @end deffn
 
+@node Inlinable Procedures
+@subsection Inlinable Procedures
+
+You can define an ``inlinable procedure'' by using
+@code{define-inlinable} instead of @code{define}. An inlinable procedure
+behaves the same as a regular procedure, but direct calls will result in
+the procedure body being inlined into the caller.
+
+Making a procedure inlinable eliminates the overhead of the call, but at
+the same time means that the caller will not transparently use the new
+definition if the inline procedure is redefined.  Inlinable procedures
+will also not deal nicely with debugging and tracing.  Therefore, you
+should not make a procedure inlinable unless it demonstrably improves
+performance in a crucial way.
+
+In general, only small procedures should be considered for inlining, as
+making large procedures inlinable will probably result in an increase in
+code size.  Additionally, the elimination of the call overhead rarely
+matters for for large procedures.
+
+@deffn {Scheme Syntax} define-inlinable (name parameter ...) . body
+Define @var{name} as a procedure with parameters @var{parameter}s and
+body @var{body}.
+@end deffn
 
 @c Local Variables:
 @c TeX-master: "guile.texi"
diff --git a/module/ice-9/boot-9.scm b/module/ice-9/boot-9.scm
index 33aa333..327e3fa 100644
--- a/module/ice-9/boot-9.scm
+++ b/module/ice-9/boot-9.scm
@@ -3497,6 +3497,42 @@ module '(ice-9 q) '(make-q q-length))}."
                          x)))))
 
 \f
+;;; Defining transparently inlinable procedures
+;;;
+
+(define-syntax define-inlinable
+  ;; Define a macro and a procedure such that direct calls are inlined, via
+  ;; the macro expansion, whereas references in non-call contexts refer to
+  ;; the procedure.  Inspired by the `define-integrable' macro by Dybvig et al.
+  (lambda (x)
+    ;; Use a space in the prefix to avoid potential -Wunused-toplevel
+    ;; warning
+    (define prefix (string->symbol "% "))
+    (define (make-procedure-name name)
+      (datum->syntax name
+                     (symbol-append prefix (syntax->datum name)
+                                    '-procedure)))
+
+    (syntax-case x ()
+      ((_ (name formals ...) body ...)
+       (identifier? #'name)
+       (with-syntax ((proc-name  (make-procedure-name #'name))
+                     ((args ...) (generate-temporaries #'(formals ...))))
+         #`(begin
+             (define (proc-name formals ...)
+               body ...)
+             (define-syntax name
+               (lambda (x)
+                 (syntax-case x ()
+                   ((_ args ...)
+                    #'((lambda (formals ...)
+                         body ...)
+                       args ...))
+                   (_
+                    (identifier? x)
+                    #'proc-name))))))))))
+
+\f
 
 (define using-readline?
   (let ((using-readline? (make-fluid)))
diff --git a/module/srfi/srfi-9.scm b/module/srfi/srfi-9.scm
index f9449a6..ad9e95d 100644
--- a/module/srfi/srfi-9.scm
+++ b/module/srfi/srfi-9.scm
@@ -64,38 +64,6 @@
 
 (cond-expand-provide (current-module) '(srfi-9))
 
-(define-syntax define-inlinable
-  ;; Define a macro and a procedure such that direct calls are inlined, via
-  ;; the macro expansion, whereas references in non-call contexts refer to
-  ;; the procedure.  Inspired by the `define-integrable' macro by Dybvig et al.
-  (lambda (x)
-    ;; Use a space in the prefix to avoid potential -Wunused-toplevel
-    ;; warning
-    (define prefix (string->symbol "% "))
-    (define (make-procedure-name name)
-      (datum->syntax name
-                     (symbol-append prefix (syntax->datum name)
-                                    '-procedure)))
-
-    (syntax-case x ()
-      ((_ (name formals ...) body ...)
-       (identifier? #'name)
-       (with-syntax ((proc-name  (make-procedure-name #'name))
-                     ((args ...) (generate-temporaries #'(formals ...))))
-         #`(begin
-             (define (proc-name formals ...)
-               body ...)
-             (define-syntax name
-               (lambda (x)
-                 (syntax-case x ()
-                   ((_ args ...)
-                    #'((lambda (formals ...)
-                         body ...)
-                       args ...))
-                   (_
-                    (identifier? x)
-                    #'proc-name))))))))))
-
 (define-syntax define-record-type
   (lambda (x)
     (define (field-identifiers field-specs)
-- 
tg: (78d1be4..) t/refactor-define-inlinable (depends on: stable-2.0)

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: smart-fixnum-p.diff --]
[-- Type: text/x-diff, Size: 1384 bytes --]

From: Andreas Rottmann <a.rottmann@gmx.at>
Subject: Implement R6RS' `fixnum?' in a smarter way

* module/rnrs/arithmetic/fixnums.scm (fixnum?): Implemented using
  bit-twiddling, and using `define-inlinable'.

---
 module/rnrs/arithmetic/fixnums.scm |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/module/rnrs/arithmetic/fixnums.scm b/module/rnrs/arithmetic/fixnums.scm
index 03511ed..0ce2458 100644
--- a/module/rnrs/arithmetic/fixnums.scm
+++ b/module/rnrs/arithmetic/fixnums.scm
@@ -76,6 +76,7 @@
 	  fxreverse-bit-field)
   (import (only (guile) ash
 		        cons*
+			define-inlinable
 			inexact->exact
 			logand
 			logbit?
@@ -84,7 +85,8 @@
 			lognot
 			logxor
 			most-positive-fixnum 
-			most-negative-fixnum)
+			most-negative-fixnum
+			object-address)
 	  (ice-9 optargs)
 	  (rnrs base (6))
 	  (rnrs control (6))
@@ -99,12 +101,9 @@
 
   (define (greatest-fixnum) most-positive-fixnum)
   (define (least-fixnum) most-negative-fixnum)
-  
-  (define (fixnum? obj) 
-    (and (integer? obj) 
-	 (exact? obj) 
-	 (>= obj most-negative-fixnum) 
-	 (<= obj most-positive-fixnum)))
+
+  (define-inlinable (fixnum? obj)
+    (not (= 0 (logand 2 (object-address obj)))))
 
   (define-syntax assert-fixnum
     (syntax-rules ()
-- 
tg: (dfabfe2..) t/smart-fixnum-p (depends on: t/rnrs-arithmetic-benchmark t/refactor-define-inlinable)

[-- Attachment #4: Type: text/plain, Size: 64 bytes --]



Regards, Rotty
-- 
Andreas Rottmann -- <http://rotty.yi.org/>

define-inlinable

[Ludovic Courtès]

Hello!

Andreas Rottmann <a.rottmann@gmx.at> writes:

> From: Andreas Rottmann <a.rottmann@gmx.at>
> Subject: Move `define-inlinable' into the default namespace
>
> * module/ice-9/boot-9.scm (define-inlineable): Moved here from SRFI-9.
> * module/srfi/srfi-9 (define-inlinable): Removed here.
>
> * doc/ref/api-procedures.texi (Inlinable Procedures): Add subsection
>   about `define-inlinable'.

Looks great to me.

Some nitpicking:

[...]

> +@node Inlinable Procedures
> +@subsection Inlinable Procedures
> +
> +You can define an ``inlinable procedure'' by using

Use @dfn{inlinable procedure} here.

> +@code{define-inlinable} instead of @code{define}. An inlinable procedure

I prefer two-spaces-after-period, but there’s no consensus.

> +behaves the same as a regular procedure, but direct calls will result in
> +the procedure body being inlined into the caller.
> +
> +Making a procedure inlinable eliminates the overhead of the call,

How about:

  Procedures defined with @code{define-inlinable} are @emph{always}
  inlined, at all call sites.  This eliminates function call overhead at
  the expense of an increase in code size.

> but at
> +the same time means that the caller will not transparently use the new
> +definition if the inline procedure is redefined.

  ... redefined using @code{set!}.

> Inlinable procedures
> +will also not deal nicely with debugging and tracing.

Instead of “not deal nicely”, what about something like:

  It is not possible to trace an inlined procedures or install a
  breakpoint in it (@pxref{Traps}).

> Therefore, you
> +should not make a procedure inlinable unless it demonstrably improves
> +performance in a crucial way.
> +
> +In general, only small procedures should be considered for inlining, as
> +making large procedures inlinable will probably result in an increase in
> +code size.  Additionally, the elimination of the call overhead rarely
> +matters for for large procedures.
> +
> +@deffn {Scheme Syntax} define-inlinable (name parameter ...) . body

I’d write “body ...” instead of “. body”.  Besides being aesthetically
nicer, the former matches a proper list whereas the latter matches a
pair.

Thanks,
Ludo’.

Re: define-inlinable

[Andreas Rottmann]

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

ludo@gnu.org (Ludovic Courtès) writes:

> Hello!
>
> Andreas Rottmann <a.rottmann@gmx.at> writes:
>
>> Subject: Move `define-inlinable' into the default namespace
>>
>> +@node Inlinable Procedures
>> +@subsection Inlinable Procedures
>> +
>> +You can define an ``inlinable procedure'' by using
>
> Use @dfn{inlinable procedure} here.
>
Done.

>> +@code{define-inlinable} instead of @code{define}. An inlinable procedure
>
> I prefer two-spaces-after-period, but there’s no consensus.
>
I try to use two spaces in English text as well, but being a German
native speaker (where this is against typographical rules, AFAIK), I
slip sometimes.  Fixed.

>> +behaves the same as a regular procedure, but direct calls will result in
>> +the procedure body being inlined into the caller.
>> +
>> +Making a procedure inlinable eliminates the overhead of the call,
>
> How about:
>
>   Procedures defined with @code{define-inlinable} are @emph{always}
>   inlined, at all call sites.  This eliminates function call overhead at
>   the expense of an increase in code size.
>
Folded in, with the addition of using ".., at _direct_ call sites.".
There's no inlining happening when you use `apply', or rebind the
procedure with `let'.  Should this be made more explicit?

>> but at
>> +the same time means that the caller will not transparently use the new
>> +definition if the inline procedure is redefined.
>
>   ... redefined using @code{set!}.
>
I don't agree with that one: there are multiple ways a procedure can get
"redefined", `set!' being just one of them.  I was actually thinking
more of re-evaluating the procedure definition or something like
`geiser-compile-file', hence I left the text like it was, being more
vague.

>> Inlinable procedures
>> +will also not deal nicely with debugging and tracing.
>
> Instead of “not deal nicely”, what about something like:
>
>   It is not possible to trace an inlined procedures or install a
>   breakpoint in it (@pxref{Traps}).
>
Done.

>> Therefore, you
>> +should not make a procedure inlinable unless it demonstrably improves
>> +performance in a crucial way.
>> +
>> +In general, only small procedures should be considered for inlining, as
>> +making large procedures inlinable will probably result in an increase in
>> +code size.  Additionally, the elimination of the call overhead rarely
>> +matters for for large procedures.
>> +
>> +@deffn {Scheme Syntax} define-inlinable (name parameter ...) . body
>
> I’d write “body ...” instead of “. body”.  Besides being aesthetically
> nicer, the former matches a proper list whereas the latter matches a
> pair.
>
Agreed, and done.

Updated patch attached, is it OK to push this way?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: refactor-define-inlinable.diff --]
[-- Type: text/x-diff, Size: 6178 bytes --]

From: Andreas Rottmann <a.rottmann@gmx.at>
Subject: Move `define-inlinable' into the default namespace

* module/ice-9/boot-9.scm (define-inlineable): Moved here from SRFI-9.
* module/srfi/srfi-9 (define-inlinable): Removed here.

* doc/ref/api-procedures.texi (Inlinable Procedures): Add subsection
  about `define-inlinable'.

---
 doc/ref/api-procedures.texi |   29 ++++++++++++++++++++++++++++-
 module/ice-9/boot-9.scm     |   36 ++++++++++++++++++++++++++++++++++++
 module/srfi/srfi-9.scm      |   32 --------------------------------
 3 files changed, 64 insertions(+), 33 deletions(-)

diff --git a/doc/ref/api-procedures.texi b/doc/ref/api-procedures.texi
index 02889c4..5c6d380 100644
--- a/doc/ref/api-procedures.texi
+++ b/doc/ref/api-procedures.texi
@@ -1,6 +1,6 @@
 @c -*-texinfo-*-
 @c This is part of the GNU Guile Reference Manual.
-@c Copyright (C)  1996, 1997, 2000, 2001, 2002, 2003, 2004, 2009, 2010
+@c Copyright (C)  1996, 1997, 2000, 2001, 2002, 2003, 2004, 2009, 2010, 2011
 @c   Free Software Foundation, Inc.
 @c See the file guile.texi for copying conditions.
 
@@ -16,6 +16,7 @@
 * Higher-Order Functions::      Function that take or return functions.
 * Procedure Properties::        Procedure properties and meta-information.
 * Procedures with Setters::     Procedures with setters.
+* Inlinable Procedures::        Procedures that can be inlined.
 @end menu
 
 
@@ -797,6 +798,32 @@ Return the setter of @var{proc}, which must be either a procedure with
 setter or an operator struct.
 @end deffn
 
+@node Inlinable Procedures
+@subsection Inlinable Procedures
+
+You can define an @dfn{inlinable procedure} by using
+@code{define-inlinable} instead of @code{define}.  An inlinable
+procedure behaves the same as a regular procedure, but direct calls will
+result in the procedure body being inlined into the caller.
+
+Procedures defined with @code{define-inlinable} are @emph{always}
+inlined, at all direct call sites.  This eliminates function call
+overhead at the expense of an increase in code size.  Additionally, the
+caller will not transparently use the new definition if the inline
+procedure is redefined.  It is not possible to trace an inlined
+procedures or install a breakpoint in it (@pxref{Traps}).  For these
+reasons, you should not make a procedure inlinable unless it
+demonstrably improves performance in a crucial way.
+
+In general, only small procedures should be considered for inlining, as
+making large procedures inlinable will probably result in an increase in
+code size.  Additionally, the elimination of the call overhead rarely
+matters for for large procedures.
+
+@deffn {Scheme Syntax} define-inlinable (name parameter ...) body ...
+Define @var{name} as a procedure with parameters @var{parameter}s and
+body @var{body}.
+@end deffn
 
 @c Local Variables:
 @c TeX-master: "guile.texi"
diff --git a/module/ice-9/boot-9.scm b/module/ice-9/boot-9.scm
index 33aa333..327e3fa 100644
--- a/module/ice-9/boot-9.scm
+++ b/module/ice-9/boot-9.scm
@@ -3497,6 +3497,42 @@ module '(ice-9 q) '(make-q q-length))}."
                          x)))))
 
 \f
+;;; Defining transparently inlinable procedures
+;;;
+
+(define-syntax define-inlinable
+  ;; Define a macro and a procedure such that direct calls are inlined, via
+  ;; the macro expansion, whereas references in non-call contexts refer to
+  ;; the procedure.  Inspired by the `define-integrable' macro by Dybvig et al.
+  (lambda (x)
+    ;; Use a space in the prefix to avoid potential -Wunused-toplevel
+    ;; warning
+    (define prefix (string->symbol "% "))
+    (define (make-procedure-name name)
+      (datum->syntax name
+                     (symbol-append prefix (syntax->datum name)
+                                    '-procedure)))
+
+    (syntax-case x ()
+      ((_ (name formals ...) body ...)
+       (identifier? #'name)
+       (with-syntax ((proc-name  (make-procedure-name #'name))
+                     ((args ...) (generate-temporaries #'(formals ...))))
+         #`(begin
+             (define (proc-name formals ...)
+               body ...)
+             (define-syntax name
+               (lambda (x)
+                 (syntax-case x ()
+                   ((_ args ...)
+                    #'((lambda (formals ...)
+                         body ...)
+                       args ...))
+                   (_
+                    (identifier? x)
+                    #'proc-name))))))))))
+
+\f
 
 (define using-readline?
   (let ((using-readline? (make-fluid)))
diff --git a/module/srfi/srfi-9.scm b/module/srfi/srfi-9.scm
index f9449a6..ad9e95d 100644
--- a/module/srfi/srfi-9.scm
+++ b/module/srfi/srfi-9.scm
@@ -64,38 +64,6 @@
 
 (cond-expand-provide (current-module) '(srfi-9))
 
-(define-syntax define-inlinable
-  ;; Define a macro and a procedure such that direct calls are inlined, via
-  ;; the macro expansion, whereas references in non-call contexts refer to
-  ;; the procedure.  Inspired by the `define-integrable' macro by Dybvig et al.
-  (lambda (x)
-    ;; Use a space in the prefix to avoid potential -Wunused-toplevel
-    ;; warning
-    (define prefix (string->symbol "% "))
-    (define (make-procedure-name name)
-      (datum->syntax name
-                     (symbol-append prefix (syntax->datum name)
-                                    '-procedure)))
-
-    (syntax-case x ()
-      ((_ (name formals ...) body ...)
-       (identifier? #'name)
-       (with-syntax ((proc-name  (make-procedure-name #'name))
-                     ((args ...) (generate-temporaries #'(formals ...))))
-         #`(begin
-             (define (proc-name formals ...)
-               body ...)
-             (define-syntax name
-               (lambda (x)
-                 (syntax-case x ()
-                   ((_ args ...)
-                    #'((lambda (formals ...)
-                         body ...)
-                       args ...))
-                   (_
-                    (identifier? x)
-                    #'proc-name))))))))))
-
 (define-syntax define-record-type
   (lambda (x)
     (define (field-identifiers field-specs)
-- 
tg: (ce60660..) t/refactor-define-inlinable (depends on: stable-2.0)

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


Thanks for the review, Rotty
-- 
Andreas Rottmann -- <http://rotty.yi.org/>

Re: define-inlinable

[Ludovic Courtès]

Hi!

Andreas Rottmann <a.rottmann@gmx.at> writes:

[...]

>>> +behaves the same as a regular procedure, but direct calls will result in
>>> +the procedure body being inlined into the caller.
>>> +
>>> +Making a procedure inlinable eliminates the overhead of the call,
>>
>> How about:
>>
>>   Procedures defined with @code{define-inlinable} are @emph{always}
>>   inlined, at all call sites.  This eliminates function call overhead at
>>   the expense of an increase in code size.
>>
> Folded in, with the addition of using ".., at _direct_ call sites.".
> There's no inlining happening when you use `apply', or rebind the
> procedure with `let'.  Should this be made more explicit?

That’s fine IMO.

>>> but at
>>> +the same time means that the caller will not transparently use the new
>>> +definition if the inline procedure is redefined.
>>
>>   ... redefined using @code{set!}.
>>
> I don't agree with that one: there are multiple ways a procedure can get
> "redefined", `set!' being just one of them.  I was actually thinking
> more of re-evaluating the procedure definition or something like
> `geiser-compile-file', hence I left the text like it was, being more
> vague.

Right, good point.

> Updated patch attached, is it OK to push this way?

Yes, please go ahead!

Thanks,
Ludo’.

Re: [PATCH 3/3] Add `fixnum?' VM primitive

[Ludovic Courtès]

Hi!

Andreas Rottmann <a.rottmann@gmx.at> writes:

> From: Andreas Rottmann <a.rottmann@gmx.at>
> Subject: Implement R6RS' `fixnum?' in a smarter way
>
> * module/rnrs/arithmetic/fixnums.scm (fixnum?): Implemented using
>   bit-twiddling, and using `define-inlinable'.

Fine with me, feel free to commit.

Ludo’.

Re: define-inlinable

[Andy Wingo]

On Thu 07 Apr 2011 00:24, ludo@gnu.org (Ludovic Courtès) writes:

>> Updated patch attached, is it OK to push this way?
>
> Yes, please go ahead!

FWIW:

Running r6rs-arithmetic-flonums.test
ERROR: r6rs-arithmetic-flonums.test: fixnum->flonum: simple - arguments: ((wrong-type-arg #f "Wrong type to apply: ~S" (#<syntax-transformer fixnum?>) (#<syntax-transformer fixnum?>)))

In practice this means that anyone that compiled something that uses
fixnum? against Guile 2.0.0 now has broken code.  I don't know if this
is important, as R6RS users probably have lots of other carnage to deal
with, but it is strictly an ABI break.

Andy
-- 
http://wingolog.org/

Re: define-inlinable

[Ludovic Courtès]

Hello!

Andy Wingo <wingo@pobox.com> writes:

> Running r6rs-arithmetic-flonums.test
> ERROR: r6rs-arithmetic-flonums.test: fixnum->flonum: simple - arguments: ((wrong-type-arg #f "Wrong type to apply: ~S" (#<syntax-transformer fixnum?>) (#<syntax-transformer fixnum?>)))
>
> In practice this means that anyone that compiled something that uses
> fixnum? against Guile 2.0.0 now has broken code.

Ouch, right.

> I don't know if this is important, as R6RS users probably have lots of
> other carnage to deal with, but it is strictly an ABI break.

Well well, they’ll need to recompile.  My feeling is that it’s
acceptable, but I don’t have a strong opinion.

Thanks,
Ludo’.

Re: define-inlinable

[Andy Wingo]

Heya :)

On Mon 11 Apr 2011 22:01, ludo@gnu.org (Ludovic Courtès) writes:

>> I don't know if this is important, as R6RS users probably have lots of
>> other carnage to deal with, but it is strictly an ABI break.
>
> Well well, they’ll need to recompile.  My feeling is that it’s
> acceptable, but I don’t have a strong opinion.

In this case it probably is, but we don't really know what users there
are.  But let's please keep this general issue in mind in the future.

Cheers,

Andy
-- 
http://wingolog.org/

Re: define-inlinable

[Andreas Rottmann]

Andy Wingo <wingo@pobox.com> writes:

> Heya :)
>
> On Mon 11 Apr 2011 22:01, ludo@gnu.org (Ludovic Courtès) writes:
>
>>> I don't know if this is important, as R6RS users probably have lots of
>>> other carnage to deal with, but it is strictly an ABI break.
>>
>> Well well, they’ll need to recompile.  My feeling is that it’s
>> acceptable, but I don’t have a strong opinion.
>
> In this case it probably is, but we don't really know what users there
> are.  But let's please keep this general issue in mind in the future.
>
Agreed.  I'm reluctant to using macros as a performance hack (e.g.,
define-inlinable) -- it just feels wrong, but sometimes performance
numbers are seductive...

Regards, Rotty
-- 
Andreas Rottmann -- <http://rotty.yi.org/>