[m-rev.] diff: Fix some ThreadScope issues.

Paul Bone pbone at csse.unimelb.edu.au
Thu Jun 23 18:16:18 AEST 2011


Fix some ThreadScope issues.

Firstly, this change allows the ThreadScope tool to read Mercury's .eventlog
files without aborting.  This is fixed by making THREAD_START and THREAD_STOP
events consistent.

Secondly, this change implements the missing EVENT_SLEEPING event.  This
ensures that the implementation matches the description in the ThreadScope
paper.

Thirdly, the idle engines try to run a suspended context before running a
spark.

runtime/mercury_threadscope.c:
    Don't post THREAD_START or THREAD_STOP events if it wouldn't make sense,
    ie: the thread is already stopped.  We do this to make RTS code simpler
    since an engine may hang on to a context even when that context is stopped.
    The RTS uses this for caching.

    Create a new event ENGINE_SLEEPING to be used when an engine goes to sleep.

runtime/mercury_context.c:
    Add some missing calls to threadscope, this ensures that Mercury's eventlog file
    maintains some invariants expected by the ThreadScope visualisation tool.

    Modify how idle engines look for new work: now, in all cases, an idle
    engine will attempt to resume a context first.

    Avoid taking the lock to the global run queue of contexts if the runqueue
    pointer is NULL indicating that the queue is empty.

