[pypy-commit] stmgc card-marking: Try to cope with the missing HAS_CARDS flag and the refactoring. Unclear if

Raemi noreply at buildbot.pypy.org
Thu Jun 19 10:56:54 CEST 2014


Author: Remi Meier <remi.meier at inf.ethz.ch>
Branch: card-marking
Changeset: r1251:aa5874a86f3a
Date: 2014-06-19 10:57 +0200
http://bitbucket.org/pypy/stmgc/changeset/aa5874a86f3a/

Log:	Try to cope with the missing HAS_CARDS flag and the refactoring.
	Unclear if better, since it is slower in some cases. Not sure if I
	grasped the full idea.

diff --git a/c7/stm/core.c b/c7/stm/core.c
--- a/c7/stm/core.c
+++ b/c7/stm/core.c
@@ -48,8 +48,6 @@
        i.e. it comes from before the most recent minor collection.
     */
     assert(STM_PSEGMENT->objects_pointing_to_nursery != NULL);
-    dprintf_test(("write_slowpath %p -> ovf obj_to_nurs, index:%lu\n",
-                  obj, mark_card ? index : (uintptr_t)-1));
 
     assert(obj->stm_flags & GCFLAG_WRITE_BARRIER);
     if (!mark_card) {
@@ -57,6 +55,11 @@
            into 'objects_pointing_to_nursery', and remove the flag so
            that the write_slowpath will not be called again until the
            next minor collection. */
+        if (obj->stm_flags & GCFLAG_CARDS_SET) {
+            /* if we clear this flag, we also need to clear the cards */
+            _reset_object_cards(get_priv_segment(STM_SEGMENT->segment_num),
+                                obj, CARD_CLEAR, false);
+        }
         obj->stm_flags &= ~(GCFLAG_WRITE_BARRIER | GCFLAG_CARDS_SET);
         LIST_APPEND(STM_PSEGMENT->objects_pointing_to_nursery, obj);
     }
@@ -195,6 +198,13 @@
             LIST_APPEND(STM_PSEGMENT->objects_pointing_to_nursery, obj);
         }
 
+        if (obj->stm_flags & GCFLAG_CARDS_SET) {
+            /* if we clear this flag, we have to tell sync_old_objs that
+               everything needs to be synced */
+            _reset_object_cards(get_priv_segment(STM_SEGMENT->segment_num),
+                                obj, CARD_MARKED_OLD, true); /* mark all */
+        }
+
         /* remove GCFLAG_WRITE_BARRIER if we succeeded in getting the base
            write-lock (not for card marking). */
         obj->stm_flags &= ~(GCFLAG_WRITE_BARRIER | GCFLAG_CARDS_SET);
@@ -213,7 +223,19 @@
 
 void _stm_write_slowpath(object_t *obj)
 {
-    write_slowpath_common(obj, /*mark_card=*/false, -1);
+    write_slowpath_common(obj, /*mark_card=*/false);
+}
+
+static bool obj_should_use_cards(object_t *obj)
+{
+    struct object_s *realobj = (struct object_s *)
+        REAL_ADDRESS(STM_SEGMENT->segment_base, obj);
+    size_t size = stmcb_size_rounded_up(realobj);
+
+    if (size < _STM_MIN_CARD_OBJ_SIZE)
+        return false;
+
+    return !!stmcb_should_use_cards(realobj);
 }
 
 void _stm_write_slowpath_card(object_t *obj, uintptr_t index)
@@ -223,12 +245,15 @@
        card marking instead.
     */
     if (!(obj->stm_flags & GCFLAG_CARDS_SET)) {
-        bool mark_card = obj_uses_cards(obj);
+        bool mark_card = obj_should_use_cards(obj);
         write_slowpath_common(obj, mark_card);
         if (!mark_card)
             return;
     }
 
+    dprintf_test(("write_slowpath_card %p -> index:%lu\n",
+                  obj, index));
+
     /* We reach this point if we have to mark the card.
      */
     assert(obj->stm_flags & GCFLAG_WRITE_BARRIER);
@@ -258,9 +283,10 @@
     /* More debug checks */
     dprintf(("mark %p index %lu, card:%lu with %d\n",
              obj, index, get_index_to_card_index(index), CARD_MARKED));
