* [PATCH v3] gdb/DAP Introduce new methods to to Pretty Printers
@ 2023-06-26 15:26 Simon Farre
0 siblings, 0 replies; only message in thread
From: Simon Farre @ 2023-06-26 15:26 UTC (permalink / raw)
To: gdb-patches; +Cc: tom, Simon Farre
v3.
Removed the "NoOpScalar" bug fix, since tom addresses it in his patch
here: https://sourceware.org/pipermail/gdb-patches/2023-June/200472.html
v2.
I've added some additional notes as per Eli's Request to the
documentation and fixed formatting.
v1.
This patch introduces the `num_children` method to the NoOpArray-pretty
printer as well as fixing a bug where some types accidentally uses
NoOpScalar pretty printers. It also cleans up the retrieval of variables
during a "variables" request (or an "evaluate" request) reducing the
complexity.
Caching of variables would be fine if we
actually kept variables around during step/continues that stops inside
the same frame we're currently at - this is how I did it for my
DA-implementation Midas, to be able to be faster than most other DA:s;
meaning, I kept the `gdb.Value`s around until we've left the frame they
were created in. However, I think this breaks with the
DAP-specification that states that variableReferences should only ever
be valid "during a stop". So unless we actually build some
"remembering mechanism" that keeps variableReferences and invalidates
them only once we figure out we're not in the same frame anymore,
caching the values (children) is not important. Regardless, that kind of
functionality would rely on mechanisms other than the ones I've removed
here.
Going forward, Pretty Printers of "array type" should be expected (though not required)
to provide the `num_children` method, in order to be able to provide a
performant DAP-experience. They should also provide `children_range`,
a method meant to retrieve a sub-range of children of a Pretty Printer
wrapped variable without having to iterate over all prior elements of
the range. This method is appropriate on pretty printers that wrap
std::vector, arrays, etc. Even though these methods are not required,
there's no good reason to not implement them, because it provides an
absolutely astronomical performance difference. I've provided the method
for the NoOpArrayPrinter.
After hacking the std-library pretty printer for std::vector to provide
these two methods, servicing one "variables" request of the "Locals"
scope where a std::vector<std::string> of 10k elements exist, went from
35 seconds (!!!) down to 0.4 ms. For a 10k element of std::vector<int>
it went from 100 ms down to the same 0.4ms, also. The same went for
retrieving a sub-range of elements 9800-9900; it went from 35 seconds
(due to having to iterate over _all_ prior children) down to 350ms, for
std::vector<std::string> - the still-very-high time count is because of
std::string's Pretty Printer implementation which is incredibly slow.
For the std::vector<int>, retrieving that same sub-range was in the ~10
ms range.
These two new optional methods have been added to documentation as well
as an example that mimicks the layout of `std::vector`, to show an
example of how they can be implemented.
---
gdb/doc/python.texi | 55 +++++++++++++++++++++++++
gdb/python/lib/gdb/dap/scopes.py | 70 ++++++++++++++++++++++++--------
gdb/python/lib/gdb/dap/varref.py | 68 +++++++++----------------------
gdb/python/lib/gdb/printing.py | 29 ++++++++++---
4 files changed, 150 insertions(+), 72 deletions(-)
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 9cb97a9597d..eddc1a96a94 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -1707,6 +1707,26 @@ Children may be hidden from display based on the value of @samp{set
print max-depth} (@pxref{Print Settings}).
@end defun
+@defun pretty_printer.children_range (self, start, end)
+This method is optional but @value{GDBN} will call this method to
+be able to more effeciently compute a sub-range of children
+of the pretty-printers value. If the pretty printer is for an
+array-like type, it is recommended that this method is implemented
+as the DAP interpreter will make extended use of it. For an example on how
+to best implement this method @pxref{Writing a Pretty-Printer}. Not
+implementing this and @code{num_children} can have drastic effects on
+the performance of a DAP-interpreter debug session. If this method is not
+implemented GDB will do eager evaluation of variables in a DAP-session
+which will come at a performance cost.
+@end defun
+
+@defun pretty_printer.num_children (self)
+@value{GDBN} can call this method to query how many children this pretty
+printer will return from a @code{pretty_printer.children} call. This method
+is used by the DAP interpreter to be able to display paginated results, as
+well as use @code{pretty_printer.children_range} to be more effecient.
+@end defun
+
@defun pretty_printer.display_hint (self)
The CLI may call this method and use its result to change the
formatting of a value. The result will also be supplied to an MI
@@ -1965,10 +1985,43 @@ These are the types we are going to pretty-print:
@smallexample
struct foo @{ int a, b; @};
struct bar @{ struct foo x, y; @};
+struct vec @{ foo* start; foo* current; foo* end; @}
+
@end smallexample
Here are the printers:
+@smallexample
+class vecPrinter:
+ """Print a vec of foo objects."""
+
+ def __init__(self, val):
+ self.val = val
+
+ def to_string(self):
+ return ""
+
+ def num_children(self):
+ first = self.val["start"]
+ last = self.val["current"]
+ return last - first
+
+ def children_range(self, start, end):
+ ptr = self.val["start"]
+ for i in range(start, end):
+ yield (str(i), (ptr + i).dereference())
+
+ def children(self):
+ ptr = self.val["start"]
+ size = int(self.num_children())
+ for i in range(0, size):
+ yield (str(i), (ptr + i).dereference())
+
+ def display_hint(self):
+ return "array"
+
+@end smallexample
+
@smallexample
class fooPrinter:
"""Print a foo object."""
@@ -2003,6 +2056,7 @@ def build_pretty_printer():
"my_library")
pp.add_printer('foo', '^foo$', fooPrinter)
pp.add_printer('bar', '^bar$', barPrinter)
+ pp.add_printer('vec', '^vec$', barPrinter)
return pp
@end smallexample
@@ -2025,6 +2079,7 @@ my_library.so:
my_library
foo
bar
+ vec
@end smallexample
@node Type Printing API
diff --git a/gdb/python/lib/gdb/dap/scopes.py b/gdb/python/lib/gdb/dap/scopes.py
index 9b80dd9ce80..10d9a5a6b97 100644
--- a/gdb/python/lib/gdb/dap/scopes.py
+++ b/gdb/python/lib/gdb/dap/scopes.py
@@ -49,48 +49,76 @@ def _block_vars(block):
return (args, locs)
+class ScopePrinter:
+ def __init__(self, frame, variables):
+ self.variables = variables
+ self.frame = frame
+
+ def to_string(self):
+ return ""
+
+ @in_gdb_thread
+ def children(self):
+ for sym in self.variables:
+ if sym.needs_frame:
+ val = sym.value(self.frame)
+ else:
+ val = sym.value()
+ yield ((sym.print_name, val))
+
+ def num_children(self):
+ return len(self.variables)
+
+
class ScopeReference(BaseReference):
- def __init__(self, name, hint, frame, var_list):
+ def __init__(self, name, hint, frame, printer):
super().__init__(name)
self.hint = hint
self.frame = frame
self.func = frame.function()
- self.var_list = var_list
+ self.printer = printer
def to_object(self):
result = super().to_object()
result["presentationHint"] = self.hint
# How would we know?
result["expensive"] = False
- result["namedVariables"] = len(self.var_list)
+ result["namedVariables"] = self.printer.num_children()
if self.func is not None:
result["line"] = self.func.line
# FIXME construct a Source object
return result
def child_count(self):
- return len(self.var_list)
+ return self.printer.num_children()
+
+
+class RegisterFilePrinter:
+ def __init__(self, frame, registers):
+ self.registers = registers
+ self.frame = frame
+
+ def to_string(self):
+ return ""
@in_gdb_thread
- def fetch_one_child(self, idx):
- sym = self.var_list[idx]
- if sym.needs_frame:
- val = sym.value(self.frame)
- else:
- val = sym.value()
- return (sym.print_name, val)
+ def children(self):
+ for reg in self.registers:
+ yield (reg.name, self.frame.read_register(reg))
+
+ def num_children(self):
+ return len(self.registers)
class RegisterReference(ScopeReference):
def __init__(self, name, frame):
super().__init__(
- name, "registers", frame, list(frame.architecture().registers())
+ name,
+ "registers",
+ frame,
+ RegisterFilePrinter(frame, list(frame.architecture().registers())),
)
- @in_gdb_thread
- def fetch_one_child(self, idx):
- return (self.var_list[idx].name, self.frame.read_register(self.var_list[idx]))
-
# Helper function to create a DAP scopes for a given frame ID.
@in_gdb_thread
@@ -101,9 +129,15 @@ def _get_scope(id):
if block is not None:
(args, locs) = _block_vars(block)
if args:
- scopes.append(ScopeReference("Arguments", "arguments", frame, args))
+ scopes.append(
+ ScopeReference(
+ "Arguments", "arguments", frame, ScopePrinter(frame, args)
+ )
+ )
if locs:
- scopes.append(ScopeReference("Locals", "locals", frame, locs))
+ scopes.append(
+ ScopeReference("Locals", "locals", frame, ScopePrinter(frame, locs))
+ )
scopes.append(RegisterReference("Registers", frame))
return [x.to_object() for x in scopes]
diff --git a/gdb/python/lib/gdb/dap/varref.py b/gdb/python/lib/gdb/dap/varref.py
index 213151fd3d3..bc73db7f45d 100644
--- a/gdb/python/lib/gdb/dap/varref.py
+++ b/gdb/python/lib/gdb/dap/varref.py
@@ -17,7 +17,7 @@ import gdb
from .startup import in_gdb_thread
from .server import client_bool_capability
from abc import abstractmethod
-
+from itertools import islice
# A list of all the variable references created during this pause.
all_variables = []
@@ -73,19 +73,9 @@ class BaseReference:
"""Call this to declare that this variable or scope has no children."""
self.ref = 0
- @abstractmethod
- def fetch_one_child(self, index):
- """Fetch one child of this variable.
-
- INDEX is the index of the child to fetch.
- This should return a tuple of the form (NAME, VALUE), where
- NAME is the name of the variable, and VALUE is a gdb.Value."""
- return
-
@abstractmethod
def child_count(self):
- """Return the number of children of this variable."""
- return
+ return None
@in_gdb_thread
def fetch_children(self, start, count):
@@ -95,15 +85,19 @@ class BaseReference:
COUNT is the number to return, with 0 meaning return all."""
if count == 0:
count = self.child_count()
- if self.children is None:
- self.children = [None] * self.child_count()
result = []
- for idx in range(start, start + count):
- if self.children[idx] is None:
- (name, value) = self.fetch_one_child(idx)
- self.children[idx] = VariableReference(name, value)
- result.append(self.children[idx])
- return result
+ stop = start + count if count is not None else None
+
+ if hasattr(self.printer, "children_range"):
+ # Performant path
+ for (name, value) in self.printer.children_range(start, start + count):
+ result.append(VariableReference(name, value))
+ return result
+ else:
+ # The extremely slow path, if type is a "array-like"
+ for (name, value) in islice(self.printer.children(), start, stop):
+ result.append(VariableReference(name, value))
+ return result
class VariableReference(BaseReference):
@@ -124,35 +118,17 @@ class VariableReference(BaseReference):
self.child_cache = None
if not hasattr(self.printer, "children"):
self.no_children()
- self.count = None
- else:
- self.count = -1
-
- def cache_children(self):
- if self.child_cache is None:
- # This discards all laziness. This could be improved
- # slightly by lazily evaluating children, but because this
- # code also generally needs to know the number of
- # children, it probably wouldn't help much. A real fix
- # would require an update to gdb's pretty-printer protocol
- # (though of course that is probably also inadvisable).
- self.child_cache = list(self.printer.children())
- return self.child_cache
def child_count(self):
- if self.count is None:
- return None
- if self.count == -1:
- if hasattr(self.printer, "num_children"):
- num_children = self.printer.num_children
- else:
- num_children = len(self.cache_children())
- self.count = num_children
- return self.count
+ if hasattr(self.printer, "num_children"):
+ return self.printer.num_children()
+ return None
def to_object(self):
result = super().to_object()
- result[self.result_name] = self.printer.to_string()
+ result[self.result_name] = (
+ str(self.value) if self.ref == 0 else str(self.printer.to_string())
+ )
num_children = self.child_count()
if num_children is not None:
if (
@@ -170,10 +146,6 @@ class VariableReference(BaseReference):
result["type"] = str(self.value.type)
return result
- @in_gdb_thread
- def fetch_one_child(self, idx):
- return self.cache_children()[idx]
-
@in_gdb_thread
def find_variable(ref):
diff --git a/gdb/python/lib/gdb/printing.py b/gdb/python/lib/gdb/printing.py
index 353427d000a..b6cd6aa1a66 100644
--- a/gdb/python/lib/gdb/printing.py
+++ b/gdb/python/lib/gdb/printing.py
@@ -278,18 +278,18 @@ class NoOpScalarPrinter:
def to_string(self):
return self.value.format_string(raw=True)
+ def num_children(self):
+ return 0
+
class NoOpArrayPrinter:
"""A no-op pretty printer that wraps an array value."""
- def __init__(self, value):
+ def __init__(self, ty, value):
self.value = value
- (low, high) = self.value.type.range()
+ (low, high) = ty.range()
self.low = low
self.high = high
- # This is a convenience to the DAP code and perhaps other
- # users.
- self.num_children = high - low + 1
def to_string(self):
return ""
@@ -301,11 +301,19 @@ class NoOpArrayPrinter:
for i in range(self.low, self.high + 1):
yield (i, self.value[i])
+ def num_children(self):
+ return self.high - self.low + 1
+
+ def children_range(self, start, end):
+ for i in range(start, end):
+ yield (i, self.value[start])
+
class NoOpStructPrinter:
"""A no-op pretty printer that wraps a struct or union value."""
- def __init__(self, value):
+ def __init__(self, ty, value):
+ self.ty = ty
self.value = value
def to_string(self):
@@ -316,6 +324,15 @@ class NoOpStructPrinter:
if field.name is not None:
yield (field.name, self.value[field])
+ def num_children(self):
+ return len(
+ [
+ x
+ for x in self.value.type.fields()
+ if x.name is not None and hasattr(x, "bitpos")
+ ]
+ )
+
def make_visualizer(value):
"""Given a gdb.Value, wrap it in a pretty-printer.
--
2.41.0
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2023-06-26 15:27 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-26 15:26 [PATCH v3] gdb/DAP Introduce new methods to to Pretty Printers Simon Farre
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).