[pypy-commit] pypy stmgc-c7: update to stmgc/e0ff682f6615 and fix the RPython code

arigo noreply at buildbot.pypy.org
Mon Feb 9 00:19:19 CET 2015


Author: Armin Rigo <arigo at tunes.org>
Branch: stmgc-c7
Changeset: r75777:0f6ff5506d41
Date: 2015-02-09 00:18 +0100
http://bitbucket.org/pypy/pypy/changeset/0f6ff5506d41/

Log:	update to stmgc/e0ff682f6615 and fix the RPython code

diff --git a/pypy/module/pypystm/hashtable.py b/pypy/module/pypystm/hashtable.py
--- a/pypy/module/pypystm/hashtable.py
+++ b/pypy/module/pypystm/hashtable.py
@@ -26,15 +26,14 @@
 
     @unwrap_spec(key=int)
     def setitem_w(self, key, w_value):
-        entry = self.h.lookup(key)
-        entry.object = cast_instance_to_gcref(w_value)
+        self.h.set(key, cast_instance_to_gcref(w_value))
 
     @unwrap_spec(key=int)
     def delitem_w(self, space, key):
         entry = self.h.lookup(key)
         if not entry.object:
             space.raise_key_error(space.wrap(key))
-        entry.object = rstm.NULL_GCREF
+        self.h.writeobj(entry, rstm.NULL_GCREF)
 
     @unwrap_spec(key=int)
     def contains_w(self, space, key):
@@ -53,7 +52,7 @@
         entry = self.h.lookup(key)
         gcref = entry.object
         if not gcref:
-            entry.object = cast_instance_to_gcref(w_default)
+            self.h.writeobj(entry, cast_instance_to_gcref(w_default))
             return w_default
         return cast_gcref_to_instance(W_Root, gcref)
 
diff --git a/pypy/module/pypystm/stmdict.py b/pypy/module/pypystm/stmdict.py
--- a/pypy/module/pypystm/stmdict.py
+++ b/pypy/module/pypystm/stmdict.py
@@ -43,7 +43,7 @@
         for i in range(length):
             dest[dest_start + i] = source[source_start + i]
 
-def pop_from_entry(entry, space, w_key):
+def pop_from_entry(h, entry, space, w_key):
     array = lltype.cast_opaque_ptr(PARRAY, entry.object)
     if not array:
         return None
@@ -59,7 +59,7 @@
         narray = lltype.malloc(ARRAY, L)
         ll_arraycopy(array, narray, 0, 0, i)
         ll_arraycopy(array, narray, i + 2, i, L - i)
-    entry.object = lltype.cast_opaque_ptr(llmemory.GCREF, narray)
+    h.writeobj(entry, lltype.cast_opaque_ptr(llmemory.GCREF, narray))
     return w_value
 
 
@@ -96,12 +96,12 @@
             L = 0
         narray[L] = cast_instance_to_gcref(w_key)
         narray[L + 1] = cast_instance_to_gcref(w_value)
-        entry.object = lltype.cast_opaque_ptr(llmemory.GCREF, narray)
+        self.h.writeobj(entry, lltype.cast_opaque_ptr(llmemory.GCREF, narray))
 
     def delitem_w(self, space, w_key):
         hkey = space.hash_w(w_key)
         entry = self.h.lookup(hkey)
-        if pop_from_entry(entry, space, w_key) is None:
+        if pop_from_entry(self.h, entry, space, w_key) is None:
             space.raise_key_error(w_key)
 
     def contains_w(self, space, w_key):
@@ -126,7 +126,7 @@
     def pop_w(self, space, w_key, w_default=None):
         hkey = space.hash_w(w_key)
         entry = self.h.lookup(hkey)
-        w_value = pop_from_entry(entry, space, w_key)
+        w_value = pop_from_entry(self.h, entry, space, w_key)
         if w_value is not None:
             return w_value
         elif w_default is not None:
@@ -152,7 +152,7 @@
             L = 0
         narray[L] = cast_instance_to_gcref(w_key)
         narray[L + 1] = cast_instance_to_gcref(w_default)
-        entry.object = lltype.cast_opaque_ptr(llmemory.GCREF, narray)
+        self.h.writeobj(entry, lltype.cast_opaque_ptr(llmemory.GCREF, narray))
         return w_default
 
     def get_length(self):
diff --git a/pypy/module/pypystm/stmset.py b/pypy/module/pypystm/stmset.py
--- a/pypy/module/pypystm/stmset.py
+++ b/pypy/module/pypystm/stmset.py
@@ -65,7 +65,7 @@
             narray = lltype.malloc(ARRAY, 1)
             L = 0
         narray[L] = cast_instance_to_gcref(w_key)
-        entry.object = lltype.cast_opaque_ptr(llmemory.GCREF, narray)
+        self.h.writeobj(entry, lltype.cast_opaque_ptr(llmemory.GCREF, narray))
 
     def try_remove(self, space, w_key):
         hkey = space.hash_w(w_key)
@@ -84,7 +84,7 @@
             narray = lltype.malloc(ARRAY, L)
             ll_arraycopy(array, narray, 0, 0, i)
             ll_arraycopy(array, narray, i + 1, i, L - i)
-        entry.object = lltype.cast_opaque_ptr(llmemory.GCREF, narray)
+        self.h.writeobj(entry, lltype.cast_opaque_ptr(llmemory.GCREF, narray))
         return True
 
     def remove_w(self, space, w_key):
diff --git a/rpython/rlib/rstm.py b/rpython/rlib/rstm.py
--- a/rpython/rlib/rstm.py
+++ b/rpython/rlib/rstm.py
@@ -218,6 +218,10 @@
     return llop.stm_hashtable_lookup(_STM_HASHTABLE_ENTRY_P,
                                      h, h.ll_raw_hashtable, key)
 
+ at dont_look_inside
+def _ll_hashtable_writeobj(h, entry, value):
+    llop.stm_hashtable_write_entry(lltype.Void, h, entry, value)
+
 _HASHTABLE_OBJ = lltype.GcStruct('HASHTABLE_OBJ',
                                  ('ll_raw_hashtable', _STM_HASHTABLE_P),
                                  rtti=True,
@@ -226,7 +230,8 @@
                                            'len': _ll_hashtable_len,
                                           'list': _ll_hashtable_list,
                                       'freelist': _ll_hashtable_freelist,
-                                        'lookup': _ll_hashtable_lookup})
+                                        'lookup': _ll_hashtable_lookup,
+                                      'writeobj': _ll_hashtable_writeobj})
 NULL_HASHTABLE = lltype.nullptr(_HASHTABLE_OBJ)
 
 def _ll_hashtable_trace(gc, obj, callback, arg):
@@ -303,6 +308,10 @@
         assert type(key) is int
         return EntryObjectForTest(self, key)
 
+    def writeobj(self, entry, nvalue):
+        assert isinstance(entry, EntryObjectForTest)
+        self.set(entry.key, nvalue)
+
 class EntryObjectForTest(object):
     def __init__(self, hashtable, key):
         self.hashtable = hashtable
@@ -312,6 +321,7 @@
     def _getobj(self):
         return self.hashtable.get(self.key)
     def _setobj(self, nvalue):
-        self.hashtable.set(self.key, nvalue)
+        raise Exception("can't assign to the 'object' attribute:"
+                        " use h.writeobj() instead")
 
     object = property(_getobj, _setobj)
diff --git a/rpython/rtyper/lltypesystem/lloperation.py b/rpython/rtyper/lltypesystem/lloperation.py
--- a/rpython/rtyper/lltypesystem/lloperation.py
+++ b/rpython/rtyper/lltypesystem/lloperation.py
@@ -465,6 +465,7 @@
     'stm_hashtable_read':     LLOp(),
     'stm_hashtable_write':    LLOp(),
     'stm_hashtable_lookup':   LLOp(),
+    'stm_hashtable_write_entry':        LLOp(),
     'stm_hashtable_length_upper_bound': LLOp(),
     'stm_hashtable_list'  :   LLOp(),
     'stm_hashtable_tracefn':  LLOp(),
