[m-rev.] for review: speed up the declarative debugger by 127%

Ian MacLarty maclarty at cs.mu.OZ.AU
Sat Aug 13 20:59:27 AEST 2005


I got a further speed improvement (about 3%) by converting all the functions
called from MR_trace_decl_debug to macros, but decided to reverse that change
since the performance gain didn't seem worth the headache of maintaining a lot
of macros, especially since the change below gives such a huge improvement.

For review by anyone.

Speed up the declarative debugger by factoring out the code responsible for
building the histogram in implicit subtrees into a seperate function.
Call this new function directly from MR_trace when inside an implicit subtree.
Because this function doesn't have to do very much and will be called for
most events when the search space is large, this is a big speed improvement.

A session which finds a bug in a leaf node of a run of the Mercury
compiler on a small program using divide-and-query, takes 56% less time.

trace/mercury_trace.c:
trace/mercury_trace.h:
	Add MR_trace_real_decl_implicit_subtree.

trace/mercury_trace_declarative.c:
trace/mercury_trace_declarative.h:
	Export the global variables that keep track of the depth of events
	in the EDT and the histgram so they can be accessed by
	MR_trace_real_decl_implicit_subtree.

	Remove the MR_TRACE_EVENT_TOO_DEEP macro.  This macro used to be
	more complicated, but now it's simple and obvious enough that we
	don't need a macro.

	In MR_trace_decl_debug avoid multiply dereferencing the same field
	in event_info, by dereferencing MR_trace_port and MR_trace_seqno once
	and then storing them in variables.

	Rename event_depth to node_depth, since this gives the depth of the
	corresponding EDT node.

	Replace MR_trace_calculate_event_depth with a macro
	MR_DD_CALC_NODE_DEPTH.  We need it to be a macro to avoid function
	calls in MR_trace_real_decl_implicit_subtree.

	Call the MR_DD_CALC_NODE_DEPTH macro.  If entering an implicit subtree
	set the callback function to MR_trace_real_decl_implicit_subtree.

	Move the check for suppressed events to MR_trace_include_event.

	Delete MR_trace_count_event_in_implicit_subtree since
	MR_trace_real_decl_implicit_subtree now does this job.

Index: trace/mercury_trace.c
===================================================================
RCS file: /home/mercury1/repository/mercury/trace/mercury_trace.c,v
retrieving revision 1.84
diff -u -b -r1.84 mercury_trace.c
--- trace/mercury_trace.c	12 Aug 2005 02:01:27 -0000	1.84
+++ trace/mercury_trace.c	13 Aug 2005 10:47:08 -0000
@@ -121,8 +121,8 @@
 ** in runtime/mercury_trace_base.c, which in turn is called from
 ** compiled code whenever an event to be traced occurs.
 **
-** The initial part, MR_TRACE_REAL_SETUP_CODE, is shared by MR_trace_real and
-** MR_trace_real_decl.
+** The initial part, MR_TRACE_REAL_SETUP_CODE, is shared by MR_trace_real,
+** MR_trace_real_decl and MR_trace_real_decl_implicit_subtree.
 */

 #if defined(MR_USE_MINIMAL_MODEL_STACK_COPY) && defined(MR_MINIMAL_MODEL_DEBUG)
@@ -461,7 +461,8 @@

 /*
 ** The MR_TRACE_EVENT_DECL_AND_SETUP macro is the initial part of
-** MR_trace_event, shared also by MR_trace_real_decl.
+** MR_trace_event, shared also by MR_trace_real_decl and
+** MR_trace_real_decl_implicit_subtree.
 **
 ** It must be included in a context that defines layout, port, seqno and depth.
 */
