[m-rev.] for review: speed up the declarative debugger by 127%
Ian MacLarty
maclarty at cs.mu.OZ.AU
Sat Aug 13 23:44:20 AEST 2005
> For review by anyone.
>
Forgot this bit:
Estimated hours taken: 5
Branches: main
> 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
> --------------------------------------------------------------------------
>
--------------------------------------------------------------------------
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