-    assert(IMPLY(IS_OVERFLOW_OBJ(obj), write_locks[base_lock_idx] == 0));
-    assert(IMPLY(!IS_OVERFLOW_OBJ(obj), write_locks[base_lock_idx] ==
-                                            STM_PSEGMENT->write_lock_num));
+    assert(IMPLY(IS_OVERFLOW_OBJ(STM_PSEGMENT, obj),
+                 write_locks[base_lock_idx] == 0));
+    assert(IMPLY(!IS_OVERFLOW_OBJ(STM_PSEGMENT, obj),
+                 write_locks[base_lock_idx] == STM_PSEGMENT->write_lock_num));
 }
 
 static void reset_transaction_read_version(void)
@@ -530,7 +556,7 @@
 
 static void _card_wise_synchronize_object_now(object_t *obj)
 {
-    assert(obj->stm_flags & GCFLAG_HAS_CARDS);
+    assert(obj_should_use_cards(obj));
     assert(!(obj->stm_flags & GCFLAG_CARDS_SET));
     assert(!IS_OVERFLOW_OBJ(STM_PSEGMENT, obj));
 
@@ -563,6 +589,7 @@
        of pages (except _has_private_page_in_range) */
     uintptr_t base_offset;
     ssize_t item_size;
+    bool all_cards_were_cleared = true;
     stmcb_get_card_base_itemsize(realobj, &base_offset, &item_size);
 
     uintptr_t start_card_index = -1;
@@ -573,6 +600,7 @@
         OPT_ASSERT(card_value != CARD_MARKED); /* always only MARKED_OLD or CLEAR */
 
         if (card_value == CARD_MARKED_OLD) {
+            all_cards_were_cleared = false;
             write_locks[card_lock_idx] = CARD_CLEAR;
 
             if (start_card_index == -1) {   /* first marked card */
@@ -637,6 +665,13 @@
         card_index++;
     }
 
+    if (all_cards_were_cleared) {
+        /* well, seems like we never called stm_write_card() on it, so actually
+           we need to fall back to synchronize the whole object */
+        _page_wise_synchronize_object_now(obj);
+        return;
+    }
+
 #ifndef NDEBUG
     char *src = REAL_ADDRESS(stm_object_pages, (uintptr_t)obj);
     char *dst;
@@ -661,9 +696,9 @@
     assert(STM_PSEGMENT->privatization_lock == 1);
 
     if (obj->stm_flags & GCFLAG_SMALL_UNIFORM) {
-        assert(!(obj->stm_flags & GCFLAG_HAS_CARDS));
+        assert(!(obj->stm_flags & GCFLAG_CARDS_SET));
         abort();//XXX WRITE THE FAST CASE
-    } else if (ignore_cards || !(obj->stm_flags & GCFLAG_HAS_CARDS)) {
+    } else if (ignore_cards || !obj_should_use_cards(obj)) {
         _page_wise_synchronize_object_now(obj);
     } else {
         _card_wise_synchronize_object_now(obj);
@@ -722,7 +757,7 @@
     /* reset these lists to NULL for the next transaction */
     _verify_cards_cleared_in_all_lists(get_priv_segment(STM_SEGMENT->segment_num));
     LIST_FREE(STM_PSEGMENT->objects_pointing_to_nursery);
-    LIST_FREE(STM_PSEGMENT->old_objects_with_cards);
+    list_clear(STM_PSEGMENT->old_objects_with_cards);
     LIST_FREE(STM_PSEGMENT->large_overflow_objects);
 
     timing_end_transaction(attribute_to);
@@ -833,7 +868,7 @@
             ssize_t size = stmcb_size_rounded_up((struct object_s *)src);
             memcpy(dst, src, size);
 
-            if (item->stm_flags & GCFLAG_HAS_CARDS)
+            if (obj_should_use_cards(item))
                 _reset_object_cards(pseg, item, CARD_CLEAR, false);
 
             /* objects in 'modified_old_objects' usually have the
@@ -910,7 +945,7 @@
 
     /* reset these lists to NULL too on abort */
     LIST_FREE(pseg->objects_pointing_to_nursery);
-    LIST_FREE(pseg->old_objects_with_cards);
+    list_clear(pseg->old_objects_with_cards);
     LIST_FREE(pseg->large_overflow_objects);
     list_clear(pseg->young_weakrefs);
 #pragma pop_macro("STM_SEGMENT")
diff --git a/c7/stm/core.h b/c7/stm/core.h
--- a/c7/stm/core.h
+++ b/c7/stm/core.h
@@ -56,9 +56,10 @@
        after the object. */
     GCFLAG_HAS_SHADOW = 0x04,
 
-    /* Set on objects that are large enough to have multiple cards
-       (at least _STM_MIN_CARD_COUNT), and that have at least one card
-       marked.  This flag implies GCFLAG_WRITE_BARRIER. */
+    /* Set on objects that are large enough (_STM_MIN_CARD_OBJ_SIZE)
+       to have multiple cards (at least _STM_MIN_CARD_COUNT), and that
+       have at least one card marked.  This flag implies
+       GCFLAG_WRITE_BARRIER. */
     GCFLAG_CARDS_SET = _STM_GCFLAG_CARDS_SET,
 
     /* All remaining bits of the 32-bit 'stm_flags' field are taken by
diff --git a/c7/stm/gcpage.c b/c7/stm/gcpage.c
--- a/c7/stm/gcpage.c
+++ b/c7/stm/gcpage.c
@@ -119,8 +119,6 @@
 
     object_t *o = (object_t *)(p - stm_object_pages);
     o->stm_flags = GCFLAG_WRITE_BARRIER;
-    if (use_cards && size_rounded_up > CARD_SIZE)
-        o->stm_flags |= GCFLAG_HAS_CARDS;
 
     if (testing_prebuilt_objs == NULL)
         testing_prebuilt_objs = list_create();
@@ -472,42 +470,39 @@
 
                     realobj->stm_flags |= GCFLAG_WRITE_BARRIER;
 
-                    /* logic corresponds to _collect_now() in nursery.c */
-                    if (realobj->stm_flags & GCFLAG_HAS_CARDS) {
-                        /* We called a normal WB on these objs. If we wrote
-                           a value to some place in them, we need to
-                           synchronise the whole object on commit */
-                        if (IS_OVERFLOW_OBJ(pseg, realobj)) {
-                            /* we do not need the old cards for overflow objects */
+                    if (realobj->stm_flags & GCFLAG_CARDS_SET) {
+                        /* we called a normal WB on this object, so all cards
+                           need to be marked OLD */
+                        if (!IS_OVERFLOW_OBJ(pseg, realobj)) {
+                            _reset_object_cards(pseg, item, CARD_MARKED_OLD, true); /* mark all */
+                        } else {
+                            /* simply clear overflow */
                             _reset_object_cards(pseg, item, CARD_CLEAR, false);
-                        } else {
-                            _reset_object_cards(pseg, item, CARD_MARKED_OLD, true); /* mark all */
                         }
                     }
                 }));
             list_clear(lst);
-
-            lst = pseg->old_objects_with_cards;
-            LIST_FOREACH_R(lst, object_t* /*item*/,
-                ({
-                    struct object_s *realobj = (struct object_s *)
-                        REAL_ADDRESS(pseg->pub.segment_base, item);
-                    OPT_ASSERT(realobj->stm_flags & GCFLAG_CARDS_SET);
-                    OPT_ASSERT(realobj->stm_flags & GCFLAG_WRITE_BARRIER);
-
-                    /* logic corresponds to _trace_card_object() in nursery.c */
-                    uint8_t mark_value = IS_OVERFLOW_OBJ(pseg, realobj) ?
-                        CARD_CLEAR : CARD_MARKED_OLD;
-                    _reset_object_cards(pseg, item, mark_value, false);
-                }));
-            list_clear(lst);
-
         } else {
             /* if here MINOR_NOTHING_TO_DO() was true before, it's like
                we "didn't do a collection" at all. So nothing to do on
                modified_old_objs. */
         }
 
+        lst = pseg->old_objects_with_cards;
+        LIST_FOREACH_R(lst, object_t* /*item*/,
+            ({
+                struct object_s *realobj = (struct object_s *)
+                    REAL_ADDRESS(pseg->pub.segment_base, item);
+                OPT_ASSERT(realobj->stm_flags & GCFLAG_CARDS_SET);
+                OPT_ASSERT(realobj->stm_flags & GCFLAG_WRITE_BARRIER);
+
+                /* clear cards if overflow, or mark marked cards as old otherwise */
+                uint8_t mark_value = IS_OVERFLOW_OBJ(pseg, realobj) ?
+                    CARD_CLEAR : CARD_MARKED_OLD;
+                _reset_object_cards(pseg, item, mark_value, false);
+            }));
+        list_clear(lst);
+
         /* Remove from 'large_overflow_objects' all objects that die */
         lst = pseg->large_overflow_objects;
         if (lst != NULL) {
diff --git a/c7/stm/nursery.c b/c7/stm/nursery.c
--- a/c7/stm/nursery.c
+++ b/c7/stm/nursery.c
@@ -114,8 +114,6 @@
          copy_large_object:;
             char *realnobj = REAL_ADDRESS(STM_SEGMENT->segment_base, nobj);
             memcpy(realnobj, realobj, size);
-            if (size > CARD_SIZE && stmcb_should_use_cards((struct object_s*)realnobj))
-                nobj->stm_flags |= GCFLAG_HAS_CARDS;
 
             nobj_sync_now = ((uintptr_t)nobj) | FLAG_SYNC_LARGE;
         }
@@ -141,11 +139,6 @@
         nobj = obj;
         tree_delete_item(STM_PSEGMENT->young_outside_nursery, (uintptr_t)nobj);
         nobj_sync_now = ((uintptr_t)nobj) | FLAG_SYNC_LARGE;
-
-        realobj = REAL_ADDRESS(STM_SEGMENT->segment_base, obj);
-        size = stmcb_size_rounded_up((struct object_s *)realobj);
-        if (size > CARD_SIZE && stmcb_should_use_cards((struct object_s*)realobj))
-            nobj->stm_flags |= GCFLAG_HAS_CARDS;
     }
 
     /* Set the overflow_number if nedeed */
