Author: Remi Meier <remi.me...@inf.ethz.ch>
Branch: card-marking
Changeset: r1218:36e751d8b104
Date: 2014-05-20 13:12 +0200
http://bitbucket.org/pypy/stmgc/changeset/36e751d8b104/

Log:    change the interface from offset to index (and finally add the
        forgotten tests)

diff --git a/c7/stm/core.c b/c7/stm/core.c
--- a/c7/stm/core.c
+++ b/c7/stm/core.c
@@ -40,7 +40,29 @@
 #endif
 }
 
-static bool _stm_write_slowpath_overflow_objs(object_t *obj, uintptr_t offset)
+static void _stm_mark_card(object_t *obj, uintptr_t card_index)
+{
+    if (!(obj->stm_flags & GCFLAG_CARDS_SET)) {
+        /* not yet in the list */
+        if (STM_PSEGMENT->old_objects_with_cards) {
+            /* if we never had a minor collection in this transaction,
+               this list doesn't exist */
+            LIST_APPEND(STM_PSEGMENT->old_objects_with_cards, obj);
+        }
+        obj->stm_flags |= GCFLAG_CARDS_SET;
+    }
+
+    /* Just acquire the corresponding lock for the next minor_collection
+       to know what may have changed.
+       We already own the object here or it is an overflow obj. */
+    uintptr_t card_lock_idx = get_write_lock_idx((uintptr_t)obj) + card_index;
+    assert(write_locks[card_lock_idx] == 0
+           || write_locks[card_lock_idx] == STM_PSEGMENT->write_lock_num);
+    if (!write_locks[card_lock_idx])
+        write_locks[card_lock_idx] = STM_PSEGMENT->write_lock_num;
+}
+
+static bool _stm_write_slowpath_overflow_objs(object_t *obj, uintptr_t 
card_index)
 {
     /* is this an object from the same transaction, outside the nursery? */
     if ((obj->stm_flags & -GCFLAG_OVERFLOW_NUMBER_bit0)
@@ -49,24 +71,15 @@
         assert(STM_PSEGMENT->objects_pointing_to_nursery != NULL);
         dprintf_test(("write_slowpath %p -> ovf obj_to_nurs\n", obj));
 
-        if (!offset) {
-            /* no card to be marked */
+        if (!card_index) {
+            /* no card to be marked, don't call again until next collection */
             obj->stm_flags &= ~GCFLAG_WRITE_BARRIER;
             LIST_APPEND(STM_PSEGMENT->objects_pointing_to_nursery, obj);
         } else {
             /* don't remove GCFLAG_WRITE_BARRIER because we need to be
                here for every card to mark */
-            if (!(obj->stm_flags & GCFLAG_CARDS_SET)) {
-                /* not yet in the list */
-                LIST_APPEND(STM_PSEGMENT->old_objects_with_cards, obj);
-                obj->stm_flags |= GCFLAG_CARDS_SET;
-            }
-
-            /* just acquire the corresponding lock for the next 
minor_collection
-               to know what may have changed. only we know about this object: 
*/
-            uintptr_t lock_idx = get_write_lock_idx((uintptr_t)obj + offset);
-            assert(!write_locks[lock_idx]);
-            write_locks[lock_idx] = STM_PSEGMENT->write_lock_num;
+            assert(STM_PSEGMENT->old_objects_with_cards);
+            _stm_mark_card(obj, card_index);
         }
 
         /* We don't need to do anything in the STM part of the WB slowpath: */
@@ -77,14 +90,18 @@
     return false;
 }
 
-void _stm_write_slowpath(object_t *obj, uintptr_t offset)
+void _stm_write_slowpath(object_t *obj, uintptr_t card_index)
 {
-    assert(IMPLY(!(obj->stm_flags & GCFLAG_HAS_CARDS), offset == 0));
+    assert(IMPLY(!(obj->stm_flags & GCFLAG_HAS_CARDS), card_index == 0));
+    assert(
+        IMPLY(card_index, (card_index - 1) * CARD_SIZE < stmcb_size_rounded_up(
+                  (struct object_s*)REAL_ADDRESS(STM_SEGMENT->segment_base,
+                                                 obj))));
     assert(_seems_to_be_running_transaction());
     assert(!_is_young(obj));
     assert(obj->stm_flags & GCFLAG_WRITE_BARRIER);
 
-    if (_stm_write_slowpath_overflow_objs(obj, offset))
+    if (_stm_write_slowpath_overflow_objs(obj, card_index))
         return;
 
     /* do a read-barrier now.  Note that this must occur before the
@@ -98,9 +115,7 @@
        'modified_old_objects' (but, because it had GCFLAG_WRITE_BARRIER,
        not in 'objects_pointing_to_nursery').  We'll detect this case
        by finding that we already own the write-lock. */
-    bool lock_whole = offset == 0;
     uintptr_t base_lock_idx = get_write_lock_idx((uintptr_t)obj);
-    //uintptr_t card_lock_idx = get_write_lock_idx((uintptr_t)obj + offset);
     uint8_t lock_num = STM_PSEGMENT->write_lock_num;
     assert(base_lock_idx < sizeof(write_locks));
  retry:
@@ -175,43 +190,36 @@
         goto retry;
     }
 
+
+    /* check that we really have a private page */
+    assert(is_private_page(STM_SEGMENT->segment_num,
+                           ((uintptr_t)obj) / 4096));
+
+    /* check that so far all copies of the object have the flag */
+    check_flag_write_barrier(obj);
+
     /* A common case for write_locks[] that was either 0 or lock_num:
        we need to add the object to the appropriate list if there is one. */
-    if (lock_whole) {
+    if (!card_index) {
         if (STM_PSEGMENT->objects_pointing_to_nursery != NULL) {
             dprintf_test(("write_slowpath %p -> old obj_to_nurs\n", obj));
             LIST_APPEND(STM_PSEGMENT->objects_pointing_to_nursery, obj);
         }
 
-        /* check that we really have a private page */
-        assert(is_private_page(STM_SEGMENT->segment_num,
-                               ((uintptr_t)obj) / 4096));
-        /* check that so far all copies of the object have the flag */
-        check_flag_write_barrier(obj);
-
         /* remove GCFLAG_WRITE_BARRIER if we succeeded in getting the base
            write-lock (not for card marking). */
         assert(obj->stm_flags & GCFLAG_WRITE_BARRIER);
         obj->stm_flags &= ~GCFLAG_WRITE_BARRIER;
 
-        /* for sanity, check again that all other segment copies of this
-           object still have the flag (so privatization worked) */
-        check_flag_write_barrier(obj);
+    } else {
+        /* don't remove WRITE_BARRIER */
+        _stm_mark_card(obj, card_index);
+    }
 
-    } else { /* card marking case */
+    /* for sanity, check again that all other segment copies of this
+       object still have the flag (so privatization worked) */
+    check_flag_write_barrier(obj);
 
-        if (!(obj->stm_flags & GCFLAG_CARDS_SET)) {
-            /* not yet in the list (may enter here multiple times) */
-            if (STM_PSEGMENT->old_objects_with_cards != NULL) {
-                LIST_APPEND(STM_PSEGMENT->old_objects_with_cards, obj);
-            }
-            obj->stm_flags |= GCFLAG_CARDS_SET;
-        }
-
-        /* check that we really have a private page */
-        assert(is_private_page(STM_SEGMENT->segment_num,
-                               ((uintptr_t)obj + offset) / 4096));
-    }
 }
 
 static void reset_transaction_read_version(void)
diff --git a/c7/stm/core.h b/c7/stm/core.h
--- a/c7/stm/core.h
+++ b/c7/stm/core.h
@@ -226,6 +226,10 @@
 
 #define REAL_ADDRESS(segment_base, src)   ((segment_base) + (uintptr_t)(src))
 
+static inline uintptr_t get_card_index(uintptr_t byte_offset) {
+    assert(_STM_CARD_SIZE == 32);
+    return (byte_offset >> 5) + 1;
+}
 static inline uintptr_t get_write_lock_idx(uintptr_t obj) {
     return (obj >> 4) - WRITELOCK_START;
 }
@@ -262,15 +266,6 @@
     return rm == other_transaction_read_version;
 }
 
