[Guile-commits] GNU Guile branch, master, updated. release_1-9-8-76-gbd5a75d

[Ludovic Courtès]

This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU Guile".

http://git.savannah.gnu.org/cgit/guile.git/commit/?id=bd5a75dcd8436ebe077c9ce52300d013e9519d94

The branch, master has been updated
       via  bd5a75dcd8436ebe077c9ce52300d013e9519d94 (commit)
       via  c02924d02b4b78998b380deb20a4fd2d5fc3cfbb (commit)
       via  e2cf8eb921a4dd12af5024861e2b2bfc0b90a1ed (commit)
      from  b7ecadca7b84a5c9229001354f7b902ef94b5ac5 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit bd5a75dcd8436ebe077c9ce52300d013e9519d94
Author: Ludovic Courtès <address@hidden>
Date:   Fri Mar 5 13:38:28 2010 +0100

    Recycle fluid numbers.
    
    * libguile/fluids.c: Remove outdated comment on the complexity of fluid GC.
      (FLUID_GROW): Increase to 128.
      (allocated_fluids): Change to `void **'.
      (allocated_fluids_num): Remove.
      (grow_dynamic_state): Remove race-condition checker.  Grow to
      ALLOCATED_FLUIDS_LEN.
      (next_fluid_num): Rename to...
      (new_fluid): ... this.  Return a fluid instead of a fluid number.
      Assume ALLOCATED_FLUIDS is sparse and search for a free number in it.
      Make ALLOCATED_FLUIDS point to pointerless memory and initialize
      individual elements with a pointer to the new fluid.  Register a
      disappearing link from there to the fluid.
      (scm_make_fluid): Use `new_fluid ()'.
      (scm_fluid_ref, scm_fluid_set_x, scm_i_swap_with_fluids): Remove
      assertion that wasn't checked in a thread-safe way.

