[pypy-commit] stmgc c7-full-profiling: in-progress

arigo noreply at buildbot.pypy.org
Sat Oct 4 14:06:13 CEST 2014


Author: Armin Rigo <arigo at tunes.org>
Branch: c7-full-profiling
Changeset: r1444:233e7fec61ee
Date: 2014-10-04 14:06 +0200
http://bitbucket.org/pypy/stmgc/changeset/233e7fec61ee/

Log:	in-progress

diff --git a/c7/stm/contention.c b/c7/stm/contention.c
--- a/c7/stm/contention.c
+++ b/c7/stm/contention.c
@@ -3,34 +3,50 @@
 #endif
 
 
-enum contention_kind_e {
+/* Here are the possible kinds of contention:
 
-    /* A write-write contention occurs when we running our transaction
-       and detect that we are about to write to an object that another
-       thread is also writing to.  This kind of contention must be
-       resolved before continuing.  This *must* abort one of the two
-       threads: the caller's thread is not at a safe-point, so cannot
-       wait! */
-    WRITE_WRITE_CONTENTION,
+   STM_CONTENTION_WRITE_WRITE
 
-    /* A write-read contention occurs when we are trying to commit: it
+       A write-write contention occurs when we are running our
+       transaction and detect that we are about to write to an object
+       that another thread is also writing to.  This kind of
+       contention must be resolved before continuing.  This *must*
+       abort one of the two threads: the caller's thread is not at a
+       safe-point, so cannot wait!
+
+       It is reported as a timing event with the following two markers:
+       the current thread (i.e. where the second-in-time write occurs);
+       and the other thread (from its 'modified_old_objects_markers',
+       where the first-in-time write occurred).
+
+   STM_CONTENTION_WRITE_READ
+
+       A write-read contention occurs when we are trying to commit: it
        means that an object we wrote to was also read by another
        transaction.  Even though it would seem obvious that we should
        just abort the other thread and proceed in our commit, a more
        subtle answer would be in some cases to wait for the other thread
        to commit first.  It would commit having read the old value, and
-       then we can commit our change to it. */
-    WRITE_READ_CONTENTION,
+       then we can commit our change to it.
 
-    /* An inevitable contention occurs when we're trying to become
+       It is reported as a timing event with only one marker: the
+       older location of the write that was done by the current thread.
+
+    STM_CONTENTION_INEVITABLE
+
+       An inevitable contention occurs when we're trying to become
        inevitable but another thread already is.  We can never abort the
        other thread in this case, but we still have the choice to abort
-       ourselves or pause until the other thread commits. */
-    INEVITABLE_CONTENTION,
-};
+       ourselves or pause until the other thread commits.
+
+       It is reported with two markers, one for the current thread and
+       one for the other thread.  Each marker gives the location that
+       attempts to make the transaction inevitable.
+*/
+
 
 struct contmgr_s {
-    enum contention_kind_e kind;
+    enum stm_event_e kind;
     struct stm_priv_segment_info_s *other_pseg;
     bool abort_other;
     bool try_sleep;  // XXX add a way to timeout, but should handle repeated
@@ -99,7 +115,7 @@
 
 
 static bool contention_management(uint8_t other_segment_num,
-                                  enum contention_kind_e kind,
+                                  enum stm_event_e kind,
                                   object_t *obj)
 {
     assert(_has_mutex());
@@ -109,6 +125,9 @@
     if (must_abort())
         abort_with_mutex();
 
+    /* Report the contention */
+    timing_contention(kind, other_segment_num, obj);
+
     /* Who should abort here: this thread, or the other thread? */
     struct contmgr_s contmgr;
     contmgr.kind = kind;
@@ -138,19 +157,8 @@
         contmgr.abort_other = false;
     }
 
-
-    int wait_category =
-        kind == WRITE_READ_CONTENTION ? STM_TIME_WAIT_WRITE_READ :
-        kind == INEVITABLE_CONTENTION ? STM_TIME_WAIT_INEVITABLE :
-        STM_TIME_WAIT_OTHER;
-
-    int abort_category =
-        kind == WRITE_WRITE_CONTENTION ? STM_TIME_RUN_ABORTED_WRITE_WRITE :
-        kind == WRITE_READ_CONTENTION ? STM_TIME_RUN_ABORTED_WRITE_READ :
-        kind == INEVITABLE_CONTENTION ? STM_TIME_RUN_ABORTED_INEVITABLE :
-        STM_TIME_RUN_ABORTED_OTHER;
-
-
+    /* Do one of three things here...
+     */
     if (contmgr.try_sleep && kind != WRITE_WRITE_CONTENTION &&
         contmgr.other_pseg->safe_point != SP_WAIT_FOR_C_TRANSACTION_DONE) {
         others_may_have_run = true;
@@ -166,12 +174,26 @@
         contmgr.other_pseg->signal_when_done = true;
         marker_contention(kind, false, other_segment_num, obj);
 
-        change_timing_state(wait_category);
-
         /* tell the other to commit ASAP */
         signal_other_to_commit_soon(contmgr.other_pseg);
 
+        enum stm_event_e wait_category;
+        switch (kind) {
+        case WRITE_READ_CONTENTION:
+            wait_category = STM_WT_WRITE_READ;
+            break;
+        case INEVITABLE_CONTENTION:
+            wait_category = STM_WT_INEVITABLE;
+            break;
+        default:
+            stm_fatalerror("unexpected wait kind: %d", kind);
+        }
+
         dprintf(("pausing...\n"));
+
+        timing_event(STM_SEGMENT->running_thread, wait_category, .., ..);
+        ...;
+        
         cond_signal(C_AT_SAFE_POINT);
         STM_PSEGMENT->safe_point = SP_WAIT_FOR_C_TRANSACTION_DONE;
         cond_wait(C_TRANSACTION_DONE);
diff --git a/c7/stm/core.c b/c7/stm/core.c
--- a/c7/stm/core.c
+++ b/c7/stm/core.c
@@ -124,17 +124,13 @@
 
         dprintf_test(("write_slowpath %p -> mod_old\n", obj));
 
-        /* First change to this old object from this transaction.
+        /* Add the current marker, recording where we wrote to this object */
+        timing_record_write();
+
+        /* Change to this old object from this transaction.
            Add it to the list 'modified_old_objects'. */
         LIST_APPEND(STM_PSEGMENT->modified_old_objects, obj);
 
-        /* Add the current marker, recording where we wrote to this object */
-        uintptr_t marker[2];
-        marker_fetch(STM_SEGMENT->running_thread, marker);
-        STM_PSEGMENT->modified_old_objects_markers =
-            list_append2(STM_PSEGMENT->modified_old_objects_markers,
-                         marker[0], marker[1]);
-
         release_marker_lock(STM_SEGMENT->segment_base);
 
         /* We need to privatize the pages containing the object, if they
@@ -328,29 +324,24 @@
     STM_SEGMENT->transaction_read_version = 1;
 }
 
-static void _stm_start_transaction(stm_thread_local_t *tl, bool inevitable)
+static uint64_t _global_start_time = 0;
+
+static void _stm_start_transaction(stm_thread_local_t *tl)
 {
     assert(!_stm_in_transaction(tl));
 
-  retry:
-    if (inevitable) {
-        wait_for_end_of_inevitable_transaction(tl);
-    }
-
-    if (!acquire_thread_segment(tl))
-        goto retry;
+    while (!acquire_thread_segment(tl))
+        ;
     /* GS invalid before this point! */
 
     assert(STM_PSEGMENT->safe_point == SP_NO_TRANSACTION);
     assert(STM_PSEGMENT->transaction_state == TS_NONE);
-    timing_event(tl, STM_TRANSACTION_START, NULL, NULL);
-    STM_PSEGMENT->start_time = tl->_timing_cur_start;
+    timing_event(tl, STM_TRANSACTION_START);
+    STM_PSEGMENT->start_time = _global_start_time++;
     STM_PSEGMENT->signalled_to_commit_soon = false;
     STM_PSEGMENT->safe_point = SP_RUNNING;
-    STM_PSEGMENT->marker_inev[1] = 0;
-    if (inevitable)
-        marker_fetch_inev();
-    STM_PSEGMENT->transaction_state = (inevitable ? TS_INEVITABLE : TS_REGULAR);
+    STM_PSEGMENT->marker_inev.object = NULL;
+    STM_PSEGMENT->transaction_state = TS_REGULAR;
 #ifndef NDEBUG
     STM_PSEGMENT->running_pthread = pthread_self();
 #endif
@@ -399,14 +390,17 @@
 #else
     long repeat_count = stm_rewind_jmp_setjmp(tl);
 #endif
-    _stm_start_transaction(tl, false);
+    _stm_start_transaction(tl);
     return repeat_count;
 }
 
 void stm_start_inevitable_transaction(stm_thread_local_t *tl)
 {
+    /* used to be more efficient, starting directly an inevitable transaction,
+       but there is no real point any more, I believe */
     s_mutex_lock();
-    _stm_start_transaction(tl, true);
+    _stm_start_transaction(tl);
+    _stm_become_inevitable("start_inevitable_transaction");
 }
 
 
@@ -449,7 +443,10 @@
                         return true;
                     }
                     /* we aborted the other transaction without waiting, so
-                       we can just continue */
+                       we can just break out of this loop on
+                       modified_old_objects and continue with the next
+                       segment */
+                    break;
                 }
             }));
     }
