Re: [PATCH] Improve heap shrinkage calculation

2019-12-01 Thread Peter Bex
On Sun, Dec 01, 2019 at 11:22:37PM +0100, felix.winkelm...@bevuta.com wrote:
> It seems to work fine, but "-:hs0" doesn't seem to work. I tried
> 
> csi -:d -:hs0
> > (define x (make-vector 1000))
> > (##sys#gc)
> > (##sys#gc)
> > ...
> 
> The heap is still shrunk on each GC. "-:hs100" works, but isn't really
> useful, as it resizes the heap to the previous value.

I think megane mentioned this on IRC the other day as well.  It looks
like this has been broken for a while already.

In C_rereclaim2:

  if(size < MINIMAL_HEAP_SIZE) size = MINIMAL_HEAP_SIZE;

If the percentage is zero, "size" will be zero and the line above will
reset the heap size to the minimal heap size.

Why is -:hs100 not useful?  You don't want it to resize at all?

Cheers,
Peter


signature.asc
Description: PGP signature


Re: [PATCH] Improve heap shrinkage calculation

2019-12-01 Thread felix . winkelmann
> Hi all,
>
> As suggested by Sven Hartrumpf in #1379, the current logic which
> decides when the heap should be shrunk is confusing and looks
> "obviously wrong".
>
> For one, the "count" variable indicates the actual used memory.
> This is in one half of the heap, so it seems incorrect to me to
> calculate the watermark on the full heap size (which is what the
> current code does).  And taking a percentage of a percentage seems
> wrong as well.
>
> The attached patch fixes the code as suggested by Sven and adds
> a parameter to allow the user to tweak the low watermark as well.

It seems to work fine, but "-:hs0" doesn't seem to work. I tried

csi -:d -:hs0
> (define x (make-vector 1000))
> (##sys#gc)
> (##sys#gc)
> ...

The heap is still shrunk on each GC. "-:hs100" works, but isn't really
useful, as it resizes the heap to the previous value.


felix




[PATCH] Improve heap shrinkage calculation

2019-12-01 Thread Peter Bex
Hi all,

As suggested by Sven Hartrumpf in #1379, the current logic which
decides when the heap should be shrunk is confusing and looks
"obviously wrong".

For one, the "count" variable indicates the actual used memory.
This is in one half of the heap, so it seems incorrect to me to
calculate the watermark on the full heap size (which is what the
current code does).  And taking a percentage of a percentage seems
wrong as well.

The attached patch fixes the code as suggested by Sven and adds
a parameter to allow the user to tweak the low watermark as well.

Cheers,
Peter
From db141cfc0ac4e8ca37c823749c24e5acbe128e80 Mon Sep 17 00:00:00 2001
From: Peter Bex 
Date: Sun, 1 Dec 2019 15:36:36 +0100
Subject: [PATCH] Improve heap shrinkage factor.

The old calculation was confused because it took a percentage
of a percentage.

This adds a new option to set heap shrinkage watermark and uses
that as-is to decide when to change the heap size.

By default, the heap will be shrunk by 50% when the used amount
is at 25% of the heap.  This looks closest to what seems to be
the intended behaviour of the code, and is also what was suggested
in #1379 by Sven Hartrumpf.
---
 NEWS  |  3 +++
 manual/Using the compiler |  8 +---
 runtime.c | 12 ++--
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index 0890e8fd..05b4dfe9 100644
--- a/NEWS
+++ b/NEWS
@@ -30,6 +30,9 @@
 an error instead of hanging forever (fixes #1586).
   - define-record-type will now give an error if the constructor
 definition refers to field that's not listed elsewhere (see #1633)
+  - Added new -:hu option to set the memory usage low watermark
+percentage at which the heap should be shrunk, and changed the
+calculation to actually reflect this (see #1379).
 
 - Compiler
   - Fixed a bug in lfa2 pass which caused "if" or "cond" nodes to be
diff --git a/manual/Using the compiler b/manual/Using the compiler
index da9f1808..9b096153 100644
--- a/manual/Using the compiler	
+++ b/manual/Using the compiler	
@@ -219,11 +219,13 @@ by the startup code and will not be contained in the result of
 
 ; {{-:hgPERCENTAGE}} : Sets the growth rate of the heap in percent. If the heap is exhausted, then it will grow by {{PERCENTAGE}}. The default is 200.
 
-; {{-:hiNUMBER}} : Specifies the initial heap size
+; {{-:hiNUMBER}} : Specifies the initial heap size (this number includes both heap semispaces, therefore only half of it is actually available to the program).
 
-; {{-:hmNUMBER}} : Specifies a maximal heap size. The default is (2GB - 15).
+; {{-:hmNUMBER}} : Specifies a maximal heap size (including both semispaces). The default is (2GB - 15).
 
-; {{-:hsPERCENTAGE}} : Sets the shrink rate of the heap in percent. If no more than a quarter of {{PERCENTAGE}} of the heap is used, then it will shrink to {{PERCENTAGE}}. The default is 50.  Note: If you want to make sure that the heap never shrinks, specify a value of {{0}}.  (this can be useful in situations where an optimal heap-size is known in advance). 
+; {{-:hsPERCENTAGE}} : Sets the shrink rate of the heap in percent. The heap is shrunk to {{PERCENTAGE}} when the watermark is reached. The default is 50.  Note: If you want to make sure that the heap never shrinks, specify a value of {{0}}.  (this can be useful in situations where an optimal heap-size is known in advance). 
+
+; {{-:huPERCENTAGE}} : Sets the memory usage watermark below which heap shrinking is triggered. The default is 25.
 
 ; {{-:o}} : Disables detection of stack overflows at run-time.
 
diff --git a/runtime.c b/runtime.c
index a3cffeb6..4a190df5 100644
--- a/runtime.c
+++ b/runtime.c
@@ -358,7 +358,8 @@ C_TLS int
   C_main_argc;
 C_TLS C_uword 
   C_heap_growth,
-  C_heap_shrinkage;
+  C_heap_shrinkage,
+  C_heap_shrinkage_used;
 C_TLS C_uword C_maximal_heap_size;
 C_TLS time_t
   C_startup_time_seconds,
@@ -782,6 +783,8 @@ int CHICKEN_initialize(int heap, int stack, int symbols, void *toplevel)
 
   if(C_heap_shrinkage <= 0) C_heap_shrinkage = DEFAULT_HEAP_SHRINKAGE;
 
+  if(C_heap_shrinkage_used <= 0) C_heap_shrinkage_used = DEFAULT_HEAP_SHRINKAGE_USED;
+
   if(C_maximal_heap_size <= 0) C_maximal_heap_size = DEFAULT_MAXIMAL_HEAP_SIZE;
 
 #if !defined(NO_DLOAD2) && defined(HAVE_DLFCN_H)
@@ -1370,6 +1373,7 @@ void CHICKEN_parse_command_line(int argc, char *argv[], C_word *heap, C_word *st
 		 " -:hmSIZE set maximal heap size\n"
 		 " -:hgPERCENTAGE   set heap growth percentage\n"
 		 " -:hsPERCENTAGE   set heap shrink percentage\n"
+		 " -:huPERCENTAGE   set percentage of memory used at which heap will be shrunk\n"
 		 " -:hSIZE  set fixed heap size\n"
 		 " -:r  write trace output to stderr\n"
 		 " -:p  collect statistical profile and write to file at exit\n"
@@ -1404,6 +1408,9 @@ void CHICKEN_parse_command_line(int argc, char *argv[], C_word *heap, C_word *st
 	  case 's':
 	C_heap_shrinkage = arg_val(ptr + 1);
 	

[PATCH 3/3] Let scrutinizer infer types for foreign types with retconv/argconv given

2019-12-01 Thread megane
Hi,

Here's some more improvement on the #1649 issue.

This patch just drops the procedure annotation if scrutinizer can infer
it anyway. This gives the scrutinizer a change to make the type a bit
more accurate.

It's still not optimal, though.

The first two commits are just straight-forward refactoring. The last
one is the meat.

Here's a simple example:

  (import scheme
  (chicken base)
  (chicken string)
  (chicken type)
  (chicken foreign))

  (define-foreign-type foo int string->number list)

  (define inch (foreign-lambda foo "rand" foo))

  (compiler-typecase inch ((not *) 1))

;; Before:
;;
;; Error: No typecase match
;;   In file `foreign-lambda-and-retconvert.scm:11',
;;   At the toplevel,
;;   In `compiler-typecase' expression:
;;
;; (compiler-typecase g21 ((not *) 1) (else (##core#undefined)))
;;
;;   Tested expression does not match any case.
;;
;;   The expression has this type:
;;
;; (* -> *)
;;
;;   The specified type cases are these:
;;
;; (not *)

;; After:
;;
;; Error: No typecase match
;;   In file `foreign-lambda-and-retconvert.scm:11',
;;   At the toplevel,
;;   In `compiler-typecase' expression:
;;
;; (compiler-typecase g21 ((not *) 1) (else (##core#undefined)))
;;
;;   Tested expression does not match any case.
;;
;;   The expression has this type:
;;
;; (string -> list)
;;
;;   The specified type cases are these:
;;
;; (not *)

>From 91fcb2a2863856b66bb24dedde4a3b40e7f47f4d Mon Sep 17 00:00:00 2001
From: megane 
Date: Sun, 1 Dec 2019 09:23:29 +0200
Subject: [PATCH 1/3] * chicken-ffi-syntax.scm: Add annotate-foreign-procedure
 helper function

---
 chicken-ffi-syntax.scm | 53 +-
 1 file changed, 21 insertions(+), 32 deletions(-)

diff --git a/chicken-ffi-syntax.scm b/chicken-ffi-syntax.scm
index 1ba5348b..e11a6a28 100644
--- a/chicken-ffi-syntax.scm
+++ b/chicken-ffi-syntax.scm
@@ -213,6 +213,15 @@
 
 ;;; Aliases for internal forms
 
+(define (annotate-foreign-procedure e argtypes rtype)
+  `(##core#the
+(procedure ,(map (cut chicken.compiler.support#foreign-type->scrutiny-type 
<> 'arg)
+(chicken.syntax#strip-syntax argtypes))
+  ,(chicken.compiler.support#foreign-type->scrutiny-type
+(chicken.syntax#strip-syntax rtype) 'result))
+#f
+,e))
+
 (##sys#extend-macro-environment
  'define-foreign-type
  '()
@@ -254,13 +263,9 @@
  (compiler-only-er-transformer
   (lambda (form r c)
 (##sys#check-syntax 'foreign-lambda form '(_ _ _ . _))
-`(##core#the
-  (procedure ,(map (cut 
chicken.compiler.support#foreign-type->scrutiny-type <> 'arg)
-  (chicken.syntax#strip-syntax (cdddr form)))
-,(chicken.compiler.support#foreign-type->scrutiny-type
-  (chicken.syntax#strip-syntax (cadr form)) 'result))
-  #f
-  (##core#foreign-lambda ,@(cdr form))
+(annotate-foreign-procedure `(##core#foreign-lambda ,@(cdr form))
+   (cdddr form)
+   (cadr form)
 
 (##sys#extend-macro-environment
  'foreign-lambda*
@@ -268,16 +273,9 @@
  (compiler-only-er-transformer
   (lambda (form r c)
 (##sys#check-syntax 'foreign-lambda* form '(_ _ _ _ . _))
-`(##core#the
-  (procedure ,(map (lambda (a)
-(chicken.compiler.support#foreign-type->scrutiny-type
- (car a)
- 'arg))
-   (chicken.syntax#strip-syntax (caddr form)))
- ,(chicken.compiler.support#foreign-type->scrutiny-type
-   (chicken.syntax#strip-syntax (cadr form)) 'result))
-  #f
-  (##core#foreign-lambda* ,@(cdr form))
+(annotate-foreign-procedure `(##core#foreign-lambda* ,@(cdr form))
+   (map car (caddr form))
+   (cadr form)
 
 (##sys#extend-macro-environment
  'foreign-safe-lambda
@@ -285,13 +283,9 @@
  (compiler-only-er-transformer
   (lambda (form r c)
 (##sys#check-syntax 'foreign-safe-lambda form '(_ _ _ . _))
-`(##core#the
-  (procedure ,(map (cut 
chicken.compiler.support#foreign-type->scrutiny-type <> 'arg)
-   (chicken.syntax#strip-syntax (cdddr form)))
- ,(chicken.compiler.support#foreign-type->scrutiny-type
-   (chicken.syntax#strip-syntax (cadr form)) 'result))
-  #f
-  (##core#foreign-safe-lambda ,@(cdr form))
+(annotate-foreign-procedure `(##core#foreign-safe-lambda ,@(cdr form))
+   (cdddr form)
+   (cadr form)
 
 (##sys#extend-macro-environment
  'foreign-safe-lambda*
@@ -299,14 +293,9 @@
  (compiler-only-er-transformer
   (lambda (form r c)
 (##sys#check-syntax 'foreign-safe-lambda* form '(_ _ _ _ . _))
-`(##core#the
-  (procedure ,(map (lambda (a)
-