[m-rev.] for review: show cliques in mdb stack traces less obtrusively

[Peter Wang]

On Tue, 01 Oct 2024 20:21:30 +0200 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> For review by anyone.
> 
> The suggestion that this diff acts upon is Peter's, and it is
> from 2012, so better late than never.
> 
> The diff includes debugging output that shows the boundaries
> of cliques, to make the correctness of the updated output easier
> to see. I intend to delete these before commit.
> 
> Zoltan.

> diff --git a/runtime/mercury_stack_trace.c b/runtime/mercury_stack_trace.c
> index f28db366d..36422db0c 100644
> --- a/runtime/mercury_stack_trace.c
> +++ b/runtime/mercury_stack_trace.c
> @@ -585,16 +591,23 @@ MR_dump_stack_from_layout_clique(FILE *fp, const MR_LabelLayout *label_layout,
>                  &params, &dump_info, walked_stack[level].ste_label_layout,
>                  walked_stack[level].ste_trace_sp,
>                  walked_stack[level].ste_trace_curfr,
> -                walked_stack[level].ste_reused_frames, print_stack_record,
> -                lines_dumped_so_far >= line_limit);
> +                // The stack frame contains some cliques, which means that
> +                // all its lines have space for the boxes. Add the same amount
> +                // of padding, to make the columns of in-clique and
> +                // not-in-clique calls line up.
> +                walked_stack[level].ste_reused_frames, MR_CLIQUE_FRAME_OUTSIDE,
> +                print_stack_record, lines_dumped_so_far >= line_limit);
>          }
>          MR_dump_stack_record_flush(fp, &params, &dump_info,
>              print_stack_record);
>  
> -        fprintf(fp, "<mutually recursive set of stack frames start>\n");
> +        // ZZZ fprintf(fp, "<mutually recursive set of stack frames start>\n");
> +        clique_frame_bytes_ptr = MR_CLIQUE_FRAME_TOP;
>          lines_dumped_before_clique = lines_dumped_so_far;
>          for (; level <= cl->cl_last_level; level++) {
>              if (lines_dumped_so_far >= line_limit) {
> +                dump_info.sdi_prev_frame_dump_info.MR_sdi_clique_frame_marker =
> +                    MR_CLIQUE_FRAME_BOTTOM;

I think incompletely dumped cliques should end with
MR_CLIQUE_FRAME_MIDDLE or a different character (see below).

>                  MR_dump_stack_record_flush(fp, &params, &dump_info,
>                      print_stack_record);
>                  fprintf(fp, "<more stack frames snipped>\n");

> @@ -605,6 +618,8 @@ MR_dump_stack_from_layout_clique(FILE *fp, const MR_LabelLayout *label_layout,
>              if (lines_dumped_so_far - lines_dumped_before_clique
>                  >= clique_line_limit)
>              {
> +                dump_info.sdi_prev_frame_dump_info.MR_sdi_clique_frame_marker =
> +                    MR_CLIQUE_FRAME_BOTTOM;
>                  MR_dump_stack_record_flush(fp, &params, &dump_info,
>                      print_stack_record);
>                  fprintf(fp, "<more stack frames in clique snipped>\n");

Same here.

> diff --git a/runtime/mercury_stack_trace.h b/runtime/mercury_stack_trace.h
> index f543cc9f1..3b4ddafbc 100644
> --- a/runtime/mercury_stack_trace.h
> +++ b/runtime/mercury_stack_trace.h
> @@ -72,6 +59,13 @@ typedef struct {
>      // recursive calls.
>      //
>      // If include_trace_data is TRUE, frame_count should be 1.
> +    //
> +    // If the clique frame marker field is non-NULL, then it points to
> +    // a short (two-character) string that indicates whether the call or calls
> +    // denoted by this structure is inside a clique or not, and if it is,
> +    // then whether it the first call in the clique, the last call in the
> +    // clique, or neither. Note that the characters will be UTF-8, but
> +    // will often be non-ASCII.

Say that the string will be UTF-8 encoded, and will often contain
non-ASCII characters.

> +clique: 9 to 14
> +   0         pred mutrec.r0c/3-0 (det) (mutrec.m:207)
> +   1 ┌    3* pred mutrec.q3/3-0 (det) (mutrec.m:188 and others)
> +   4 │       pred mutrec.q2/3-0 (det) (mutrec.m:164)
> +   5 └       pred mutrec.q3/3-0 (det) (mutrec.m:184)
>  <more stack frames in clique snipped>

Using U+2506 BOX DRAWINGS LIGHT TRIPLE DASH VERTICAL, for example,
on the last dumped line would look like this:

      0         pred mutrec.r0c/3-0 (det) (mutrec.m:207)
      1 ┌    3* pred mutrec.q3/3-0 (det) (mutrec.m:188 and others)
      4 │       pred mutrec.q2/3-0 (det) (mutrec.m:164)
      5 ┆       pred mutrec.q3/3-0 (det) (mutrec.m:184)
   <more stack frames in clique snipped>

The patch looks good, otherwise.

Peter