unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
* GC + Java finalization
@ 2021-07-03 12:05 Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library
  2021-07-03 17:14 ` Maxime Devos
  2021-07-15 18:44 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library
  0 siblings, 2 replies; 6+ messages in thread
From: Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library @ 2021-07-03 12:05 UTC (permalink / raw)
  To: guile-devel


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

Hi Guile devs,

I'm hacking on GNU LilyPond and recently wondered if Guile could run
without Java finalization that prevents collection of chains of
unreachable objects. I found that the functionality is only needed once
the first guardian is created, so it's possible to delay enabling the
mode until then. This required some fixes to free functions that
assumed dependent objects to be freed only later (see first two
patches).
The third patch delays ensuring Java finalization to scm_make_guardian,
but doesn't disable it explicitly (it's on by default in bdwgc). This
could now be done right after GC_INIT(), but it's not clear (at least
to me) whether client applications actually rely it, so I think it's
better if that's not done in Guile itself.

Please consider applying, the fixes potentially also to stable-2.2.

Thanks
Jonas

[-- Attachment #1.2: 0001-Fix-test-smob-mark.patch --]
[-- Type: text/x-patch, Size: 1854 bytes --]

From c28444893ab14bd35381c7b60545fd248cc7d877 Mon Sep 17 00:00:00 2001
From: Jonas Hahnfeld <hahnjo@hahnjo.de>
Date: Fri, 2 Jul 2021 23:52:13 +0200
Subject: [PATCH 1/3] Fix test-smob-mark

* test-suite/standalone/test-smob-mark.c
  (init_smob_type): Correct size of smob type.
  (free_x): Clear smob data instead of local variable.
  (test_scm_smob_mark): Put smobs in array to ensure marking.
---
 test-suite/standalone/test-smob-mark.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/test-suite/standalone/test-smob-mark.c b/test-suite/standalone/test-smob-mark.c
index d0bb5336a..ee7e02029 100644
--- a/test-suite/standalone/test-smob-mark.c
+++ b/test-suite/standalone/test-smob-mark.c
@@ -79,7 +79,7 @@ free_x (SCM x)
   x_t *c_x;
   c_x = (x_t *) SCM_SMOB_DATA (x);
   scm_gc_free (c_x, sizeof (x_t), "x");
-  c_x = NULL;
+  SCM_SET_SMOB_DATA (x, NULL);
   return 0;
 }
 
@@ -100,14 +100,17 @@ print_x (SCM x, SCM port, scm_print_state * pstate SCM_UNUSED)
 static void
 test_scm_smob_mark ()
 {
+  SCM *smobs = scm_gc_malloc (sizeof(SCM) * SMOBS_COUNT, "smobs");
+
   int i;
   mark_call_count = 0;
   for (i = 0; i < SMOBS_COUNT; i++)
-    make_x ();
+    smobs[i] = make_x ();
   scm_gc ();
   if (mark_call_count < SMOBS_COUNT)
     {
-      fprintf (stderr, "FAIL: SMOB mark function called for each SMOB\n");
+      // Print pointer so it cannot be collected before.
+      fprintf (stderr, "FAIL: SMOB mark function called for each SMOB (smobs = %p)\n", smobs);
       exit (EXIT_FAILURE);
     }
 }
@@ -115,7 +118,7 @@ test_scm_smob_mark ()
 static void
 init_smob_type ()
 {
-  x_tag = scm_make_smob_type ("x", sizeof (x_t));
+  x_tag = scm_make_smob_type ("x", sizeof (x_t *));
   scm_set_smob_free (x_tag, free_x);
   scm_set_smob_print (x_tag, print_x);
   scm_set_smob_mark (x_tag, mark_x);
-- 
2.32.0


[-- Attachment #1.3: 0002-Avoid-matching-calls-of-scm_gc_free.patch --]
[-- Type: text/x-patch, Size: 3938 bytes --]

From 7127986755f4500c0b000f769009d1240d163468 Mon Sep 17 00:00:00 2001
From: Jonas Hahnfeld <hahnjo@hahnjo.de>
Date: Fri, 2 Jul 2021 23:03:17 +0200
Subject: [PATCH 2/3] Avoid matching calls of scm_gc_free

There is no point in registering memory with the garbage collector
if it doesn't need to do its job. In fact, calling scm_gc_free in
a free function is wrong because it relies on Java finalization.

* libguile/random.c (scm_c_random_bignum): Replace matching calls of
  scm_gc_calloc and scm_gc_free.
* libguile/regex-posix.c (scm_make_regexp, regex_free): Avoid call
  of scm_gc_free in free function.
* test-suite/standalone/test-smob-mark.c (make_x, free_x): Avoid
  call of scm_gc_free in free function.
---
 THANKS                                 | 1 +
 libguile/random.c                      | 8 ++------
 libguile/regex-posix.c                 | 6 +++---
 test-suite/standalone/test-smob-mark.c | 4 ++--
 4 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/THANKS b/THANKS
index aa4877e95..786b65d1a 100644
--- a/THANKS
+++ b/THANKS
@@ -13,6 +13,7 @@ Contributors since the last release:
          Volker Grabsch
          Julian Graham
         Michael Gran
+          Jonas Hahnfeld
          Daniel Hartwig
              No Itisnt
            Neil Jerram
diff --git a/libguile/random.c b/libguile/random.c
index 63da7f5d6..ac400a9fd 100644
--- a/libguile/random.c
+++ b/libguile/random.c
@@ -324,9 +324,7 @@ scm_c_random_bignum (scm_t_rstate *state, SCM m)
   /* we know the result will be this big */
   mpz_realloc2 (SCM_I_BIG_MPZ (result), m_bits);
 
-  random_chunks =
-    (uint32_t *) scm_gc_calloc (num_chunks * sizeof (uint32_t),
-                                     "random bignum chunks");
+  random_chunks = (uint32_t *) scm_calloc (num_chunks * sizeof (uint32_t));
 
   do
     {
@@ -363,9 +361,7 @@ scm_c_random_bignum (scm_t_rstate *state, SCM m)
       /* if result >= m, regenerate it (it is important to regenerate
 	 all bits in order not to get a distorted distribution) */
     } while (mpz_cmp (SCM_I_BIG_MPZ (result), SCM_I_BIG_MPZ (m)) >= 0);
-  scm_gc_free (random_chunks,
-               num_chunks * sizeof (uint32_t),
-               "random bignum chunks");
+  free (random_chunks);
   return scm_i_normbig (result);
 }
 
diff --git a/libguile/regex-posix.c b/libguile/regex-posix.c
index a08da02db..36bb639e0 100644
--- a/libguile/regex-posix.c
+++ b/libguile/regex-posix.c
@@ -67,7 +67,7 @@ static size_t
 regex_free (SCM obj)
 {
   regfree (SCM_RGX (obj));
-  scm_gc_free (SCM_RGX (obj), sizeof(regex_t), "regex");
+  free (SCM_RGX (obj));
   return 0;
 }
 
@@ -164,7 +164,7 @@ SCM_DEFINE (scm_make_regexp, "make-regexp", 1, 0, 1,
       flag = SCM_CDR (flag);
     }
 
-  rx = scm_gc_malloc_pointerless (sizeof (regex_t), "regex");
+  rx = scm_malloc (sizeof (regex_t));
   c_pat = scm_to_locale_string (pat);
   status = regcomp (rx, c_pat,
 		    /* Make sure they're not passing REG_NOSUB;
@@ -174,7 +174,7 @@ SCM_DEFINE (scm_make_regexp, "make-regexp", 1, 0, 1,
   if (status != 0)
     {
       SCM errmsg = scm_regexp_error_msg (status, rx);
-      scm_gc_free (rx, sizeof(regex_t), "regex");
+      free (rx);
       scm_error_scm (scm_regexp_error_key,
 		     scm_from_utf8_string (FUNC_NAME),
 		     errmsg,
diff --git a/test-suite/standalone/test-smob-mark.c b/test-suite/standalone/test-smob-mark.c
index ee7e02029..d7995d6f4 100644
--- a/test-suite/standalone/test-smob-mark.c
+++ b/test-suite/standalone/test-smob-mark.c
@@ -56,7 +56,7 @@ make_x ()
   x_t *c_x;
 
   i++;
-  c_x = (x_t *) scm_gc_malloc (sizeof (x_t), "x");
+  c_x = (x_t *) scm_malloc (sizeof (x_t));
   c_x->scm_value = scm_from_int (i);
   c_x->c_value = i;
   SCM_NEWSMOB (s_x, x_tag, c_x);
@@ -78,7 +78,7 @@ free_x (SCM x)
 {
   x_t *c_x;
   c_x = (x_t *) SCM_SMOB_DATA (x);
-  scm_gc_free (c_x, sizeof (x_t), "x");
+  free (c_x);
   SCM_SET_SMOB_DATA (x, NULL);
   return 0;
 }
-- 
2.32.0


[-- Attachment #1.4: 0003-Delay-Java-finalization-for-guardians.patch --]
[-- Type: text/x-patch, Size: 1695 bytes --]

From 02204f9a57e6608a23db6944f9176059966f9fd6 Mon Sep 17 00:00:00 2001
From: Jonas Hahnfeld <hahnjo@hahnjo.de>
Date: Fri, 2 Jul 2021 23:16:32 +0200
Subject: [PATCH 3/3] Delay Java finalization for guardians

It is only needed once the first guardian is created.

* libguile/guardians.c (scm_make_guardian): Move call to enable
  Java finalization from scm_init_guardians.
---
 libguile/guardians.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/libguile/guardians.c b/libguile/guardians.c
index fa8c8b8f7..357c8dbd1 100644
--- a/libguile/guardians.c
+++ b/libguile/guardians.c
@@ -306,6 +306,8 @@ guardian_apply (SCM guardian, SCM obj, SCM throw_p)
     return scm_i_get_one_zombie (guardian);
 }
 
+static scm_i_pthread_mutex_t gc_java_mutex;
+
 SCM_DEFINE (scm_make_guardian, "make-guardian", 0, 0, 0, 
 	    (),
 "Create a new guardian.  A guardian protects a set of objects from\n"
@@ -349,6 +351,11 @@ SCM_DEFINE (scm_make_guardian, "make-guardian", 0, 0, 0,
 	    "value, it is eligible to be returned from a guardian.\n")
 #define FUNC_NAME s_scm_make_guardian
 {
+  /* For guardians, we use unordered finalization `a la Java. */
+  scm_i_pthread_mutex_lock (&gc_java_mutex);
+  GC_set_java_finalization (1);
+  scm_i_pthread_mutex_unlock (&gc_java_mutex);
+
   t_guardian *g = scm_gc_malloc (sizeof (t_guardian), "guardian");
   SCM z;
 