Index: runtime/mercury_context.c
===================================================================
RCS file: /home/mercury1/repository/mercury/runtime/mercury_context.c,v
retrieving revision 1.95
diff -u -p -b -r1.95 mercury_context.c
--- runtime/mercury_context.c	2 Jun 2011 05:59:20 -0000	1.95
+++ runtime/mercury_context.c	23 Jun 2011 08:05:34 -0000
@@ -1500,15 +1500,19 @@ MR_BEGIN_MODULE(scheduler_module_idle_cl
 MR_BEGIN_CODE
 MR_define_entry(MR_do_idle_clean_context);
 {
+#ifdef MR_THREADSCOPE
+    MR_threadscope_post_stop_context(MR_TS_STOP_REASON_FINISHED);
+#endif
+
+    MR_MAYBE_TRAMPOLINE(do_get_context());
     MR_MAYBE_TRAMPOLINE(do_local_spark(NULL));
+    MR_MAYBE_TRAMPOLINE(do_work_steal(NULL));
 
+    /*
+     * XXX: I'm not convinced that the idle state is useful.
+     */
     advertise_engine_state_idle();
 
-    MR_MAYBE_TRAMPOLINE_AND_ACTION(do_work_steal(NULL),
-        advertise_engine_state_working());
-
-    MR_MAYBE_TRAMPOLINE_AND_ACTION(do_get_context(),
-        advertise_engine_state_working());
     MR_GOTO(MR_ENTRY(MR_do_sleep));
 }
 MR_END_MODULE
@@ -1522,22 +1526,43 @@ MR_define_entry(MR_do_idle_dirty_context
 {
     MR_Code *join_label = (MR_Code*)MR_r1;
 
-    MR_MAYBE_TRAMPOLINE(do_local_spark(join_label));
-
-    advertise_engine_state_idle();
+    if (MR_runqueue_head != NULL) {
+        /*
+        ** There's a context in the run queue, save this context and attempt to
+        ** get a context from the run queue.
+        */
+        save_dirty_context(join_label);
+        MR_ENGINE(MR_eng_this_context) = NULL;
+        MR_MAYBE_TRAMPOLINE(do_get_context());
 
-    MR_MAYBE_TRAMPOLINE_AND_ACTION(do_work_steal(join_label),
-        advertise_engine_state_working());
+        /*
+        ** If execution reaches this point then we lost a race for a free context.
+        ** Tell threadscope about it,
+        ** XXX: Consider making this a first-class event in the future.
+        */
+#ifdef MR_THREADSCOPE
+        MR_threadscope_post_log_msg("Runtime: Lost a race for a runnable context.");
+#endif
+        join_label = NULL;
+    }
 
+#ifdef MR_THREADSCOPE
+    MR_threadscope_post_stop_context(MR_TS_STOP_REASON_BLOCKED);
+#endif
+    MR_MAYBE_TRAMPOLINE(do_local_spark(join_label));
+    MR_MAYBE_TRAMPOLINE(do_work_steal(join_label));
+    if (join_label != NULL) {
     /*
     ** Save the dirty context, we can't take it to sleep and it won't be used
     ** if do_get_context() succeeds.
     */
     save_dirty_context(join_label);
     MR_ENGINE(MR_eng_this_context) = NULL;
+        MR_MAYBE_TRAMPOLINE(do_get_context());
+    }
+
+    advertise_engine_state_idle();
 
-    MR_MAYBE_TRAMPOLINE_AND_ACTION(do_get_context(),
-        advertise_engine_state_working());
     MR_GOTO(MR_ENTRY(MR_do_sleep));
 }
 MR_END_MODULE
@@ -1563,6 +1588,9 @@ MR_define_entry(MR_do_sleep);
     while (1) {
         engine_sleep_sync_data[engine_id].d.es_state = ENGINE_STATE_SLEEPING;
         MR_CPU_SFENCE;
+#ifdef MR_THREADSCOPE
+        MR_threadscope_post_engine_sleeping();
+#endif
         result = MR_SEM_WAIT(
             &(engine_sleep_sync_data[engine_id].d.es_sleep_semaphore),
             "MR_do_sleep sleep_sem");
@@ -1662,6 +1690,7 @@ do_get_context(void)
     MR_threadscope_post_looking_for_global_context();
     #endif
 
+    if (MR_runqueue_head != NULL) {
     MR_LOCK(&MR_runqueue_lock, "do_get_context (i)");
     ready_context = MR_find_ready_context();
     MR_UNLOCK(&MR_runqueue_lock, "do_get_context (ii)");
@@ -1678,6 +1707,7 @@ do_get_context(void)
 
         return resume_point;
     }
+    }
 
     return NULL;
 }
@@ -1745,14 +1775,14 @@ prepare_engine_for_spark(volatile MR_Spa
 #endif
 */
         MR_load_context(MR_ENGINE(MR_eng_this_context));
-#ifdef MR_THREADSCOPE
-        MR_threadscope_post_run_context();
-#endif
 #ifdef MR_DEBUG_STACK_SEGMENTS
         MR_debug_log_message("created new context for spark: %p",
             MR_ENGINE(MR_eng_this_context));
 #endif
     }
+#ifdef MR_THREADSCOPE
+    MR_threadscope_post_run_context();
+#endif
 
     /*
     ** At this point we have a context, either a dirty context that's
Index: runtime/mercury_threadscope.c
===================================================================
RCS file: /home/mercury1/repository/mercury/runtime/mercury_threadscope.c,v
retrieving revision 1.15
diff -u -p -b -r1.15 mercury_threadscope.c
--- runtime/mercury_threadscope.c	2 Jun 2011 05:59:21 -0000	1.15
+++ runtime/mercury_threadscope.c	23 Jun 2011 08:05:33 -0000
@@ -177,7 +177,8 @@
                                             108 /* () */
 #define MR_TS_MER_EVENT_WORK_STEALING       109 /* () */
 #define MR_TS_MER_EVENT_RELEASE_CONTEXT     110 /* (context id) */
-#define MR_TS_NUM_MER_EVENTS                11
+#define MR_TS_MER_EVENT_ENGINE_SLEEPING     111 /* () */
+#define MR_TS_NUM_MER_EVENTS                 12
 
 #if 0  /* DEPRECATED EVENTS: */
 #define EVENT_CREATE_SPARK        13 /* (cap, thread) */
@@ -500,6 +501,11 @@ static EventTypeDesc event_type_descs[] 
         SZ_CONTEXT_ID
     },
     {
+        MR_TS_MER_EVENT_ENGINE_SLEEPING,
+        "This engine is going to sleep",
+        0
+    },
+    {
         /* Mark the end of this array. */
         MR_TS_NUM_EVENT_TAGS,
         NULL,
@@ -1211,8 +1217,10 @@ MR_threadscope_post_run_context(void)
 
     if (context) {
         MR_US_SPIN_LOCK(&(buffer->MR_tsbuffer_lock));
+        if (buffer->MR_tsbuffer_ctxt_is_stopped) {
         MR_threadscope_post_run_context_locked(buffer, context);
-        buffer->MR_tsbuffer_ctxt_is_stopped = 0;
+            buffer->MR_tsbuffer_ctxt_is_stopped = MR_FALSE;
+        }
         MR_US_UNLOCK(&(buffer->MR_tsbuffer_lock));
     }
 }
