unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
* [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
@ 2011-05-31 19:48 Mark H Weaver
  2011-05-31 20:07 ` Mark H Weaver
  2011-05-31 21:52 ` Ludovic Courtès
  0 siblings, 2 replies; 25+ messages in thread
From: Mark H Weaver @ 2011-05-31 19:48 UTC (permalink / raw)
  To: guile-devel

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

Hello all,

This patch changes the way that bignums are allocated, so that digits
are now allocated using the GC allocation functions instead of standard
malloc/realloc/free.

Without this patch, computing (factorial 100000), where factorial is
implemented using the straightforward iterative approach below, causes
Guile to allocate a huge amount of memory.  This happens because the
majority of the memory used by large bignums (the digits themselves) is
not allocated by the GC, and so the GC doesn't run until a huge amount
of memory has been allocated.

  (define (factorial n)
    (define (fac n acc)
      (if (<= n 1)
        accum
        (fac (1- n) (* n acc))))
    (fac n 1))

With this patch, (factorial 100000) takes a long time but memory usage
is modest.

The main reason I haven't already pushed this patch is that there is a
slight complication: when you register custom allocation functions for
use by GMP, they get used for _all_ allocation, not just for digits.  In
particular, they get used to allocate the block returned by mpz_get_str
(when the first argument is NULL).  This means that if the user of
mpz_get_str uses the standard "free" to deallocate this block, it will
fail badly.  This could potentially affect programs which use libguile
but also use the GMP functions directly.

What do you think?

   Best,
    Mark



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Configure GMP to use GC allocation functions, remove bignum finalizers --]
[-- Type: text/x-diff, Size: 4110 bytes --]

From 964e43e579d9be310d5eaecde4c560c01c4c1afc Mon Sep 17 00:00:00 2001
From: Mark H Weaver <mhw@netris.org>
Date: Tue, 31 May 2011 15:26:41 -0400
Subject: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers

* libguile/numbers.c (custom_gmp_malloc, custom_gmp_realloc,
  custom_gmp_free): New static functions used by GMP for allocation.
  These are just wrappers for scm_gc_malloc, scm_gc_realloc, and
  scm_gc_free.

  (scm_init_numbers): Use mp_set_memory_functions to configure GMP to
  use custom_gmp_{malloc,realloc,free} for memory allocation.

  (make_bignum): Allocate the mpz_t (and type tag) using scm_gc_malloc,
  so that the GC will see the pointer to the digits, which are now
  allocated using custom_gmp_malloc.  Previously,
  scm_gc_malloc_pointerless was used to allocate the mpz_t, and a
  finalizer was used to free the digits.

  (finalize_bignum): Remove this static function.

  (scm_bigprint): Use custom_gmp_free to deallocate the string returned
  by mpz_get_str.
---
 libguile/numbers.c |   53 ++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 37 insertions(+), 16 deletions(-)

diff --git a/libguile/numbers.c b/libguile/numbers.c
index 5aeced6..1bf894a 100644
--- a/libguile/numbers.c
+++ b/libguile/numbers.c
@@ -172,14 +172,32 @@ scm_from_complex_double (complex double z)
 static mpz_t z_negative_one;
 
 \f
-/* Clear the `mpz_t' embedded in bignum PTR.  */
-static void
-finalize_bignum (GC_PTR ptr, GC_PTR data)
+
+/* The next three functions (custom_libgmp_*) are passed to
+   mp_set_memory_functions (in GMP) so that memory used by the digits
+   themselves are allocated by the garbage collector.  This is needed so
+   that GC will be run at appropriate times.  Otherwise, a program which
+   creates many large bignums would malloc a huge amount of memory
+   before the GC runs. */
+static void *
+custom_gmp_malloc (size_t alloc_size)
+{
+  /* Unfortunately we cannot safely use scm_gc_malloc_pointerless here,
+     because the GMP docs specifically warns that it may use allocated
+     blocks to hold pointers to other allocated blocks. */
+  return scm_gc_malloc (alloc_size, "bignum-digits");
+}
+
+static void *
+custom_gmp_realloc (void *old_ptr, size_t old_size, size_t new_size)
 {
-  SCM bignum;
+  return scm_gc_realloc (old_ptr, old_size, new_size, "bignum-digits");
+}
 
-  bignum = PTR2SCM (ptr);
-  mpz_clear (SCM_I_BIG_MPZ (bignum));
+static void
+custom_gmp_free (void *ptr, size_t size)
+{
+  scm_gc_free (ptr, size, "bignum-digits");
 }
 
 /* Return a new uninitialized bignum.  */
@@ -187,18 +205,12 @@ static inline SCM
 make_bignum (void)
 {
   scm_t_bits *p;
-  GC_finalization_proc prev_finalizer;
-  GC_PTR prev_finalizer_data;
 
   /* Allocate one word for the type tag and enough room for an `mpz_t'.  */
-  p = scm_gc_malloc_pointerless (sizeof (scm_t_bits) + sizeof (mpz_t),
-				 "bignum");
+  p = scm_gc_malloc (sizeof (scm_t_bits) + sizeof (mpz_t),
+		     "bignum");
   p[0] = scm_tc16_big;
 
-  GC_REGISTER_FINALIZER_NO_ORDER (p, finalize_bignum, NULL,
-				  &prev_finalizer,
-				  &prev_finalizer_data);
-
   return SCM_PACK (p);
 }
 
@@ -5360,9 +5372,10 @@ int
 scm_bigprint (SCM exp, SCM port, scm_print_state *pstate SCM_UNUSED)
 {
   char *str = mpz_get_str (NULL, 10, SCM_I_BIG_MPZ (exp));
+  size_t len = (size_t) strlen (str);
   scm_remember_upto_here_1 (exp);
-  scm_lfwrite (str, (size_t) strlen (str), port);
-  free (str);
+  scm_lfwrite (str, len, port);
+  custom_gmp_free (str, len + 1);
   return !0;
 }
 /*** END nums->strs ***/
@@ -9668,6 +9681,14 @@ scm_init_numbers ()
 {
   int i;
 
+  /* IMPORTANT: mp_set_memory_functions _must_ be called before any GMP
+     functions are called, or else custom_gmp_realloc and/or
+     custom_gmp_free could be called on a memory block allocated with
+     plain malloc, which would be bad. */
+  mp_set_memory_functions (custom_gmp_malloc,
+			   custom_gmp_realloc,
+			   custom_gmp_free);
+
   mpz_init_set_si (z_negative_one, -1);
 
   /* It may be possible to tune the performance of some algorithms by using
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-05-31 19:48 [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers Mark H Weaver
@ 2011-05-31 20:07 ` Mark H Weaver
  2011-05-31 21:52 ` Ludovic Courtès
  1 sibling, 0 replies; 25+ messages in thread
From: Mark H Weaver @ 2011-05-31 20:07 UTC (permalink / raw)
  To: guile-devel

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

Here is a revised version of the patch, which fixes a typo in a comment
and a poorly chosen "what" tag for allocation.

Also, the definition of factorial in my email had a typo.  It should be:

  (define (factorial n)
    (define (fac n acc)
      (if (<= n 1)
        acc
        (fac (1- n) (* n acc))))
    (fac n 1))

Apologies for the sloppiness,

       Mark



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Configure GMP to use GC allocation functions, remove bignum finalizers --]
[-- Type: text/x-diff, Size: 4104 bytes --]

From 2a8c5a5b464f2af723476585f620a108ac1cc37b Mon Sep 17 00:00:00 2001
From: Mark H Weaver <mhw@netris.org>
Date: Tue, 31 May 2011 15:26:41 -0400
Subject: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers

* libguile/numbers.c (custom_gmp_malloc, custom_gmp_realloc,
  custom_gmp_free): New static functions used by GMP for allocation.
  These are just wrappers for scm_gc_malloc, scm_gc_realloc, and
  scm_gc_free.

  (scm_init_numbers): Use mp_set_memory_functions to configure GMP to
  use custom_gmp_{malloc,realloc,free} for memory allocation.

  (make_bignum): Allocate the mpz_t (and type tag) using scm_gc_malloc,
  so that the GC will see the pointer to the digits, which are now
  allocated using custom_gmp_malloc.  Previously,
  scm_gc_malloc_pointerless was used to allocate the mpz_t, and a
  finalizer was used to free the digits.

  (finalize_bignum): Remove this static function.

  (scm_bigprint): Use custom_gmp_free to deallocate the string returned
  by mpz_get_str.
---
 libguile/numbers.c |   53 ++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 37 insertions(+), 16 deletions(-)

diff --git a/libguile/numbers.c b/libguile/numbers.c
index 5aeced6..d14e5ea 100644
--- a/libguile/numbers.c
+++ b/libguile/numbers.c
@@ -172,14 +172,32 @@ scm_from_complex_double (complex double z)
 static mpz_t z_negative_one;
 
 \f
-/* Clear the `mpz_t' embedded in bignum PTR.  */
-static void
-finalize_bignum (GC_PTR ptr, GC_PTR data)
+
+/* The next three functions (custom_gmp_*) are passed to
+   mp_set_memory_functions (in GMP) so that memory used by the digits
+   themselves are allocated by the garbage collector.  This is needed so
+   that GC will be run at appropriate times.  Otherwise, a program which
+   creates many large bignums would malloc a huge amount of memory
+   before the GC runs. */
+static void *
+custom_gmp_malloc (size_t alloc_size)
+{
+  /* Unfortunately we cannot safely use scm_gc_malloc_pointerless here,
+     because the GMP docs specifically warns that it may use allocated
+     blocks to hold pointers to other allocated blocks. */
+  return scm_gc_malloc (alloc_size, "gmp-internal");
+}
+
+static void *
+custom_gmp_realloc (void *old_ptr, size_t old_size, size_t new_size)
 {
-  SCM bignum;
+  return scm_gc_realloc (old_ptr, old_size, new_size, "gmp-internal");
+}
 
-  bignum = PTR2SCM (ptr);
-  mpz_clear (SCM_I_BIG_MPZ (bignum));
+static void
+custom_gmp_free (void *ptr, size_t size)
+{
+  scm_gc_free (ptr, size, "gmp-internal");
 }
 
 /* Return a new uninitialized bignum.  */
@@ -187,18 +205,12 @@ static inline SCM
 make_bignum (void)
 {
   scm_t_bits *p;
-  GC_finalization_proc prev_finalizer;
-  GC_PTR prev_finalizer_data;
 
   /* Allocate one word for the type tag and enough room for an `mpz_t'.  */
-  p = scm_gc_malloc_pointerless (sizeof (scm_t_bits) + sizeof (mpz_t),
-				 "bignum");
+  p = scm_gc_malloc (sizeof (scm_t_bits) + sizeof (mpz_t),
+		     "bignum");
   p[0] = scm_tc16_big;
 
-  GC_REGISTER_FINALIZER_NO_ORDER (p, finalize_bignum, NULL,
-				  &prev_finalizer,
-				  &prev_finalizer_data);
-
   return SCM_PACK (p);
 }
 
@@ -5360,9 +5372,10 @@ int
 scm_bigprint (SCM exp, SCM port, scm_print_state *pstate SCM_UNUSED)
 {
   char *str = mpz_get_str (NULL, 10, SCM_I_BIG_MPZ (exp));
+  size_t len = (size_t) strlen (str);
   scm_remember_upto_here_1 (exp);
-  scm_lfwrite (str, (size_t) strlen (str), port);
-  free (str);
+  scm_lfwrite (str, len, port);
+  custom_gmp_free (str, len + 1);
   return !0;
 }
 /*** END nums->strs ***/
@@ -9668,6 +9681,14 @@ scm_init_numbers ()
 {
   int i;
 
+  /* IMPORTANT: mp_set_memory_functions _must_ be called before any GMP
+     functions are called, or else custom_gmp_realloc and/or
+     custom_gmp_free could be called on a memory block allocated with
+     plain malloc, which would be bad. */
+  mp_set_memory_functions (custom_gmp_malloc,
+			   custom_gmp_realloc,
+			   custom_gmp_free);
+
   mpz_init_set_si (z_negative_one, -1);
 
   /* It may be possible to tune the performance of some algorithms by using
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-05-31 19:48 [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers Mark H Weaver
  2011-05-31 20:07 ` Mark H Weaver
@ 2011-05-31 21:52 ` Ludovic Courtès
  2011-05-31 23:18   ` Neil Jerram
  1 sibling, 1 reply; 25+ messages in thread
From: Ludovic Courtès @ 2011-05-31 21:52 UTC (permalink / raw)
  To: guile-devel

Hi!

Mark H Weaver <mhw@netris.org> skribas:

> This patch changes the way that bignums are allocated, so that digits
> are now allocated using the GC allocation functions instead of standard
> malloc/realloc/free.
>
> Without this patch, computing (factorial 100000), where factorial is
> implemented using the straightforward iterative approach below, causes
> Guile to allocate a huge amount of memory.  This happens because the
> majority of the memory used by large bignums (the digits themselves) is
> not allocated by the GC, and so the GC doesn't run until a huge amount
> of memory has been allocated.
>
>   (define (factorial n)
>     (define (fac n acc)
>       (if (<= n 1)
>         accum
>         (fac (1- n) (* n acc))))
>     (fac n 1))

Ouch!  Thanks for tracking this down.

> With this patch, (factorial 100000) takes a long time but memory usage
> is modest.

OK, interesting.

Reminds me of the malloc hook trick:
<http://thread.gmane.org/gmane.comp.programming.garbage-collection.boehmgc/4371>.
It may still be worth considering for libguile, no?

> The main reason I haven't already pushed this patch is that there is a
> slight complication: when you register custom allocation functions for
> use by GMP, they get used for _all_ allocation, not just for digits.  In
> particular, they get used to allocate the block returned by mpz_get_str
> (when the first argument is NULL).  This means that if the user of
> mpz_get_str uses the standard "free" to deallocate this block, it will
> fail badly.  This could potentially affect programs which use libguile
> but also use the GMP functions directly.

Yes, that's a problem, probably even be a showstopper for 2.0.  :-(

What do others think?

> +static void
> +custom_gmp_free (void *ptr, size_t size)
> +{
> +  scm_gc_free (ptr, size, "bignum-digits");
>  }

May be safer to do nothing here (see <gc.h>.)

Thanks,
Ludo'.




^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-05-31 21:52 ` Ludovic Courtès
@ 2011-05-31 23:18   ` Neil Jerram
  2011-05-31 23:32     ` Neil Jerram
  2011-06-01  8:34     ` Andy Wingo
  0 siblings, 2 replies; 25+ messages in thread
From: Neil Jerram @ 2011-05-31 23:18 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

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

> Hi!
>
> Mark H Weaver <mhw@netris.org> skribas:
>
>> The main reason I haven't already pushed this patch is that there is a
>> slight complication: when you register custom allocation functions for
>> use by GMP, they get used for _all_ allocation, not just for digits.  In
>> particular, they get used to allocate the block returned by mpz_get_str
>> (when the first argument is NULL).  This means that if the user of
>> mpz_get_str uses the standard "free" to deallocate this block, it will
>> fail badly.  This could potentially affect programs which use libguile
>> but also use the GMP functions directly.
>
> Yes, that's a problem, probably even be a showstopper for 2.0.  :-(
>
> What do others think?

Provide an API to allow the trade-off to be decided by the application
developer?

The application developer would know if they were using GMP other than
via Guile, and in that case they'd choose to have GMP use GC
allocation.  Alternatively they might know that their application's use
of GMP/Guile was not such as to generate lots of GMP garbage as the
factorial example does, and so choose to use normal malloc.

Of course, that's only if there really is no more perfect solution here.

      Neil



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-05-31 23:18   ` Neil Jerram
@ 2011-05-31 23:32     ` Neil Jerram
  2011-06-01  8:34     ` Andy Wingo
  1 sibling, 0 replies; 25+ messages in thread
From: Neil Jerram @ 2011-05-31 23:32 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

Neil Jerram <neil@ossau.uklinux.net> writes:

> The application developer would know if they were using GMP other than
> via Guile, and in that case they'd choose to have GMP use GC
> allocation.  Alternatively they might know that their application's use
> of GMP/Guile was not such as to generate lots of GMP garbage as the
> factorial example does, and so choose to use normal malloc.

I missed a "not" there.  But I'm sure you get the idea anyway.

       Neil



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-05-31 23:18   ` Neil Jerram
  2011-05-31 23:32     ` Neil Jerram
@ 2011-06-01  8:34     ` Andy Wingo
  2011-06-02 23:14       ` Ludovic Courtès
  2011-11-27 19:33       ` Andy Wingo
  1 sibling, 2 replies; 25+ messages in thread
From: Andy Wingo @ 2011-06-01  8:34 UTC (permalink / raw)
  To: Neil Jerram; +Cc: Ludovic Courtès, guile-devel

Hey folks,

Thanks for the debugging and the patch, Mark!

On Wed 01 Jun 2011 01:18, Neil Jerram <neil@ossau.uklinux.net> writes:

> ludo@gnu.org (Ludovic Courtès) writes:
>
>> Mark H Weaver <mhw@netris.org> skribas:
>>
>>> The main reason I haven't already pushed this patch is that there is a
>>> slight complication: when you register custom allocation functions for
>>> use by GMP, they get used for _all_ allocation, not just for digits.  In
>>> particular, they get used to allocate the block returned by mpz_get_str
>>> (when the first argument is NULL).  This means that if the user of
>>> mpz_get_str uses the standard "free" to deallocate this block, it will
>>> fail badly.  This could potentially affect programs which use libguile
>>> but also use the GMP functions directly.
>>
>> Yes, that's a problem, probably even be a showstopper for 2.0.  :-(
>>
>> What do others think?
>
> Provide an API to allow the trade-off to be decided by the application
> developer?

I think this only makes sense as an interim measure, like if we want to
change GMP to allocate only via GC_malloc in 2.2, then provide the
option in 2.0.  There has to be a better long-term solution though.

What if we get libgc to track total heap size (whether the user has
malloc, g_slice_alloc, or whatever), and start doing GC more frequently
if it sees the total heap size going up?

Andy
-- 
http://wingolog.org/



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-06-01  8:34     ` Andy Wingo
@ 2011-06-02 23:14       ` Ludovic Courtès
  2011-11-27 19:33       ` Andy Wingo
  1 sibling, 0 replies; 25+ messages in thread
From: Ludovic Courtès @ 2011-06-02 23:14 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guile-devel, Neil Jerram

Hi,

Andy Wingo <wingo@pobox.com> skribas:

> What if we get libgc to track total heap size (whether the user has
> malloc, g_slice_alloc, or whatever), and start doing GC more frequently
> if it sees the total heap size going up?

That would be ideal, of course.

Ludo'.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-06-01  8:34     ` Andy Wingo
  2011-06-02 23:14       ` Ludovic Courtès
@ 2011-11-27 19:33       ` Andy Wingo
  2011-11-27 21:25         ` Ludovic Courtès
  1 sibling, 1 reply; 25+ messages in thread
From: Andy Wingo @ 2011-11-27 19:33 UTC (permalink / raw)
  To: Neil Jerram; +Cc: Ludovic Courtès, guile-devel

Hi,

An old thread, but one that would be good to fix.  To recap, Mark Weaver
wrote:

> Without this patch, computing (factorial 100000), where factorial is
> implemented using the straightforward iterative approach below, causes
> Guile to allocate a huge amount of memory.  This happens because the
> majority of the memory used by large bignums (the digits themselves) is
> not allocated by the GC, and so the GC doesn't run until a huge amount
> of memory has been allocated.

We talked about allocating the gmp memory with libgc, but there was
concern about other gmp users in the same Guile process.

On Wed 01 Jun 2011 10:34, Andy Wingo <wingo@pobox.com> writes:

> On Wed 01 Jun 2011 01:18, Neil Jerram <neil@ossau.uklinux.net> writes:
>
>> Provide an API to allow the trade-off to be decided by the application
>> developer?
>
> I think this only makes sense as an interim measure, like if we want to
> change GMP to allocate only via GC_malloc in 2.2, then provide the
> option in 2.0.  There has to be a better long-term solution though.
>
> What if we get libgc to track total heap size (whether the user has
> malloc, g_slice_alloc, or whatever), and start doing GC more frequently
> if it sees the total heap size going up?

I still this this is a good idea.  The key would be to GC more often
when the heap image grows faster than GC'd memory is allocated.  We can
detect this situation by computing (ImageSize_t+1 - ImageSize_t) /
(LiveBytes_t+1 - LiveBytes_t), where ImageSize is some measure of the
total process size, perhaps via getrusage(), and LiveBytes is
GC_get_heap_size() - GC_get_free_bytes().

We could probably prototype this ourselves, outside of libgc, using
GC_set_free_space_divisor as the knob to tweak.  If it is generally
applicable we could make a patch to incorporate this in libgc proper.

WDYT?

Andy
-- 
http://wingolog.org/



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-11-27 19:33       ` Andy Wingo
@ 2011-11-27 21:25         ` Ludovic Courtès
  2011-11-28 22:23           ` Andy Wingo
  2011-12-02 11:10           ` Andy Wingo
  0 siblings, 2 replies; 25+ messages in thread
From: Ludovic Courtès @ 2011-11-27 21:25 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guile-devel, Neil Jerram

Hi!

Andy Wingo <wingo@pobox.com> skribis:

> I still this this is a good idea.  The key would be to GC more often
> when the heap image grows faster than GC'd memory is allocated.  We can
> detect this situation by computing (ImageSize_t+1 - ImageSize_t) /
> (LiveBytes_t+1 - LiveBytes_t), where ImageSize is some measure of the
> total process size, perhaps via getrusage(), and LiveBytes is
> GC_get_heap_size() - GC_get_free_bytes().

The problem is that this measurement doesn’t allow us to differentiate
between a growing heap with objects that may be freed as a result of
running the GC, and a growing heap just because the application needs
more malloc’d objects.

(I think this is the argument that was made against my __malloc_hook
patch back then. [0])

A longer term option may be to augment libgc with something akin to our
old scm_gc_register_collectable_memory.

WDYT?

Thanks,
Ludo’.

[0] http://thread.gmane.org/gmane.comp.programming.garbage-collection.boehmgc/4371



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-11-27 21:25         ` Ludovic Courtès
@ 2011-11-28 22:23           ` Andy Wingo
  2011-11-28 23:10             ` Ludovic Courtès
  2011-12-02 11:10           ` Andy Wingo
  1 sibling, 1 reply; 25+ messages in thread
From: Andy Wingo @ 2011-11-28 22:23 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel, Neil Jerram

On Sun 27 Nov 2011 22:25, ludo@gnu.org (Ludovic Courtès) writes:

> The problem is that this measurement doesn’t allow us to differentiate
> between a growing heap with objects that may be freed as a result of
> running the GC, and a growing heap just because the application needs
> more malloc’d objects.

This is true, but typically the heap stabilizes at some point.

Here is the problem: if you are allocating at a GC-managed heap size H,
garbage collection will tend to limit your process image size to H*N.
But if at the same time you are mallocating at a rate M bytes per
GC-allocated byte, then your process stabilizes at H*N*M -- assuming
that collecting data will result in malloc'd data being freed.  It
doesn't take a very large M for this to be a bad situation.  If you
would like to limit your image size, you should GC more often -- the
bigger the M, the more often.

The original iterative factorial case that Mark gave is pessimal,
because M is an increasing function of time.

Now, what happens if the process is not growing because of GC, but for
other reasons?  In that case M will be estimated as artificially high
for a while, and so GC will happen more often on the Guile side.  But
when it stabilizes, we can ease back the GC frequency.

The key is to measure process image growth, not mallocation rate.

I'm going to give this a try and see what happens.

Andy
-- 
http://wingolog.org/



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-11-28 22:23           ` Andy Wingo
@ 2011-11-28 23:10             ` Ludovic Courtès
  2011-11-28 23:50               ` Andy Wingo
  0 siblings, 1 reply; 25+ messages in thread
From: Ludovic Courtès @ 2011-11-28 23:10 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guile-devel, Neil Jerram

Andy Wingo <wingo@pobox.com> skribis:

> On Sun 27 Nov 2011 22:25, ludo@gnu.org (Ludovic Courtès) writes:
>
>> The problem is that this measurement doesn’t allow us to differentiate
>> between a growing heap with objects that may be freed as a result of
>> running the GC, and a growing heap just because the application needs
>> more malloc’d objects.
>
> This is true, but typically the heap stabilizes at some point.

Ooh, re-reading your previous message, I now see what you mean, and it
makes sense to me.

Thanks,
Ludo’.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-11-28 23:10             ` Ludovic Courtès
@ 2011-11-28 23:50               ` Andy Wingo
  2011-11-29 11:06                 ` Ludovic Courtès
  0 siblings, 1 reply; 25+ messages in thread
From: Andy Wingo @ 2011-11-28 23:50 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel, Neil Jerram

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

On Tue 29 Nov 2011 00:10, ludo@gnu.org (Ludovic Courtès) writes:

> Andy Wingo <wingo@pobox.com> skribis:
>
>> On Sun 27 Nov 2011 22:25, ludo@gnu.org (Ludovic Courtès) writes:
>>
>>> The problem is that this measurement doesn’t allow us to differentiate
>>> between a growing heap with objects that may be freed as a result of
>>> running the GC, and a growing heap just because the application needs
>>> more malloc’d objects.
>>
>> This is true, but typically the heap stabilizes at some point.
>
> Ooh, re-reading your previous message, I now see what you mean, and it
> makes sense to me.

WDYT?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-increase-garbage-collection-rate-if-the-process-is-g.patch --]
[-- Type: text/x-diff, Size: 6389 bytes --]

From 5dc96678aba01297a127aa6f3781ecedec3b24f1 Mon Sep 17 00:00:00 2001
From: Andy Wingo <wingo@pobox.com>
Date: Tue, 29 Nov 2011 00:48:56 +0100
Subject: [PATCH] increase garbage collection rate if the process is growing

* configure.ac: Check for GC_get_free_space_divisor.
* libguile/gc.c (GC_get_free_space_divisor): Define an implementation,
  if needed.
  (accumulate_gc_timer): Fix indentation.
  (get_image_size): New terrible hack.  Needs implementations on other
  platforms.
  (adjust_gc_frequency): Attempt to adjust the GC frequency based on
  process image growth.  Needs more comments.
  (scm_init_gc): Add the adjust_gc_frequency to the after_gc_c_hook.
---
 configure.ac  |    2 +-
 libguile/gc.c |  108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 107 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index 3a56cda..d63dd63 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1259,7 +1259,7 @@ save_LIBS="$LIBS"
 LIBS="$BDW_GC_LIBS $LIBS"
 CFLAGS="$BDW_GC_CFLAGS $CFLAGS"
 
-AC_CHECK_FUNCS([GC_do_blocking GC_call_with_gc_active GC_pthread_exit GC_pthread_cancel GC_allow_register_threads GC_pthread_sigmask GC_set_start_callback GC_get_heap_usage_safe])
+AC_CHECK_FUNCS([GC_do_blocking GC_call_with_gc_active GC_pthread_exit GC_pthread_cancel GC_allow_register_threads GC_pthread_sigmask GC_set_start_callback GC_get_heap_usage_safe GC_get_free_space_divisor])
 
 # Though the `GC_do_blocking ()' symbol is present in GC 7.1, it is not
 # declared, and has a different type (returning void instead of
diff --git a/libguile/gc.c b/libguile/gc.c
index 4895b7d..281efde 100644
--- a/libguile/gc.c
+++ b/libguile/gc.c
@@ -27,6 +27,7 @@
 #include <stdio.h>
 #include <errno.h>
 #include <string.h>
+#include <math.h>
 
 #ifdef __ia64__
 #include <ucontext.h>
@@ -194,6 +195,8 @@ SCM_DEFINE (scm_set_debug_cell_accesses_x, "set-debug-cell-accesses!", 1, 0, 0,
 
 \f
 
+/* Compatibility.  */
+
 #ifndef HAVE_GC_GET_HEAP_USAGE_SAFE
 static void
 GC_get_heap_usage_safe (GC_word *pheap_size, GC_word *pfree_bytes,
@@ -208,6 +211,14 @@ GC_get_heap_usage_safe (GC_word *pheap_size, GC_word *pfree_bytes,
 }
 #endif
 
+#ifndef HAVE_GC_GET_FREE_SPACE_DIVISOR
+static GC_word
+GC_get_free_space_divisor (void)
+{
+  return GC_free_space_divisor;
+}
+#endif
+
 \f
 /* Hooks.  */
 scm_t_c_hook scm_before_gc_c_hook;
@@ -230,6 +241,9 @@ unsigned long scm_gc_ports_collected = 0;
 static long gc_time_taken = 0;
 static long gc_start_time = 0;
 
+static unsigned long free_space_divisor;
+static unsigned long minimum_free_space_divisor;
+static double target_free_space_divisor;
 
 static unsigned long protected_obj_count = 0;
 
@@ -598,7 +612,10 @@ void
 scm_storage_prehistory ()
 {
   GC_all_interior_pointers = 0;
-  GC_set_free_space_divisor (scm_getenv_int ("GC_FREE_SPACE_DIVISOR", 3));
+  free_space_divisor = scm_getenv_int ("GC_FREE_SPACE_DIVISOR", 3);
+  minimum_free_space_divisor = free_space_divisor;
+  target_free_space_divisor = free_space_divisor;
+  GC_set_free_space_divisor (free_space_divisor);
 
   GC_INIT ();
 
@@ -742,7 +759,8 @@ accumulate_gc_timer (void * hook_data SCM_UNUSED,
                 void *data SCM_UNUSED)
 {
   if (gc_start_time)
-    { long now = scm_c_get_internal_run_time ();
+    {
+      long now = scm_c_get_internal_run_time ();
       gc_time_taken += now - gc_start_time;
       gc_start_time = 0;
     }
@@ -750,6 +768,91 @@ accumulate_gc_timer (void * hook_data SCM_UNUSED,
   return NULL;
 }
 
+static size_t
+get_image_size (void)
+{
+  unsigned long size, resident, share;
+  size_t ret;
+
+  FILE *fp = fopen ("/proc/self/statm", "r");
+
+  if (fp && fscanf (fp, "%lu %lu %lu", &size, &resident, &share) == 3)
+    ret = resident * 4096;
+
+  if (fp)
+    fclose (fp);
+
+  return ret;
+}
+
+static void *
+adjust_gc_frequency (void * hook_data SCM_UNUSED,
+                     void *fn_data SCM_UNUSED,
+                     void *data SCM_UNUSED)
+{
+  static size_t prev_image_size = 0;
+  static size_t prev_bytes_alloced = 0;
+  size_t image_size;
+  size_t bytes_alloced;
+  
+  image_size = get_image_size ();
+  bytes_alloced = GC_get_total_bytes ();
+
+#define HEURISTICS_DEBUG 0
+
+#if HEURISTICS_DEBUG
+  fprintf (stderr, "prev image / alloced: %lu / %lu\n", prev_image_size, prev_bytes_alloced);
+  fprintf (stderr, "     image / alloced: %lu / %lu\n", image_size, bytes_alloced);
+  fprintf (stderr, "divisor %lu / %f\n", free_space_divisor, target_free_space_divisor);
+#endif
+
+  if (prev_image_size && bytes_alloced != prev_bytes_alloced)
+    {
+      double growth_rate, new_target_free_space_divisor;
+      double damping_factor = 0.5;
+      double hysteresis = 0.1;
+
+      growth_rate = ((double) image_size - prev_image_size)
+        / ((double)bytes_alloced - prev_bytes_alloced);
+      
+#if HEURISTICS_DEBUG
+      fprintf (stderr, "growth rate %f\n", growth_rate);
+#endif
+
+      new_target_free_space_divisor = minimum_free_space_divisor;
+
+      if (growth_rate > 0)
+        new_target_free_space_divisor *= 1.0 + growth_rate;
+
+#if HEURISTICS_DEBUG
+      fprintf (stderr, "new divisor %f\n", new_target_free_space_divisor);
+#endif
+
+      target_free_space_divisor =
+        (damping_factor * target_free_space_divisor
+         + (1.0 - damping_factor) * new_target_free_space_divisor);
+
+#if HEURISTICS_DEBUG
+      fprintf (stderr, "new target divisor %f\n", target_free_space_divisor);
+#endif
+
+      if (free_space_divisor + 0.5 + hysteresis < target_free_space_divisor
+          || free_space_divisor - 0.5 - hysteresis > target_free_space_divisor)
+        {
+          free_space_divisor = lround (target_free_space_divisor);
+#if HEURISTICS_DEBUG
+          fprintf (stderr, "new divisor %lu\n", free_space_divisor);
+#endif
+          GC_set_free_space_divisor (free_space_divisor);
+        }
+    }
+
+  prev_image_size = image_size;
+  prev_bytes_alloced = bytes_alloced;
+
+  return NULL;
+}
+
 
 \f
 
@@ -847,6 +950,7 @@ scm_init_gc ()
   scm_c_hook_add (&scm_before_gc_c_hook, queue_after_gc_hook, NULL, 0);
   scm_c_hook_add (&scm_before_gc_c_hook, start_gc_timer, NULL, 0);
   scm_c_hook_add (&scm_after_gc_c_hook, accumulate_gc_timer, NULL, 0);
+  scm_c_hook_add (&scm_after_gc_c_hook, adjust_gc_frequency, NULL, 0);
 
 #ifdef HAVE_GC_SET_START_CALLBACK
   GC_set_start_callback (run_before_gc_c_hook);
-- 
1.7.7.3


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


-- 
http://wingolog.org/

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-11-28 23:50               ` Andy Wingo
@ 2011-11-29 11:06                 ` Ludovic Courtès
  2011-11-29 11:35                   ` Andy Wingo
  2011-11-29 12:45                   ` Andy Wingo
  0 siblings, 2 replies; 25+ messages in thread
From: Ludovic Courtès @ 2011-11-29 11:06 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guile-devel, Neil Jerram

Hi Andy!

Andy Wingo <wingo@pobox.com> skribis:

> On Tue 29 Nov 2011 00:10, ludo@gnu.org (Ludovic Courtès) writes:
>
>> Andy Wingo <wingo@pobox.com> skribis:
>>
>>> On Sun 27 Nov 2011 22:25, ludo@gnu.org (Ludovic Courtès) writes:
>>>
>>>> The problem is that this measurement doesn’t allow us to differentiate
>>>> between a growing heap with objects that may be freed as a result of
>>>> running the GC, and a growing heap just because the application needs
>>>> more malloc’d objects.
>>>
>>> This is true, but typically the heap stabilizes at some point.
>>
>> Ooh, re-reading your previous message, I now see what you mean, and it
>> makes sense to me.
>
> WDYT?

Nice!

> +static size_t
> +get_image_size (void)
> +{
> +  unsigned long size, resident, share;
> +  size_t ret;
> +
> +  FILE *fp = fopen ("/proc/self/statm", "r");
> +
> +  if (fp && fscanf (fp, "%lu %lu %lu", &size, &resident, &share) == 3)
> +    ret = resident * 4096;
> +
> +  if (fp)
> +    fclose (fp);
> +
> +  return ret;
> +}

RET should never be left uninitialized, and the caller should be
prepared to deal with RET == 0.

(The code itself doesn’t have to #ifdef __linux__ because other OSes
provides an implementation that mimics Linux’s /proc.)

> +static void *
> +adjust_gc_frequency (void * hook_data SCM_UNUSED,
> +                     void *fn_data SCM_UNUSED,
> +                     void *data SCM_UNUSED)
> +{

[...]

> +      if (free_space_divisor + 0.5 + hysteresis < target_free_space_divisor
> +          || free_space_divisor - 0.5 - hysteresis > target_free_space_divisor)
> +        {
> +          free_space_divisor = lround (target_free_space_divisor);
> +#if HEURISTICS_DEBUG
> +          fprintf (stderr, "new divisor %lu\n", free_space_divisor);
> +#endif
> +          GC_set_free_space_divisor (free_space_divisor);
> +        }
> +    }
> +

My impression was that FREE_SPACE_DIVISOR was quite coarse-grain.
What’s your experience with that?

Could you run the stuff under gc-benchmarks/ with and without the
heuristic, as in [0]?

Also, could you check with a program that actually mixes malloc +
GC-alloc how it behaves?

I tried to reproduce the infamous iconv issue, which would have been a
simple way to check, but failed:

  (with-fluids ((%default-port-encoding "UTF-16BE"))
    (let loop () (open-output-string ) (loop)))

Memory growth appears to be bounded here.

Thanks,
Ludo’.

[0] http://thread.gmane.org/gmane.lisp.guile.devel/7803



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-11-29 11:06                 ` Ludovic Courtès
@ 2011-11-29 11:35                   ` Andy Wingo
  2011-11-29 13:20                     ` Ludovic Courtès
  2011-11-29 12:45                   ` Andy Wingo
  1 sibling, 1 reply; 25+ messages in thread
From: Andy Wingo @ 2011-11-29 11:35 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel, Neil Jerram

On Tue 29 Nov 2011 12:06, ludo@gnu.org (Ludovic Courtès) writes:

> RET should never be left uninitialized, and the caller should be
> prepared to deal with RET == 0.

Oops, will fix.

> (The code itself doesn’t have to #ifdef __linux__ because other OSes
> provides an implementation that mimics Linux’s /proc.)

Interesting, didn't know that.  In any case it should DTRT if statm
isn't there.

> My impression was that FREE_SPACE_DIVISOR was quite coarse-grain.
> What’s your experience with that?

It has varying precision.  For example in the (factorial 100000)
example, it gets to 1800 at some point.  Quantizing the
target_free_space_divisor is an issue.

> Could you run the stuff under gc-benchmarks/ with and without the
> heuristic, as in [0]?

Sure.

> Also, could you check with a program that actually mixes malloc +
> GC-alloc how it behaves?

Does the factorial example not count?

> I tried to reproduce the infamous iconv issue, which would have been a
> simple way to check, but failed:
>
>   (with-fluids ((%default-port-encoding "UTF-16BE"))
>     (let loop () (open-output-string ) (loop)))
>
> Memory growth appears to be bounded here.

What do you mean?  Do you mean that this patch fixes the iconv issue, or
that you couldn't reproduce it before?

Regards,

Andy
-- 
http://wingolog.org/



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-11-29 11:06                 ` Ludovic Courtès
  2011-11-29 11:35                   ` Andy Wingo
@ 2011-11-29 12:45                   ` Andy Wingo
  2011-11-29 13:18                     ` Ludovic Courtès
  1 sibling, 1 reply; 25+ messages in thread
From: Andy Wingo @ 2011-11-29 12:45 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel, Neil Jerram

On Tue 29 Nov 2011 12:06, ludo@gnu.org (Ludovic Courtès) writes:

> Could you run the stuff under gc-benchmarks/ with and without the
> heuristic, as in [0]?
>
> [0] http://thread.gmane.org/gmane.lisp.guile.devel/7803

Adding a FSD=2 entry:

   benchmark: `gc-benchmarks/gcbench.scm'
                          heap size (MiB)  execution time (s.)
   Guile                    53.17 (1.00x)      1.965 (1.00x)
   BDW-GC, FSD=2            55.76 (1.05x)      1.953 (0.99x) -
   BDW-GC, FSD=3            44.94 (0.85x)      2.017 (1.03x) ++
   BDW-GC, FSD=6            37.74 (0.71x)      2.361 (1.20x) ++++
   BDW-GC, FSD=9            37.77 (0.71x)      2.913 (1.48x) ++++
   BDW-GC, FSD=2 incr.      54.32 (1.02x)      2.049 (1.04x) 
   BDW-GC, FSD=3 incr.      51.21 (0.96x)      2.164 (1.10x) +
   BDW-GC, FSD=3 gene.      54.50 (1.02x)      2.131 (1.08x) 

   benchmark: `gc-benchmarks/string.scm'
                          heap size (MiB)  execution time (s.)
   Guile                   356.60 (1.00x)      0.564 (1.00x)
   BDW-GC, FSD=2           380.20 (1.07x)      0.498 (0.88x) -
   BDW-GC, FSD=3           265.62 (0.74x)      0.463 (0.82x) ++++
   BDW-GC, FSD=6           319.94 (0.90x)      0.485 (0.86x) ++
   BDW-GC, FSD=9           318.98 (0.89x)      0.489 (0.87x) ++
   BDW-GC, FSD=2 incr.     477.82 (1.34x)      0.624 (1.11x) -----
   BDW-GC, FSD=3 incr.     418.91 (1.17x)      0.643 (1.14x) ---
   BDW-GC, FSD=3 gene.     338.70 (0.95x)      0.615 (1.09x) +

Win!

Andy
-- 
http://wingolog.org/



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-11-29 12:45                   ` Andy Wingo
@ 2011-11-29 13:18                     ` Ludovic Courtès
  2011-11-29 14:44                       ` Andy Wingo
  0 siblings, 1 reply; 25+ messages in thread
From: Ludovic Courtès @ 2011-11-29 13:18 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guile-devel, Neil Jerram

Hey!

Andy Wingo <wingo@pobox.com> skribis:

> On Tue 29 Nov 2011 12:06, ludo@gnu.org (Ludovic Courtès) writes:
>
>> Could you run the stuff under gc-benchmarks/ with and without the
>> heuristic, as in [0]?
>>
>> [0] http://thread.gmane.org/gmane.lisp.guile.devel/7803

IIRC the “Guile” lines below correspond to whatever guile is in $PATH.
Do you know what that is in your case?

> Adding a FSD=2 entry:
>
>    benchmark: `gc-benchmarks/gcbench.scm'
>                           heap size (MiB)  execution time (s.)
>    Guile                    53.17 (1.00x)      1.965 (1.00x)
>    BDW-GC, FSD=2            55.76 (1.05x)      1.953 (0.99x) -
>    BDW-GC, FSD=3            44.94 (0.85x)      2.017 (1.03x) ++
>    BDW-GC, FSD=6            37.74 (0.71x)      2.361 (1.20x) ++++
>    BDW-GC, FSD=9            37.77 (0.71x)      2.913 (1.48x) ++++
>    BDW-GC, FSD=2 incr.      54.32 (1.02x)      2.049 (1.04x) 
>    BDW-GC, FSD=3 incr.      51.21 (0.96x)      2.164 (1.10x) +
>    BDW-GC, FSD=3 gene.      54.50 (1.02x)      2.131 (1.08x) 
>
>    benchmark: `gc-benchmarks/string.scm'
>                           heap size (MiB)  execution time (s.)
>    Guile                   356.60 (1.00x)      0.564 (1.00x)
>    BDW-GC, FSD=2           380.20 (1.07x)      0.498 (0.88x) -
>    BDW-GC, FSD=3           265.62 (0.74x)      0.463 (0.82x) ++++
>    BDW-GC, FSD=6           319.94 (0.90x)      0.485 (0.86x) ++
>    BDW-GC, FSD=9           318.98 (0.89x)      0.489 (0.87x) ++
>    BDW-GC, FSD=2 incr.     477.82 (1.34x)      0.624 (1.11x) -----
>    BDW-GC, FSD=3 incr.     418.91 (1.17x)      0.643 (1.14x) ---
>    BDW-GC, FSD=3 gene.     338.70 (0.95x)      0.615 (1.09x) +
>
> Win!

Yeah, looks pleasant!  :-)

The funny thing is that the initial FSD (which is what is shown here)
has an impact, although one might think the initial value doesn’t matter
much since adjust_gc_something changes it anyway.

The other interesting thing is that it seems to be noticeably better,
even on pure Scheme code–i.e., code that only does GC_malloc, not
malloc.

Thanks,
Ludo’.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-11-29 11:35                   ` Andy Wingo
@ 2011-11-29 13:20                     ` Ludovic Courtès
  0 siblings, 0 replies; 25+ messages in thread
From: Ludovic Courtès @ 2011-11-29 13:20 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guile-devel, Neil Jerram

Andy Wingo <wingo@pobox.com> skribis:

> On Tue 29 Nov 2011 12:06, ludo@gnu.org (Ludovic Courtès) writes:

[...]

>> I tried to reproduce the infamous iconv issue, which would have been a
>> simple way to check, but failed:
>>
>>   (with-fluids ((%default-port-encoding "UTF-16BE"))
>>     (let loop () (open-output-string ) (loop)))
>>
>> Memory growth appears to be bounded here.
>
> What do you mean?  Do you mean that this patch fixes the iconv issue, or
> that you couldn't reproduce it before?

The latter.

Thanks,
Ludo’.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-11-29 13:18                     ` Ludovic Courtès
@ 2011-11-29 14:44                       ` Andy Wingo
  2011-11-29 16:22                         ` Ludovic Courtès
  0 siblings, 1 reply; 25+ messages in thread
From: Andy Wingo @ 2011-11-29 14:44 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel, Neil Jerram

Greets,

On Tue 29 Nov 2011 14:18, ludo@gnu.org (Ludovic Courtès) writes:

> Andy Wingo <wingo@pobox.com> skribis:
>
>> On Tue 29 Nov 2011 12:06, ludo@gnu.org (Ludovic Courtès) writes:
>>
>>> Could you run the stuff under gc-benchmarks/ with and without the
>>> heuristic, as in [0]?
>>>
>>> [0] http://thread.gmane.org/gmane.lisp.guile.devel/7803
>
> IIRC the “Guile” lines below correspond to whatever guile is in $PATH.
> Do you know what that is in your case?

I fixed the environment to make it so that it was something post-2.0.3,
but before these heuristics.

> The funny thing is that the initial FSD (which is what is shown here)
> has an impact, although one might think the initial value doesn’t matter
> much since adjust_gc_something changes it anyway.

The FSD set in the env. var is the minimum FSD, so it does matter.

> The other interesting thing is that it seems to be noticeably better,
> even on pure Scheme code–i.e., code that only does GC_malloc, not
> malloc.

It could be that having a better baseline memory usage affects the
specific benchmarks being run.  Dunno tho.  Growing the image size can
also happen due to GC allocations though, so perhaps increasing
allocation in those times does also make sense.

Cheers,

Andy
-- 
http://wingolog.org/



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-11-29 14:44                       ` Andy Wingo
@ 2011-11-29 16:22                         ` Ludovic Courtès
  0 siblings, 0 replies; 25+ messages in thread
From: Ludovic Courtès @ 2011-11-29 16:22 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guile-devel, Neil Jerram

Howdy!

Andy Wingo <wingo@pobox.com> skribis:

> On Tue 29 Nov 2011 14:18, ludo@gnu.org (Ludovic Courtès) writes:
>
>> Andy Wingo <wingo@pobox.com> skribis:
>>
>>> On Tue 29 Nov 2011 12:06, ludo@gnu.org (Ludovic Courtès) writes:
>>>
>>>> Could you run the stuff under gc-benchmarks/ with and without the
>>>> heuristic, as in [0]?
>>>>
>>>> [0] http://thread.gmane.org/gmane.lisp.guile.devel/7803
>>
>> IIRC the “Guile” lines below correspond to whatever guile is in $PATH.
>> Do you know what that is in your case?
>
> I fixed the environment to make it so that it was something post-2.0.3,
> but before these heuristics.

OK.

>> The funny thing is that the initial FSD (which is what is shown here)
>> has an impact, although one might think the initial value doesn’t matter
>> much since adjust_gc_something changes it anyway.
>
> The FSD set in the env. var is the minimum FSD, so it does matter.

Oh, right.

>> The other interesting thing is that it seems to be noticeably better,
>> even on pure Scheme code–i.e., code that only does GC_malloc, not
>> malloc.
>
> It could be that having a better baseline memory usage affects the
> specific benchmarks being run.  Dunno tho.  Growing the image size can
> also happen due to GC allocations though, so perhaps increasing
> allocation in those times does also make sense.

Food for thought.  :-)

Thanks,
Ludo’.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-11-27 21:25         ` Ludovic Courtès
  2011-11-28 22:23           ` Andy Wingo
@ 2011-12-02 11:10           ` Andy Wingo
  2011-12-02 17:02             ` Ludovic Courtès
  1 sibling, 1 reply; 25+ messages in thread
From: Andy Wingo @ 2011-12-02 11:10 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel, Neil Jerram

On Sun 27 Nov 2011 22:25, ludo@gnu.org (Ludovic Courtès) writes:

> A longer term option may be to augment libgc with something akin to our
> old scm_gc_register_collectable_memory.

This is also necessary, as it turns out.  I added
scm_gc_register_allocation, which will simply run a GC every so often.

Currently the heuristic is that when GC runs, a counter is reset to be
equal to the current GC heap size.  scm_gc_register_allocation(size_t)
decrements this counter.  When it wraps around, we run GC.

I made scm_realloc call scm_gc_register_allocation.  I also installed
custom gmp allocators that call scm_malloc and friends -- so the same
allocators, but instrumented.  (Because we hard-coded `free' already in
the printer, we know that this shouldn't break anything.)

That makes the `(factorial 100000)' test take twice as long to run (6
seconds vs 3 seconds), because GC ran 1000 times instead of 15 times,
but it kept the memory image size to 18 MB instead of 1800 MB.

Andy
-- 
http://wingolog.org/



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-12-02 11:10           ` Andy Wingo
@ 2011-12-02 17:02             ` Ludovic Courtès
  2011-12-02 18:12               ` Andy Wingo
  0 siblings, 1 reply; 25+ messages in thread
From: Ludovic Courtès @ 2011-12-02 17:02 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guile-devel, Neil Jerram

Hello!

Andy Wingo <wingo@pobox.com> skribis:

> On Sun 27 Nov 2011 22:25, ludo@gnu.org (Ludovic Courtès) writes:
>
>> A longer term option may be to augment libgc with something akin to our
>> old scm_gc_register_collectable_memory.
>
> This is also necessary, as it turns out.  I added
> scm_gc_register_allocation, which will simply run a GC every so often.
>
> Currently the heuristic is that when GC runs, a counter is reset to be
> equal to the current GC heap size.  scm_gc_register_allocation(size_t)
> decrements this counter.  When it wraps around, we run GC.

OK, sounds good.  I guess scm_gc_register_collectable_memory could be
changed to just call it, ignoring its first argument?

> I made scm_realloc call scm_gc_register_allocation.  I also installed
> custom gmp allocators that call scm_malloc and friends

I was about to say “we can’t do that in 2.0!”, but then saw your
scm_install_gmp_memory_functions trick.  Cool!  :-)

Could you make it SCM_INTERNAL instead of SCM_API?

> That makes the `(factorial 100000)' test take twice as long to run (6
> seconds vs 3 seconds), because GC ran 1000 times instead of 15 times,
> but it kept the memory image size to 18 MB instead of 1800 MB.

Could you check how it affects gc-benchmarks/?  :-)

Thank you for being both an efficient and a brave hacker!

Ludo’.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-12-02 17:02             ` Ludovic Courtès
@ 2011-12-02 18:12               ` Andy Wingo
  2011-12-02 22:17                 ` Ludovic Courtès
  0 siblings, 1 reply; 25+ messages in thread
From: Andy Wingo @ 2011-12-02 18:12 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Neil Jerram, guile-devel

Heya,

On Fri 02 Dec 2011 18:02, ludo@gnu.org (Ludovic Courtès) writes:

> I guess scm_gc_register_collectable_memory could be
> changed to just call it, ignoring its first argument?

Done.

>> I made scm_realloc call scm_gc_register_allocation.  I also installed
>> custom gmp allocators that call scm_malloc and friends
>
> I was about to say “we can’t do that in 2.0!”, but then saw your
> scm_install_gmp_memory_functions trick.  Cool!  :-)

Did you also see that we used to^H^H still `free' the memory returned
from mpz_get_str?  That means that in practice, since the 1.8 days we
did not support other allocators for mp_memory_functions.  For that
reason I defaulted scm_install_gmp_memory_functions to 1.  I will fix
the other instance of scm_take_locale_string.

> Could you make it SCM_INTERNAL instead of SCM_API?

Sure, but you don't want to allow users to set it?

> Could you check how it affects gc-benchmarks/?  :-)

Are there tests for numbers there?  This change would only affect that,
I think.

Cheers,

Andy
-- 
http://wingolog.org/



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-12-02 18:12               ` Andy Wingo
@ 2011-12-02 22:17                 ` Ludovic Courtès
  2011-12-03 10:57                   ` Andy Wingo
  0 siblings, 1 reply; 25+ messages in thread
From: Ludovic Courtès @ 2011-12-02 22:17 UTC (permalink / raw)
  To: Andy Wingo; +Cc: Neil Jerram, guile-devel

Hello!

Andy Wingo <wingo@pobox.com> skribis:

> On Fri 02 Dec 2011 18:02, ludo@gnu.org (Ludovic Courtès) writes:
>
>> I guess scm_gc_register_collectable_memory could be
>> changed to just call it, ignoring its first argument?
>
> Done.

Thanks!

>>> I made scm_realloc call scm_gc_register_allocation.  I also installed
>>> custom gmp allocators that call scm_malloc and friends
>>
>> I was about to say “we can’t do that in 2.0!”, but then saw your
>> scm_install_gmp_memory_functions trick.  Cool!  :-)
>
> Did you also see that we used to^H^H still `free' the memory returned
> from mpz_get_str?  That means that in practice, since the 1.8 days we
> did not support other allocators for mp_memory_functions.

Oh, OK.

> For that reason I defaulted scm_install_gmp_memory_functions to 1.

Yes, makes sense.

>> Could you make it SCM_INTERNAL instead of SCM_API?
>
> Sure, but you don't want to allow users to set it?

I’d say no, because that will fail gracelessly if it gets set or cleared
in the middle of a run, won’t it?

>> Could you check how it affects gc-benchmarks/?  :-)
>
> Are there tests for numbers there?

I think so, but I’m not sure.  :-)

(There are some details at <http://www.ccs.neu.edu/home/will/GC/sourcecode.html>.)

Thanks,
Ludo’.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-12-02 22:17                 ` Ludovic Courtès
@ 2011-12-03 10:57                   ` Andy Wingo
  2011-12-03 17:57                     ` Ludovic Courtès
  0 siblings, 1 reply; 25+ messages in thread
From: Andy Wingo @ 2011-12-03 10:57 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Neil Jerram, guile-devel

On Fri 02 Dec 2011 23:17, ludo@gnu.org (Ludovic Courtès) writes:

>> For that reason I defaulted scm_install_gmp_memory_functions to 1.
>
>>> Could you make it SCM_INTERNAL instead of SCM_API?
>>
>> Sure, but you don't want to allow users to set it?
>
> I’d say no, because that will fail gracelessly if it gets set or cleared
> in the middle of a run, won’t it?

To use it, you have to set it before scm_init_numbers() runs, as that is
the only place it is checked.  So it should be OK.  We could define a
more procedural interface, if that's useful, but I punted.

Andy
-- 
http://wingolog.org/



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers
  2011-12-03 10:57                   ` Andy Wingo
@ 2011-12-03 17:57                     ` Ludovic Courtès
  0 siblings, 0 replies; 25+ messages in thread
From: Ludovic Courtès @ 2011-12-03 17:57 UTC (permalink / raw)
  To: Andy Wingo; +Cc: Neil Jerram, guile-devel

Andy Wingo <wingo@pobox.com> skribis:

> On Fri 02 Dec 2011 23:17, ludo@gnu.org (Ludovic Courtès) writes:
>
>>> For that reason I defaulted scm_install_gmp_memory_functions to 1.
>>
>>>> Could you make it SCM_INTERNAL instead of SCM_API?
>>>
>>> Sure, but you don't want to allow users to set it?
>>
>> I’d say no, because that will fail gracelessly if it gets set or cleared
>> in the middle of a run, won’t it?
>
> To use it, you have to set it before scm_init_numbers() runs, as that is
> the only place it is checked.

OK, it doesn’t hurt to leave it public, then.

Thanks,
Ludo’.



^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2011-12-03 17:57 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-31 19:48 [PATCH] Configure GMP to use GC allocation functions, remove bignum finalizers Mark H Weaver
2011-05-31 20:07 ` Mark H Weaver
2011-05-31 21:52 ` Ludovic Courtès
2011-05-31 23:18   ` Neil Jerram
2011-05-31 23:32     ` Neil Jerram
2011-06-01  8:34     ` Andy Wingo
2011-06-02 23:14       ` Ludovic Courtès
2011-11-27 19:33       ` Andy Wingo
2011-11-27 21:25         ` Ludovic Courtès
2011-11-28 22:23           ` Andy Wingo
2011-11-28 23:10             ` Ludovic Courtès
2011-11-28 23:50               ` Andy Wingo
2011-11-29 11:06                 ` Ludovic Courtès
2011-11-29 11:35                   ` Andy Wingo
2011-11-29 13:20                     ` Ludovic Courtès
2011-11-29 12:45                   ` Andy Wingo
2011-11-29 13:18                     ` Ludovic Courtès
2011-11-29 14:44                       ` Andy Wingo
2011-11-29 16:22                         ` Ludovic Courtès
2011-12-02 11:10           ` Andy Wingo
2011-12-02 17:02             ` Ludovic Courtès
2011-12-02 18:12               ` Andy Wingo
2011-12-02 22:17                 ` Ludovic Courtès
2011-12-03 10:57                   ` Andy Wingo
2011-12-03 17:57                     ` Ludovic Courtès

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).