@@ -369,8 +376,7 @@ SCM_DEFINE (scm_make_guardian, "make-guardian", 0, 0, 0,
 void
 scm_init_guardians ()
 {
-  /* We use unordered finalization `a la Java.  */
-  GC_set_java_finalization (1);
+  scm_i_pthread_mutex_init (&gc_java_mutex, NULL);
 
   tc16_guardian = scm_make_smob_type ("guardian", 0);
 
-- 
2.32.0


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: GC + Java finalization
  2021-07-03 12:05 GC + Java finalization Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library
@ 2021-07-03 17:14 ` Maxime Devos
  2021-07-03 17:26   ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library
  2021-07-15 18:44 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library
  1 sibling, 1 reply; 6+ messages in thread
From: Maxime Devos @ 2021-07-03 17:14 UTC (permalink / raw)
  To: Jonas Hahnfeld, guile-devel

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

Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library schreef op za 03-07-2021 om 14:05 [+0200]:
> Hi Guile devs,
> 

Hi, I'm not really a Guile dev but I'll respond anyway.

> I'm hacking on GNU LilyPond and recently wondered if Guile could run
> without Java finalization that prevents collection of chains of
> unreachable objects.

Do you have an example where this is a problem?
I.e., did you encounter ‘chains of unreachable objects’ that were
uncollectable, and so, where?

> I found that the functionality is only needed once
> the first guardian is created, so it's possible to delay enabling the
> mode until then. This required some fixes to free functions that
> assumed dependent objects to be freed only later (see first two
> patches).
> The third patch delays ensuring Java finalization to scm_make_guardian,
> but doesn't disable it explicitly (it's on by default in bdwgc). This
> could now be done right after GC_INIT(), but it's not clear (at least
> to me) whether client applications actually rely it, so I think it's
> better if that's not done in Guile itself.
> 
> Please consider applying, the fixes potentially also to stable-2.2.
> 
> Thanks
> Jonas

I would need to look more closely at how ‘Java-style’ finalisation
works. Some comments anyway:

(first patch)

> * test-suite/standalone/test-smob-mark.c
>   (init_smob_type): Correct size of smob type.
>   (free_x): Clear smob data instead of local variable.
>   (test_scm_smob_mark): Put smobs in array to ensure marking.
>
> -      fprintf (stderr, "FAIL: SMOB mark function called for each SMOB\n");
> +      // Print pointer so it cannot be collected before.
> +      fprintf (stderr, "FAIL: SMOB mark function called for each SMOB (smobs = %p)\n", smobs);
>        exit (EXIT_FAILURE);

Normally scm_remember_upto_here is used for that.
Also, I believe "/* */"-style comments are used customarily used in Guile
instead of "//"-style comments.

> static void
> init_smob_type ()
> {
> -  x_tag = scm_make_smob_type ("x", sizeof (x_t));
> +  x_tag = scm_make_smob_type ("x", sizeof (x_t *));

This change seems to be a fix independent of the ‘do we want Java-style finalization’
question.

(third patch)

Note that guardians are used in (ice-9 popen).
They are also used by some guile libraries (e.g. guile-fibers),
so you can't use (ice-9 popen) or any library using guardians
if Java-style finalization is undesirable.

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

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

* Re: GC + Java finalization
  2021-07-03 17:14 ` Maxime Devos
@ 2021-07-03 17:26   ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library
  2021-07-03 18:49     ` Maxime Devos
  2021-07-03 18:54     ` Maxime Devos
  0 siblings, 2 replies; 6+ messages in thread
From: Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library @ 2021-07-03 17:26 UTC (permalink / raw)
  To: Maxime Devos, guile-devel

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

Am Samstag, dem 03.07.2021 um 19:14 +0200 schrieb Maxime Devos:
> Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library schreef op za 03-07-2021 om 14:05 [+0200]:
> > Hi Guile devs,
> > 
> 
> Hi, I'm not really a Guile dev but I'll respond anyway.
> 
> > I'm hacking on GNU LilyPond and recently wondered if Guile could run
> > without Java finalization that prevents collection of chains of
> > unreachable objects.
> 
> Do you have an example where this is a problem?
> I.e., did you encounter ‘chains of unreachable objects’ that were
> uncollectable, and so, where?

Sorry, I should have been clearer: Chains don't become uncollectable,
but a chain of N objects takes N collections to be completely reclaimed
(because Java finalization prepares for the possibility that a free
function makes an object live again, as Guile does for guardians). This
leads to unnecessary waste on the heap, and more work for the collector
(even though I haven't been able to measure so far).

> 
> > I found that the functionality is only needed once
> > the first guardian is created, so it's possible to delay enabling the
> > mode until then. This required some fixes to free functions that
> > assumed dependent objects to be freed only later (see first two
> > patches).
> > The third patch delays ensuring Java finalization to scm_make_guardian,
> > but doesn't disable it explicitly (it's on by default in bdwgc). This
> > could now be done right after GC_INIT(), but it's not clear (at least
> > to me) whether client applications actually rely it, so I think it's
> > better if that's not done in Guile itself.
> > 
> > Please consider applying, the fixes potentially also to stable-2.2.
> > 
> > Thanks
> > Jonas
> 
> I would need to look more closely at how ‘Java-style’ finalisation
> works. Some comments anyway:
> 
> (first patch)
> 
> > * test-suite/standalone/test-smob-mark.c
> >   (init_smob_type): Correct size of smob type.
> >   (free_x): Clear smob data instead of local variable.
> >   (test_scm_smob_mark): Put smobs in array to ensure marking.
> > 
> > -      fprintf (stderr, "FAIL: SMOB mark function called for each SMOB\n");
> > +      // Print pointer so it cannot be collected before.
> > +      fprintf (stderr, "FAIL: SMOB mark function called for each SMOB (smobs = %p)\n", smobs);
> >        exit (EXIT_FAILURE);
> 
> Normally scm_remember_upto_here is used for that.

I think I tried, but it wasn't available. Or I mistyped, not sure.

> Also, I believe "/* */"-style comments are used customarily used in Guile
> instead of "//"-style comments.
> 
> > static void
> > init_smob_type ()
> > {
> > -  x_tag = scm_make_smob_type ("x", sizeof (x_t));
> > +  x_tag = scm_make_smob_type ("x", sizeof (x_t *));
> 
> This change seems to be a fix independent of the ‘do we want Java-style finalization’
> question.

Yes, that's why it's a separate patch.

> 
> (third patch)
> 
> Note that guardians are used in (ice-9 popen).
> They are also used by some guile libraries (e.g. guile-fibers),
> so you can't use (ice-9 popen) or any library using guardians
> if Java-style finalization is undesirable.

Yes, I'm aware and that's why any constructed guardian force-enables
Java finalization. But applications might not use it, and at least for
LilyPond I'm sure it's not used.

Jonas

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: GC + Java finalization
  2021-07-03 17:26   ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library
@ 2021-07-03 18:49     ` Maxime Devos
  2021-07-03 18:54     ` Maxime Devos
  1 sibling, 0 replies; 6+ messages in thread
From: Maxime Devos @ 2021-07-03 18:49 UTC (permalink / raw)
  To: Jonas Hahnfeld, guile-devel

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

Jonas Hahnfeld schreef op za 03-07-2021 om 19:26 [+0200]:
> Sorry, I should have been clearer: Chains don't become uncollectable,
> but a chain of N objects takes N collections to be completely reclaimed
> (because Java finalization prepares for the possibility that a free
> function makes an object live again, as Guile does for guardians). This
> leads to unnecessary waste on the heap, and more work for the collector
> (even though I haven't been able to measure so far).

Disabling Java-style finalization would be an optimisation, ok!

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

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

* Re: GC + Java finalization
  2021-07-03 17:26   ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library
  2021-07-03 18:49     ` Maxime Devos
@ 2021-07-03 18:54     ` Maxime Devos
  1 sibling, 0 replies; 6+ messages in thread
From: Maxime Devos @ 2021-07-03 18:54 UTC (permalink / raw)
  To: Jonas Hahnfeld; +Cc: guile-devel

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

Jonas Hahnfeld schreef op za 03-07-2021 om 19:26 [+0200]:

> > Normally scm_remember_upto_here is used for that.
> 
> I think I tried, but it wasn't available. Or I mistyped, not sure.

It is defined in <libguile/gc.h>.

Actually, the special case scm_remember_upto_here_1 would be prefered,
because it theoretically is more efficient.

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

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

* Re: GC + Java finalization
  2021-07-03 12:05 GC + Java finalization Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library
  2021-07-03 17:14 ` Maxime Devos
@ 2021-07-15 18:44 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library
  1 sibling, 0 replies; 6+ messages in thread
From: Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library @ 2021-07-15 18:44 UTC (permalink / raw)
  To: guile-devel; +Cc: Maxime Devos


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

Am Samstag, dem 03.07.2021 um 14:05 +0200 schrieb Jonas Hahnfeld via
Developers list for Guile, the GNU extensibility library:
> Hi Guile devs,
> 
> I'm hacking on GNU LilyPond and recently wondered if Guile could run
> without Java finalization that prevents collection of chains of
> unreachable objects. I found that the functionality is only needed once
> the first guardian is created, so it's possible to delay enabling the
> mode until then. This required some fixes to free functions that
> assumed dependent objects to be freed only later (see first two
> patches).
> The third patch delays ensuring Java finalization to scm_make_guardian,
> but doesn't disable it explicitly (it's on by default in bdwgc). This
> could now be done right after GC_INIT(), but it's not clear (at least
> to me) whether client applications actually rely it, so I think it's
> better if that's not done in Guile itself.
> 
> Please consider applying, the fixes potentially also to stable-2.2.

I didn't receive other comments than those by Maxime, so here is an
updated version of the first patch.

Jonas

[-- Attachment #1.2: 0001-Fix-test-smob-mark.patch --]
[-- Type: text/x-patch, Size: 1694 bytes --]

From f89a8271c3cbb775a83db3e59fa66ea82f6239a1 Mon Sep 17 00:00:00 2001
From: Jonas Hahnfeld <hahnjo@hahnjo.de>
Date: Fri, 2 Jul 2021 23:52:13 +0200
Subject: [PATCH 1/3] Fix test-smob-mark

* test-suite/standalone/test-smob-mark.c
  (init_smob_type): Correct size of smob type.
  (free_x): Clear smob data instead of local variable.
  (test_scm_smob_mark): Put smobs in array to ensure marking.
---
 test-suite/standalone/test-smob-mark.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/test-suite/standalone/test-smob-mark.c b/test-suite/standalone/test-smob-mark.c
index d0bb5336a..586a2b081 100644
--- a/test-suite/standalone/test-smob-mark.c
+++ b/test-suite/standalone/test-smob-mark.c
@@ -79,7 +79,7 @@ free_x (SCM x)
   x_t *c_x;
   c_x = (x_t *) SCM_SMOB_DATA (x);
   scm_gc_free (c_x, sizeof (x_t), "x");
-  c_x = NULL;
+  SCM_SET_SMOB_DATA (x, NULL);
   return 0;
 }
 
@@ -100,22 +100,25 @@ print_x (SCM x, SCM port, scm_print_state * pstate SCM_UNUSED)
 static void
 test_scm_smob_mark ()
 {
+  SCM *smobs = scm_gc_malloc (sizeof(SCM) * SMOBS_COUNT, "smobs");
+
   int i;
   mark_call_count = 0;
   for (i = 0; i < SMOBS_COUNT; i++)
-    make_x ();
+    smobs[i] = make_x ();
   scm_gc ();
   if (mark_call_count < SMOBS_COUNT)
     {
       fprintf (stderr, "FAIL: SMOB mark function called for each SMOB\n");
       exit (EXIT_FAILURE);
     }
+  scm_remember_upto_here_1 (smobs);
 }
 
 static void
 init_smob_type ()
 {
-  x_tag = scm_make_smob_type ("x", sizeof (x_t));
+  x_tag = scm_make_smob_type ("x", sizeof (x_t *));
   scm_set_smob_free (x_tag, free_x);
   scm_set_smob_print (x_tag, print_x);
   scm_set_smob_mark (x_tag, mark_x);
-- 
2.32.0


[-- Attachment #1.3: 0002-Avoid-matching-calls-of-scm_gc_free.patch --]
[-- Type: text/x-patch, Size: 3938 bytes --]

From 33af6a98c6801e7b4880d1d3f78f7e2097c2174e Mon Sep 17 00:00:00 2001
From: Jonas Hahnfeld <hahnjo@hahnjo.de>
Date: Fri, 2 Jul 2021 23:03:17 +0200
Subject: [PATCH 2/3] Avoid matching calls of scm_gc_free

There is no point in registering memory with the garbage collector
if it doesn't need to do its job. In fact, calling scm_gc_free in
a free function is wrong because it relies on Java finalization.

* libguile/random.c (scm_c_random_bignum): Replace matching calls of
  scm_gc_calloc and scm_gc_free.
* libguile/regex-posix.c (scm_make_regexp, regex_free): Avoid call
  of scm_gc_free in free function.
* test-suite/standalone/test-smob-mark.c (make_x, free_x): Avoid
  call of scm_gc_free in free function.
---
 THANKS                                 | 1 +
 libguile/random.c                      | 8 ++------
 libguile/regex-posix.c                 | 6 +++---
 test-suite/standalone/test-smob-mark.c | 4 ++--
 4 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/THANKS b/THANKS
index aa4877e95..786b65d1a 100644
--- a/THANKS
+++ b/THANKS
@@ -13,6 +13,7 @@ Contributors since the last release:
          Volker Grabsch
          Julian Graham
         Michael Gran
+          Jonas Hahnfeld
          Daniel Hartwig
              No Itisnt
            Neil Jerram
diff --git a/libguile/random.c b/libguile/random.c
index 63da7f5d6..ac400a9fd 100644
--- a/libguile/random.c
+++ b/libguile/random.c
@@ -324,9 +324,7 @@ scm_c_random_bignum (scm_t_rstate *state, SCM m)
   /* we know the result will be this big */
   mpz_realloc2 (SCM_I_BIG_MPZ (result), m_bits);
 
-  random_chunks =
-    (uint32_t *) scm_gc_calloc (num_chunks * sizeof (uint32_t),
-                                     "random bignum chunks");
+  random_chunks = (uint32_t *) scm_calloc (num_chunks * sizeof (uint32_t));
 
   do
     {
@@ -363,9 +361,7 @@ scm_c_random_bignum (scm_t_rstate *state, SCM m)
       /* if result >= m, regenerate it (it is important to regenerate
 	 all bits in order not to get a distorted distribution) */
     } while (mpz_cmp (SCM_I_BIG_MPZ (result), SCM_I_BIG_MPZ (m)) >= 0);
-  scm_gc_free (random_chunks,
-               num_chunks * sizeof (uint32_t),
-               "random bignum chunks");
+  free (random_chunks);
   return scm_i_normbig (result);
 }
 
diff --git a/libguile/regex-posix.c b/libguile/regex-posix.c
index a08da02db..36bb639e0 100644
--- a/libguile/regex-posix.c
+++ b/libguile/regex-posix.c
@@ -67,7 +67,7 @@ static size_t
 regex_free (SCM obj)
 {
   regfree (SCM_RGX (obj));
-  scm_gc_free (SCM_RGX (obj), sizeof(regex_t), "regex");
+  free (SCM_RGX (obj));
   return 0;
 }
 
@@ -164,7 +164,7 @@ SCM_DEFINE (scm_make_regexp, "make-regexp", 1, 0, 1,
       flag = SCM_CDR (flag);
     }
 
-  rx = scm_gc_malloc_pointerless (sizeof (regex_t), "regex");
+  rx = scm_malloc (sizeof (regex_t));
   c_pat = scm_to_locale_string (pat);
   status = regcomp (rx, c_pat,
 		    /* Make sure they're not passing REG_NOSUB;
@@ -174,7 +174,7 @@ SCM_DEFINE (scm_make_regexp, "make-regexp", 1, 0, 1,
   if (status != 0)
     {
       SCM errmsg = scm_regexp_error_msg (status, rx);
-      scm_gc_free (rx, sizeof(regex_t), "regex");
+      free (rx);
       scm_error_scm (scm_regexp_error_key,
 		     scm_from_utf8_string (FUNC_NAME),
 		     errmsg,
diff --git a/test-suite/standalone/test-smob-mark.c b/test-suite/standalone/test-smob-mark.c
index 586a2b081..edbb51f89 100644
--- a/test-suite/standalone/test-smob-mark.c
+++ b/test-suite/standalone/test-smob-mark.c
@@ -56,7 +56,7 @@ make_x ()
   x_t *c_x;
 
   i++;
-  c_x = (x_t *) scm_gc_malloc (sizeof (x_t), "x");
+  c_x = (x_t *) scm_malloc (sizeof (x_t));
   c_x->scm_value = scm_from_int (i);
   c_x->c_value = i;
   SCM_NEWSMOB (s_x, x_tag, c_x);
@@ -78,7 +78,7 @@ free_x (SCM x)
 {
   x_t *c_x;
   c_x = (x_t *) SCM_SMOB_DATA (x);
-  scm_gc_free (c_x, sizeof (x_t), "x");
+  free (c_x);
   SCM_SET_SMOB_DATA (x, NULL);
   return 0;
 }
-- 
2.32.0


[-- Attachment #1.4: 0003-Delay-Java-finalization-for-guardians.patch --]
[-- Type: text/x-patch, Size: 1695 bytes --]

From 6bbb76692a26090918dfed42d67c6ba42e54e034 Mon Sep 17 00:00:00 2001
From: Jonas Hahnfeld <hahnjo@hahnjo.de>
Date: Fri, 2 Jul 2021 23:16:32 +0200
Subject: [PATCH 3/3] Delay Java finalization for guardians

It is only needed once the first guardian is created.

* libguile/guardians.c (scm_make_guardian): Move call to enable
  Java finalization from scm_init_guardians.
---
 libguile/guardians.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/libguile/guardians.c b/libguile/guardians.c
index fa8c8b8f7..357c8dbd1 100644
--- a/libguile/guardians.c
+++ b/libguile/guardians.c
@@ -306,6 +306,8 @@ guardian_apply (SCM guardian, SCM obj, SCM throw_p)
     return scm_i_get_one_zombie (guardian);
 }
 
+static scm_i_pthread_mutex_t gc_java_mutex;
+
 SCM_DEFINE (scm_make_guardian, "make-guardian", 0, 0, 0, 
 	    (),
 "Create a new guardian.  A guardian protects a set of objects from\n"
@@ -349,6 +351,11 @@ SCM_DEFINE (scm_make_guardian, "make-guardian", 0, 0, 0,
 	    "value, it is eligible to be returned from a guardian.\n")
 #define FUNC_NAME s_scm_make_guardian
 {
+  /* For guardians, we use unordered finalization `a la Java. */
+  scm_i_pthread_mutex_lock (&gc_java_mutex);
+  GC_set_java_finalization (1);
+  scm_i_pthread_mutex_unlock (&gc_java_mutex);
+
   t_guardian *g = scm_gc_malloc (sizeof (t_guardian), "guardian");
   SCM z;
 
@@ -369,8 +376,7 @@ SCM_DEFINE (scm_make_guardian, "make-guardian", 0, 0, 0,
 void
 scm_init_guardians ()
 {
-  /* We use unordered finalization `a la Java.  */
-  GC_set_java_finalization (1);
+  scm_i_pthread_mutex_init (&gc_java_mutex, NULL);
 
   tc16_guardian = scm_make_smob_type ("guardian", 0);
 
-- 
2.32.0


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

end of thread, other threads:[~2021-07-15 18:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-03 12:05 GC + Java finalization Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library
2021-07-03 17:14 ` Maxime Devos
2021-07-03 17:26   ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library
2021-07-03 18:49     ` Maxime Devos
2021-07-03 18:54     ` Maxime Devos
2021-07-15 18:44 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library

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 NNTP newsgroup(s).