@@ -783,13 +780,13 @@
     list_clear(STM_PSEGMENT->modified_old_objects_markers);
 }
 
-static void _finish_transaction(enum stm_event_e final_event)
+static void _finish_transaction(enum stm_event_e event)
 {
     STM_PSEGMENT->safe_point = SP_NO_TRANSACTION;
     STM_PSEGMENT->transaction_state = TS_NONE;
 
     /* marker_inev is not needed anymore */
-    STM_PSEGMENT->marker_inev[1] = 0;
+    STM_PSEGMENT->marker_inev.object = NULL;
 
     /* reset these lists to NULL for the next transaction */
     _verify_cards_cleared_in_all_lists(get_priv_segment(STM_SEGMENT->segment_num));
@@ -797,9 +794,9 @@
     list_clear(STM_PSEGMENT->old_objects_with_cards);
     LIST_FREE(STM_PSEGMENT->large_overflow_objects);
 
-    timing_end_transaction(attribute_to);
+    stm_thread_local_t *tl = STM_SEGMENT->running_thread;
+    timing_event(tl, event);
 
-    stm_thread_local_t *tl = STM_SEGMENT->running_thread;
     release_thread_segment(tl);
     /* cannot access STM_SEGMENT or STM_PSEGMENT from here ! */
 }
@@ -835,9 +832,9 @@
 
     /* if a major collection is required, do it here */
     if (is_major_collection_requested()) {
-        timing_event(NULL, STM_GC_MAJOR_START, NULL, NULL);
+        timing_event(STM_SEGMENT->running_thread, STM_GC_MAJOR_START);
         major_collection_now_at_safe_point();
-        timing_event(NULL, STM_GC_MAJOR_STOP, NULL, NULL);
+        timing_event(STM_SEGMENT->running_thread, STM_GC_MAJOR_DONE);
     }
 
     /* synchronize modified old objects to other threads */