-static inline bool was_read_remote_card(char *base, object_t *obj, uintptr_t 
offset,
-                                        uint8_t other_transaction_read_version)
-{
-    uint8_t rm = ((struct stm_read_marker_s *)
-                  (base + (((uintptr_t)obj + offset) >> 4)))->rm;
-    assert(rm <= other_transaction_read_version);
-    return rm == other_transaction_read_version;
-}
-
 static inline void _duck(void) {
     /* put a call to _duck() between two instructions that set 0 into
        a %gs-prefixed address and that may otherwise be replaced with
diff --git a/c7/stm/misc.c b/c7/stm/misc.c
--- a/c7/stm/misc.c
+++ b/c7/stm/misc.c
@@ -40,12 +40,6 @@
     return (obj->stm_flags & _STM_GCFLAG_WRITE_BARRIER) == 0;
 }
 
-bool _stm_was_read_card(object_t *obj, uintptr_t offset)
-{
-    return was_read_remote_card(
-        STM_SEGMENT->segment_base, obj, offset,
-        STM_SEGMENT->transaction_read_version);
-}
 
 bool _stm_was_written_card(object_t *obj)
 {
diff --git a/c7/stm/nursery.c b/c7/stm/nursery.c
--- a/c7/stm/nursery.c
+++ b/c7/stm/nursery.c
@@ -183,16 +183,32 @@
     minor_trace_if_young(&tl->thread_local_obj);
 }
 
+static __thread object_t *_card_base_obj;
 static void minor_trace_if_young_cards(object_t **pobj)
 {
-    /* XXX: maybe add a specialised stmcb_trace_cards() */
-    uintptr_t obj = (uintptr_t)((char*)pobj - STM_SEGMENT->segment_base);
-    if (write_locks[get_write_lock_idx(obj)]) {
+    /* XXX: add a specialised stmcb_trace_cards() that
+       also gives the obj-base */
+    assert(_card_base_obj);
+    uintptr_t base_lock_idx = get_write_lock_idx((uintptr_t)_card_base_obj);
+    uintptr_t card_lock_idx = base_lock_idx;
+    card_lock_idx += get_card_index(
+        (uintptr_t)((char*)pobj - STM_SEGMENT->segment_base) - 
(uintptr_t)_card_base_obj);
+
+    if (write_locks[card_lock_idx]) {
         dprintf(("minor_trace_if_young_cards: trace %p\n", *pobj));
         minor_trace_if_young(pobj);
     }
 }
 
+static void _trace_card_object(object_t *obj)
+{
+    /* XXX HACK XXX: */
+    _card_base_obj = obj;
+    assert(!_is_in_nursery(obj));
+    char *realobj = REAL_ADDRESS(STM_SEGMENT->segment_base, obj);
+    stmcb_trace((struct object_s *)realobj, &minor_trace_if_young_cards);
+}
+
 static inline void _collect_now(object_t *obj)
 {
     assert(!_is_young(obj));
@@ -212,9 +228,7 @@
     } else {
         /* only trace cards */
         dprintf(("-> has cards\n"));
-        assert(!_is_in_nursery(obj));
-        char *realobj = REAL_ADDRESS(STM_SEGMENT->segment_base, obj);
-        stmcb_trace((struct object_s *)realobj, &minor_trace_if_young_cards);
+        _trace_card_object(obj);
     }
 
     /* clear the CARDS_SET, but not the real cards since they are
diff --git a/c7/stm/setup.c b/c7/stm/setup.c
--- a/c7/stm/setup.c
+++ b/c7/stm/setup.c
@@ -84,7 +84,7 @@
     /* Check that some values are acceptable */
     assert(NB_SEGMENTS <= NB_SEGMENTS_MAX);
     assert(CARD_SIZE > 0 && CARD_SIZE % 16 == 0);
-    assert(CARD_SIZE == 16);    /* actually, it is hardcoded in some places 
right now.. */
+    assert(CARD_SIZE == 32);    /* actually, it is hardcoded in some places 
right now.. */
     assert(4096 <= ((uintptr_t)STM_SEGMENT));
     assert((uintptr_t)STM_SEGMENT == (uintptr_t)STM_PSEGMENT);
     assert(((uintptr_t)STM_PSEGMENT) + sizeof(*STM_PSEGMENT) <= 8192);
diff --git a/c7/stmgc.h b/c7/stmgc.h
--- a/c7/stmgc.h
+++ b/c7/stmgc.h
@@ -120,7 +120,6 @@
 #include <stdbool.h>
 bool _stm_was_read(object_t *obj);
 bool _stm_was_written(object_t *obj);
-bool _stm_was_read_card(object_t *obj, uintptr_t offset);
 bool _stm_was_written_card(object_t *obj);
 uintptr_t _stm_get_private_page(uintptr_t pagenum);
 bool _stm_in_transaction(stm_thread_local_t *tl);
@@ -147,7 +146,7 @@
 #define _STM_GCFLAG_WRITE_BARRIER      0x01
 #define _STM_GCFLAG_HAS_CARDS          0x08
 #define _STM_GCFLAG_CARDS_SET          0x10
-#define _STM_CARD_SIZE                 16 /* modulo 16 == 0! */
+#define _STM_CARD_SIZE                 32 /* 16 may be safe too */
 #define _STM_NSE_SIGNAL_MAX     _STM_TIME_N
 #define _STM_FAST_ALLOC           (66*1024)
 
@@ -218,21 +217,18 @@
         _stm_write_slowpath(obj, 0);
 }
 