@@ -1245,9 +1253,10 @@ MR_threadscope_post_stop_context(MR_Cont
     context = MR_thread_engine_base->MR_eng_this_context;
 
     MR_US_SPIN_LOCK(&(buffer->MR_tsbuffer_lock));
+    if (!buffer->MR_tsbuffer_ctxt_is_stopped) {
     MR_threadscope_post_stop_context_locked(buffer, context, reason);
-
-    buffer->MR_tsbuffer_ctxt_is_stopped = 1;
+        buffer->MR_tsbuffer_ctxt_is_stopped = MR_TRUE;
+    }
     MR_US_UNLOCK(&(buffer->MR_tsbuffer_lock));
 }
 
@@ -1691,6 +1700,24 @@ MR_threadscope_post_signal_future(MR_Fut
     MR_US_UNLOCK(&(buffer->MR_tsbuffer_lock));
 }
 
+void MR_threadscope_post_engine_sleeping(void)
+{
+    struct MR_threadscope_event_buffer *buffer = MR_ENGINE(MR_eng_ts_buffer);
+
+    MR_US_SPIN_LOCK(&(buffer->MR_tsbuffer_lock));
+    if (!enough_room_for_event(buffer, MR_TS_MER_EVENT_ENGINE_SLEEPING)) {
+        flush_event_buffer(buffer);
+        open_block(buffer, MR_ENGINE(MR_eng_id));
+    } else if (!block_is_open(buffer)) {
+        open_block(buffer, MR_ENGINE(MR_eng_id));
+    }
+
+    put_event_header(buffer, MR_TS_MER_EVENT_ENGINE_SLEEPING,
+        get_current_time_nanosecs());
+
+    MR_US_UNLOCK(&(buffer->MR_tsbuffer_lock));
+}
+
 /***************************************************************************/
 
 static struct MR_threadscope_event_buffer*
@@ -1701,7 +1728,7 @@ MR_create_event_buffer(void)
     buffer = MR_GC_NEW(MR_threadscope_event_buffer_t);
     buffer->MR_tsbuffer_pos = 0;
     buffer->MR_tsbuffer_block_open_pos = -1;
-    buffer->MR_tsbuffer_ctxt_is_stopped = 1;
+    buffer->MR_tsbuffer_ctxt_is_stopped = MR_TRUE;
     buffer->MR_tsbuffer_lock = MR_US_LOCK_INITIAL_VALUE;
 
     return buffer;
Index: runtime/mercury_threadscope.h
===================================================================
RCS file: /home/mercury1/repository/mercury/runtime/mercury_threadscope.h,v
retrieving revision 1.12
diff -u -p -b -r1.12 mercury_threadscope.h
--- runtime/mercury_threadscope.h	2 Jun 2011 05:59:21 -0000	1.12
+++ runtime/mercury_threadscope.h	23 Jun 2011 08:05:33 -0000
@@ -200,6 +200,11 @@ extern void MR_threadscope_post_wait_fut
 extern void MR_threadscope_post_signal_future(MR_Future* future_id);
 
 /*
+** Post this event when the engine is going to sleep.
+*/
+extern void MR_threadscope_post_engine_sleeping(void);
+
+/*
 ** Register all the strings in an array and save their IDs in the array.
 */
 extern void MR_threadscope_register_strings_array(MR_Threadscope_String *array,
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 489 bytes
Desc: Digital signature
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20110623/ea424fe1/attachment.sig>


More information about the reviews mailing list