Re: [PATCH] Improve heap shrinkage calculation
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
> 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
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
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) -