-/* The following are barriers that work on the granularity of CARD_SIZE.
-   They can only be used on objects one called stm_use_cards() on. */
+/* The following is a GC-optimized barrier that works on the granularity
+   of CARD_SIZE. It can only be used on objects one called stm_use_cards()
+   on. It has the same purpose as stm_write() for TM.
+   'index' is the byte-offset into the object divided by _STM_CARD_SIZE
+   plus 1: (offset // CARD_SIZE) + 1
+*/
 __attribute__((always_inline))
-static inline void stm_read_card(object_t *obj, uintptr_t offset)
-{
-    OPT_ASSERT(obj->stm_flags & _STM_GCFLAG_HAS_CARDS);
-    ((stm_read_marker_t *)(((uintptr_t)obj + offset) >> 4))->rm =
-        STM_SEGMENT->transaction_read_version;
-}
-__attribute__((always_inline))
-static inline void stm_write_card(object_t *obj, uintptr_t offset)
+static inline void stm_write_card(object_t *obj, uintptr_t index)
 {
     OPT_ASSERT(obj->stm_flags & _STM_GCFLAG_HAS_CARDS);
     if (UNLIKELY((obj->stm_flags & _STM_GCFLAG_WRITE_BARRIER) != 0))
-        _stm_write_slowpath(obj, offset);
+        _stm_write_slowpath(obj, index);
 }
 
 /* Must be provided by the user of this library.
diff --git a/c7/test/support.py b/c7/test/support.py
--- a/c7/test/support.py
+++ b/c7/test/support.py
@@ -44,7 +44,6 @@
 object_t *_stm_allocate_old(ssize_t size_rounded_up);
 
 void stm_use_cards(object_t* o);
-void stm_read_card(object_t *obj, uintptr_t offset);
 /*void stm_write_card(); use _checked_stm_write_card() instead */
 
 