diff --git a/rpython/translator/stm/funcgen.py b/rpython/translator/stm/funcgen.py
--- a/rpython/translator/stm/funcgen.py
+++ b/rpython/translator/stm/funcgen.py
@@ -312,6 +312,13 @@
     return ('stm_hashtable_write((object_t *)%s, %s, %s, (object_t *)%s, '
             '&stm_thread_local);' % (arg0, arg1, arg2, arg3))
 
+def stm_hashtable_write_entry(funcgen, op):
+    arg0 = funcgen.expr(op.args[0])
+    arg1 = funcgen.expr(op.args[1])
+    arg2 = funcgen.expr(op.args[2])
+    return ('stm_hashtable_write_entry((object_t *)%s, %s, (object_t *)%s);' % (
+        arg0, arg1, arg2))
+
 def stm_hashtable_lookup(funcgen, op):
     arg0 = funcgen.expr(op.args[0])
     arg1 = funcgen.expr(op.args[1])
diff --git a/rpython/translator/stm/src_stm/revision b/rpython/translator/stm/src_stm/revision
--- a/rpython/translator/stm/src_stm/revision
+++ b/rpython/translator/stm/src_stm/revision
@@ -1,1 +1,1 @@
-005668d99755
+e0ff682f6615
diff --git a/rpython/translator/stm/src_stm/stm/core.c b/rpython/translator/stm/src_stm/stm/core.c
--- a/rpython/translator/stm/src_stm/stm/core.c
+++ b/rpython/translator/stm/src_stm/stm/core.c
@@ -374,6 +374,7 @@
 
     assert(list_is_empty(STM_PSEGMENT->modified_old_objects));
     assert(list_is_empty(STM_PSEGMENT->modified_old_objects_markers));
+    assert(list_is_empty(STM_PSEGMENT->modified_old_hashtables));
     assert(list_is_empty(STM_PSEGMENT->young_weakrefs));
     assert(tree_is_cleared(STM_PSEGMENT->young_outside_nursery));
     assert(tree_is_cleared(STM_PSEGMENT->nursery_objects_shadows));
@@ -438,29 +439,45 @@
             continue;    /* no need to check: is pending immediate abort */
 
         char *remote_base = get_segment_base(i);
+        object_t *conflicting_obj;
 
         LIST_FOREACH_R(
             STM_PSEGMENT->modified_old_objects,
             object_t * /*item*/,
             ({
                 if (was_read_remote(remote_base, item)) {
-                    /* A write-read conflict! */
-                    dprintf(("write-read conflict on %p, our seg: %d, other: %ld\n",
-                             item, STM_SEGMENT->segment_num, i));
-                    if (write_read_contention_management(i, item)) {
-                        /* If we reach this point, we didn't abort, but we
-                           had to wait for the other thread to commit.  If we
-                           did, then we have to restart committing from our call
-                           to synchronize_all_threads(). */
-                        return true;
-                    }
-                    /* we aborted the other transaction without waiting, so
-                       we can just break out of this loop on
-                       modified_old_objects and continue with the next
-                       segment */
-                    break;
+                    conflicting_obj = item;
+                    goto found_conflict;
                 }
             }));
+
+        LIST_FOREACH_R(
+            STM_PSEGMENT->modified_old_hashtables,
+            object_t * /*item*/,
+            ({
+                if (was_read_remote(remote_base, item)) {
+                    conflicting_obj = item;
+                    goto found_conflict;
+                }
+            }));
+
+        continue;
+
+     found_conflict:
+        /* A write-read conflict! */
+        dprintf(("write-read conflict on %p, our seg: %d, other: %ld\n",
+                 conflicting_obj, STM_SEGMENT->segment_num, i));
+        if (write_read_contention_management(i, conflicting_obj)) {
+            /* If we reach this point, we didn't abort, but we
+               had to wait for the other thread to commit.  If we
+               did, then we have to restart committing from our call
+               to synchronize_all_threads(). */
+            return true;
+        }
+        /* we aborted the other transaction without waiting, so we can
+           just ignore the rest of this (now aborted) segment.  Let's
+           move on to the next one. */
+        continue;
     }
 
     return false;
@@ -790,6 +807,7 @@
 
     list_clear(STM_PSEGMENT->modified_old_objects);
     list_clear(STM_PSEGMENT->modified_old_objects_markers);