@@ -864,7 +861,7 @@
     }
 
     /* done */
-    _finish_transaction(STM_TR_COMMIT);
+    _finish_transaction(STM_TRANSACTION_COMMIT);
     /* cannot access STM_SEGMENT or STM_PSEGMENT from here ! */
 
     s_mutex_unlock();
@@ -957,10 +954,6 @@
                        (int)pseg->transaction_state);
     }
 
-    /* if we don't have marker information already, look up and preserve
-       the marker information from the shadowstack as a string */
-    marker_default_for_abort(pseg);
-
     /* throw away the content of the nursery */
     long bytes_in_nursery = throw_away_nursery(pseg);
 
@@ -1049,16 +1042,13 @@
     /* invoke the callbacks */
     invoke_and_clear_user_callbacks(1);   /* for abort */
 
-    enum stm_event_e final_event = STM_TR_ABORT_OTHER;
-
     if (is_abort(STM_SEGMENT->nursery_end)) {
         /* done aborting */
-        final_event = STM_SEGMENT->nursery_end;
         STM_SEGMENT->nursery_end = pause_signalled ? NSE_SIGPAUSE
                                                    : NURSERY_END;
     }
 
-    _finish_transaction(final_event);
+    _finish_transaction(STM_TRANSACTION_ABORT);
     /* cannot access STM_SEGMENT or STM_PSEGMENT from here ! */
 
     /* Broadcast C_ABORTED to wake up contention.c */
@@ -1100,8 +1090,8 @@
     if (STM_PSEGMENT->transaction_state == TS_REGULAR) {
         dprintf(("become_inevitable: %s\n", msg));
 
-        marker_fetch_inev();
-        wait_for_end_of_inevitable_transaction(NULL);
+        timing_fetch_inev();
+        wait_for_end_of_inevitable_transaction();
         STM_PSEGMENT->transaction_state = TS_INEVITABLE;
         stm_rewind_jmp_forget(STM_SEGMENT->running_thread);
         invoke_and_clear_user_callbacks(0);   /* for commit */
diff --git a/c7/stm/core.h b/c7/stm/core.h
--- a/c7/stm/core.h
+++ b/c7/stm/core.h
@@ -138,7 +138,7 @@
 
     /* Start time: to know approximately for how long a transaction has
        been running, in contention management */
-    double start_time;
+    uint64_t start_time;
 
     /* This is the number stored in the overflowed objects (a multiple of
        GCFLAG_OVERFLOW_NUMBER_bit0).  It is incremented when the
@@ -196,10 +196,8 @@
     pthread_t running_pthread;
 #endif
 
-    /* Temporarily stores the marker information */
-    char marker_self[_STM_MARKER_LEN];
-    char marker_other[_STM_MARKER_LEN];
-    uintptr_t marker_inev[2];  /* marker where this thread became inevitable */
+    /* marker where this thread became inevitable */
+    stm_loc_marker_t marker_inev;
 };
 
 enum /* safe_point */ {
diff --git a/c7/stm/forksupport.c b/c7/stm/forksupport.c
--- a/c7/stm/forksupport.c
+++ b/c7/stm/forksupport.c
@@ -187,7 +187,6 @@
 #ifndef NDEBUG
     pr->running_pthread = pthread_self();
 #endif
-    strcpy(pr->marker_self, "fork");
     tl->shadowstack = NULL;
     pr->shadowstack_at_start_of_transaction = NULL;
     stm_rewind_jmp_forget(tl);
diff --git a/c7/stm/gcpage.c b/c7/stm/gcpage.c
--- a/c7/stm/gcpage.c
+++ b/c7/stm/gcpage.c
@@ -141,7 +141,7 @@
 
     if (is_major_collection_requested()) {   /* if still true */
 
-        timing_event(NULL, STM_GC_MAJOR_START, NULL, NULL);
+        timing_event(STM_SEGMENT->running_thread, STM_GC_MAJOR_START);
 
         synchronize_all_threads(STOP_OTHERS_UNTIL_MUTEX_UNLOCK);
 
@@ -149,7 +149,7 @@
             major_collection_now_at_safe_point();
         }
 
-        timing_event(NULL, STM_GC_MAJOR_STOP, NULL, NULL);
+        timing_event(STM_SEGMENT->running_thread, STM_GC_MAJOR_DONE);
     }
 
     s_mutex_unlock();
@@ -446,9 +446,9 @@
         for (i = list_count(lst); i > 0; i -= 2) {
             mark_visit_object((object_t *)list_item(lst, i - 1), base);
         }
-        if (get_priv_segment(j)->marker_inev[1]) {
-            uintptr_t marker_inev_obj = get_priv_segment(j)->marker_inev[1];
-            mark_visit_object((object_t *)marker_inev_obj, base);
+        if (get_priv_segment(j)->marker_inev.segment_base) {
+            object_t *marker_inev_obj = get_priv_segment(j)->marker_inev.object;
+            mark_visit_object(marker_inev_obj, base);
         }
     }
 }
diff --git a/c7/stm/marker.c b/c7/stm/marker.c
--- a/c7/stm/marker.c
+++ b/c7/stm/marker.c
@@ -3,18 +3,11 @@
 #endif
 
 