@@ -197,8 +190,8 @@
     struct object_s *realobj = (struct object_s *)REAL_ADDRESS(pseg->pub.segment_base, obj);
     size_t size = stmcb_size_rounded_up(realobj);
 
-    if (!(realobj->stm_flags & GCFLAG_HAS_CARDS))
-        return;
+    if (size < _STM_MIN_CARD_OBJ_SIZE)
+        return;                 /* too small for cards */
 
     uintptr_t first_card_index = get_write_lock_idx((uintptr_t)obj);
     uintptr_t card_index = 1;
@@ -233,11 +226,9 @@
             pseg->objects_pointing_to_nursery, object_t * /*item*/,
             _cards_cleared_in_object(pseg, item));
     }
-    if (pseg->old_objects_with_cards) {
-        LIST_FOREACH_R(
-            pseg->old_objects_with_cards, object_t * /*item*/,
-            _cards_cleared_in_object(pseg, item));
-    }
+    LIST_FOREACH_R(
+        pseg->old_objects_with_cards, object_t * /*item*/,
+        _cards_cleared_in_object(pseg, item));
 #endif
 }
 
@@ -252,8 +243,7 @@
     struct object_s *realobj = (struct object_s *)REAL_ADDRESS(pseg->pub.segment_base, obj);
     size_t size = stmcb_size_rounded_up(realobj);
 
