[pypy-commit] stmgc marker: hg merge default

arigo noreply at buildbot.pypy.org
Mon Apr 28 17:16:17 CEST 2014


Author: Armin Rigo <arigo at tunes.org>
Branch: marker
Changeset: r1187:06fa05eeb305
Date: 2014-04-28 17:15 +0200
http://bitbucket.org/pypy/stmgc/changeset/06fa05eeb305/

Log:	hg merge default

diff --git a/c7/demo/demo2.c b/c7/demo/demo2.c
--- a/c7/demo/demo2.c
+++ b/c7/demo/demo2.c
@@ -44,6 +44,8 @@
     visit((object_t **)&n->next);
 }
 
+void stmcb_commit_soon() {}
+
 static void expand_marker(char *base, uintptr_t odd_number,
                           object_t *following_object,
                           char *outputbuf, size_t outputbufsize)
diff --git a/c7/demo/demo_largemalloc.c b/c7/demo/demo_largemalloc.c
--- a/c7/demo/demo_largemalloc.c
+++ b/c7/demo/demo_largemalloc.c
@@ -23,6 +23,8 @@
     abort();
 }
 
+void stmcb_commit_soon() {}
+
 /************************************************************/
 
 #define ARENA_SIZE  (1024*1024*1024)
diff --git a/c7/demo/demo_random.c b/c7/demo/demo_random.c
--- a/c7/demo/demo_random.c
+++ b/c7/demo/demo_random.c
@@ -79,6 +79,8 @@
     assert(n->next == *last_next);
 }
 
+void stmcb_commit_soon() {}
+
 int get_rand(int max)
 {
     if (max == 0)
diff --git a/c7/demo/demo_simple.c b/c7/demo/demo_simple.c
--- a/c7/demo/demo_simple.c
+++ b/c7/demo/demo_simple.c
@@ -39,6 +39,8 @@
     visit((object_t **)&n->next);
 }
 
+void stmcb_commit_soon() {}
+
 
 
 static sem_t done;
diff --git a/c7/stm/contention.c b/c7/stm/contention.c
--- a/c7/stm/contention.c
+++ b/c7/stm/contention.c
@@ -165,7 +165,8 @@
 
         change_timing_state(wait_category);
 
-        /* XXX should also tell other_pseg "please commit soon" */
+        /* tell the other to commit ASAP */
+        signal_other_to_commit_soon(contmgr.other_pseg);
 
         dprintf(("pausing...\n"));
         cond_signal(C_AT_SAFE_POINT);
@@ -181,6 +182,9 @@
     }
 
     else if (!contmgr.abort_other) {
+        /* tell the other to commit ASAP, since it causes aborts */
+        signal_other_to_commit_soon(contmgr.other_pseg);
+
         dprintf(("abort in contention\n"));
         STM_SEGMENT->nursery_end = abort_category;
         if (kind == WRITE_WRITE_CONTENTION)
@@ -267,6 +271,13 @@
             abort_data_structures_from_segment_num(other_segment_num);
         }
         dprintf(("killed other thread\n"));
+
+        /* we should commit soon, we caused an abort */
+        //signal_other_to_commit_soon(get_priv_segment(STM_SEGMENT->segment_num));
+        if (!STM_PSEGMENT->signalled_to_commit_soon) {
+            STM_PSEGMENT->signalled_to_commit_soon = true;
+            stmcb_commit_soon();
+        }
     }
 }
 
