microsoft / MIEngine

The Visual Studio MI Debug Engine ("MIEngine") provides an open-source Visual Studio Debugger extension that works with MI-enabled debuggers such as gdb and lldb.
MIT License
818 stars 218 forks source link

[#1349] Update the default stack trace format to include values. #1350

Closed gareth-rees closed 2 years ago

gareth-rees commented 2 years ago

Summary

This reduces the number of round-trips to the debugger, because when values are requested, the -stack-list-arguments command also returns the type of each argument immediately, avoiding the need for subsequent -var-create and -var-delete commands to get these types.

Fixes #1349 (Unnecessary -var-create and -var-delete commands during a stack trace can cause noticeable pause each time the debuggee stops)

Details

I repeated the reproduction procedure from #1349, took a copy of the commands issued by a single Step Over operation with ten frames on the stack and attached them here: stack-list-arguments-after.gz. Here's a comparison of the before and after counts of commands:

Command Counts (main) Counts (this branch)
-stack-list-arguments 2 1
-stack-list-frames 1 1
-stack-list-variables 1 1
-stack-select-frame 20 0
-var-create 210 10
-var-delete 210 10

Notes

A side-effect of this change is that we get values in the CALL STACK window in Visual Studio Code (see screenshots below). I think this is an improvement, but since it is a big UX change you need to take it into consideration.

main this branch
Screenshot from 2022-09-06 16-02-07 Screenshot from 2022-09-06 16-03-40

If this is not acceptable, let me know, and I can implement solution 2 from #1349 instead.

gregg-miskelly commented 2 years ago

I believe this is going to regress the scenario fixed in https://github.com/Microsoft/MIEngine/pull/673 where the problem was that GDB could take a long time to obtain values.

I wonder if Gareth's scenario could be fixed by kicking off all the -var-create's in parallel, and not waiting on the results of the -var-deletes.

gregg-miskelly commented 2 years ago

I haven't had time to test it, but I created a branch to play with that kind of fix to see if it helps your scenario: https://github.com/gregg-miskelly/MIEngine/pull/new/ParallelGetTypeOfArgAsync

gareth-rees commented 2 years ago

Thanks for the link to #673, this explains why the default format excludes values. But this is surprising to me, since the GDB documentation says that

if print-values is 2 or --simple-values, print the name, type and value for simple data types, and the name and type for arrays, structures and unions

so I didn't expect that values of large objects would be printed by --simple-values. The relevant GDB source code is here:

case PRINT_SIMPLE_VALUES:
  type = check_typedef (sym2->type ());
  if (type->code () != TYPE_CODE_ARRAY
      && type->code () != TYPE_CODE_STRUCT
      && type->code () != TYPE_CODE_UNION)
    {

which means that C++ reference types (TYPE_CODE_REF and TYPE_CODE_RVALUE_REF) get treated as "simple" values. Possibly this is something that can be fixed in GDB, which would allow MIEngine to add values to the default format (guarded by a suitable GDB feature). I will investigate whether this is possible.

(I doubt that parallelizing the -var-create commands will help my case, because I'm in the "GDB before-prompt hook" case and so all the hooks have to be run before any new operations can take place. I can pursue other options here, for example disabling the hook during the stack trace. So don't spend too much time on this.)

gareth-rees commented 2 years ago

I think it makes sense to close this PR: I will open another one if I come up with a better approach.