+    list_clear(STM_PSEGMENT->modified_old_hashtables);
 }
 
 static void _finish_transaction(enum stm_event_e event)
@@ -951,6 +969,7 @@
 
     list_clear(pseg->modified_old_objects);
     list_clear(pseg->modified_old_objects_markers);
+    list_clear(pseg->modified_old_hashtables);
 }
 
 static void abort_data_structures_from_segment_num(int segment_num)
diff --git a/rpython/translator/stm/src_stm/stm/core.h b/rpython/translator/stm/src_stm/stm/core.h
--- a/rpython/translator/stm/src_stm/stm/core.h
+++ b/rpython/translator/stm/src_stm/stm/core.h
@@ -96,6 +96,14 @@
     struct list_s *modified_old_objects_markers;
     uintptr_t modified_old_objects_markers_num_old;
 
+    /* This list contains all old hashtables that have entries that we
+       modified.  Note that several transactions can all commit if
+       they have the same hashtable listed here.  The point of this
+       list is only that if another segment does a global "read" of
+       the hashtable (stm_hashtable_list), then it conflicts with this
+       segment if it has got any change to the hashtable. */
+    struct list_s *modified_old_hashtables;
+
     /* List of out-of-nursery objects that may contain pointers to
        nursery objects.  This is used to track the GC status: they are
        all objects outside the nursery on which an stm_write() occurred
@@ -282,17 +290,13 @@
 static stm_thread_local_t *abort_with_mutex_no_longjmp(void);
 static void abort_data_structures_from_segment_num(int segment_num);
 
-static inline struct stm_read_marker_s *get_read_marker(char *base,
-                                                        object_t *obj) {
-    return (struct stm_read_marker_s *)(base + (((uintptr_t)obj) >> 4));
-}
-
 static inline bool was_read_remote(char *base, object_t *obj)
 {
     uint8_t other_transaction_read_version =
         ((struct stm_segment_info_s *)REAL_ADDRESS(base, STM_PSEGMENT))
             ->transaction_read_version;
-    uint8_t rm = get_read_marker(base, obj)->rm;
+    uint8_t rm = ((struct stm_read_marker_s *)
+                  (base + (((uintptr_t)obj) >> 4)))->rm;
     assert(rm <= other_transaction_read_version);
     return rm == other_transaction_read_version;
 }
diff --git a/rpython/translator/stm/src_stm/stm/gcpage.c b/rpython/translator/stm/src_stm/stm/gcpage.c
--- a/rpython/translator/stm/src_stm/stm/gcpage.c
+++ b/rpython/translator/stm/src_stm/stm/gcpage.c
@@ -476,13 +476,26 @@
     }
 }
 
-static void clean_up_segment_lists(void)
-{
 #pragma push_macro("STM_PSEGMENT")
 #pragma push_macro("STM_SEGMENT")
 #undef STM_PSEGMENT
 #undef STM_SEGMENT
 
+static void remove_objects_that_die(struct list_s *lst)
+{
+    if (lst != NULL) {
+        uintptr_t n = list_count(lst);
+        while (n > 0) {
+            object_t *obj = (object_t *)list_item(lst, --n);
+            if (!mark_visited_test(obj)) {
+                list_set_item(lst, n, list_pop_item(lst));
+            }
+        }
+    }
+}
+
+static void clean_up_segment_lists(void)
+{
     long i;
     for (i = 1; i <= NB_SEGMENTS; i++) {
         struct stm_priv_segment_info_s *pseg = get_priv_segment(i);
@@ -541,21 +554,14 @@
             }));
         list_clear(lst);
 
-        /* Remove from 'large_overflow_objects' all objects that die */
-        lst = pseg->large_overflow_objects;
-        if (lst != NULL) {
-            uintptr_t n = list_count(lst);
-            while (n > 0) {
-                object_t *obj = (object_t *)list_item(lst, --n);
-                if (!mark_visited_test(obj)) {
-                    list_set_item(lst, n, list_pop_item(lst));
-                }
-            }
-        }
+        /* Remove from 'large_overflow_objects' and 'modified_old_hashtables'
+           all objects that die */
+        remove_objects_that_die(pseg->large_overflow_objects);
+        remove_objects_that_die(pseg->modified_old_hashtables);
     }
