[pypy-commit] stmgc default: apply fixes from last few commit to C8

Raemi noreply at buildbot.pypy.org
Tue Feb 24 15:20:04 CET 2015


Author: Remi Meier <remi.meier at gmail.com>
Branch: 
Changeset: r1653:d0ca59d95e84
Date: 2015-02-24 15:20 +0100
http://bitbucket.org/pypy/stmgc/changeset/d0ca59d95e84/

Log:	apply fixes from last few commit to C8

diff --git a/c8/stm/extra.c b/c8/stm/extra.c
--- a/c8/stm/extra.c
+++ b/c8/stm/extra.c
@@ -6,15 +6,24 @@
 static long register_callbacks(stm_thread_local_t *tl,
                                void *key, void callback(void *), long index)
 {
-    if (!_stm_in_transaction(tl)) {
-        /* check that the current thread-local is really running a
+    dprintf(("register_callbacks: tl=%p key=%p callback=%p index=%ld\n",
+             tl, key, callback, index));
+    if (tl->associated_segment_num == -1) {
+        /* check that the provided thread-local is really running a
            transaction, and do nothing otherwise. */
+        dprintf(("  NOT IN TRANSACTION\n"));
         return -1;
     }
-
-    if (STM_PSEGMENT->transaction_state == TS_INEVITABLE) {
+    /* The tl was only here to check that.  We're really using
+       STM_PSEGMENT below, which is often but not always the
+       segment corresponding to the tl.  One case where it's not
+       the case is if this gets called from stmcb_light_finalizer()
+       from abort_finalizers() from major collections or contention.
+    */
+    if (STM_PSEGMENT->transaction_state != TS_REGULAR) {
         /* ignore callbacks if we're in an inevitable transaction
-           (which cannot abort) */
+           (which cannot abort) or no transaction at all in this segment */
+        dprintf(("  STATE = %d\n", (int)STM_PSEGMENT->transaction_state));
         return -1;
     }
 
@@ -23,15 +32,19 @@
 
     if (callback == NULL) {
         /* double-unregistering works, but return 0 */
-        return tree_delete_item(callbacks, (uintptr_t)key);
+        long res = tree_delete_item(callbacks, (uintptr_t)key);
+        dprintf(("  DELETED %ld\n", res));
+        return res;
     }
     else {
         /* double-registering the same key will crash */
+        dprintf(("  INSERTING\n"));
         tree_insert(callbacks, (uintptr_t)key, (uintptr_t)callback);
         return 1;
     }
 }
 
+
 long stm_call_on_commit(stm_thread_local_t *tl,
                        void *key, void callback(void *))
 {
@@ -39,6 +52,7 @@
     if (result < 0 && callback != NULL) {
         /* no regular transaction running, invoke the callback
            immediately */
+        dprintf(("stm_call_on_commit calls now: %p(%p)\n", callback, key));
         callback(key);
     }
     return result;
@@ -72,8 +86,11 @@
         assert(key != NULL);
         assert(callback != NULL);
 
-        /* The callback may call stm_call_on_abort(key, NULL).  It is ignored,
-           because 'callbacks_on_commit_and_abort' was cleared already. */
+        /* The callback may call stm_call_on_abort(key, NULL)
+           (so with callback==NULL).  It is ignored, because
+           'callbacks_on_commit_and_abort' was cleared already. */
+        dprintf(("invoke_and_clear_user_callbacks(%ld): %p(%p)\n",
+                 index, callback, key));
         callback(key);
 
     } TREE_LOOP_END;
diff --git a/c8/stm/gcpage.c b/c8/stm/gcpage.c
--- a/c8/stm/gcpage.c
+++ b/c8/stm/gcpage.c
@@ -388,10 +388,10 @@
         */
 
         /* only for new, uncommitted objects:
-           If 'tl' is currently running, its 'associated_segment_num'
+           If 'tl' is currently running, its 'last_associated_segment_num'
            field is the segment number that contains the correct
            version of its overflowed objects. */
-        char *segment_base = get_segment_base(tl->associated_segment_num);
+        char *segment_base = get_segment_base(tl->last_associated_segment_num);
 
         struct stm_shadowentry_s *current = tl->shadowstack;
         struct stm_shadowentry_s *base = tl->shadowstack_base;
diff --git a/c8/stm/setup.c b/c8/stm/setup.c
--- a/c8/stm/setup.c
+++ b/c8/stm/setup.c
@@ -223,14 +223,15 @@
         tl->prev = stm_all_thread_locals->prev;
         stm_all_thread_locals->prev->next = tl;
         stm_all_thread_locals->prev = tl;
-        num = (tl->prev->associated_segment_num) % (NB_SEGMENTS-1);
+        num = (tl->prev->last_associated_segment_num) % (NB_SEGMENTS-1);
     }
     tl->thread_local_obj = NULL;
 
     /* assign numbers consecutively, but that's for tests; we could also
        assign the same number to all of them and they would get their own
        numbers automatically. */