commit c02924d02b4b78998b380deb20a4fd2d5fc3cfbb
Author: Ludovic Courtès <address@hidden>
Date:   Fri Mar 5 11:41:42 2010 +0100

    Add new fluid tests.
    
    * test-suite/tests/fluids.test ("initial fluid values")["initial value
      is inherited from parent thread"]: New test.
      ("fluid values are thread-local"): New test.

commit e2cf8eb921a4dd12af5024861e2b2bfc0b90a1ed
Author: Ludovic Courtès <address@hidden>
Date:   Fri Mar 5 09:49:04 2010 +0100

    Don't gratuitously over-engineer things...
    
    * libguile/eval.c (eval)[SCM_M_CALL]: Always use `alloca' for ARGV.

-----------------------------------------------------------------------

Summary of changes:
 libguile/eval.c              |   12 +---
 libguile/fluids.c            |  146 ++++++++++++++++-------------------------
 test-suite/tests/fluids.test |   26 +++++++-
 3 files changed, 84 insertions(+), 100 deletions(-)

diff --git a/libguile/eval.c b/libguile/eval.c
index 72ab774..707144b 100644
--- a/libguile/eval.c
+++ b/libguile/eval.c
@@ -315,18 +315,10 @@ eval (SCM x, SCM env)
         }
       else
         {
+         SCM *argv;
          unsigned int i;
 
-         /* Using `alloca' would make the stack grow until this function
-            returns.  Thus we use C99 variable-length arrays where available,
-            so that stack space is freed when ARGV goes out of scope.  */
-#if __STDC_VERSION__ >= 199901L
-         SCM argv[argc];
-#else
-         SCM argv[128];
-         assert (argc < 128);
-#endif
-
+         argv = alloca (argc * sizeof (SCM));
          for (i = 0; i < argc; i++, mx = CDR (mx))
            argv[i] = eval (CAR (mx), env);
 
diff --git a/libguile/fluids.c b/libguile/fluids.c
index 765df7b..17d67e9 100644
--- a/libguile/fluids.c
+++ b/libguile/fluids.c
@@ -22,7 +22,6 @@
 
 #include <stdio.h>
 #include <string.h>
-#include <assert.h>
 
 #include "libguile/_scm.h"
 #include "libguile/print.h"
@@ -34,36 +33,17 @@
 #include "libguile/deprecation.h"
 #include "libguile/lang.h"
 #include "libguile/validate.h"
+#include "libguile/bdw-gc.h"
 
-#define FLUID_GROW 20
-
-/* A lot of the complexity below stems from the desire to reuse fluid
-   slots.  Normally, fluids should be pretty global and long-lived
-   things, so that reusing their slots should not be overly critical,
-   but it is the right thing to do nevertheless.  The code therefore
-   puts the burdon on allocating and collection fluids and keeps
-   accessing fluids lock free.  This is achieved by manipulating the
-   global state of the fluid machinery mostly in single threaded
-   sections.
-
-   Reusing a fluid slot means that it must be reset to #f in all
-   dynamic states.  We do this by maintaining a weak list of all
-   dynamic states, which is used after a GC to do the resetting.
-
-   Also, the fluid vectors in the dynamic states need to grow from
-   time to time when more fluids are created.  We do this in a single
-   threaded section so that threads do not need to lock when accessing
-   a fluid in the normal way.
-*/
-
-static scm_i_pthread_mutex_t fluid_admin_mutex = 
SCM_I_PTHREAD_MUTEX_INITIALIZER;
+/* Number of additional slots to allocate when ALLOCATED_FLUIDS is full.  */
+#define FLUID_GROW 128
 
-/* Protected by fluid_admin_mutex, but also accessed during GC.  See
-   next_fluid_num for a discussion of this.
- */
+/* Vector of allocated fluids indexed by fluid numbers.  Access is protected by
+   FLUID_ADMIN_MUTEX.  */
+static void **allocated_fluids = NULL;
 static size_t allocated_fluids_len = 0;
-static size_t allocated_fluids_num = 0;
-static char *allocated_fluids = NULL;
+
+static scm_i_pthread_mutex_t fluid_admin_mutex = 
SCM_I_PTHREAD_MUTEX_INITIALIZER;
 
 #define IS_FLUID(x)         SCM_I_FLUID_P (x)
 #define FLUID_NUM(x)        SCM_I_FLUID_NUM (x)
@@ -74,34 +54,26 @@ static char *allocated_fluids = NULL;
 
 
 
-/* Grow STATE so that it can hold up to ALLOCATED_FLUIDS_NUM fluids.  */
+/* Grow STATE so that it can hold up to ALLOCATED_FLUIDS_LEN fluids.  This may
+   be more than necessary since ALLOCATED_FLUIDS is sparse and the current
+   thread may not access all the fluids anyway.  Memory usage could be improved
+   by using a 2-level array as is done in glibc for pthread keys (TODO).  */
 static void
 grow_dynamic_state (SCM state)
 {
   SCM new_fluids;
   SCM old_fluids = DYNAMIC_STATE_FLUIDS (state);
-  size_t i, new_len, old_len = SCM_SIMPLE_VECTOR_LENGTH (old_fluids);
+  size_t i, len, old_len = SCM_SIMPLE_VECTOR_LENGTH (old_fluids);
 
- retry:
-  new_len = allocated_fluids_num;
-  new_fluids = scm_c_make_vector (new_len, SCM_BOOL_F);
+  /* Assume the assignment below is atomic.  */
+  len = allocated_fluids_len;
 
-  scm_i_pthread_mutex_lock (&fluid_admin_mutex);
-  if (new_len != allocated_fluids_num)
-    {
-      /* We lost the race.  */
-      scm_i_pthread_mutex_unlock (&fluid_admin_mutex);
-      goto retry;
-    }
-
-  assert (allocated_fluids_num > old_len);
+  new_fluids = scm_c_make_vector (len, SCM_BOOL_F);
 
   for (i = 0; i < old_len; i++)
     SCM_SIMPLE_VECTOR_SET (new_fluids, i,
                           SCM_SIMPLE_VECTOR_REF (old_fluids, i));
   SET_DYNAMIC_STATE_FLUIDS (state, new_fluids);
-
-  scm_i_pthread_mutex_unlock (&fluid_admin_mutex);
 }
 
 void
@@ -128,45 +100,53 @@ scm_i_with_fluids_print (SCM exp, SCM port, 
scm_print_state *pstate SCM_UNUSED)
   scm_putc ('>', port);
 }
 