-void (*stmcb_expand_marker)(char *segment_base, uintptr_t odd_number,
-                            object_t *following_object,
-                            char *outputbuf, size_t outputbufsize);
-
-void (*stmcb_debug_print)(const char *cause, double time,
-                          const char *marker);
-
-
-static void marker_fetch(stm_thread_local_t *tl, uintptr_t marker[2])
+static void marker_fetch(stm_loc_marker_t *out_marker)
 {
-    /* fetch the current marker from the tl's shadow stack,
-       and return it in 'marker[2]'. */
+    /* Fetch the current marker from the 'out_marker->tl's shadow stack,
+       and return it in 'out_marker->odd_number' and 'out_marker->object'. */
+    stm_thread_local_t *tl = out_marker->tl;
     struct stm_shadowentry_s *current = tl->shadowstack - 1;
     struct stm_shadowentry_s *base = tl->shadowstack_base;
 
@@ -28,87 +21,32 @@
     }
     if (current != base) {
         /* found the odd marker */
-        marker[0] = (uintptr_t)current[0].ss;
-        marker[1] = (uintptr_t)current[1].ss;
+        out_marker->odd_number = (uintptr_t)current[0].ss;
+        out_marker->object = current[1].ss;
     }
     else {
         /* no marker found */
-        marker[0] = 0;
-        marker[1] = 0;
+        out_marker->odd_number = 0;
+        out_marker->object = NULL;
     }
 }
 
-static void marker_expand(uintptr_t marker[2], char *segment_base,
-                          char *outmarker)
+static void _timing_fetch_inev(void)
 {
-    /* Expand the marker given by 'marker[2]' into a full string.  This
-       works assuming that the marker was produced inside the segment
-       given by 'segment_base'.  If that's from a different thread, you
-       must first acquire the corresponding 'marker_lock'. */
-    assert(_has_mutex());
-    outmarker[0] = 0;
-    if (marker[0] == 0)
-        return;   /* no marker entry found */
-    if (stmcb_expand_marker != NULL) {
-        stmcb_expand_marker(segment_base, marker[0], (object_t *)marker[1],
-                            outmarker, _STM_MARKER_LEN);
-    }
+    stm_loc_marker_t marker;
+    marker.tl = STM_SEGMENT->running_thread;
+    marker_fetch(&marker);
+    STM_PSEGMENT->marker_inev.odd_number = marker.odd_number;
+    STM_PSEGMENT->marker_inev.object = marker.object;
 }
 