-    tl->associated_segment_num = num + 1;
+    tl->associated_segment_num = -1;
+    tl->last_associated_segment_num = num + 1;
     *_get_cpth(tl) = pthread_self();
     _init_shadow_stack(tl);
     set_gs_register(get_segment_base(num + 1));
diff --git a/c8/stm/sync.c b/c8/stm/sync.c
--- a/c8/stm/sync.c
+++ b/c8/stm/sync.c
@@ -110,7 +110,7 @@
     assert(_has_mutex());
     assert(_is_tl_registered(tl));
 
-    int num = tl->associated_segment_num - 1; // 0..NB_SEG-1
+    int num = tl->last_associated_segment_num - 1; // 0..NB_SEG-1
     OPT_ASSERT(num >= 0);
     if (sync_ctl.in_use1[num+1] == 0) {
         /* fast-path: we can get the same segment number than the one
@@ -130,8 +130,9 @@
         num = (num+1) % (NB_SEGMENTS-1);
         if (sync_ctl.in_use1[num+1] == 0) {
             /* we're getting 'num', a different number. */
-            dprintf(("acquired different segment: %d->%d\n", tl->associated_segment_num, num+1));
-            tl->associated_segment_num = num+1;
+            dprintf(("acquired different segment: %d->%d\n",
+                     tl->last_associated_segment_num, num+1));
+            tl->last_associated_segment_num = num+1;
             set_gs_register(get_segment_base(num+1));
             goto got_num;
         }
@@ -147,6 +148,7 @@
     sync_ctl.in_use1[num+1] = 1;
     assert(STM_SEGMENT->segment_num == num+1);
     assert(STM_SEGMENT->running_thread == NULL);
+    tl->associated_segment_num = tl->last_associated_segment_num;
     STM_SEGMENT->running_thread = tl;
     return true;
 }
@@ -158,10 +160,12 @@
     cond_signal(C_SEGMENT_FREE);
 
     assert(STM_SEGMENT->running_thread == tl);
+    assert(tl->associated_segment_num == tl->last_associated_segment_num);
+    tl->associated_segment_num = -1;
     STM_SEGMENT->running_thread = NULL;
 
-    assert(sync_ctl.in_use1[tl->associated_segment_num] == 1);
-    sync_ctl.in_use1[tl->associated_segment_num] = 0;
+    assert(sync_ctl.in_use1[tl->last_associated_segment_num] == 1);
+    sync_ctl.in_use1[tl->last_associated_segment_num] = 0;
 }
 
 __attribute__((unused))
@@ -172,9 +176,16 @@
 
 bool _stm_in_transaction(stm_thread_local_t *tl)
 {
-    int num = tl->associated_segment_num;
-    assert(1 <= num && num < NB_SEGMENTS);
-    return get_segment(num)->running_thread == tl;
+    if (tl->associated_segment_num == -1) {
+        return false;
+    }
+    else {
+        int num = tl->associated_segment_num;
+        OPT_ASSERT(1 <= num && num < NB_SEGMENTS);
+        OPT_ASSERT(num == tl->last_associated_segment_num);
+        OPT_ASSERT(get_segment(num)->running_thread == tl);
+        return true;
+    }
 }
 
 void _stm_test_switch(stm_thread_local_t *tl)
diff --git a/c8/stmgc.h b/c8/stmgc.h
--- a/c8/stmgc.h
+++ b/c8/stmgc.h
@@ -67,6 +67,7 @@
     long last_abort__bytes_in_nursery;
     /* the next fields are handled internally by the library */
     int associated_segment_num;
+    int last_associated_segment_num;
     struct stm_thread_local_s *prev, *next;
     void *creating_pthread[2];
 } stm_thread_local_t;


More information about the pypy-commit mailing list