+}
 #pragma pop_macro("STM_SEGMENT")
 #pragma pop_macro("STM_PSEGMENT")
-}
 
 static inline bool largemalloc_keep_object_at(char *data)
 {
diff --git a/rpython/translator/stm/src_stm/stm/hashtable.c b/rpython/translator/stm/src_stm/stm/hashtable.c
--- a/rpython/translator/stm/src_stm/stm/hashtable.c
+++ b/rpython/translator/stm/src_stm/stm/hashtable.c
@@ -298,34 +298,12 @@
                synchronization with other pieces of the code that may
                change.
             */
-
-            /* First fetch the read marker of 'hashtableobj' in all
-               segments, before allocate_outside_nursery_large() which
-               might trigger a GC.  Synchronization guarantee: if
-               stm_read(hobj) in stm_hashtable_list() has set the read
-               marker, then it did synchronize with us here by
-               acquiring and releasing this hashtable' lock.  However,
-               the interval of time between reading the readmarkers of
-               hobj and copying them to the new entry object might be
-               enough for the other threads to do anything, including
-               a reset_transaction_read_version(), so that we might in
-               theory write bogus read markers that are not valid any
-               more.  To prevent this, reset_transaction_read_version()
-               acquires the privatization_lock too.
-            */
-            long j;
-            uint8_t readmarkers[NB_SEGMENTS];
-
             acquire_privatization_lock();
-            for (j = 1; j <= NB_SEGMENTS; j++) {
-                readmarkers[j - 1] = get_read_marker(get_segment_base(j),
-                                                     hashtableobj)->rm;
-            }
-
             char *p = allocate_outside_nursery_large(
                           sizeof(stm_hashtable_entry_t));
             entry = (stm_hashtable_entry_t *)(p - stm_object_pages);
 
+            long j;
             for (j = 0; j <= NB_SEGMENTS; j++) {
                 struct stm_hashtable_entry_s *e;
                 e = (struct stm_hashtable_entry_s *)
@@ -336,11 +314,6 @@
                 e->object = NULL;
             }
             hashtable->additions += 0x100;
-
-            for (j = 1; j <= NB_SEGMENTS; j++) {
-                get_read_marker(get_segment_base(j), (object_t *)entry)->rm =
-                    readmarkers[j - 1];
-            }
             release_privatization_lock();
         }
         write_fence();     /* make sure 'entry' is fully initialized here */
@@ -370,15 +343,44 @@
     return e->object;
 }
 
+void stm_hashtable_write_entry(object_t *hobj, stm_hashtable_entry_t *entry,
+                               object_t *nvalue)
+{
+    if (stm_write((object_t *)entry)) {
+        uintptr_t i = list_count(STM_PSEGMENT->modified_old_objects);
+        if (i > 0 && list_item(STM_PSEGMENT->modified_old_objects, i - 1)
+                     == (uintptr_t)entry) {
+            /* 'modified_old_hashtables' is always obtained by taking
+               a subset of 'modified_old_objects' which contains only
+               stm_hashtable_entry_t objects, and then replacing the
+               stm_hashtable_entry_t objects with the hobj they come
+               from.  It's possible to have duplicates in
+               'modified_old_hashtables'; here we only try a bit to
+               avoid them --- at least the list should never be longer
+               than 'modified_old_objects'. */
+            i = list_count(STM_PSEGMENT->modified_old_hashtables);
+            if (i > 0 && list_item(STM_PSEGMENT->modified_old_hashtables, i - 1)
+                         == (uintptr_t)hobj) {
+                /* already added */
+            }
+            else {
+                LIST_APPEND(STM_PSEGMENT->modified_old_hashtables, hobj);
+            }
+        }
+    }
+    entry->object = nvalue;
+}
+
 void stm_hashtable_write(object_t *hobj, stm_hashtable_t *hashtable,
                          uintptr_t key, object_t *nvalue,
                          stm_thread_local_t *tl)
 {
     STM_PUSH_ROOT(*tl, nvalue);
+    STM_PUSH_ROOT(*tl, hobj);
     stm_hashtable_entry_t *e = stm_hashtable_lookup(hobj, hashtable, key);
-    stm_write((object_t *)e);
+    STM_POP_ROOT(*tl, hobj);
     STM_POP_ROOT(*tl, nvalue);
-    e->object = nvalue;
+    stm_hashtable_write_entry(hobj, e, nvalue);
 }
 
 long stm_hashtable_length_upper_bound(stm_hashtable_t *hashtable)