diff --git a/c7/stm/contention.h b/c7/stm/contention.h
--- a/c7/stm/contention.h
+++ b/c7/stm/contention.h
@@ -6,7 +6,8 @@
 static void inevitable_contention_management(uint8_t other_segment_num);
 
 static inline bool is_abort(uintptr_t nursery_end) {
-    return (nursery_end <= _STM_NSE_SIGNAL_MAX && nursery_end != NSE_SIGPAUSE);
+    return (nursery_end <= _STM_NSE_SIGNAL_MAX && nursery_end != NSE_SIGPAUSE
+            && nursery_end != NSE_SIGCOMMITSOON);
 }
 
 static inline bool is_aborting_now(uint8_t other_segment_num) {
diff --git a/c7/stm/core.c b/c7/stm/core.c
--- a/c7/stm/core.c
+++ b/c7/stm/core.c
@@ -14,13 +14,10 @@
 #define EVENTUALLY(condition)                                   \
     {                                                           \
         if (!(condition)) {                                     \
-            int _i;                                             \
-            for (_i = 1; _i <= NB_SEGMENTS; _i++)               \
-                spinlock_acquire(lock_pages_privatizing[_i]);   \
+            acquire_privatization_lock();                       \
             if (!(condition))                                   \
                 stm_fatalerror("fails: " #condition);           \
-            for (_i = 1; _i <= NB_SEGMENTS; _i++)               \
-                spinlock_release(lock_pages_privatizing[_i]);   \
+            release_privatization_lock();                       \
         }                                                       \
     }
 #endif
@@ -78,11 +75,11 @@
     if (write_locks[lock_idx] == 0) {
         /* A lock to prevent reading garbage from
            lookup_other_thread_recorded_marker() */
-        acquire_segment_lock(STM_SEGMENT->segment_base);
+        acquire_marker_lock(STM_SEGMENT->segment_base);
 
         if (UNLIKELY(!__sync_bool_compare_and_swap(&write_locks[lock_idx],
                                                    0, lock_num))) {
-            release_segment_lock(STM_SEGMENT->segment_base);
+            release_marker_lock(STM_SEGMENT->segment_base);
             goto retry;
         }
 
@@ -99,7 +96,7 @@
             list_append2(STM_PSEGMENT->modified_old_objects_markers,
                          marker[0], marker[1]);
 
-        release_segment_lock(STM_SEGMENT->segment_base);
+        release_marker_lock(STM_SEGMENT->segment_base);
 
         /* We need to privatize the pages containing the object, if they
            are still SHARED_PAGE.  The common case is that there is only
@@ -210,6 +207,7 @@
     assert(STM_PSEGMENT->transaction_state == TS_NONE);
     change_timing_state(STM_TIME_RUN_CURRENT);
     STM_PSEGMENT->start_time = tl->_timing_cur_start;
+    STM_PSEGMENT->signalled_to_commit_soon = false;
     STM_PSEGMENT->safe_point = SP_RUNNING;
 #ifndef NDEBUG
     STM_PSEGMENT->marker_inev[1] = 99999999999999999L;
@@ -362,9 +360,12 @@
     /* Copy around the version of 'obj' that lives in our own segment.
        It is first copied into the shared pages, and then into other
        segments' own private pages.
+
+       Must be called with the privatization lock acquired.
     */
     assert(!_is_young(obj));
     assert(obj->stm_flags & GCFLAG_WRITE_BARRIER);
+    assert(STM_PSEGMENT->privatization_lock == 1);
 
     uintptr_t start = (uintptr_t)obj;
     uintptr_t first_page = start / 4096UL;
@@ -406,26 +407,9 @@
                     memcpy(dst, src, copy_size);
             }
             else {
-                EVENTUALLY(memcmp(dst, src, copy_size) == 0);  /* same page */
+                assert(memcmp(dst, src, copy_size) == 0);  /* same page */
             }
 
-            /* Do a full memory barrier.  We must make sure that other
-               CPUs see the changes we did to the shared page ("S",
-               above) before we check the other segments below with
-               is_private_page().  Otherwise, we risk the following:
-               this CPU writes "S" but the writes are not visible yet;
-               then it checks is_private_page() and gets false, and does
-               nothing more; just afterwards another CPU sets its own
-               private_page bit and copies the page; but it risks doing
-               so before seeing the "S" writes.
-
-               XXX what is the cost of this?  If it's high, then we
-               should reorganize the code so that we buffer the second
-               parts and do them by bunch of N, after just one call to
-               __sync_synchronize()...
-            */
-            __sync_synchronize();
-
             for (i = 1; i <= NB_SEGMENTS; i++) {
                 if (i == myself)
                     continue;
@@ -442,7 +426,7 @@
                         memcpy(dst, src, copy_size);
                 }
                 else {
-                    EVENTUALLY(!memcmp(dst, src, copy_size));  /* same page */
+                    assert(!memcmp(dst, src, copy_size));  /* same page */
                 }
             }
 
@@ -456,12 +440,15 @@
     if (STM_PSEGMENT->large_overflow_objects == NULL)
         return;
 
+    acquire_privatization_lock();
     LIST_FOREACH_R(STM_PSEGMENT->large_overflow_objects, object_t *,
                    synchronize_object_now(item));
+    release_privatization_lock();
 }
 
 static void push_modified_to_other_segments(void)
 {
+    acquire_privatization_lock();
     LIST_FOREACH_R(
         STM_PSEGMENT->modified_old_objects,
         object_t * /*item*/,
@@ -481,6 +468,7 @@
                private pages as needed */
             synchronize_object_now(item);
         }));