@@ -56,9 +55,8 @@
 object_t *stm_setup_prebuilt_weakref(object_t *);
 
 bool _checked_stm_write(object_t *obj);
-bool _checked_stm_write_card(object_t *obj, uintptr_t offset);
+bool _checked_stm_write_card(object_t *obj, uintptr_t index);
 bool _stm_was_read(object_t *obj);
-bool _stm_was_read_card(object_t *obj, uintptr_t offset);
 bool _stm_was_written(object_t *obj);
 bool _stm_was_written_card(object_t *obj);
 char *_stm_real_address(object_t *obj);
@@ -79,7 +77,7 @@
 uint32_t _get_type_id(object_t *obj);
 void _set_ptr(object_t *obj, int n, object_t *v);
 object_t * _get_ptr(object_t *obj, int n);
-uintptr_t _index_to_offset(object_t *obj, int n);
+uintptr_t _index_to_card_index(object_t *obj, int n);
 
 void _set_weakref(object_t *obj, object_t *v);
 object_t* _get_weakref(object_t *obj);
@@ -192,8 +190,8 @@
     CHECKED(stm_write(object));
 }
 
-bool _checked_stm_write_card(object_t *object, uintptr_t offset) {
-    CHECKED(stm_write_card(object, offset));
+bool _checked_stm_write_card(object_t *object, uintptr_t index) {
+    CHECKED(stm_write_card(object, index));
 }
 
 bool _check_stop_safe_point(void) {
@@ -268,7 +266,7 @@
     return *field;
 }
 
-uintptr_t _index_to_offset(object_t *obj, int n)
+uintptr_t _index_to_card_index(object_t *obj, int n)
 {
     long nrefs = (long)((myobj_t*)obj)->type_id - 421420;
     assert(n < nrefs);
@@ -276,7 +274,7 @@
     stm_char *field_addr = NULL;
     field_addr += SIZEOF_MYOBJ; /* header */
     field_addr += n * sizeof(void*); /* field */
-    return (uintptr_t)field_addr;
+    return ((uintptr_t)field_addr / _STM_CARD_SIZE) + 1;
 }
 
 ssize_t stmcb_size_rounded_up(struct object_s *obj)
@@ -358,6 +356,8 @@
 
 class EmptyStack(Exception):
     pass
+def byte_offset_to_card_index(offset):
+    return (offset // CARD_SIZE) + 1
 
 def is_in_nursery(o):
     return lib.stm_can_move(o)
@@ -408,32 +408,27 @@
 
 def stm_set_ref(obj, idx, ref, use_cards=False):
     if use_cards:
-        stm_write_card(obj, lib._index_to_offset(obj, idx))
+        stm_write_card(obj, lib._index_to_card_index(obj, idx))
     else:
         stm_write(obj)
     lib._set_ptr(obj, idx, ref)
 
-def stm_get_ref(obj, idx, use_cards=False):
-    if use_cards:
-        stm_read_card(obj, lib._index_to_offset(obj, idx))
-    else:
-        stm_read(obj)
+def stm_get_ref(obj, idx):
+    stm_read(obj)
     return lib._get_ptr(obj, idx)
 
 def stm_set_char(obj, c, offset=HDR, use_cards=False):
     assert HDR <= offset < stm_get_obj_size(obj)
     if use_cards:
-        stm_write_card(obj, offset)
+        index = byte_offset_to_card_index(offset)
+        stm_write_card(obj, index)
     else:
         stm_write(obj)
     stm_get_real_address(obj)[offset] = c
 
-def stm_get_char(obj, offset=HDR, use_cards=False):
+def stm_get_char(obj, offset=HDR):
     assert HDR <= offset < stm_get_obj_size(obj)
-    if use_cards:
-        stm_read_card(obj, offset)
-    else:
-        stm_read(obj)
+    stm_read(obj)
     return stm_get_real_address(obj)[offset]
 
 def stm_get_real_address(obj):
@@ -442,19 +437,14 @@
 def stm_read(o):
     lib.stm_read(o)
 
-def stm_read_card(o, offset):
-    assert stm_get_flags(o) & GCFLAG_HAS_CARDS
-    assert offset < stm_get_obj_size(o)
-    lib.stm_read_card(o, offset)
 
 def stm_write(o):
     if lib._checked_stm_write(o):
         raise Conflict()
 
-def stm_write_card(o, offset):
+def stm_write_card(o, index):
     assert stm_get_flags(o) & GCFLAG_HAS_CARDS
-    assert offset < stm_get_obj_size(o)
-    if lib._checked_stm_write_card(o, offset):
+    if lib._checked_stm_write_card(o, index):
         raise Conflict()
 
 def stm_was_read(o):
@@ -463,9 +453,6 @@
 def stm_was_written(o):
     return lib._stm_was_written(o)
 
-def stm_was_read_card(o, offset):
-    return lib._stm_was_read_card(o, offset)
-
 def stm_was_written_card(o):
     return lib._stm_was_written_card(o)
 
diff --git a/c7/test/test_card_marking.py b/c7/test/test_card_marking.py
new file mode 100644
--- /dev/null
+++ b/c7/test/test_card_marking.py
@@ -0,0 +1,56 @@
+from support import *
+import py
+
+class TestBasic(BaseTest):
+
+    def test_simple(self):
+        o = stm_allocate_old(1024, True)
+        self.start_transaction()
+        stm_read(o)
+        stm_write(o)
+        self.commit_transaction()
+
+
+    def test_simple2(self):
+        o = stm_allocate_old(1024, True)
+        self.start_transaction()
+        stm_write_card(o, 5)
+        assert not stm_was_written(o) # don't remove GCFLAG_WRITE_BARRIER
+        assert stm_was_written_card(o)
+        self.commit_transaction()
+
+    def test_overflow(self):
+        self.start_transaction()
+        o = stm_allocate(1024, True)
+        self.push_root(o)
+        stm_minor_collect()
+        o = self.pop_root()
+        stm_write_card(o, 5)
+        # don't remove GCFLAG_WB
+        assert not stm_was_written(o)
+        stm_write(o)
+        assert stm_was_written(o)
+        self.commit_transaction()
+
+    def test_nursery(self):
+        o = stm_allocate_old_refs(200, True)
+        self.start_transaction()
+        p = stm_allocate(64, True)
+        d = stm_allocate(64, True)
+        stm_set_ref(o, 199, p, True)
+
+        # without a write-barrier:
+        lib._set_ptr(o, 0, d)
+
+        self.push_root(o)
+        stm_minor_collect()
+        o = self.pop_root()
+
+        pn = stm_get_ref(o, 199)
+        assert not is_in_nursery(pn)
+        assert pn != p
+
+        # d was not traced!
+        dn = stm_get_ref(o, 0)
+        assert is_in_nursery(dn)
+        assert dn == d
_______________________________________________
pypy-commit mailing list
pypy-commit@python.org
https://mail.python.org/mailman/listinfo/pypy-commit

Reply via email to