@@ -402,28 +404,15 @@
 long stm_hashtable_list(object_t *hobj, stm_hashtable_t *hashtable,
                         stm_hashtable_entry_t **results)
 {
-    stm_hashtable_table_t *table;
-    intptr_t rc;
-
     /* Set the read marker.  It will be left as long as we're running
        the same transaction.
     */
     stm_read(hobj);
 
-    /* Acquire and immediately release the lock.  We don't actually
-       need to do anything while we hold the lock, but the point is to
-       wait until the lock is available, and to synchronize other
-       threads with the stm_read() done above.
-     */
- restart:
-    table = VOLATILE_HASHTABLE(hashtable)->table;
-    rc = VOLATILE_TABLE(table)->resize_counter;
-    if (IS_EVEN(rc)) {
-        spin_loop();
-        goto restart;
-    }
-    if (!__sync_bool_compare_and_swap(&table->resize_counter, rc, rc))
-        goto restart;
+    /* Get the table.  No synchronization is needed: we may miss some
+       entries that are being added, but they would contain NULL in
+       this segment anyway. */
+    stm_hashtable_table_t *table = VOLATILE_HASHTABLE(hashtable)->table;
 
     /* Read all entries, check which ones are not NULL, count them,
        and optionally list them in 'results'.
diff --git a/rpython/translator/stm/src_stm/stm/setup.c b/rpython/translator/stm/src_stm/stm/setup.c
--- a/rpython/translator/stm/src_stm/stm/setup.c
+++ b/rpython/translator/stm/src_stm/stm/setup.c
@@ -123,6 +123,7 @@
         pr->large_overflow_objects = NULL;
         pr->modified_old_objects = list_create();
         pr->modified_old_objects_markers = list_create();
+        pr->modified_old_hashtables = list_create();
         pr->young_weakrefs = list_create();
         pr->old_weakrefs = list_create();
         pr->young_outside_nursery = tree_create();
@@ -167,6 +168,7 @@
         assert(pr->large_overflow_objects == NULL);
         list_free(pr->modified_old_objects);
         list_free(pr->modified_old_objects_markers);
+        list_free(pr->modified_old_hashtables);
         list_free(pr->young_weakrefs);
         list_free(pr->old_weakrefs);
         tree_free(pr->young_outside_nursery);
diff --git a/rpython/translator/stm/src_stm/stmgc.h b/rpython/translator/stm/src_stm/stmgc.h
--- a/rpython/translator/stm/src_stm/stmgc.h
+++ b/rpython/translator/stm/src_stm/stmgc.h
@@ -188,10 +188,13 @@
    necessary to call it immediately after stm_allocate().
 */
 __attribute__((always_inline))
-static inline void stm_write(object_t *obj)
+static inline int stm_write(object_t *obj)
 {
-    if (UNLIKELY((obj->stm_flags & _STM_GCFLAG_WRITE_BARRIER) != 0))
+    if (UNLIKELY((obj->stm_flags & _STM_GCFLAG_WRITE_BARRIER) != 0)) {
         _stm_write_slowpath(obj);
+        return 1;
+    }
+    return 0;
 }
 
 /* The following is a GC-optimized barrier that works on the granularity
@@ -545,6 +548,8 @@
 object_t *stm_hashtable_read(object_t *, stm_hashtable_t *, uintptr_t key);
 void stm_hashtable_write(object_t *, stm_hashtable_t *, uintptr_t key,
                          object_t *nvalue, stm_thread_local_t *);
+void stm_hashtable_write_entry(object_t *hobj, stm_hashtable_entry_t *entry,
+                               object_t *nvalue);
 long stm_hashtable_length_upper_bound(stm_hashtable_t *);
 long stm_hashtable_list(object_t *, stm_hashtable_t *,
                         stm_hashtable_entry_t **results);


More information about the pypy-commit mailing list