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

Ian MacLarty maclarty at cs.mu.OZ.AU
Sun Aug 14 14:07:49 AEST 2005


On Sun, 14 Aug 2005, Zoltan Somogyi wrote:

> On 13-Aug-2005, Ian MacLarty <maclarty at cs.mu.OZ.AU> wrote:
> > 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.
>
> Is this 3% before or after the 56%? What is 3% before the 56% becomes 6% after,
> which is worthwhile.
>

It's 3% faster after the 56%.

> > +        /*
> > +        ** 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++;
>
> It would be nice if we could avoid games with decrement/increment/decrement
> again. At least, you should document (a) where the initial decrement is, since
> it is not visible here, and (b) why you want an overall decrement in the
> first place.
>

Okay.  What I've done is define a separate global which keeps track of the
depth of nodes in implicit subtrees and use this in
MR_trace_real_decl_implicit_subtree.  This also saves a minus.
See the interdiff below.

> > +        {
> > +            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;
>
> Please add a comment such as
>
> 	/* MR_TRACE_EVENT_TEARDOWN ends by returning jumpaddr to the caller */
>

Done.

Here's the interdiff.

diff -bu trace/mercury_trace.c trace/mercury_trace.c
--- trace/mercury_trace.c	13 Aug 2005 10:47:08 -0000
+++ trace/mercury_trace.c	14 Aug 2005 03:36:49 -0000
@@ -575,7 +575,6 @@
     MR_Unsigned         depth;
     MR_Trace_Port       port;
     MR_Unsigned         node_depth;
-    MR_Unsigned         depth_in_imp_subtree;

     MR_TRACE_REAL_SETUP_CODE();

@@ -590,30 +589,22 @@
     ** 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);
+    MR_DD_CALC_NODE_DEPTH(port, node_depth, MR_edt_implicit_subtree_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]++;
+    if ((node_depth == 0) && MR_port_is_final(port)) {
+        MR_edt_implicit_subtree_counters[node_depth]++;

         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 returns jumpaddr */
             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]++;
+        if (node_depth < MR_edt_implicit_subtree_num_counters) {
+            MR_edt_implicit_subtree_counters[node_depth]++;
         }
     }
     return NULL;
diff -bu trace/mercury_trace_declarative.c trace/mercury_trace_declarative.c
--- trace/mercury_trace_declarative.c	13 Aug 2005 10:10:46 -0000
+++ trace/mercury_trace_declarative.c	14 Aug 2005 03:30:34 -0000
@@ -450,6 +450,7 @@

 MR_Integer      MR_edt_depth;
 MR_Unsigned     MR_edt_max_depth;
+MR_Integer      MR_edt_implicit_subtree_depth;

 MR_Unsigned     *MR_edt_implicit_subtree_counters;
 MR_Unsigned     MR_edt_implicit_subtree_num_counters;
@@ -484,7 +485,7 @@
         return jumpaddr;
     }

-    MR_DD_CALC_NODE_DEPTH(port, node_depth);
+    MR_DD_CALC_NODE_DEPTH(port, node_depth, MR_edt_depth);

     if (node_depth == MR_edt_max_depth
             && (port == MR_PORT_CALL || port == MR_PORT_REDO))
@@ -494,7 +495,8 @@
         ** top of an implicit subtree.
         */
         MR_trace_reset_implicit_subtree_counters();
-        MR_edt_implicit_subtree_counters[node_depth - MR_edt_max_depth]++;
+        MR_edt_implicit_subtree_counters[0]++;
+        MR_edt_implicit_subtree_depth = 0;
         MR_selected_trace_func_ptr = MR_trace_real_decl_implicit_subtree;
     }

diff -bu trace/mercury_trace_declarative.h trace/mercury_trace_declarative.h
--- trace/mercury_trace_declarative.h	13 Aug 2005 10:30:52 -0000
+++ trace/mercury_trace_declarative.h	14 Aug 2005 03:44:15 -0000
@@ -109,16 +109,28 @@

 /*
 ** 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
+** the call depth is not guaranteed to be increase by one each time
+** -- 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.
+** the depth of events in the subtree we are materializing.
+** MR_edt_depth keeps track of the depth of nodes in the EDT by storing the
+** depth of the next event if it is a final or internal event.  If the next
+** event is a CALL or REDO it will have a depth of MR_edt_depth + 1.
+** The CALL and final events of the root of the subtree being materialized
+** have depth 0.
 */

 extern  MR_Integer          MR_edt_depth;

 /*
+** MR_edt_implicit_subtree_depth performs the same role as MR_edt_depth,
+** except that it keeps track of the depth of nodes in implicit subtrees of
+** the tree being materialized.
+*/
+
+extern	MR_Integer	MR_edt_implicit_subtree_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.
 */
@@ -203,23 +215,25 @@

 /*
 ** The following macro works out the depth of a node in the EDT.
-** It has the side-effect of updating MR_edt_depth.
+** It also updates next_depth to the depth of the next node if that node is
+** a final or internal event.
+** If the next node is a CALL or REDO it will have a depth of next_depth + 1.
 */

-#define MR_DD_CALC_NODE_DEPTH(port, node_depth)                               \
+#define MR_DD_CALC_NODE_DEPTH(port, node_depth, next_depth)                   \
     do {                                                                      \
         if (port == MR_PORT_CALL || port == MR_PORT_REDO) {                   \
-            node_depth = ++MR_edt_depth;                                      \
+            node_depth = ++next_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.     \
+                ** next_depth (not next_depth - 1), however                   \
+                ** we need to adjust next_depth here for future events.       \
                 */                                                            \
-                node_depth = MR_edt_depth--;                                  \
+                node_depth = next_depth--;                                    \
             } else {                                                          \
-                node_depth = MR_edt_depth;                                    \
+                node_depth = next_depth;                                      \
             }                                                                 \
         }                                                                     \
     } while(0)

--------------------------------------------------------------------------
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