-    OPT_ASSERT(size >= 32);
-    assert(realobj->stm_flags & GCFLAG_HAS_CARDS);
+    OPT_ASSERT(size >= _STM_MIN_CARD_OBJ_SIZE);
     assert(IMPLY(mark_value == CARD_CLEAR, !mark_all)); /* not necessary */
     assert(IMPLY(mark_all, mark_value == CARD_MARKED_OLD)); /* set *all* to OLD */
     assert(IMPLY(IS_OVERFLOW_OBJ(pseg, realobj),
@@ -349,29 +339,24 @@
         stmcb_trace((struct object_s *)realobj, &minor_trace_if_young);
 
         obj->stm_flags |= GCFLAG_WRITE_BARRIER;
-        if (obj->stm_flags & GCFLAG_HAS_CARDS) {
+        if (obj->stm_flags & GCFLAG_CARDS_SET) {
             /* all objects that had WB cleared need to be fully synchronised
                on commit, so we have to mark all their cards */
             struct stm_priv_segment_info_s *pseg = get_priv_segment(
                 STM_SEGMENT->segment_num);
 
-            if (was_definitely_young) {
-                /* stm_wb-slowpath should never have triggered for young objs */
-                assert(!(obj->stm_flags & GCFLAG_CARDS_SET));
-                return;
-            }
+            /* stm_wb-slowpath should never have triggered for young objs */
+            assert(!was_definitely_young);
 
-            if (IS_OVERFLOW_OBJ(STM_PSEGMENT, obj)) {
-                /* we do not need the old cards for overflow objects */
+            if (!IS_OVERFLOW_OBJ(STM_PSEGMENT, obj)) {
+                _reset_object_cards(pseg, obj, CARD_MARKED_OLD, true); /* mark all */
+            } else {
+                /* simply clear overflow */
                 _reset_object_cards(pseg, obj, CARD_CLEAR, false);
-            } else {
-                _reset_object_cards(pseg, obj, CARD_MARKED_OLD, true); /* mark all */
             }
         }
-    } if (obj->stm_flags & GCFLAG_CARDS_SET) {
-        assert(!was_definitely_young);
-        _trace_card_object(obj);
     }
+    /* else traced in collect_cardrefs_to_nursery if necessary */
 }
 
 
@@ -384,8 +369,16 @@
         object_t *obj = (object_t*)list_pop_item(lst);
 
         assert(!_is_young(obj));
-        assert(obj->stm_flags & GCFLAG_CARDS_SET);
-        _collect_now(obj, false);
+
+        if (!(obj->stm_flags & GCFLAG_CARDS_SET)) {
+            /* handled in _collect_now() */
+            continue;
+        }
+
+        /* traces cards, clears marked cards or marks them old if
+           necessary */
+        _trace_card_object(obj);
+
         assert(!(obj->stm_flags & GCFLAG_CARDS_SET));
     }
 }
