Re: [Chicken-hackers] [PATCH] Make (time) annonce the maximum heap size used

[Peter Bex]

On Sat, Sep 17, 2016 at 04:21:24PM +0200, Kooda wrote:
> Hi!
> 
> I’ve been discussing this on the IRC channel for some time, and
> sjamaan created a ticket about it: http://bugs.call-cc.org/ticket/1318
> so I gave it a go.
> 
> This will be useful for our benchmark runs, among other things.
> 
> What this patch basically does is recording the amount of heap used
> after a major GC.
> 
> It’s my first time looking at the GC code so it took me a little while
> to understand what to do. I hope it’s clear.

Hi Kooda,

Good job!  I have made a few minor tweaks:

- C_number() is a bit of a strange function that we should probably
get rid of.  I've replaced it with C_unsigned_int_to_num, which,
despite its name, accepts a C_uword and converts it to a fixnum or
flonum (or in CHICKEN 5, a bignum).
- Printing a very large number in bytes is not very user-friendly so
I've added a quick and dirty conversion to KiB/MiB/GiB.
This will make parsing it a bit more difficult for chicken-benchmarks,
but it's worthwhile, I think, because time is useful on its own.
- Added a NEWS entry.

In CHICKEN 5, the C_number function is only used for the "number" foreign
type specifier, for which the documentation says:

  A floating-point number. Similar to double, but when used as a result
  type, then either an exact integer or a floating-point number is
  returned, depending on whether the result fits into an exact integer
  or not.

We might want to consider getting rid of that, because it makes very
little sense to do that in a Scheme with bignums.  And if it's truly
a floating-point number that is returned, it makes more sense to put
it in a flonum.  Objections, anyone?

Cheers,
Peter
From 402344764fc31f84f2aeee1ecdfb63bed392f23e Mon Sep 17 00:00:00 2001
From: Kooda 
Date: Fri, 16 Sep 2016 17:20:30 +0200
Subject: [PATCH] Make (time) show the maximum heap usage

Signed-off-by: Peter Bex 
---
 NEWS|  3 +++
 library.scm | 22 +-
 runtime.c   | 20 +---
 3 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/NEWS b/NEWS
index ebe102c..2ddfb39 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,8 @@
 4.11.2
 
+- Runtime system:
+  - "time" macro now shows peak memory usage (#1318, thanks to Kooda).
+
 4.11.1
 
 - Security fixes
diff --git a/library.scm b/library.scm
index 6676d03..ef5bdb0 100644
--- a/library.scm
+++ b/library.scm
@@ -221,7 +221,14 @@ EOF
   (##sys#gc #t)
   (##core#inline "C_start_timer"))
 
-(define ##sys#stop-timer (##core#primitive "C_stop_timer"))
+(define (##sys#stop-timer)
+  (let ((info ((##core#primitive "C_stop_timer"
+;; Run a major GC one more time to get memory usage information in
+;; case there was no major GC while the timer was running
+(##sys#gc #t)
+(##sys#setslot info 6 (##sys#slot ((##core#primitive "C_stop_timer")) 6))
+info))
+
 (define (##sys#immediate? x) (not (##core#inline "C_blockp" x)))
 (define (##sys#message str) (##core#inline "C_message" str))
 (define (##sys#byte x i) (##core#inline "C_subbyte" x i))
@@ -5023,6 +5030,16 @@ EOF
   (define (pchr chr) (##sys#write-char-0 chr ##sys#standard-error))
   (define (pnum num)
 (##sys#print (if (zero? num) "0" (##sys#number->string num)) #f ##sys#standard-error))
+  (define (round-to x y) ; Convert to fp with y digits after the point
+(/ (round (* x (expt 10 y))) (expt 10.0 y)))
+  (define (pmem bytes)
+(cond ((> bytes (expt 1024 3))
+	   (pnum (round-to (/ bytes (expt 1024 3)) 2)) (pstr " GiB"))
+	  ((> bytes (expt 1024 2))
+	   (pnum (round-to (/ bytes (expt 1024 2)) 2)) (pstr " MiB"))
+	  ((> bytes 1024)
+	   (pnum (round-to (/ bytes 1024) 2)) (pstr " KiB"))
+	  (else (pnum bytes) (pstr " bytes"
   (##sys#flush-output ##sys#standard-output)
   (pnum (##sys#slot info 0))
   (pstr "s CPU time")
@@ -5047,6 +5064,9 @@ EOF
   (pchr #\/)
   (pnum minor)
   (pstr " GCs (major/minor)")))
+  (let ((maximum-heap-usage (##sys#slot info 6)))
+(pstr ", maximum live heap: ")
+(pmem maximum-heap-usage))
   (##sys#write-char-0 #\newline ##sys#standard-error)
   (##sys#flush-output ##sys#standard-error))
 
diff --git a/runtime.c b/runtime.c
index 320c730..4b6300f 100644
--- a/runtime.c
+++ b/runtime.c
@@ -400,8 +400,9 @@ static C_TLS C_uword
   heapspace2_size,
   heap_size,
   temporary_stack_size,
-  fixed_temporary_stack_size = 0;
-static C_TLS C_char 
+  fixed_temporary_stack_size = 0,
+  maximum_heap_usage;
+static C_TLS C_char
   buffer[ STRING_BUFFER_SIZE ],
   *private_repository = NULL,
   *current_module_name,
@@ -767,7 +768,7 @@ int CHICKEN_initialize(int heap, int stack, int symbols, void *toplevel)
 #endif
   }
 
-  tracked_mutation_count = mutation_count = gc_count_1 = gc_count_1_total = gc_count_2 = 0;
+  tracked_mutation_count = mutation_count = gc_count_1 = gc_count_1_total = gc_count_2 = maximum_heap_usage = 0;
   lf_list = NULL;
   C_register_lf2(NULL, 0, create_initial_ptable());

Re: [Chicken-hackers] [PATCH] Make (time) annonce the maximum heap size used

[Kooda]

On Sun, 18 Sep 2016 14:32:51 +0200,
Peter Bex wrote:
> - C_number() is a bit of a strange function that we should probably
> get rid of.  I've replaced it with C_unsigned_int_to_num, which,
> despite its name, accepts a C_uword and converts it to a fixnum or
> flonum (or in CHICKEN 5, a bignum).

Ok, I didn’t know about that one.


> - Printing a very large number in bytes is not very user-friendly so
> I've added a quick and dirty conversion to KiB/MiB/GiB.
> This will make parsing it a bit more difficult for chicken-benchmarks,
> but it's worthwhile, I think, because time is useful on its own.

Yep, that’s fine by me.

I was considering making chicken-benchmarks use the vector returned by
##sys#stop-timer instead of parsing time’s textual output anyways, so
it might not be so much of a problem.


> - Added a NEWS entry.

Ah, right. I always forget about that.


> In CHICKEN 5, the C_number function is only used for the "number" foreign
> type specifier, for which the documentation says:
> 
>   A floating-point number. Similar to double, but when used as a result
>   type, then either an exact integer or a floating-point number is
>   returned, depending on whether the result fits into an exact integer
>   or not.
> 
> We might want to consider getting rid of that, because it makes very
> little sense to do that in a Scheme with bignums.  And if it's truly
> a floating-point number that is returned, it makes more sense to put
> it in a flonum.  Objections, anyone?

Sounds good to me. :)


Thanks a lot for reviewing this patch! :D

___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers