[RFC,PATCH] Do not GC_INIT the Boehm GC if already initialized

[RFC,PATCH] Do not GC_INIT the Boehm GC if already initialized

[Jose E. Marchesi]

Hello Guile hackers.

We are in the process of integrating GNU poke[1] in GDB by mean of
libpoke.

Problem is, libpoke uses the Boehm GC, as guile does.  We are working on
switching to an ad-hoc exact collector, but it will get some time.

So, in the interim, we may:

1) Make both libguile and libpoke to do GC_INIT conditionally, only if
   no one else has initialized the collector before.  This is already in
   poke master.  A suggested (untested!) patch for guile below.

2) Test to see if the collector works properly.

3) Do bug-fix releases of both libguile and libpoke so the GDB people
   can check for the right version in configure.ac in order to allow
   configuring GDB with both libpoke and libguile support.

Does this sound like a plan?
Thanks in advance!

[1] https://jemarch.net/poke

diff --git a/libguile/gc.c b/libguile/gc.c
index 7717e9bef..36653d373 100644
--- a/libguile/gc.c
+++ b/libguile/gc.c
@@ -462,7 +462,8 @@ scm_storage_prehistory ()
   setenv ("GC_MARKERS", "1", 1);
 #endif
 
-  GC_INIT ();
+  if(!GC_is_init_called ())
+    GC_INIT ();
 
   size_t heap_size = GC_get_heap_size ();
   if (heap_size < DEFAULT_INITIAL_HEAP_SIZE)

Re: [RFC,PATCH] Do not GC_INIT the Boehm GC if already initialized

[Jean Abou Samra]

[-- Attachment #1.1: Type: text/plain, Size: 1670 bytes --]

On 06/02/2023 19:34, Jose E. Marchesi wrote:
> 
> Hello Guile hackers.
> 
> We are in the process of integrating GNU poke[1] in GDB by mean of
> libpoke.
> 
> Problem is, libpoke uses the Boehm GC, as guile does.  We are working on
> switching to an ad-hoc exact collector, but it will get some time.
> 
> So, in the interim, we may:
> 
> 1) Make both libguile and libpoke to do GC_INIT conditionally, only if
>    no one else has initialized the collector before.  This is already in
>    poke master.  A suggested (untested!) patch for guile below.
> 
> 2) Test to see if the collector works properly.
> 
> 3) Do bug-fix releases of both libguile and libpoke so the GDB people
>    can check for the right version in configure.ac in order to allow
>    configuring GDB with both libpoke and libguile support.
> 
> Does this sound like a plan?
> Thanks in advance!
> 
> [1] https://jemarch.net/poke
> 
> diff --git a/libguile/gc.c b/libguile/gc.c
> index 7717e9bef..36653d373 100644
> --- a/libguile/gc.c
> +++ b/libguile/gc.c
> @@ -462,7 +462,8 @@ scm_storage_prehistory ()
>    setenv ("GC_MARKERS", "1", 1);
>  #endif
>  
> -  GC_INIT ();
> +  if(!GC_is_init_called ())
> +    GC_INIT ();
>  
>    size_t heap_size = GC_get_heap_size ();
>    if (heap_size < DEFAULT_INITIAL_HEAP_SIZE)
> 



Isn't that going to cause problems if the configurations clash?
For example, the scm_storage_prehistory () function that you
are modifying does

GC_set_all_interior_pointers (0);

If the application that previously initialized the GC relies
on interior pointer tracing, Guile disabling it is going to
cause trouble.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [RFC,PATCH] Do not GC_INIT the Boehm GC if already initialized

[Jose E. Marchesi]

> On 06/02/2023 19:34, Jose E. Marchesi wrote:
>> 
>> Hello Guile hackers.
>> 
>> We are in the process of integrating GNU poke[1] in GDB by mean of
>> libpoke.
>> 
>> Problem is, libpoke uses the Boehm GC, as guile does.  We are working on
>> switching to an ad-hoc exact collector, but it will get some time.
>> 
>> So, in the interim, we may:
>> 
>> 1) Make both libguile and libpoke to do GC_INIT conditionally, only if
>>    no one else has initialized the collector before.  This is already in
>>    poke master.  A suggested (untested!) patch for guile below.
>> 
>> 2) Test to see if the collector works properly.
>> 
>> 3) Do bug-fix releases of both libguile and libpoke so the GDB people
>>    can check for the right version in configure.ac in order to allow
>>    configuring GDB with both libpoke and libguile support.
>> 
>> Does this sound like a plan?
>> Thanks in advance!
>> 
>> [1] https://jemarch.net/poke
>> 
>> diff --git a/libguile/gc.c b/libguile/gc.c
>> index 7717e9bef..36653d373 100644
>> --- a/libguile/gc.c
>> +++ b/libguile/gc.c
>> @@ -462,7 +462,8 @@ scm_storage_prehistory ()
>>    setenv ("GC_MARKERS", "1", 1);
>>  #endif
>>  
>> -  GC_INIT ();
>> +  if(!GC_is_init_called ())
>> +    GC_INIT ();
>>  
>>    size_t heap_size = GC_get_heap_size ();
>>    if (heap_size < DEFAULT_INITIAL_HEAP_SIZE)
>> 
>
>
>
> Isn't that going to cause problems if the configurations clash?
> For example, the scm_storage_prehistory () function that you
> are modifying does
>
> GC_set_all_interior_pointers (0);
>
> If the application that previously initialized the GC relies
> on interior pointer tracing, Guile disabling it is going to
> cause trouble.

Indeed, we would need to agree on these details and come with a
configuration that works with both configurations.

Re: [RFC,PATCH] Do not GC_INIT the Boehm GC if already initialized

[Maxime Devos]

[-- Attachment #1.1.1: Type: text/plain, Size: 1033 bytes --]


 > [RFC,PATCH] Do not GC_INIT the Boehm GC if already initialized
On 06-02-2023 19:34, Jose E. Marchesi wrote:
> 
> Hello Guile hackers.
> 
> We are in the process of integrating GNU poke[1] in GDB by mean of
> libpoke.
> 
> Problem is, libpoke uses the Boehm GC, as guile does.  We are working on
> switching to an ad-hoc exact collector, but it will get some time.
> 
> So, in the interim, we may:
> 
> 1) Make both libguile and libpoke to do GC_INIT conditionally, only if
>     no one else has initialized the collector before.  This is already in
>     poke master.  A suggested (untested!) patch for guile below. > [...]

According to the Boehm GC documentation, this 'conditional 
initialisation' is unnecessary:

/* Portable clients should call this at the program start-up.  More   */
/* over, some platforms require this call to be done strictly from the*/
/* primordial thread.  **Multiple invocations are harmless.**         */
#define GC_INIT() [...]

(emphasis added).

Greetings,
Maxime

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [RFC,PATCH] Do not GC_INIT the Boehm GC if already initialized

[Ludovic Courtès]

Hi Maxime,

Maxime Devos <maximedevos@telenet.be> skribis:

>> [RFC,PATCH] Do not GC_INIT the Boehm GC if already initialized
> On 06-02-2023 19:34, Jose E. Marchesi wrote:
>> Hello Guile hackers.
>> We are in the process of integrating GNU poke[1] in GDB by mean of
>> libpoke.
>> Problem is, libpoke uses the Boehm GC, as guile does.  We are
>> working on
>> switching to an ad-hoc exact collector, but it will get some time.
>> So, in the interim, we may:
>> 1) Make both libguile and libpoke to do GC_INIT conditionally, only
>> if
>>     no one else has initialized the collector before.  This is already in
>>     poke master.  A suggested (untested!) patch for guile below. > [...]
>
> According to the Boehm GC documentation, this 'conditional
> initialisation' is unnecessary:
>
> /* Portable clients should call this at the program start-up.  More   */
> /* over, some platforms require this call to be done strictly from the*/
> /* primordial thread.  **Multiple invocations are harmless.**         */
> #define GC_INIT() [...]
>
> (emphasis added).

The “Multiple invocations” bit isn’t in libgc 8.0.4.  Which version are
you looking at?

Thanks,
Ludo’.

Re: [RFC,PATCH] Do not GC_INIT the Boehm GC if already initialized

[Maxime Devos]

[-- Attachment #1.1.1: Type: text/plain, Size: 512 bytes --]



On 20-02-2023 11:06, Ludovic Courtès wrote:
> [...]
> The “Multiple invocations” bit isn’t in libgc 8.0.4.  Which version are
> you looking at?

Commit 151b49a5302eea89ffd9efd9cdb82b75ac4f5d35 (include/gc/gc.h).
Looking at the changelog, that part is there since at least 8.0.6:

[...]
* Refine GC_INIT documentation about its multiple invocation

As it only a documentation change (going by the changelog), possibly the 
same behaviour existed in libgc 8.0.4 too.

Greetings,
Maxime.

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [RFC,PATCH] Do not GC_INIT the Boehm GC if already initialized

[Hans Åberg]

> On 20 Feb 2023, at 11:06, Ludovic Courtès <ludo@gnu.org> wrote:
> 
>> According to the Boehm GC documentation, this 'conditional
>> initialisation' is unnecessary:
>> 
>> /* Portable clients should call this at the program start-up.  More   */
>> /* over, some platforms require this call to be done strictly from the*/
>> /* primordial thread.  **Multiple invocations are harmless.**         */
>> #define GC_INIT() [...]
>> 
>> (emphasis added).
> 
> The “Multiple invocations” bit isn’t in libgc 8.0.4.  Which version are
> you looking at?

It is in MacPorts boehmgc @8.2.2 (devel), which Guile lists as a dependency.