-static void marker_default_for_abort(struct stm_priv_segment_info_s *pseg)
+static void marker_fetch_obj_write(object_t *obj,
+                                   struct stm_loc_marker_t *out_marker)
 {
-    if (pseg->marker_self[0] != 0)
-        return;   /* already collected an entry */
-
-    uintptr_t marker[2];
-    marker_fetch(pseg->pub.running_thread, marker);
-    marker_expand(marker, pseg->pub.segment_base, pseg->marker_self);
-    pseg->marker_other[0] = 0;
-}
-
-char *_stm_expand_marker(void)
-{
-    /* for tests only! */
-    static char _result[_STM_MARKER_LEN];
-    uintptr_t marker[2];
-    _result[0] = 0;
-    s_mutex_lock();
-    marker_fetch(STM_SEGMENT->running_thread, marker);
-    marker_expand(marker, STM_SEGMENT->segment_base, _result);
-    s_mutex_unlock();
-    return _result;
-}
-
-#if 0   // XXX
-static void marker_copy(stm_thread_local_t *tl,
-                        struct stm_priv_segment_info_s *pseg,
-                        enum stm_time_e attribute_to, double time)
-{
-    /* Copies the marker information from pseg to tl.  This is called
-       indirectly from abort_with_mutex(), but only if the lost time is
-       greater than that of the previous recorded marker.  By contrast,
-       pseg->marker_self has been filled already in all cases.  The
-       reason for the two steps is that we must fill pseg->marker_self
-       earlier than now (some objects may be GCed), but we only know
-       here the total time it gets attributed.
+    /* From 'out_marker->tl', fill in 'out_marker->segment_base' and
+       'out_marker->odd_number' and 'out_marker->object' from the
+       marker associated with writing the 'obj'.
     */
-    if (stmcb_debug_print) {
-        stmcb_debug_print(timer_names[attribute_to], time, pseg->marker_self);
-    }
-    if (time * 0.99 > tl->longest_marker_time) {
-        tl->longest_marker_state = attribute_to;
-        tl->longest_marker_time = time;
-        memcpy(tl->longest_marker_self, pseg->marker_self, _STM_MARKER_LEN);
-        memcpy(tl->longest_marker_other, pseg->marker_other, _STM_MARKER_LEN);
-    }
-    pseg->marker_self[0] = 0;
-    pseg->marker_other[0] = 0;
-}
-#endif
-
-static void marker_fetch_obj_write(uint8_t in_segment_num, object_t *obj,
-                                   uintptr_t marker[2])
-{
     assert(_has_mutex());
 
     /* here, we acquired the other thread's marker_lock, which means that:
@@ -120,80 +58,83 @@
            the global mutex_lock at this point too).
     */
     long i;
+    int in_segment_num = out_marker->tl->associated_segment_num;
     struct stm_priv_segment_info_s *pseg = get_priv_segment(in_segment_num);
     struct list_s *mlst = pseg->modified_old_objects;
     struct list_s *mlstm = pseg->modified_old_objects_markers;
-    for (i = list_count(mlst); --i >= 0; ) {
+    assert(list_count(mlstm) <= 2 * list_count(mlst));
+    for (i = list_count(mlstm) / 2; --i >= 0; ) {
         if (list_item(mlst, i) == (uintptr_t)obj) {
-            assert(list_count(mlstm) == 2 * list_count(mlst));
-            marker[0] = list_item(mlstm, i * 2 + 0);
-            marker[1] = list_item(mlstm, i * 2 + 1);
+            out_marker->odd_number = list_item(mlstm, i * 2 + 0);
+            out_marker->object = (object_t *)list_item(mlstm, i * 2 + 1);
             return;
         }
     }
-    marker[0] = 0;
-    marker[1] = 0;
+    out_marker->odd_number = 0;
+    out_marker->object = NULL;
 }
 
-static void marker_contention(int kind, bool abort_other,
-                              uint8_t other_segment_num, object_t *obj)
+static void _timing_record_write(void)
 {
-    uintptr_t self_marker[2];
-    uintptr_t other_marker[2];
-    struct stm_priv_segment_info_s *my_pseg, *other_pseg;
+    stm_loc_marker_t marker;
+    marker.tl = STM_SEGMENT->running_thread;
+    marker_fetch(&marker);
 
-    my_pseg = get_priv_segment(STM_SEGMENT->segment_num);
+    long base_count = list_count(STM_PSEGMENT->modified_old_objects);
+    struct list_s *mlstm = STM_PSEGMENT->modified_old_objects_markers;
+    while (list_count(mlstm) < 2 * base_count) {
+        mlstm = list_append2(mlstm, 0, 0);
+    }
+    mlstm = list_append2(mlstm, marker.odd_number, (uintptr_t)marker.object);
+    STM_PSEGMENT->modified_old_objects_markers = mlstm;
+}
+
+static void _timing_contention(enum stm_event_e kind,
+                               uint8_t other_segment_num, object_t *obj)
+{
+    struct stm_priv_segment_info_s *other_pseg;
     other_pseg = get_priv_segment(other_segment_num);
 
-    char *my_segment_base = STM_SEGMENT->segment_base;
-    char *other_segment_base = get_segment_base(other_segment_num);
+    char *other_segment_base = other_pseg->pub.segment_base;
+    acquire_marker_lock(other_segment_base);
 
-    acquire_marker_lock(other_segment_base);
+    stm_loc_marker_t markers[2];
 
     /* Collect the location for myself.  It's usually the current
        location, except in a write-read abort, in which case it's the
        older location of the write. */
-    if (kind == WRITE_READ_CONTENTION)
-        marker_fetch_obj_write(my_pseg->pub.segment_num, obj, self_marker);
+    markers[0].tl = STM_SEGMENT->running_thread;
+    markers[0].segment_base = STM_SEGMENT->segment_base;
+
+    if (kind == STM_CONTENTION_WRITE_READ)
+        marker_fetch_obj_write(obj, &markers[0]);
     else
-        marker_fetch(my_pseg->pub.running_thread, self_marker);
-
-    /* Expand this location into either my_pseg->marker_self or
-       other_pseg->marker_other, depending on who aborts. */
-    marker_expand(self_marker, my_segment_base,
-                  abort_other ? other_pseg->marker_other
-                              : my_pseg->marker_self);
+        marker_fetch(&markers[0]);
 
     /* For some categories, we can also collect the relevant information
        for the other segment. */
-    char *outmarker = abort_other ? other_pseg->marker_self
-                                  : my_pseg->marker_other;
+    markers[1].tl = other_pseg->pub.running_thread;
+    markers[1].segment_base = other_pseg->pub.segment_base;
+
     switch (kind) {
     case WRITE_WRITE_CONTENTION:
-        marker_fetch_obj_write(other_segment_num, obj, other_marker);
-        marker_expand(other_marker, other_segment_base, outmarker);
+        marker_fetch_obj_write(obj, &markers[1]);
         break;
     case INEVITABLE_CONTENTION:
-        assert(abort_other == false);
-        other_marker[0] = other_pseg->marker_inev[0];
-        other_marker[1] = other_pseg->marker_inev[1];
-        marker_expand(other_marker, other_segment_base, outmarker);
-        break;
-    case WRITE_READ_CONTENTION:
-        strcpy(outmarker, "<read at unknown location>");
+        markers[1].odd_number = other_pseg->marker_inev[0];
+        markers[1].object = (object_t *)other_pseg->marker_inev[1];
         break;
     default:
-        outmarker[0] = 0;
+        markers[1].odd_number = 0;
+        markers[1].object = NULL;
         break;
     }
 
     release_marker_lock(other_segment_base);
+
+    stmcb_timing_event(markers[0].tl, kind, markers);
 }
 
-static void marker_fetch_inev(void)
-{
-    uintptr_t marker[2];
-    marker_fetch(STM_SEGMENT->running_thread, marker);
-    STM_PSEGMENT->marker_inev[0] = marker[0];
-    STM_PSEGMENT->marker_inev[1] = marker[1];
-}
+
+void (*stmcb_timing_event)(enum stm_event_e event,
+                           stm_loc_marker_t *markers[2]);
diff --git a/c7/stm/marker.h b/c7/stm/marker.h
--- a/c7/stm/marker.h
+++ b/c7/stm/marker.h
@@ -1,9 +1,19 @@
 
-static void marker_fetch(stm_thread_local_t *tl, uintptr_t marker[2]);
-static void marker_fetch_inev(void);
-static void marker_expand(uintptr_t marker[2], char *segment_base,
-                          char *outmarker);
-static void marker_default_for_abort(struct stm_priv_segment_info_s *pseg);
+static void _timing_record_write(void);
+static void _timing_fetch_inev(void);
+static void _timing_contention(enum stm_event_e kind,
+                               uint8_t other_segment_num, object_t *obj);
 
-static void marker_contention(int kind, bool abort_other,
-                              uint8_t other_segment_num, object_t *obj);
+
+#define timing_event(tl, event)                                         \
+    (stmcb_timing_event != NULL ? stmcb_timing_event(tl, event, NULL) : (void)0)
+
+#define timing_record_write()                                           \
+    (stmcb_timing_event != NULL ? _timing_record_write() : (void)0)
+
+#define timing_fetch_inev()                                             \
+    (stmcb_timing_event != NULL ? _timing_fetch_inev() : (void)0)
+
+#define timing_contention(kind, other_segnum, obj)                      \
+    (stmcb_timing_event != NULL ?                                       \
+        _timing_contention(kind, other_segnum, obj) : (void)0)
diff --git a/c7/stm/nursery.c b/c7/stm/nursery.c
--- a/c7/stm/nursery.c
+++ b/c7/stm/nursery.c
@@ -425,11 +425,13 @@
     for (i = num_old + 1; i < total; i += 2) {
         minor_trace_if_young((object_t **)list_ptr_to_item(mlst, i));
     }
-    if (STM_PSEGMENT->marker_inev[1]) {
-        uintptr_t *pmarker_inev_obj = (uintptr_t *)
+    if (STM_PSEGMENT->marker_inev.segment_base) {
+        assert(STM_PSEGMENT->marker_inev.segment_base ==
+               STM_SEGMENT->segment_base);
+        object_t **pmarker_inev_obj = (object_t **)
             REAL_ADDRESS(STM_SEGMENT->segment_base,
-                         &STM_PSEGMENT->marker_inev[1]);
-        minor_trace_if_young((object_t **)pmarker_inev_obj);
+                         &STM_PSEGMENT->marker_inev.object);
+        minor_trace_if_young(pmarker_inev_obj);
     }
 }
 
