[pypy-commit] stmgc c8-new-page-handling: don't apply undo copies in stm_validate() if we have our own modified version (probably not the whole story yet..)

[Raemi]

Author: Remi Meier <remi.meier at inf.ethz.ch>
Branch: c8-new-page-handling
Changeset: r1432:e565123efe0d
Date: 2014-09-26 16:01 +0200
http://bitbucket.org/pypy/stmgc/changeset/e565123efe0d/

Log:	don't apply undo copies in stm_validate() if we have our own
	modified version (probably not the whole story yet..)

diff --git a/c8/stm/core.c b/c8/stm/core.c
--- a/c8/stm/core.c
+++ b/c8/stm/core.c
@@ -9,7 +9,8 @@
    inner loop.
 */
 static void import_objects(
-        int from_segnum,            /* or -1: from undo->backup */
+        int from_segnum,            /* or -1: from undo->backup,
+                                       or -2: from undo->backup if not stm_was_read(obj) */
         uintptr_t pagenum,          /* or -1: "all accessible" */
         struct stm_undo_s *undo,
         struct stm_undo_s *end)
@@ -32,6 +33,9 @@
                 continue;
         }
 
+        if (from_segnum == -2 && _stm_was_read(obj))
+            continue;           /* only copy unmodified */
+
         dprintf(("import slice seg=%d obj=%p off=%lu sz=%d pg=%lu\n",
                  from_segnum, obj, SLICE_OFFSET(undo->slice),
                  SLICE_SIZE(undo->slice), current_page_num));
@@ -48,7 +52,8 @@
 
 /* ############# signal handler ############# */
 
-static void copy_bk_objs_in_page_from(int from_segnum, uintptr_t pagenum)
+static void copy_bk_objs_in_page_from(int from_segnum, uintptr_t pagenum,
+                                      bool only_if_not_modified)
 {
     /* looks at all bk copies of objects overlapping page 'pagenum' and
        copies the part in 'pagenum' back to the current segment */
@@ -58,7 +63,8 @@
     struct stm_undo_s *undo = (struct stm_undo_s *)list->items;
     struct stm_undo_s *end = (struct stm_undo_s *)(list->items + list->count);
 
-    import_objects(-1, pagenum, undo, end);
+    import_objects(only_if_not_modified ? -2 : -1,
+                   pagenum, undo, end);
 }
 
 static void go_to_the_past(uintptr_t pagenum,
@@ -138,7 +144,7 @@
              (char*)(get_virt_page_of(copy_from_segnum, pagenum) * 4096UL));
 
     /* if there were modifications in the page, revert them. */
-    copy_bk_objs_in_page_from(copy_from_segnum, pagenum);
+    copy_bk_objs_in_page_from(copy_from_segnum, pagenum, false);
 
     /* we need to go from 'src_version' to 'target_version'.  This
        might need a walk into the past. */
@@ -290,14 +296,15 @@
             }
         }
 
-        struct stm_undo_s *undo = cl->written;
-        struct stm_undo_s *end = cl->written + cl->written_count;
+        if (cl->written_count) {
+            struct stm_undo_s *undo = cl->written;
+            struct stm_undo_s *end = cl->written + cl->written_count;
 
-        segment_copied_from |= (1UL << cl->segment_num);
-        import_objects(cl->segment_num, -1, undo, end);
+            segment_copied_from |= (1UL << cl->segment_num);
+            import_objects(cl->segment_num, -1, undo, end);
+        }
 
         /* last fully validated entry */
-
         STM_PSEGMENT->last_commit_log_entry = cl;
     }
 
@@ -308,7 +315,10 @@
     for (segnum = 0; segment_copied_from != 0; segnum++) {
         if (segment_copied_from & (1UL << segnum)) {
             segment_copied_from &= ~(1UL << segnum);
-            copy_bk_objs_in_page_from(segnum, -1);
+            /* here we can actually have our own modified version, so
+               make sure to only copy things that are not modified in our
+               segment... */
+            copy_bk_objs_in_page_from(segnum, -1, true);
         }
     }
 
diff --git a/c8/test/test_basic.py b/c8/test/test_basic.py
--- a/c8/test/test_basic.py
+++ b/c8/test/test_basic.py
@@ -763,3 +763,23 @@
 
         # seg1 segfaults, validates, and aborts:
         py.test.raises(Conflict, stm_get_char, lp2)
+
+    def test_bug(self):
+        lp_char_5 = stm_allocate_old(384)
+
+        self.start_transaction() # R1
+        stm_set_char(lp_char_5, 'i', 384 - 1, False)
+        stm_set_char(lp_char_5, 'i', HDR, False)
+        #
+        #
+        self.switch(3)
+        self.start_transaction()  # R1
+        self.commit_transaction() # R2
+
+        self.start_transaction()  # R2
+        stm_set_char(lp_char_5, 'o', 384 - 1, False) # bk_copy
+        stm_set_char(lp_char_5, 'o', HDR, False)
+        #
+        #
+        self.switch(0) # validate -> R2
+        assert stm_get_char(lp_char_5, 384 - 1) == 'i'
diff --git a/c8/test/test_random.py b/c8/test/test_random.py
--- a/c8/test/test_random.py
+++ b/c8/test/test_random.py
@@ -497,7 +497,8 @@
 
 def op_switch_thread(ex, global_state, thread_state, new_thread_state=None):
     if new_thread_state is None:
-        new_thread_state = global_state.rnd.choice(global_state.thread_states)
+        new_thread_state = global_state.rnd.choice(
+            global_state.thread_states + [thread_state] * 3) # more likely not switch
 
     if new_thread_state != thread_state:
         if thread_state.transaction_state: