Hi hackers, A while ago I was digging into CHICKEN's worrisome I/O performance and I noticed that read-char isn't compiled as efficiently as one would expect it to be: the argument type check isn't inlined as it should, even though there is a specialization in types.db, and it calls read-char/port which is a full CPS call. See also ticket #1219.
So today I took a look into why this is happening, and the reason seems pretty simple: normally, when you have a file that contains code and type declarations plus specializations, the type is marked as "declared" rather than the default, which I guess is "inferred". Types declarations loaded via an external database should be treated identically to inline declarations, I think, but those are currently not marked as "declared". The missing marking causes the code in "initial-argument-types" and "variable-result" (in scrutinizer.scm) to use the "*" wildcard rather than the actual declared type. This somehow means specializations won't be triggered. I'm not 100% sure why that's the case. There was another ticket made by Felix, #745, which indicates that really we want a user definition to invalidate type specialization for a procedure of the same name. This was closed after I noted that specializations already don't occur. However, the current behaviour is more of an accident than an actual feature: the specializations list on the identifier isn't removed, and in fact, if the procedure's type is declared inside the file, this will cause the specializations to be triggered again, even if the type is different (I think). So I re-opened #745 as this will need a "proper" fix, obliterating the specializations and declared procedure type. The first attached patch simply adds the "declared-type" variable mark to all types that were loaded via an external database. Adding this also found some inconsistent types between types.db and the definitions in library.scm and tcp.scm, so I've fixed those as well (the third patch). The second patch simply copies the body of read-char/port into read-char, exactly like we already do in write-char and write-char/port. I'm not sure that's the best solution, but it's better than the current situation. Attached is also a diff of the benchmarks, and you can clearly see the benefit this has in the difference between "test" and "master" on the kernwyk-wc and kernwyk-cat benchmarks, as I hoped it would. The slatex benchmark doesn't show much of an improvement, but it does a lot more than just reading, of course. I can't really explain the differences in the other benchmarks, but I guess some of those are just noise, or the result of changing stacks due to the specializations in library.scm. Cheers, Peter
From d02bf544c8ff15c33c16a15574ef243fdcf05ceb Mon Sep 17 00:00:00 2001
From: Peter Bex <pe...@more-magic.net>
Date: Wed, 18 Nov 2015 19:28:08 +0100
Subject: [PATCH 1/3] Mark external type declarations as declared.
By not being marked as "declared", types loaded from a types database
would be considered to be inferred via flow analysis. When scrutinizing
procedure definitions, "initial-argument-types" and "variable-result"
would simply return '* or '(*) as the type, which doesn't match the
loaded declaration. This had the effect of blocking specialization.
This fixes the most important part of #1219.
---
distribution/manifest | 1 +
scrutinizer.scm | 1 +
tests/runtests.bat | 2 +-
tests/runtests.sh | 2 +-
tests/specialization-test-2.scm | 6 ++++++
tests/specialization-test-2.types | 4 ++++
6 files changed, 14 insertions(+), 2 deletions(-)
create mode 100644 tests/specialization-test-2.types
diff --git a/distribution/manifest b/distribution/manifest
index f8546ef..69aabb8 100644
--- a/distribution/manifest
+++ b/distribution/manifest
@@ -172,6 +172,7 @@ tests/loopy-loop.scm
tests/r5rs_pitfalls.scm
tests/specialization-test-1.scm
tests/specialization-test-2.scm
+tests/specialization-test-2.types
tests/test-irregex.scm
tests/re-tests.txt
tests/lolevel-tests.scm
diff --git a/scrutinizer.scm b/scrutinizer.scm
index 81c2f82..0212527 100644
--- a/scrutinizer.scm
+++ b/scrutinizer.scm
@@ -1803,6 +1803,7 @@
"type-definition `~a' for toplevel binding `~a' conflicts with previously loaded type `~a'"
name new old)))
(mark-variable name '##compiler#type t)
+ (mark-variable name '##compiler#declared-type)
(when specs
(install-specializations name specs)))))
(read-file dbfile))
diff --git a/tests/runtests.bat b/tests/runtests.bat
index 9511ca5..7458989 100644
--- a/tests/runtests.bat
+++ b/tests/runtests.bat
@@ -86,7 +86,7 @@ del /f /q foo.types foo.import.*
if errorlevel 1 exit /b 1
a.out
if errorlevel 1 exit /b 1
-%compile% specialization-test-2.scm -types foo.types -specialize -debug ox
+%compile% specialization-test-2.scm -types foo.types -types specialization-test-2.types -specialize -debug ox
if errorlevel 1 exit /b 1
a.out
if errorlevel 1 exit /b 1
diff --git a/tests/runtests.sh b/tests/runtests.sh
index 5b61404..b02b716 100755
--- a/tests/runtests.sh
+++ b/tests/runtests.sh
@@ -126,7 +126,7 @@ rm -f foo.types foo.import.*
$compile specialization-test-1.scm -emit-type-file foo.types -specialize \
-debug ox -emit-import-library foo
./a.out
-$compile specialization-test-2.scm -types foo.types -specialize -debug ox
+$compile specialization-test-2.scm -types foo.types -types specialization-test-2.types -specialize -debug ox
./a.out
rm -f foo.types foo.import.*
diff --git a/tests/specialization-test-2.scm b/tests/specialization-test-2.scm
index e24e5cb..9b80922 100644
--- a/tests/specialization-test-2.scm
+++ b/tests/specialization-test-2.scm
@@ -26,3 +26,9 @@ return n;}
(assert (handle-exceptions ex #t (bug855 '(#f)) #f))
+;; #1219: Specializations from databases loaded with "-types" should
+;; be applied.
+(define (specialize-me x)
+ (error "Not specialized!"))
+
+(assert (= (specialize-me 123) 123))
diff --git a/tests/specialization-test-2.types b/tests/specialization-test-2.types
new file mode 100644
index 0000000..8686522
--- /dev/null
+++ b/tests/specialization-test-2.types
@@ -0,0 +1,4 @@
+;; -*- Scheme -*-
+(specialize-me (procedure specialize-me (fixnum) fixnum)
+ ((fixnum) #(1)))
+
--
2.1.4
From 1a048682320b8a9e5a964f79cd0d670cb9ba3e94 Mon Sep 17 00:00:00 2001 From: Peter Bex <pe...@more-magic.net> Date: Wed, 18 Nov 2015 19:28:21 +0100 Subject: [PATCH 2/3] Avoid CPS call in read-char to read-char/port. This should improve performance somewhat for those cases where the procedure is called indirectly, or with zero arguments. This fixes the remaining part of #1219. --- library.scm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library.scm b/library.scm index bb49d8b..8c7cec5 100644 --- a/library.scm +++ b/library.scm @@ -3061,7 +3061,8 @@ EOF ((##sys#slot (##sys#slot port 2) 6) port) ) ; char-ready? (define (read-char #!optional (port ##sys#standard-input)) - (##sys#read-char/port port) ) + (##sys#check-input-port port #t 'read-char) + (##sys#read-char-0 port) ) (define (##sys#read-char-0 p) (let ([c (if (##sys#slot p 6) -- 2.1.4
From 861ae33f84857e74af802287d88d2184b8d9fee2 Mon Sep 17 00:00:00 2001
From: Peter Bex <pe...@more-magic.net>
Date: Wed, 18 Nov 2015 19:29:59 +0100
Subject: [PATCH 3/3] Fix a few incorrect type declarations.
These were found by simply compiling with DEBUGBUILD after rebuilding
CHICKEN with the declared-type fix in the scrutinizer.
Conflicts:
types.db
---
types.db | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/types.db b/types.db
index 050824a..198af1d 100644
--- a/types.db
+++ b/types.db
@@ -866,7 +866,7 @@
((*) (number)
(##core#inline_allocate ("C_s_a_i_plus" 36) #(1) '1)))
-(argc+argv (#(procedure #:clean) argc+argv () fixnum (list-of string) fixnum))
+(argc+argv (#(procedure #:clean) argc+argv () fixnum pointer))
(argv (#(procedure #:clean) argv () (list-of string)))
(integer-length (#(procedure #:clean #:enforce #:foldable) integer-length (integer) fixnum)
((fixnum) (##core#inline "C_i_fixnum_length" #(1)))
@@ -935,7 +935,7 @@
(condition->list (#(procedure #:clean #:enforce) condition->list ((struct condition)) (list-of (pair symbol *))))
(continuation-capture (#(procedure #:enforce) continuation-capture ((procedure ((struct continuation)) . *)) *))
(continuation-graft (#(procedure #:clean #:enforce) continuation-graft ((struct continuation) (procedure () . *)) *))
-(continuation-return (#(procedure #:enforce) continuation-return (procedure #!rest) . *)) ;XXX make return type more specific?
+(continuation-return (#(procedure #:enforce) continuation-return ((struct continuation) #!rest) . *)) ;XXX make return type more specific?
(continuation? (#(procedure #:pure #:predicate (struct continuation)) continuation? (*) boolean))
@@ -2207,7 +2207,7 @@
(chicken.tcp#tcp-accept-ready? (#(procedure #:clean #:enforce) chicken.tcp#tcp-accept-ready? ((struct tcp-listener)) boolean))
(chicken.tcp#tcp-accept-timeout (#(procedure #:clean #:enforce) chicken.tcp#tcp-accept-timeout (#!optional (or false integer)) (or false integer)))
(chicken.tcp#tcp-addresses (#(procedure #:clean #:enforce) chicken.tcp#tcp-addresses (port) string string))
-(chicken.tcp#tcp-buffer-size (#(procedure #:clean #:enforce) chicken.tcp#tcp-buffer-size (#!optional fixnum) fixnum))
+(chicken.tcp#tcp-buffer-size (#(procedure #:clean #:enforce) chicken.tcp#tcp-buffer-size (#!optional (or false fixnum)) (or false fixnum)))
(chicken.tcp#tcp-close (#(procedure #:clean #:enforce) chicken.tcp#tcp-close ((struct tcp-listener)) undefined))
(chicken.tcp#tcp-connect (#(procedure #:clean #:enforce) chicken.tcp#tcp-connect (string #!optional fixnum) input-port output-port))
(chicken.tcp#tcp-connect-timeout (#(procedure #:clean #:enforce) chicken.tcp#tcp-connect-timeout (#!optional (or false integer)) (or false integer)))
--
2.1.4
From 0812009d1b90ed75cd9ab7f4d5b8c0454ea89685 Mon Sep 17 00:00:00 2001
From: Peter Bex <pe...@more-magic.net>
Date: Wed, 18 Nov 2015 17:04:57 +0100
Subject: [PATCH 1/3] Mark external type declarations as declared.
By not being marked as "declared", types loaded from a types database
would be considered to be inferred via flow analysis. When scrutinizing
procedure definitions, "initial-argument-types" and "variable-result"
would simply return '* or '(*) as the type, which doesn't match the
loaded declaration. This had the effect of blocking specialization.
This fixes the most important part of #1219.
---
distribution/manifest | 1 +
scrutinizer.scm | 1 +
tests/runtests.bat | 2 +-
tests/runtests.sh | 2 +-
tests/specialization-test-2.scm | 6 ++++++
tests/specialization-test-2.types | 4 ++++
6 files changed, 14 insertions(+), 2 deletions(-)
create mode 100644 tests/specialization-test-2.types
diff --git a/distribution/manifest b/distribution/manifest
index 34c6ae3..c40671e 100644
--- a/distribution/manifest
+++ b/distribution/manifest
@@ -174,6 +174,7 @@ tests/loopy-loop.scm
tests/r5rs_pitfalls.scm
tests/specialization-test-1.scm
tests/specialization-test-2.scm
+tests/specialization-test-2.types
tests/test-irregex.scm
tests/re-tests.txt
tests/lolevel-tests.scm
diff --git a/scrutinizer.scm b/scrutinizer.scm
index 99da823..8cf2d14 100644
--- a/scrutinizer.scm
+++ b/scrutinizer.scm
@@ -1778,6 +1778,7 @@
"type-definition `~a' for toplevel binding `~a' conflicts with previously loaded type `~a'"
name new old)))
(mark-variable name '##compiler#type t)
+ (mark-variable name '##compiler#declared-type)
(when specs
(install-specializations name specs)))))
(read-file dbfile))
diff --git a/tests/runtests.bat b/tests/runtests.bat
index 9539bd4..e32aace 100644
--- a/tests/runtests.bat
+++ b/tests/runtests.bat
@@ -73,7 +73,7 @@ del /f /q foo.types foo.import.*
if errorlevel 1 exit /b 1
a.out
if errorlevel 1 exit /b 1
-%compile% specialization-test-2.scm -types foo.types -specialize -debug ox
+%compile% specialization-test-2.scm -types foo.types -types specialization-test-2.types -specialize -debug ox
if errorlevel 1 exit /b 1
a.out
if errorlevel 1 exit /b 1
diff --git a/tests/runtests.sh b/tests/runtests.sh
index 4bbd171..612e562 100755
--- a/tests/runtests.sh
+++ b/tests/runtests.sh
@@ -113,7 +113,7 @@ rm -f foo.types foo.import.*
$compile specialization-test-1.scm -emit-type-file foo.types -specialize \
-debug ox -emit-import-library foo
./a.out
-$compile specialization-test-2.scm -types foo.types -specialize -debug ox
+$compile specialization-test-2.scm -types foo.types -types specialization-test-2.types -specialize -debug ox
./a.out
rm -f foo.types foo.import.*
diff --git a/tests/specialization-test-2.scm b/tests/specialization-test-2.scm
index e24e5cb..9b80922 100644
--- a/tests/specialization-test-2.scm
+++ b/tests/specialization-test-2.scm
@@ -26,3 +26,9 @@ return n;}
(assert (handle-exceptions ex #t (bug855 '(#f)) #f))
+;; #1219: Specializations from databases loaded with "-types" should
+;; be applied.
+(define (specialize-me x)
+ (error "Not specialized!"))
+
+(assert (= (specialize-me 123) 123))
diff --git a/tests/specialization-test-2.types b/tests/specialization-test-2.types
new file mode 100644
index 0000000..8686522
--- /dev/null
+++ b/tests/specialization-test-2.types
@@ -0,0 +1,4 @@
+;; -*- Scheme -*-
+(specialize-me (procedure specialize-me (fixnum) fixnum)
+ ((fixnum) #(1)))
+
--
2.1.4
From 420f930d45fc505dde3529c1414c62cb5e63d21b Mon Sep 17 00:00:00 2001 From: Peter Bex <pe...@more-magic.net> Date: Wed, 18 Nov 2015 19:14:16 +0100 Subject: [PATCH 2/3] Avoid CPS call in read-char to read-char/port. This should improve performance somewhat for those cases where the procedure is called indirectly, or with zero arguments. This fixes the remaining part of #1219. --- library.scm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library.scm b/library.scm index 377c882..18d1195 100644 --- a/library.scm +++ b/library.scm @@ -2297,7 +2297,8 @@ EOF ((##sys#slot (##sys#slot port 2) 6) port) ) ; char-ready? (define (read-char #!optional (port ##sys#standard-input)) - (##sys#read-char/port port) ) + (##sys#check-input-port port #t 'read-char) + (##sys#read-char-0 port) ) (define (##sys#read-char-0 p) (let ([c (if (##sys#slot p 6) -- 2.1.4
From e268e34137cd2a3b828e0ba1a0a86eca1bd10cbb Mon Sep 17 00:00:00 2001
From: Peter Bex <pe...@more-magic.net>
Date: Wed, 18 Nov 2015 19:26:05 +0100
Subject: [PATCH 3/3] Fix a few incorrect type declarations.
These were found by simply compiling with DEBUGBUILD after rebuilding
CHICKEN with the declared-type fix in the scrutinizer.
---
types.db | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/types.db b/types.db
index 4058872..8b09d37 100644
--- a/types.db
+++ b/types.db
@@ -726,7 +726,7 @@
((float) (float)
(##core#inline_allocate ("C_a_i_flonum_plus" 4) #(1) '1.0)))
-(argc+argv (#(procedure #:clean) argc+argv () fixnum (list-of string) fixnum))
+(argc+argv (#(procedure #:clean) argc+argv () fixnum pointer))
(argv (#(procedure #:clean) argv () (list-of string)))
(arithmetic-shift (#(procedure #:clean #:enforce) arithmetic-shift (number number) number))
@@ -770,7 +770,7 @@
(condition->list (#(procedure #:clean #:enforce) condition->list ((struct condition)) (list-of (pair symbol *))))
(continuation-capture (#(procedure #:enforce) continuation-capture ((procedure ((struct continuation)) . *)) *))
(continuation-graft (#(procedure #:clean #:enforce) continuation-graft ((struct continuation) (procedure () . *)) *))
-(continuation-return (#(procedure #:enforce) continuation-return (procedure #!rest) . *)) ;XXX make return type more specific?
+(continuation-return (#(procedure #:enforce) continuation-return ((struct continuation) #!rest) . *)) ;XXX make return type more specific?
(continuation? (#(procedure #:pure #:predicate (struct continuation)) continuation? (*) boolean))
@@ -2626,7 +2626,7 @@
(tcp-accept-ready? (#(procedure #:clean #:enforce) tcp-accept-ready? ((struct tcp-listener)) boolean))
(tcp-accept-timeout (#(procedure #:clean #:enforce) tcp-accept-timeout (#!optional (or false number)) (or false number)))
(tcp-addresses (#(procedure #:clean #:enforce) tcp-addresses (port) string string))
-(tcp-buffer-size (#(procedure #:clean #:enforce) tcp-buffer-size (#!optional fixnum) fixnum))
+(tcp-buffer-size (#(procedure #:clean #:enforce) tcp-buffer-size (#!optional (or false fixnum)) (or false fixnum)))
(tcp-close (#(procedure #:clean #:enforce) tcp-close ((struct tcp-listener)) undefined))
(tcp-connect (#(procedure #:clean #:enforce) tcp-connect (string #!optional fixnum) input-port output-port))
(tcp-connect-timeout (#(procedure #:clean #:enforce) tcp-connect-timeout (#!optional (or false number)) (or false number)))
--
2.1.4
+---[1]: |-> installation-prefix: /home/sjamaan/chickens/master |-> csc-options: |-> repetitions: 10 +---[2]: |-> installation-prefix: /home/sjamaan/chickens/test |-> csc-options: |-> repetitions: 10 Displaying normalized results (larger numbers indicate better results) Programs [1] [2] ======================================== 0_________________________1.00______1.00 binarytrees_______________1.00______1.00 boyer_____________________1.00______1.01 browse____________________1.12______1.00 conform___________________1.00______1.03 cpstak____________________1.00______1.10 ctak______________________1.00______1.02 dderiv____________________1.02______1.00 deriv_____________________1.04______1.00 destructive_______________1.11______1.00 div-iter__________________1.00______1.41 div-rec___________________1.00______1.08 dynamic___________________1.01______1.00 earley____________________1.04______1.00 fft_______________________1.00______1.03 fib_______________________1.00______1.00 fibc______________________1.00______1.01 fibfp_____________________1.04______1.00 fprint____________________1.02______1.00 fread_____________________1.02______1.00 gcbench___________________1.01______1.00 graphs____________________1.09______1.00 hanoi_____________________1.00______1.00 kanren____________________1.00______1.00 kernwyk-ackermann_________1.00______1.00 kernwyk-array_____________1.01______1.00 kernwyk-cat_______________1.00______1.26 kernwyk-string____________1.00______1.00 kernwyk-sum_______________1.00______1.01 kernwyk-tail______________1.00______1.02 kernwyk-wc________________1.00______1.40 knucleotide_______________1.01______1.00 lattice___________________1.00______1.00 maze______________________1.01______1.00 mazefun___________________1.00______1.01 mbrot_____________________1.11______1.00 nbody_____________________1.01______1.00 nboyer____________________1.00______1.00 nestedloop________________1.06______1.00 nfa_______________________1.00______1.02 nqueens___________________1.00______1.17 ntakl_____________________1.03______1.00 nucleic2__________________1.00______1.00 paraffins_________________1.00______1.00 parsing___________________1.03______1.00 pnpoly____________________1.07______1.00 primes____________________1.00______1.06 psyntax___________________1.00______1.01 puzzle____________________1.00______1.06 ray_______________________1.02______1.00 ray2______________________1.00______1.00 sboyer____________________1.00______1.00 scheme____________________1.03______1.00 sieves-eratosthenes_______1.00______1.00 simplex___________________1.02______1.00 slatex____________________1.00______1.08 sort1_____________________1.03______1.00 tak_______________________1.06______1.00 takl______________________1.00______1.02 takr______________________1.00______1.02 traverse__________________1.01______1.00 travinit__________________1.15______1.00 triangl___________________1.01______1.00
signature.asc
Description: Digital signature
_______________________________________________ Chicken-hackers mailing list Chicken-hackers@nongnu.org https://lists.nongnu.org/mailman/listinfo/chicken-hackers