@@ -484,7 +485,8 @@

 /*
 ** The MR_TRACE_EVENT_TEARDOWN macro is the final part of
-** MR_trace_event, shared also by MR_trace_real_decl.
+** MR_trace_event, shared also by MR_trace_real_decl and
+** MR_trace_real_decl_implicit_subtree.
 **
 ** It must be included in a context that defines jumpaddr, saved_regs and
 ** event_info.
@@ -565,6 +567,58 @@
     }
 }

+MR_Code *
+MR_trace_real_decl_implicit_subtree(const MR_Label_Layout *layout)
+{
+    MR_Integer          maybe_from_full;
+    MR_Unsigned         seqno;
+    MR_Unsigned         depth;
+    MR_Trace_Port       port;
+    MR_Unsigned         node_depth;
+    MR_Unsigned         depth_in_imp_subtree;
+
+    MR_TRACE_REAL_SETUP_CODE();
+
+    port = (MR_Trace_Port) layout->MR_sll_port;
+
+    /*
+    ** Note that we do not check for suppressed events or events from
+    ** compiler generated procedures, since we want this function to be as
+    ** fast as possible (since it will be executed the most with large search
+    ** spaces).
+    ** This has minimal impact on the accuracy of the histogram, since
+    ** event suppression is not currently used and compiler generated
+    ** procedures are very likely to be leaf nodes or close to leaf nodes.
+    */
+    MR_DD_CALC_NODE_DEPTH(port, node_depth);
+
+    depth_in_imp_subtree = node_depth - MR_edt_max_depth;
+
+    if ((depth_in_imp_subtree == 0) && MR_port_is_final(port)) {
+        MR_edt_implicit_subtree_counters[depth_in_imp_subtree]++;
+
+        MR_selected_trace_func_ptr = MR_trace_real_decl;
+
+        /*
+        ** MR_trace_decl_debug will decrement MR_edt_depth again, so we
+        ** increment it here, so that the overall effect is that it is only
+        ** decremented once.
+        */
+        MR_edt_depth++;
+
+        {
+            MR_TRACE_EVENT_DECL_AND_SETUP
+            jumpaddr = MR_trace_decl_debug(&event_info);
+            MR_TRACE_EVENT_TEARDOWN
+        }
+    } else {
+        if (depth_in_imp_subtree < MR_edt_implicit_subtree_num_counters) {
+            MR_edt_implicit_subtree_counters[depth_in_imp_subtree]++;
+        }
+    }
+    return NULL;
+}
+
 /*****************************************************************************/

 /* The initial size of arrays of argument values. */
Index: trace/mercury_trace.h
===================================================================
RCS file: /home/mercury1/repository/mercury/trace/mercury_trace.h,v
retrieving revision 1.30
diff -u -b -r1.30 mercury_trace.h
--- trace/mercury_trace.h	12 Aug 2005 02:01:27 -0000	1.30
+++ trace/mercury_trace.h	13 Aug 2005 10:13:48 -0000
@@ -66,10 +66,15 @@
 **
 ** MR_trace_real_decl is the version of MR_trace_real we use when gathering
 ** events for the annotated trace.
+**
+** MR_trace_real_decl_implicit_subtree is a specialized version of
+** MR_trace_real_decl for events which are in implicit subtrees.
 */

 extern  MR_Code *MR_trace_real_decl(const MR_Label_Layout *);

+extern  MR_Code *MR_trace_real_decl_implicit_subtree(const MR_Label_Layout *);
+
 /*
 ** Ideally, MR_trace_retry works by resetting the state of the stacks and
 ** registers to the state appropriate for the call to the selected ancestor,
Index: trace/mercury_trace_declarative.c
===================================================================
RCS file: /home/mercury1/repository/mercury/trace/mercury_trace_declarative.c,v
retrieving revision 1.95
diff -u -b -r1.95 mercury_trace_declarative.c
--- trace/mercury_trace_declarative.c	12 Aug 2005 02:01:27 -0000	1.95
+++ trace/mercury_trace_declarative.c	13 Aug 2005 10:10:46 -0000
@@ -259,24 +259,6 @@
 static  MR_Trace_Node       MR_edt_return_node;

 /*
-** The depth of the EDT is different from the call depth of the events, since
-** the call depth is not guaranteed to be the same for the children of a call
-** event - see comments in MR_trace_real in trace/mercury_trace.c.  We use the
-** following variable to keep track of the EDT depth.  We only keep track of
-** the depth of the portion of the EDT we are materializing.  MR_edt_depth is 0
-** for the root of the tree we are materializing.
-*/
-
-static  MR_Integer          MR_edt_depth;
-
-/*
-** Events where the value of MR_edt_depth above is greater than the value of
-** MR_edt_max_depth will not be included in tha annotated trace.
-*/
-
-static  MR_Unsigned         MR_edt_max_depth;
-
-/*
 ** The time (in milliseconds since the start of the program) when collection of
 ** events, for the current portion of the annotated trace being materialized,
 ** started.
@@ -298,16 +280,6 @@
 static  MR_Unsigned         MR_edt_first_event;

 /*
-** MR_edt_implicit_subtree_counters points to an array that records
-** the number of events at different depths in implicit subtrees.
-** These are used to determine what depth an implicit subtree should
-** be materialized to.
-*/
-
-static  MR_Unsigned         *MR_edt_implicit_subtree_counters;
-static  MR_Unsigned         MR_edt_implicit_subtree_num_counters;
-
-/*
 ** The declarative debugger ignores modules that were not compiled with
 ** the required information.  However, this may result in incorrect
 ** assumptions being made about the code, so the debugger gives a warning
@@ -476,13 +448,11 @@

 MR_bool    MR_trace_decl_debug_debugger_mode = MR_FALSE;

-/*
-** We filter out events which are deeper than the limit given by
-** MR_edt_max_depth.  These events are implicitly represented in the structure
-** being built.
-*/
+MR_Integer      MR_edt_depth;
+MR_Unsigned     MR_edt_max_depth;

