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; 31+ 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 related	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2022-02-22 15:12 UTC | newest]

Thread overview: 31+ 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
2021-10-10 16:22   ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library
2021-11-19 12:18     ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library
2022-02-22 10:14       ` Dr. Arne Babenhauserheide
2022-02-22 15:12         ` Mike Gran
2021-11-19 13:13   ` Maxime Devos
2021-11-19 13:32     ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library
2021-11-19 13:35       ` Maxime Devos
2021-11-19 13:40         ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library
2021-11-19 13:44           ` Maxime Devos
2021-11-19 13:53             ` Maxime Devos
2021-11-19 13:53             ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library
2021-11-19 14:01               ` Maxime Devos
2021-11-19 13:48       ` Maxime Devos
2021-11-19 13:55         ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library
2021-11-19 14:14           ` Maxime Devos
2021-11-19 14:52             ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library
2021-11-19 18:48               ` Maxime Devos
2021-11-19 19:01                 ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library
2021-11-19 13:15   ` Maxime Devos
2021-11-19 13:35     ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library
2021-11-19 14:21       ` Maxime Devos
2021-11-19 15:32         ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library
2021-11-19 13:17   ` Maxime Devos
2021-11-19 13:38     ` Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library
2021-11-20  9:19   ` Maxime Devos

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).