@@ -563,9 +556,7 @@
        to hold the ones we didn't trace so far. */
     uintptr_t num_old;
     if (STM_PSEGMENT->objects_pointing_to_nursery == NULL) {
-        assert(STM_PSEGMENT->old_objects_with_cards == NULL);
         STM_PSEGMENT->objects_pointing_to_nursery = list_create();
-        STM_PSEGMENT->old_objects_with_cards = list_create();
 
         /* See the doc of 'objects_pointing_to_nursery': if it is NULL,
            then it is implicitly understood to be equal to
diff --git a/c7/stm/setup.c b/c7/stm/setup.c
--- a/c7/stm/setup.c
+++ b/c7/stm/setup.c
@@ -118,7 +118,7 @@
         pr->pub.segment_num = i;
         pr->pub.segment_base = segment_base;
         pr->objects_pointing_to_nursery = NULL;
-        pr->old_objects_with_cards = NULL;
+        pr->old_objects_with_cards = list_create();
         pr->large_overflow_objects = NULL;
         pr->modified_old_objects = list_create();
         pr->modified_old_objects_markers = list_create();
@@ -158,7 +158,7 @@
     for (i = 1; i <= NB_SEGMENTS; i++) {
         struct stm_priv_segment_info_s *pr = get_priv_segment(i);
         assert(pr->objects_pointing_to_nursery == NULL);
-        assert(pr->old_objects_with_cards == NULL);
+        list_free(pr->old_objects_with_cards);
         assert(pr->large_overflow_objects == NULL);
         list_free(pr->modified_old_objects);
         list_free(pr->modified_old_objects_markers);
diff --git a/c7/stmgc.h b/c7/stmgc.h
--- a/c7/stmgc.h
+++ b/c7/stmgc.h
@@ -150,6 +150,7 @@
 #define _STM_GCFLAG_CARDS_SET          0x08
 #define _STM_CARD_SIZE                 32     /* must be >= 32 */
 #define _STM_MIN_CARD_COUNT            17
+#define _STM_MIN_CARD_OBJ_SIZE         (_STM_CARD_SIZE * _STM_MIN_CARD_COUNT)
 #define _STM_NSE_SIGNAL_MAX     _STM_TIME_N
 #define _STM_FAST_ALLOC           (66*1024)
 
diff --git a/c7/test/support.py b/c7/test/support.py
--- a/c7/test/support.py
+++ b/c7/test/support.py
@@ -372,7 +372,6 @@
 HDR = lib.SIZEOF_MYOBJ
 assert HDR == 8
 GCFLAG_WRITE_BARRIER = lib._STM_GCFLAG_WRITE_BARRIER
-GCFLAG_HAS_CARDS = lib._STM_GCFLAG_HAS_CARDS
 CARD_SIZE = lib._STM_CARD_SIZE # 16b at least
 NB_SEGMENTS = lib.STM_NB_SEGMENTS
 FAST_ALLOC = lib._STM_FAST_ALLOC


More information about the pypy-commit mailing list