-#define MR_TRACE_EVENT_TOO_DEEP(depth) (depth > MR_edt_max_depth)
+MR_Unsigned     *MR_edt_implicit_subtree_counters;
+MR_Unsigned     MR_edt_implicit_subtree_num_counters;

 /*
 ** This function is called for every traced event when building the
@@ -497,48 +467,48 @@
     MR_Unsigned             depth;
     MR_Event_Details        event_details;
     MR_Integer              trace_suppress;
-    MR_Unsigned             event_depth;
+    MR_Unsigned             node_depth;
+    MR_Unsigned             call_seqno;
+    MR_Trace_Port           port;
     MR_Code                 *jumpaddr;

+    call_seqno = event_info->MR_call_seqno;
+    port = event_info->MR_trace_port;
     entry = event_info->MR_event_sll->MR_sll_entry;
+
     MR_trace_edt_build_sanity_check(event_info, entry);
+
     MR_trace_maybe_show_progress(event_info->MR_event_number);

     if (! MR_trace_include_event(entry, event_info, &jumpaddr)) {
         return jumpaddr;
     }

-    event_depth = MR_trace_calculate_event_depth(event_info);
-    MR_trace_count_event_in_implicit_subtree(event_info, event_depth);
+    MR_DD_CALC_NODE_DEPTH(port, node_depth);

-    if (MR_TRACE_EVENT_TOO_DEEP(event_depth)) {
-        return NULL;
-    }
-
-    trace_suppress = entry->MR_sle_module_layout->MR_ml_suppressed_events;
-    if (trace_suppress != 0) {
+    if (node_depth == MR_edt_max_depth
+            && (port == MR_PORT_CALL || port == MR_PORT_REDO))
+    {
         /*
-        ** We ignore events from modules that were not compiled
-        ** with the necessary information.  Procedures in those
-        ** modules are effectively assumed correct, so we give
-        ** the user a warning.
+        ** Reset the accumulators, since we are entering the
+        ** top of an implicit subtree.
         */
-        MR_edt_compiler_flag_warning = MR_TRUE;
-        return NULL;
+        MR_trace_reset_implicit_subtree_counters();
+        MR_edt_implicit_subtree_counters[node_depth - MR_edt_max_depth]++;
+        MR_selected_trace_func_ptr = MR_trace_real_decl_implicit_subtree;
     }

     MR_trace_construct_node(event_info);

-    if (event_info->MR_call_seqno == MR_edt_start_seqno &&
-        MR_port_is_final(event_info->MR_trace_port))
+    if (call_seqno == MR_edt_start_seqno && MR_port_is_final(port))
     {
         MR_edt_return_node = MR_trace_current_node;
     }

     if ((!MR_edt_building_supertree &&
             MR_trace_event_number == MR_edt_last_event)
-        || (MR_edt_building_supertree && event_depth == 0
-            && MR_port_is_final(event_info->MR_trace_port)))
+        || (MR_edt_building_supertree && node_depth == 0
+            && MR_port_is_final(port)))
     {
         MR_trace_free_implicit_subtree_counters();
         MR_decl_maybe_print_edt_stats();
@@ -561,27 +531,6 @@
 }

 static void
-MR_trace_count_event_in_implicit_subtree(MR_Event_Info *event_info,
-    MR_Unsigned depth)
-{
-    if (depth == MR_edt_max_depth
-        && (event_info->MR_trace_port == MR_PORT_CALL
-        || event_info->MR_trace_port == MR_PORT_REDO))
-    {
-        /*
-        ** Reset the accumulators, since we are entering the
-        ** top of an implicit subtree.
-        */
-        MR_trace_reset_implicit_subtree_counters();
-    }
-    if ((depth >= MR_edt_max_depth) && (depth - MR_edt_max_depth <
-        MR_edt_implicit_subtree_num_counters))
-    {
-        MR_edt_implicit_subtree_counters[depth - MR_edt_max_depth]++;
-    }
-}
-
-static void
 MR_trace_edt_build_sanity_check(MR_Event_Info *event_info,
     const MR_Proc_Layout *entry)
 {
@@ -615,6 +564,18 @@
         return MR_FALSE;
     }