-static size_t
-next_fluid_num ()
+
+/* Return a new fluid.  */
+static SCM
+new_fluid ()
 {
-  size_t n;
+  SCM fluid;
+  size_t trial, n;
+
+  /* Fluids are pointerless cells: the first word is the type tag; the second
+     word is the fluid number.  */
+  fluid = PTR2SCM (scm_gc_malloc_pointerless (sizeof (scm_t_cell), "fluid"));
+  SCM_SET_CELL_TYPE (fluid, scm_tc7_fluid);
 
   scm_dynwind_begin (0);
   scm_i_dynwind_pthread_mutex_lock (&fluid_admin_mutex);
 
-  if ((allocated_fluids_len > 0) &&
-      (allocated_fluids_num == allocated_fluids_len))
-    {
-      /* All fluid numbers are in use.  Run a GC to try to free some
-        up.
-      */
-      scm_gc ();
-    }
-
-  if (allocated_fluids_num < allocated_fluids_len)
+  for (trial = 0; trial < 2; trial++)
     {
+      /* Look for a free fluid number.  */
       for (n = 0; n < allocated_fluids_len; n++)
-       if (allocated_fluids[n] == 0)
+       /* TODO: Use `__sync_bool_compare_and_swap' where available.  */
+       if (allocated_fluids[n] == NULL)
          break;
+
+      if (trial == 0 && n >= allocated_fluids_len)
+       /* All fluid numbers are in use.  Run a GC and retry.  Explicitly
+          running the GC is costly and bad-style.  We only do this because
+          dynamic state fluid vectors would grow unreasonably if fluid numbers
+          weren't reused.  */
+       scm_i_gc ("fluids");
     }
-  else
+
+  if (n >= allocated_fluids_len)
     {
       /* Grow the vector of allocated fluids.  */
-      /* FIXME: Since we use `scm_malloc ()', ALLOCATED_FLUIDS is scanned by
-        the GC; therefore, all fluids remain reachable for the entire
-        program lifetime.  Hopefully this is not a problem in practice.  */
-      char *new_allocated_fluids =
-       scm_gc_malloc (allocated_fluids_len + FLUID_GROW,
-                      "allocated fluids");
+      void **new_allocated_fluids =
+       scm_gc_malloc_pointerless ((allocated_fluids_len + FLUID_GROW)
+                                  * sizeof (*allocated_fluids),
+                                  "allocated fluids");
 
       /* Copy over old values and initialize rest.  GC can not run
         during these two operations since there is no safe point in
-        them.
-      */
-      memcpy (new_allocated_fluids, allocated_fluids, allocated_fluids_len);
-      memset (new_allocated_fluids + allocated_fluids_len, 0, FLUID_GROW);
+        them.  */
+      memcpy (new_allocated_fluids, allocated_fluids,
+             allocated_fluids_len * sizeof (*allocated_fluids));
+      memset (new_allocated_fluids + allocated_fluids_len, 0,
+             FLUID_GROW * sizeof (*allocated_fluids));
       n = allocated_fluids_len;
 
       /* Update the vector of allocated fluids.  Dynamic states will
@@ -175,12 +155,15 @@ next_fluid_num ()
       allocated_fluids = new_allocated_fluids;
       allocated_fluids_len += FLUID_GROW;
     }
-  
-  allocated_fluids_num += 1;
-  allocated_fluids[n] = 1;
-  
+
+  allocated_fluids[n] = SCM2PTR (fluid);
+  SCM_SET_CELL_WORD_1 (fluid, (scm_t_bits) n);
+
+  GC_GENERAL_REGISTER_DISAPPEARING_LINK (&allocated_fluids[n],
+                                        SCM2PTR (fluid));
+
   scm_dynwind_end ();
-  return n;
+  return fluid;
 }
 
 SCM_DEFINE (scm_make_fluid, "make-fluid", 0, 0, 0, 
@@ -194,7 +177,7 @@ SCM_DEFINE (scm_make_fluid, "make-fluid", 0, 0, 0,
            "with its own dynamic state, you can use fluids for thread local 
storage.")
 #define FUNC_NAME s_scm_make_fluid
 {
-  return scm_cell (scm_tc7_fluid, (scm_t_bits) next_fluid_num ());
+  return new_fluid ();
 }
 #undef FUNC_NAME
 
@@ -229,11 +212,6 @@ SCM_DEFINE (scm_fluid_ref, "fluid-ref", 1, 0, 0,
 
   if (SCM_UNLIKELY (FLUID_NUM (fluid) >= SCM_SIMPLE_VECTOR_LENGTH (fluids)))
     {
-      /* We should only get there when the current thread's dynamic state
-        turns out to be too small compared to the set of currently allocated
-        fluids.  */
-      assert (SCM_SIMPLE_VECTOR_LENGTH (fluids) < allocated_fluids_num);
-
       /* Lazily grow the current thread's dynamic state.  */
       grow_dynamic_state (SCM_I_CURRENT_THREAD->dynamic_state);
 
@@ -255,11 +233,6 @@ SCM_DEFINE (scm_fluid_set_x, "fluid-set!", 2, 0, 0,
 
   if (SCM_UNLIKELY (FLUID_NUM (fluid) >= SCM_SIMPLE_VECTOR_LENGTH (fluids)))
     {
-      /* We should only get there when the current thread's dynamic state
-        turns out to be too small compared to the set of currently allocated
-        fluids.  */
-      assert (SCM_SIMPLE_VECTOR_LENGTH (fluids) < allocated_fluids_num);
-
       /* Lazily grow the current thread's dynamic state.  */
       grow_dynamic_state (SCM_I_CURRENT_THREAD->dynamic_state);
 
@@ -330,11 +303,6 @@ scm_i_swap_with_fluids (SCM wf, SCM dynstate)
 
   if (SCM_UNLIKELY (max >= SCM_SIMPLE_VECTOR_LENGTH (fluids)))
     {
-      /* We should only get there when the current thread's dynamic state turns
-         out to be too small compared to the set of currently allocated
-         fluids. */
-      assert (SCM_SIMPLE_VECTOR_LENGTH (fluids) < allocated_fluids_num);
-
       /* Lazily grow the current thread's dynamic state.  */
       grow_dynamic_state (dynstate);
 
diff --git a/test-suite/tests/fluids.test b/test-suite/tests/fluids.test
index 51353fa..3784e54 100644
--- a/test-suite/tests/fluids.test
+++ b/test-suite/tests/fluids.test
@@ -27,7 +27,19 @@
 
 (with-test-prefix "initial fluid values"
   (pass-if "fluid-ref uninitialized fluid is #f"
-    (not (fluid-ref a))))
+    (not (fluid-ref a)))
+
+  (pass-if "initial value is inherited from parent thread"
+    (if (provided? 'threads)
+        (let ((f (make-fluid)))
+          (fluid-set! f 'initial)
+          (let ((child (call-with-new-thread
+                        (lambda ()
+                          (let ((init (fluid-ref f)))
+                            (fluid-set! f 'new)
+                            (list init (fluid-ref f)))))))
+            (equal? '(initial new) (join-thread child))))
+        (throw 'unresolved))))
 
 (with-test-prefix "with-fluids with non-fluid"
   (pass-if-exception "exception raised if nonfluid passed to with-fluids"
@@ -56,6 +68,18 @@
            (eqv? (fluid-ref a) 2))
          (eqv? (fluid-ref a) #f))))
 
+(pass-if "fluid values are thread-local"
+  (if (provided? 'threads)
+      (let ((f (make-fluid)))
+        (fluid-set! f 'parent)
+        (let ((child (call-with-new-thread
+                      (lambda ()
+                        (fluid-set! f 'child)
+                        (fluid-ref f)))))
+          (and (eq? (join-thread child) 'child)
+               (eq? (fluid-ref f) 'parent))))
+      (throw 'unresolved)))
+
 (pass-if "fluids are GC'd"
 
   (let ((g (make-guardian)))


hooks/post-receive
-- 
GNU Guile