@@ -572,11 +574,11 @@
 
     stm_safe_point();
 
-    timing_event(NULL, STM_GC_MINOR_START, NULL, NULL);
+    timing_event(STM_SEGMENT->running_thread, STM_GC_MINOR_START);
 
     _do_minor_collection(commit);
 
-    timing_event(NULL, STM_GC_MINOR_STOP, NULL, NULL);
+    timing_event(STM_SEGMENT->running_thread, STM_GC_MINOR_DONE);
 }
 
 void stm_collect(long level)
diff --git a/c7/stm/setup.c b/c7/stm/setup.c
--- a/c7/stm/setup.c
+++ b/c7/stm/setup.c
@@ -22,8 +22,8 @@
 static char *setup_mmap(char *reason, int *map_fd)
 {
     char name[128];
-    sprintf(name, "/stmgc-c7-bigmem-%ld-%.18e",
-            (long)getpid(), get_stm_time());
+    sprintf(name, "/stmgc-c7-bigmem-%ld",
+            (long)getpid());
 
     /* Create the big shared memory object, and immediately unlink it.
        There is a small window where if this process is killed the
@@ -241,8 +241,6 @@
         num = tl->prev->associated_segment_num;
     }
     tl->thread_local_obj = NULL;
-    tl->_timing_cur_state = STM_TIME_OUTSIDE_TRANSACTION;
-    tl->_timing_cur_start = get_stm_time();
 
     /* assign numbers consecutively, but that's for tests; we could also
        assign the same number to all of them and they would get their own
diff --git a/c7/stm/sync.c b/c7/stm/sync.c
--- a/c7/stm/sync.c
+++ b/c7/stm/sync.c
@@ -123,31 +123,19 @@
 /************************************************************/
 
 
-static void wait_for_end_of_inevitable_transaction(
-                        stm_thread_local_t *tl_or_null_if_can_abort)
+static void wait_for_end_of_inevitable_transaction(void)
 {
     long i;
  restart:
     for (i = 1; i <= NB_SEGMENTS; i++) {
         struct stm_priv_segment_info_s *other_pseg = get_priv_segment(i);
         if (other_pseg->transaction_state == TS_INEVITABLE) {
-            if (tl_or_null_if_can_abort == NULL) {
-                /* handle this case like a contention: it will either
-                   abort us (not the other thread, which is inevitable),
-                   or wait for a while.  If we go past this call, then we
-                   waited; in this case we have to re-check if no other
-                   thread is inevitable. */
-                inevitable_contention_management(i);
-            }
-            else {
-                /* wait for stm_commit_transaction() to finish this
-                   inevitable transaction */
-                stm_thread_local_t *tl = tl_or_null_if_can_abort;
-                signal_other_to_commit_soon(other_pseg);
-                timing_event_wt_inevitable(tl, other_pseg);
-                cond_wait(C_INEVITABLE);
-                timing_event(tl, STM_WAIT_DONE, NULL, NULL);
-            }
+            /* handle this case like a contention: it will either
+               abort us (not the other thread, which is inevitable),
+               or wait for a while.  If we go past this call, then we
+               waited; in this case we have to re-check if no other
+               thread is inevitable. */
+            inevitable_contention_management(i);
             goto restart;
         }
     }
@@ -187,9 +175,9 @@
     }
     /* No segment available.  Wait until release_thread_segment()
        signals that one segment has been freed. */
-    timing_event(tl, STM_WT_FREE_SEGMENT, NULL, NULL);
+    timing_event(tl, STM_WAIT_FREE_SEGMENT);
     cond_wait(C_SEGMENT_FREE);
-    timing_event(tl, STM_WAIT_DONE, NULL, NULL);
+    timing_event(tl, STM_WAIT_DONE);
 
     /* Return false to the caller, which will call us again */
     return false;
@@ -331,7 +319,6 @@
     if (STM_SEGMENT->nursery_end == NURSERY_END)
         return;    /* fast path: no safe point requested */
 
-    int previous_state = -1;
     assert(_seems_to_be_running_transaction());
     assert(_has_mutex());
     while (1) {
@@ -342,10 +329,6 @@
             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) {
@@ -362,17 +345,12 @@
 #ifdef STM_TESTS
         abort_with_mutex();
 #endif
-        if (previous_state == -1) {
-            previous_state = change_timing_state(STM_TIME_SYNC_PAUSE);
-        }
+        timing_event(STM_SEGMENT->running_thread, STM_WAIT_SYNC_PAUSE);
         cond_signal(C_AT_SAFE_POINT);
         STM_PSEGMENT->safe_point = SP_WAIT_FOR_C_REQUEST_REMOVED;
         cond_wait(C_REQUEST_REMOVED);
         STM_PSEGMENT->safe_point = SP_RUNNING;
-    }
-
-    if (previous_state != -1) {
-        change_timing_state(previous_state);
+        timing_event(STM_SEGMENT->running_thread, STM_WAIT_DONE);
     }
 }
 