+    release_privatization_lock();
 
     list_clear(STM_PSEGMENT->modified_old_objects);
     list_clear(STM_PSEGMENT->modified_old_objects_markers);
diff --git a/c7/stm/core.h b/c7/stm/core.h
--- a/c7/stm/core.h
+++ b/c7/stm/core.h
@@ -156,9 +156,21 @@
     /* For sleeping contention management */
     bool signal_when_done;
 
-    /* When we mutate 'modified_old_objects' but we don't have the
-       global mutex, we must acquire this lock. */
-    uint8_t segment_lock;
+    /* This lock is acquired when that segment calls synchronize_object_now.
+       On the rare event of a page_privatize(), the latter will acquire
+       all the locks in all segments.  Otherwise, for the common case,
+       it's cheap.  (The set of all 'privatization_lock' in all segments
+       works like one single read-write lock, with page_privatize() acquiring
+       the write lock; but this variant is more efficient for the case of
+       many reads / rare writes.) */
+    uint8_t privatization_lock;
+
+    /* This lock is acquired when we mutate 'modified_old_objects' but
+       we don't have the global mutex.  It is also acquired during minor
+       collection.  It protects against a different thread that tries to
+       get this segment's marker corresponding to some object, or to
+       expand the marker into a full description. */
+    uint8_t marker_lock;
 
     /* In case of abort, we restore the 'shadowstack' field and the
        'thread_local_obj' field. */
@@ -166,6 +178,9 @@
     object_t *threadlocal_at_start_of_transaction;
     struct stm_shadowentry_s *shadowstack_at_abort;
 
+    /* Already signalled to commit soon: */
+    bool signalled_to_commit_soon;
+
     /* For debugging */
 #ifndef NDEBUG
     pthread_t running_pthread;
@@ -245,16 +260,30 @@
 static void copy_object_to_shared(object_t *obj, int source_segment_num);
 static void synchronize_object_now(object_t *obj);
 
-static inline void acquire_segment_lock(char *segment_base)
+static inline void acquire_privatization_lock(void)
 {
-    uint8_t *lock = (uint8_t *)REAL_ADDRESS(segment_base,
-                                            &STM_PSEGMENT->segment_lock);
+    uint8_t *lock = (uint8_t *)REAL_ADDRESS(STM_SEGMENT->segment_base,
+                                            &STM_PSEGMENT->privatization_lock);
     spinlock_acquire(*lock);
 }
 
-static inline void release_segment_lock(char *segment_base)
+static inline void release_privatization_lock(void)
+{
+    uint8_t *lock = (uint8_t *)REAL_ADDRESS(STM_SEGMENT->segment_base,
+                                            &STM_PSEGMENT->privatization_lock);
+    spinlock_release(*lock);
+}
+
+static inline void acquire_marker_lock(char *segment_base)
 {
     uint8_t *lock = (uint8_t *)REAL_ADDRESS(segment_base,
-                                            &STM_PSEGMENT->segment_lock);
+                                            &STM_PSEGMENT->marker_lock);
+    spinlock_acquire(*lock);
+}
+
+static inline void release_marker_lock(char *segment_base)
+{
+    uint8_t *lock = (uint8_t *)REAL_ADDRESS(segment_base,
+                                            &STM_PSEGMENT->marker_lock);
     spinlock_release(*lock);
 }
diff --git a/c7/stm/gcpage.c b/c7/stm/gcpage.c
--- a/c7/stm/gcpage.c
+++ b/c7/stm/gcpage.c
@@ -92,17 +92,20 @@
     /* uncommon case: need to initialize some more pages */
     spinlock_acquire(lock_growth_large);
 
-    if (addr + size > uninitialized_page_start) {
+    char *start = uninitialized_page_start;
+    if (addr + size > start) {
         uintptr_t npages;
-        npages = (addr + size - uninitialized_page_start) / 4096UL;
+        npages = (addr + size - start) / 4096UL;
         npages += GCPAGE_NUM_PAGES;
-        if (uninitialized_page_stop - uninitialized_page_start <
-                npages * 4096UL) {
+        if (uninitialized_page_stop - start < npages * 4096UL) {
             stm_fatalerror("out of memory!");   /* XXX */
         }
-        setup_N_pages(uninitialized_page_start, npages);
-        __sync_synchronize();
-        uninitialized_page_start += npages * 4096UL;
+        setup_N_pages(start, npages);
+        if (!__sync_bool_compare_and_swap(&uninitialized_page_start,
+                                          start,
+                                          start + npages * 4096UL)) {
+            stm_fatalerror("uninitialized_page_start changed?");
+        }
     }
     spinlock_release(lock_growth_large);
     return addr;
diff --git a/c7/stm/largemalloc.c b/c7/stm/largemalloc.c
--- a/c7/stm/largemalloc.c
+++ b/c7/stm/largemalloc.c
@@ -353,6 +353,9 @@
     mscan->size = request_size;
     mscan->prev_size = BOTH_CHUNKS_USED;
     increment_total_allocated(request_size + LARGE_MALLOC_OVERHEAD);
+#ifndef NDEBUG
+    memset((char *)&mscan->d, 0xda, request_size);
+#endif
 
     lm_unlock();
 
diff --git a/c7/stm/marker.c b/c7/stm/marker.c
--- a/c7/stm/marker.c
+++ b/c7/stm/marker.c
@@ -92,10 +92,10 @@
                                    uintptr_t marker[2])
 {
     char *segment_base = get_segment_base(in_segment_num);
-    acquire_segment_lock(segment_base);
+    acquire_marker_lock(segment_base);
     assert(_has_mutex());
 
-    /* here, we acquired the other thread's segment_lock, which means that:
+    /* here, we acquired the other thread's marker_lock, which means that:
 
        (1) it has finished filling 'modified_old_objects' after it sets
            up the write_locks[] value that we're conflicting with
@@ -118,7 +118,7 @@
     marker[0] = 0;
     marker[1] = 0;
  done:
-    release_segment_lock(segment_base);
+    release_marker_lock(segment_base);
 }
 
 static void marker_lookup_other_thread_write_write(uint8_t other_segment_num,
diff --git a/c7/stm/nursery.c b/c7/stm/nursery.c
--- a/c7/stm/nursery.c
+++ b/c7/stm/nursery.c
@@ -215,7 +215,9 @@
                content); or add the object to 'large_overflow_objects'.
             */
             if (STM_PSEGMENT->minor_collect_will_commit_now) {
+                acquire_privatization_lock();
                 synchronize_object_now(obj);
+                release_privatization_lock();
             }
             else
                 LIST_APPEND(STM_PSEGMENT->large_overflow_objects, obj);
@@ -293,6 +295,8 @@
 
     dprintf(("minor_collection commit=%d\n", (int)commit));
 
+    acquire_marker_lock(STM_SEGMENT->segment_base);
+
     STM_PSEGMENT->minor_collect_will_commit_now = commit;
     if (!commit) {
         /* 'STM_PSEGMENT->overflow_number' is used now by this collection,
@@ -336,6 +340,8 @@
 
     assert(MINOR_NOTHING_TO_DO(STM_PSEGMENT));
     assert(list_is_empty(STM_PSEGMENT->objects_pointing_to_nursery));
+
+    release_marker_lock(STM_SEGMENT->segment_base);
 }
 
 static void minor_collection(bool commit)
diff --git a/c7/stm/nursery.h b/c7/stm/nursery.h
--- a/c7/stm/nursery.h
+++ b/c7/stm/nursery.h
@@ -1,6 +1,7 @@
 
 /* '_stm_nursery_section_end' is either NURSERY_END or NSE_SIGxxx */
 #define NSE_SIGPAUSE   STM_TIME_WAIT_OTHER
+#define NSE_SIGCOMMITSOON   STM_TIME_SYNC_COMMIT_SOON
 
 
 static uint32_t highest_overflow_number;
diff --git a/c7/stm/pages.c b/c7/stm/pages.c
--- a/c7/stm/pages.c
+++ b/c7/stm/pages.c
@@ -108,18 +108,20 @@
 {
     /* check this thread's 'pages_privatized' bit */
     uint64_t bitmask = 1UL << (STM_SEGMENT->segment_num - 1);
-    struct page_shared_s *ps = &pages_privatized[pagenum - PAGE_FLAG_START];
+    volatile struct page_shared_s *ps = (volatile struct page_shared_s *)
+        &pages_privatized[pagenum - PAGE_FLAG_START];
     if (ps->by_segment & bitmask) {
         /* the page is already privatized; nothing to do */
         return;
     }
 
-#ifndef NDEBUG
-    spinlock_acquire(lock_pages_privatizing[STM_SEGMENT->segment_num]);
-#endif
+    long i;
+    for (i = 1; i <= NB_SEGMENTS; i++) {
+        spinlock_acquire(get_priv_segment(i)->privatization_lock);
+    }
 
     /* add this thread's 'pages_privatized' bit */
-    __sync_fetch_and_add(&ps->by_segment, bitmask);
+    ps->by_segment |= bitmask;
 
     /* "unmaps" the page to make the address space location correspond
        again to its underlying file offset (XXX later we should again
@@ -133,9 +135,9 @@
     /* copy the content from the shared (segment 0) source */
     pagecopy(new_page, stm_object_pages + pagenum * 4096UL);
 
-#ifndef NDEBUG
-    spinlock_release(lock_pages_privatizing[STM_SEGMENT->segment_num]);
-#endif
+    for (i = NB_SEGMENTS; i >= 1; i--) {
+        spinlock_release(get_priv_segment(i)->privatization_lock);
+    }
 }
 
 static void _page_do_reshare(long segnum, uintptr_t pagenum)
diff --git a/c7/stm/pages.h b/c7/stm/pages.h
--- a/c7/stm/pages.h
+++ b/c7/stm/pages.h
@@ -34,20 +34,6 @@
 };
 
 static struct page_shared_s pages_privatized[PAGE_FLAG_END - PAGE_FLAG_START];
-/* Rules for concurrent access to this array, possibly with is_private_page():
-
-   - we clear bits only during major collection, when all threads are
-     synchronized anyway
-
-   - we set only the bit corresponding to our segment number, using
-     an atomic addition; and we do it _before_ we actually make the
-     page private.
-
-   - concurrently, other threads checking the bits might (rarely)
-     get the answer 'true' to is_private_page() even though it is not
-     actually private yet.  This inconsistency is in the direction
-     that we want for synchronize_object_now().
-*/
 
 static void pages_initialize_shared(uintptr_t pagenum, uintptr_t count);
 static void page_privatize(uintptr_t pagenum);
@@ -72,7 +58,3 @@
     if (pages_privatized[pagenum - PAGE_FLAG_START].by_segment != 0)
         page_reshare(pagenum);
 }
-
-#ifndef NDEBUG
-static char lock_pages_privatizing[NB_SEGMENTS + 1] = { 0 };
-#endif
diff --git a/c7/stm/sync.c b/c7/stm/sync.c
--- a/c7/stm/sync.c
+++ b/c7/stm/sync.c
@@ -2,6 +2,10 @@
 #include <sys/prctl.h>
 #include <asm/prctl.h>
 
+#ifndef _STM_CORE_H_
+# error "must be compiled via stmgc.c"
+#endif
+
 
 /* Each segment can be in one of three possible states, described by
    the segment variable 'safe_point':
@@ -260,6 +264,18 @@
 static bool _safe_points_requested = false;
 #endif
 
+static void signal_other_to_commit_soon(struct stm_priv_segment_info_s *other_pseg)
+{
+    assert(_has_mutex());
+    /* never overwrite abort signals or safepoint requests
+       (too messy to deal with) */
+    if (!other_pseg->signalled_to_commit_soon
+        && !is_abort(other_pseg->pub.nursery_end)
+        && !pause_signalled) {
+        other_pseg->pub.nursery_end = NSE_SIGCOMMITSOON;
+    }
+}
+
 static void signal_everybody_to_pause_running(void)
 {
     assert(_safe_points_requested == false);
@@ -323,7 +339,21 @@
         if (STM_SEGMENT->nursery_end == NURSERY_END)
             break;    /* no safe point requested */
 
+        if (STM_SEGMENT->nursery_end == NSE_SIGCOMMITSOON) {
+            if (previous_state == -1) {
+                previous_state = change_timing_state(STM_TIME_SYNC_COMMIT_SOON);
+            }
+
+            STM_PSEGMENT->signalled_to_commit_soon = true;
+            stmcb_commit_soon();
+            if (!pause_signalled) {
+                STM_SEGMENT->nursery_end = NURSERY_END;
+                break;
+            }
+            STM_SEGMENT->nursery_end = NSE_SIGPAUSE;
+        }
         assert(STM_SEGMENT->nursery_end == NSE_SIGPAUSE);
+        assert(pause_signalled);
 
         /* If we are requested to enter a safe-point, we cannot proceed now.
            Wait until the safe-point request is removed for us. */
diff --git a/c7/stm/timing.c b/c7/stm/timing.c
--- a/c7/stm/timing.c
+++ b/c7/stm/timing.c
@@ -58,6 +58,7 @@
     "wait write read",
     "wait inevitable",
     "wait other",
+    "sync commit soon",
     "bookkeeping",
     "minor gc",
     "major gc",
diff --git a/c7/stmgc.h b/c7/stmgc.h
--- a/c7/stmgc.h
+++ b/c7/stmgc.h
@@ -66,6 +66,7 @@
     STM_TIME_WAIT_WRITE_READ,
     STM_TIME_WAIT_INEVITABLE,
     STM_TIME_WAIT_OTHER,
+    STM_TIME_SYNC_COMMIT_SOON,
     STM_TIME_BOOKKEEPING,
     STM_TIME_MINOR_GC,
     STM_TIME_MAJOR_GC,
@@ -216,9 +217,13 @@
    The "size rounded up" must be a multiple of 8 and at least 16.
    "Tracing" an object means enumerating all GC references in it,
    by invoking the callback passed as argument.
+   stmcb_commit_soon() is called when it is advised to commit
+   the transaction as soon as possible in order to avoid conflicts
+   or improve performance in general.
 */
 extern ssize_t stmcb_size_rounded_up(struct object_s *);
 extern void stmcb_trace(struct object_s *, void (object_t **));
+extern void stmcb_commit_soon(void);
 
 
 /* Allocate an object of the given size, which must be a multiple
diff --git a/c7/test/support.py b/c7/test/support.py
--- a/c7/test/support.py
+++ b/c7/test/support.py
@@ -302,6 +302,9 @@
     STM_POP_MARKER(*tl);
 }
 
+void stmcb_commit_soon()
+{
+}
 ''', sources=source_files,
      define_macros=[('STM_TESTS', '1'),
                     ('STM_LARGEMALLOC_TEST', '1'),


More information about the pypy-commit mailing list