+    if (entry->MR_sle_module_layout->MR_ml_suppressed_events != 0) {
+        /*
+        ** We ignore events from modules that were not compiled
+        ** with the necessary information.  Procedures in those
+        ** modules are effectively assumed correct, so we give
+        ** the user a warning.
+        */
+        MR_edt_compiler_flag_warning = MR_TRUE;
+        *jumpaddr = NULL;
+        return MR_FALSE;
+    }
+
     /*
     ** Decide if we are inside or outside the subtree or supertree that
     ** needs to be materialized, ignoring for now any depth limit.
@@ -715,27 +676,6 @@
     return MR_TRUE;
 }

-static MR_Unsigned
-MR_trace_calculate_event_depth(MR_Event_Info *event_info)
-{
-    if (event_info->MR_trace_port == MR_PORT_CALL
-        || event_info->MR_trace_port == MR_PORT_REDO)
-    {
-        return ++MR_edt_depth;
-    }
-
-    if (MR_port_is_final(event_info->MR_trace_port)) {
-        /*
-        ** The depth of the EXIT, FAIL or EXCP event is
-        ** MR_edt_depth (not MR_edt_depth-1), however
-        ** we need to adjust MR_edt_depth here for future events.
-        */
-        return MR_edt_depth--;
-    }
-
-    return MR_edt_depth;
-}
-
 static void
 MR_trace_construct_node(MR_Event_Info *event_info)
 {
Index: trace/mercury_trace_declarative.h
===================================================================
RCS file: /home/mercury1/repository/mercury/trace/mercury_trace_declarative.h,v
retrieving revision 1.26
diff -u -b -r1.26 mercury_trace_declarative.h
--- trace/mercury_trace_declarative.h	12 Aug 2005 02:01:27 -0000	1.26
+++ trace/mercury_trace_declarative.h	13 Aug 2005 10:30:52 -0000
@@ -108,6 +108,34 @@
 extern	void	MR_trace_decl_set_testing_flag(MR_bool testing);

 /*
+** The depth of the EDT is different from the call depth of the events, since
+** the call depth is not guaranteed to be the same for the children of a call
+** event - see comments in MR_trace_real in trace/mercury_trace.c.  We use the
+** following variable to keep track of the EDT depth.  We only keep track of
+** the depth of events in and below the subtree we are materializing.
+** MR_edt_depth is 0 for the root of the subtree we are materializing.
+*/
+
+extern  MR_Integer          MR_edt_depth;
+
+/*
+** Events where the value of MR_edt_depth above is greater than the value of
+** MR_edt_max_depth will not be included in tha annotated trace.
+*/
+
+extern  MR_Unsigned         MR_edt_max_depth;
+
+/*
+** MR_edt_implicit_subtree_counters points to an array that records
+** the number of events at different depths in implicit subtrees.
+** These are used to determine what depth an implicit subtree should
+** be materialized to.
+*/
+
+extern  MR_Unsigned         *MR_edt_implicit_subtree_counters;
+extern  MR_Unsigned         MR_edt_implicit_subtree_num_counters;
+
+/*
 ** The following macros are provided to help C code manipulate the
 ** Mercury data structure.  The values here must match the corresponding
 ** values in the definitions in browser/declarative_execution.m.
@@ -174,6 +202,29 @@
 #define	MR_DECL_DISPLAY_PROGRESS_DELAY	1000

 /*
+** The following macro works out the depth of a node in the EDT.
+** It has the side-effect of updating MR_edt_depth.
+*/
+
+#define MR_DD_CALC_NODE_DEPTH(port, node_depth)                               \
+    do {                                                                      \
+        if (port == MR_PORT_CALL || port == MR_PORT_REDO) {                   \
+            node_depth = ++MR_edt_depth;                                      \
+        } else {                                                              \
+            if (MR_port_is_final(port)) {                                     \
+                /*                                                            \
+                ** The depth of the EXIT, FAIL or EXCP event is               \
+                ** MR_edt_depth (not MR_edt_depth-1), however                 \
+                ** we need to adjust MR_edt_depth here for future events.     \
+                */                                                            \
+                node_depth = MR_edt_depth--;                                  \
+            } else {                                                          \
+                node_depth = MR_edt_depth;                                    \
+            }                                                                 \
+        }                                                                     \
+    } while(0)
+
+/*
 ** When building a new explicit tree we build it to the maximum depth such
 ** that the number of nodes in the explicit tree is less than or equal to
 ** MR_edt_desired_nodes_in_subtree.

--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------



More information about the reviews mailing list