diff --git a/c7/stm/sync.h b/c7/stm/sync.h
--- a/c7/stm/sync.h
+++ b/c7/stm/sync.h
@@ -28,7 +28,7 @@
 static bool acquire_thread_segment(stm_thread_local_t *tl);
 static void release_thread_segment(stm_thread_local_t *tl);
 
-static void wait_for_end_of_inevitable_transaction(stm_thread_local_t *);
+static void wait_for_end_of_inevitable_transaction(void);
 
 enum sync_type_e {
     STOP_OTHERS_UNTIL_MUTEX_UNLOCK,
diff --git a/c7/stm/timing.h b/c7/stm/timing.h
deleted file mode 100644
--- a/c7/stm/timing.h
+++ /dev/null
@@ -1,28 +0,0 @@
-void (*stmcb_timing_event)(stm_thread_local_t *, enum stm_event_e,
-                           const char *, const char *);
-
-static inline void timing_event(stm_thread_local_t *tl,
-                                enum stm_event_e event,
-                                const char *marker1,
-                                const char *marker2)
-{
-    if (stmcb_timing_event != NULL)
-        stmcb_timing_event(tl, event, marker1, marker2);
-}
-
-static inline void timing_event_wt_inevitable(stm_thread_local_t *tl,
-                          struct stm_priv_segment_info_s *other_pseg)
-{
-    /* We are not running a transaction yet; can't get the 'self loc' */
-    assert(_has_mutex());
-    if (stmcb_timing_event != NULL) {
-
-        char outmarker[_STM_MARKER_LEN];
-        acquire_marker_lock(other_pseg->pub.segment_base);
-        marker_expand(other_pseg->marker_inev, other_pseg->pub.segment_base,
-                      outmarker);
-        release_marker_lock(other_pseg->pub.segment_base);
-
-        stmcb_timing_event(tl, STM_WT_INEVITABLE, NULL, outmarker);
-    }
-}
diff --git a/c7/stmgc.c b/c7/stmgc.c
--- a/c7/stmgc.c
+++ b/c7/stmgc.c
@@ -15,7 +15,6 @@
 #include "stm/fprintcolor.h"
 #include "stm/weakref.h"
 #include "stm/marker.h"
-#include "stm/timing.h"
 
 #include "stm/misc.c"
 #include "stm/list.c"
diff --git a/c7/stmgc.h b/c7/stmgc.h
--- a/c7/stmgc.h
+++ b/c7/stmgc.h
@@ -410,45 +410,51 @@
 void stm_flush_timing(stm_thread_local_t *tl, int verbose);
 
 
-/* Profiling events (in the comments: value for marker1, value for marker2) */
+/* Profiling events.  In the comments: content of the markers, if any */
 enum stm_event_e {
-    /* always STM_TRANSACTION_START followed later by one of the STM_TR_xxx */
+    /* always STM_TRANSACTION_START followed later by one of COMMIT or ABORT */
     STM_TRANSACTION_START,
-    STM_TR_COMMIT,
-    STM_TR_ABORT_WRITE_WRITE,    /* self write loc, other write loc */
-    STM_TR_ABORT_WRITE_READ,     /* self write loc, other = null; or opposite */
-    STM_TR_ABORT_INEVITABLE,     /* self cur loc?, other turned-inev loc */
-    STM_TR_ABORT_OTHER,          /* ?, ? */
+    STM_TRANSACTION_COMMIT,
+    STM_TRANSACTION_ABORT,
 
-    /* always one STM_WT_xxx followed later by STM_WAIT_DONE */
-    STM_WT_FREE_SEGMENT,
-    STM_WT_SYNC_PAUSE,
-    STM_WT_WRITE_READ,           /* self write loc, other = null; or opposite */
-    STM_WT_INEVITABLE,           /* self cur loc?, other turned-inev loc */
+    /* contention; see details at the start of contention.c */
+    STM_CONTENTION_WRITE_WRITE,  /* markers: self loc / other written loc */
+    STM_CONTENTION_WRITE_READ,   /* markers: self written loc / other missing */
+    STM_CONTENTION_INEVITABLE,   /* markers: self loc / other inev loc */
+
+    /* following a contention, we get from the same thread one of:
+       STM_ABORTING_OTHER_CONTENTION, STM_TRANSACTION_ABORT (self-abort),
+       or STM_WAIT_CONTENTION (self-wait). */
+    STM_ABORTING_OTHER_CONTENTION,
+
+    /* always one STM_WAIT_xxx followed later by STM_WAIT_DONE */
+    STM_WAIT_FREE_SEGMENT,
+    STM_WAIT_SYNC_PAUSE,
+    STM_WAIT_CONTENTION,
     STM_WAIT_DONE,
 
     /* start and end of GC cycles */
     STM_GC_MINOR_START,
-    STM_GC_MINOR_STOP,
+    STM_GC_MINOR_DONE,
     STM_GC_MAJOR_START,
-    STM_GC_MAJOR_STOP,
+    STM_GC_MAJOR_DONE,
 
     _STM_EVENT_N
 };
-#define _STM_MARKER_LEN  128
 
 /* The markers pushed in the shadowstack are an odd number followed by a
-   regular pointer.  When needed, this library invokes this callback to
-   turn this pair into a human-readable explanation. */
-extern void (*stmcb_expand_marker)(char *segment_base, uintptr_t odd_number,
-                                   object_t *following_object,
-                                   char *outputbuf, size_t outputbufsize);
-extern void (*stmcb_timing_event)(stm_thread_local_t *tl,
+   regular pointer. */
+typedef struct {
+    stm_thread_local_t *tl;
+    char *segment_base;    /* base to interpret the 'object' below */
+    uintptr_t odd_number;  /* marker odd number, or 0 if marker is missing */
+    object_t *object;      /* marker object, or NULL if marker is missing */
+} stm_loc_marker_t;
+extern void (*stmcb_timing_event)(stm_thread_local_t *tl, /* the local thread */
                                   enum stm_event_e event,
-                                  const char *marker1,
-                                  const char *marker2);
+                                  stm_loc_marker_t *markers);
 
-/* Conventience macros to push the markers into the shadowstack */
+/* Convenience macros to push the markers into the shadowstack */
 #define STM_PUSH_MARKER(tl, odd_num, p)   do {  \
     uintptr_t _odd_num = (odd_num);             \
     assert(_odd_num & 1);                       \
diff --git a/c7/test/support.py b/c7/test/support.py
--- a/c7/test/support.py
+++ b/c7/test/support.py
@@ -107,39 +107,50 @@
 long stm_call_on_abort(stm_thread_local_t *, void *key, void callback(void *));
 long stm_call_on_commit(stm_thread_local_t *, void *key, void callback(void *));
 
+/* Profiling events.  In the comments: content of the markers, if any */
 enum stm_event_e {
-    /* always STM_TRANSACTION_START followed later by one of the STM_TR_xxx */
+    /* always STM_TRANSACTION_START followed later by one of COMMIT or ABORT */
     STM_TRANSACTION_START,
-    STM_TR_COMMIT,
-    STM_TR_ABORT_WRITE_WRITE,    /* self write loc, other write loc */
-    STM_TR_ABORT_WRITE_READ,     /* self write loc, other = null; or opposite */
-    STM_TR_ABORT_INEVITABLE,     /* self cur loc, other turned-inev loc */
-    STM_TR_ABORT_OTHER,          /* ?, ? */
+    STM_TRANSACTION_COMMIT,
+    STM_TRANSACTION_ABORT,
 
-    /* always one STM_WT_xxx followed later by STM_WAIT_DONE */
-    STM_WT_FREE_SEGMENT,
-    STM_WT_SYNC_PAUSE,
-    STM_WT_WRITE_READ,           /* self write loc, other = null; or opposite */
-    STM_WT_INEVITABLE,           /* self cur loc, other turned-inev loc */
+    /* contention; see details at the start of contention.c */
+    STM_CONTENTION_WRITE_WRITE,  /* markers: self loc / other written loc */
+    STM_CONTENTION_WRITE_READ,   /* markers: self written loc / other missing */
+    STM_CONTENTION_INEVITABLE,   /* markers: self inev loc / other inev loc */
+
+    /* following a contention, we get from the same thread one of:
+       STM_ABORTING_OTHER_CONTENTION, STM_TRANSACTION_ABORT (self-abort),
+       or STM_WAIT_CONTENTION (self-wait). */
+    STM_ABORTING_OTHER_CONTENTION,
+
+    /* always one STM_WAIT_xxx followed later by STM_WAIT_DONE */
+    STM_WAIT_FREE_SEGMENT,
+    STM_WAIT_SYNC_PAUSE,
+    STM_WAIT_CONTENTION,
     STM_WAIT_DONE,
 
     /* start and end of GC cycles */
     STM_GC_MINOR_START,
-    STM_GC_MINOR_STOP,
+    STM_GC_MINOR_DONE,
     STM_GC_MAJOR_START,
-    STM_GC_MAJOR_STOP,
+    STM_GC_MAJOR_DONE,
     ...
 };
 
 void stm_flush_timing(stm_thread_local_t *, int);
 
-void (*stmcb_expand_marker)(char *segment_base, uintptr_t odd_number,
-                            object_t *following_object,
-                            char *outputbuf, size_t outputbufsize);
-void (*stmcb_timing_event)(stm_thread_local_t *tl,
+typedef struct {
+    stm_thread_local_t *tl;
+    /* If segment_base==NULL, the remaining fields are undefined.  If non-NULL,
+       the rest is a marker to interpret from this segment_base addr. */
+    char *segment_base;
+    uintptr_t odd_number;
+    object_t *object;
+} stm_loc_marker_t;
+void (*stmcb_timing_event)(stm_thread_local_t *tl, /* the local thread */
                            enum stm_event_e event,
-                           const char *marker1,
-                           const char *marker2);
+                           stm_loc_marker_t *markers);
 
 void stm_push_marker(stm_thread_local_t *, uintptr_t, object_t *);
 void stm_update_marker_num(stm_thread_local_t *, uintptr_t);


More information about the pypy-commit mailing list