Re: question about expand_symtabs_matching()

Re: question about expand_symtabs_matching()

[Tom Tromey]

>>>>> "Hannes" == Hannes Domani via gdb <gdb@sourceware.org> writes:

Hannes> And in case a shared library is unloaded, I would have thought
Hannes> that you could just removeall breakpoints in the address range
Hannes> of the shared library.

Yes.

At one point we had discussed this, but I guess it never really
happened.  So, something to do :-)

Hannes> What if I have a simple breakpoint like 'break some-file.c:123', is there even any
Hannes> symbol that can be expanded with expand_symtabs_matching()?

Nope, but then again it isn't called in this scenario; at least not if
"some-file.c" exists.

Tom

Re: question about expand_symtabs_matching()

[Hannes Domani via gdb]

Am Donnerstag, 28. Februar 2019, 00:55:50 MEZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:

> >>>>> "Hannes" == Hannes Domani via gdb <gdb@sourceware.org> writes:
> 
> Hannes> And in case a shared library is unloaded, I would have thought
> Hannes> that you could just removeall breakpoints in the address range
> Hannes> of the shared library.
> 
> Yes.
> 
> At one point we had discussed this, but I guess it never really
> happened.  So, something to do :-)
> 
> Hannes> What if I have a simple breakpoint like 'break some-file.c:123', is there even any
> Hannes> symbol that can be expanded with expand_symtabs_matching()?
> 
> Nope, but then again it isn't called in this scenario; at least not if
> "some-file.c" exists.

I should have clarified that I'm currently investigating why
*pending* breakpoints slow down gdb so much.

The last 3 days I've been staring a lot at profiling flamegraphs
of gdb for the breakpoint types (again, pending only) I use
most ('b some-file.c:123' & 'b function'), and expand_symtabs_matching()
is one of the biggest time-consumers.


Similar, for the case of a simple function name as pending breakpoint,
it's cp_canonicalize_string_no_typedefs() called by find_linespec_symbols(),
that's taking most of the time.
And I'm wondering if this call is necessary if you only use the function name
without arguments (like 'function' or Class::member_function).


If you are interested, I could also send you the profiling flamegraphs.


Regards
Hannes Domani

Re: question about expand_symtabs_matching()

[Tom Tromey]

Tom> Nope, but then again it isn't called in this scenario; at least not if
Tom> "some-file.c" exists.

Hannes> I should have clarified that I'm currently investigating why
Hannes> *pending* breakpoints slow down gdb so much.

Ok.  It's been a while, but IIRC in some cases linespec can't decide how
to parse a given string until it has seen some match.  So, sometimes
re-setting pending breakpoint can be more expensive.

For example I think gdb can't tell the difference between
"break filename:function" and "break function:label".

This is addressed by the "explicit locations" work, though I don't think
this work extends all the way to fine-grained breakpoint re-setting.

Personally I'd be fine with changing the syntax of the more obscure
linespecs ("function:label" is probably not used much in practice) in
order to make the parsing more sane, if it had a performance benefit
here.  But that's just me.

Also, a negative result is still a parse result; so if a linespec
understood which objfiles had been searched, it could still easily skip
this work on dlopen or dlclose.

Hannes> Similar, for the case of a simple function name as pending breakpoint,
Hannes> it's cp_canonicalize_string_no_typedefs() called by find_linespec_symbols(),
Hannes> that's taking most of the time.
Hannes> And I'm wondering if this call is necessary if you only use the function name
Hannes> without arguments (like 'function' or Class::member_function).

I doubt it's needed but on the other hand it may not really save much.
Maybe one way to do the experiment is check the string for
non-identifier characters before trying to canonicalize it.

Hannes> If you are interested, I could also send you the profiling flamegraphs.

It's an area I'm interested in but I'm not actively working there right
now.  If you're interested at all in gdb development ... on the one hand
tackling linespec caching is maybe a difficult project, but on the other
hand it seems fun :-)

Tom

Re: question about expand_symtabs_matching()

[Hannes Domani via gdb]

Am Freitag, 1. März 2019, 19:30:56 MEZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:

> Personally I'd be fine with changing the syntax of the more obscure
> linespecs ("function:label" is probably not used much in practice) in
> order to make the parsing more sane, if it had a performance benefit
> here.  But that's just me.

I didn't even know before I started looking into this that function:label
is possible.


> Also, a negative result is still a parse result; so if a linespec
> understood which objfiles had been searched, it could still easily skip
> this work on dlopen or dlclose.

If you mean that it should only search the newly loaded shared library,
then yes, I agree.


> Hannes> Similar, for the case of a simple function name as pending breakpoint,
> Hannes> it's cp_canonicalize_string_no_typedefs() called by find_linespec_symbols(),
> Hannes> that's taking most of the time.
> Hannes> And I'm wondering if this call is necessary if you only use the function name
> Hannes> without arguments (like 'function' or Class::member_function).
> 
> I doubt it's needed but on the other hand it may not really save much.
> Maybe one way to do the experiment is check the string for
> non-identifier characters before trying to canonicalize it.

For the case of 'b some_function', with the application I'm testing:
- startup time with the call of cp_canonicalize_string_no_typedefs(): 1m 25s
- startup time without this call: 27s


> Hannes> If you are interested, I could also send you the profiling flamegraphs.
> 
> It's an area I'm interested in but I'm not actively working there right
> now.  If you're interested at all in gdb development ... on the one hand
> tackling linespec caching is maybe a difficult project, but on the other
> hand it seems fun :-)

In my personal build I've changed this:
- only call cp_canonicalize_string_no_typedefs() if it's not a simple function
  name (like 'function_name' or 'Class::member_function')
- only call expand_symtabs_matching() if the lookup_name doesn't contain a '.'

I admit that I don't fully understand what could break with these changes,
but the speedup makes it worth for me right now.


Regards
Hannes Domani

Re: question about expand_symtabs_matching()

[Tom Tromey]

>>>>> "Hannes" == Hannes Domani via gdb <gdb@sourceware.org> writes:

Hannes> For the case of 'b some_function', with the application I'm testing:
Hannes> - startup time with the call of cp_canonicalize_string_no_typedefs(): 1m 25s
Hannes> - startup time without this call: 27s

Wow.

Hannes> In my personal build I've changed this:
Hannes> - only call cp_canonicalize_string_no_typedefs() if it's not a simple function
Hannes>   name (like 'function_name' or 'Class::member_function')
Hannes> - only call expand_symtabs_matching() if the lookup_name doesn't contain a '.'

Hannes> I admit that I don't fully understand what could break with these changes,
Hannes> but the speedup makes it worth for me right now.

Could you send the diff?

thanks,
Tom

Fw: question about expand_symtabs_matching()

[Hannes Domani via gdb]

[-- Attachment #1: Type: text/plain, Size: 1140 bytes --]

Am Freitag, 8. März 2019, 22:13:52 MEZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:

> >>>>> "Hannes" == Hannes Domani via gdb <gdb@sourceware.org> writes:
> 
> Hannes> For the case of 'b some_function', with the application I'm testing:
> Hannes> - startup time with the call of cp_canonicalize_string_no_typedefs(): 1m 25s
> Hannes> - startup time without this call: 27s
> 
> Wow.
> 
> Hannes> In my personal build I've changed this:
> Hannes> - only call cp_canonicalize_string_no_typedefs() if it's not a simple function
> Hannes>   name (like 'function_name' or 'Class::member_function')
> Hannes> - only call expand_symtabs_matching() if the lookup_name doesn't contain a '.'
>
 > Hannes> I admit that I don't fully understand what could break with these changes,
> Hannes> but the speedup makes it worth for me right now.
> 
> Could you send the diff?

Both are attached.

As said before, the first improves the startup time (with 1 pending function breakpoint) from 1m 25s to 27s.

And the second improves it (with 1 pending source:line breakpoint) from 27s to 13s.


Regards
Hannes Domani

[-- Attachment #2: 0001-Don-t-expand-typedefs-for-functions-without-argument.patch --]
[-- Type: application/octet-stream, Size: 1530 bytes --]

From 2902f8e28b5d8e3c2bf4010c8bf495ae4bc98ff3 Mon Sep 17 00:00:00 2001
From: Hannes Domani <ssbssa@yahoo.de>
Date: Fri, 1 Mar 2019 14:05:17 +0100
Subject: [PATCH 1/2] Don't expand typedefs for functions without arguments.

---
 gdb/linespec.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 0f2fcfdff0..b01afadd91 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -3899,6 +3899,23 @@ find_function_symbols (struct linespec_state *state,
 				  &info, state->search_pspace);
 }
 
+static bool
+is_simple_name (const char *name)
+{
+  do
+    {
+      if (name[0] == ':' && name[1] == ':')
+	name += 2;
+      if (*name == ':')
+	break;
+      while ((*name >= 'A' && *name <= 'Z') || (*name >= 'a' && *name <= 'z')
+	     || (*name >= '0' && *name <= '9') || *name == '_')
+	name++;
+    }
+  while (name[0] == ':' && name[1] == ':');
+  return *name == '\0';
+}
+
 /* Find all symbols named NAME in FILE_SYMTABS, returning debug symbols
    in SYMBOLS and minimal symbols in MINSYMS.  */
 
@@ -3910,7 +3927,9 @@ find_linespec_symbols (struct linespec_state *state,
 		       std::vector <block_symbol> *symbols,
 		       std::vector<bound_minimal_symbol> *minsyms)
 {
-  std::string canon = cp_canonicalize_string_no_typedefs (lookup_name);
+  std::string canon;
+  if (!is_simple_name (lookup_name))
+    canon = cp_canonicalize_string_no_typedefs (lookup_name);
   if (!canon.empty ())
     lookup_name = canon.c_str ();
 
-- 
2.15.1.windows.2


[-- Attachment #3: 0002-Don-t-expand-symtabs-when-looking-for-a-filename.patch --]
[-- Type: application/octet-stream, Size: 767 bytes --]

From 9f1e4c10b9e3a691f674b2d2a5c9d4af305f0b26 Mon Sep 17 00:00:00 2001
From: Hannes Domani <ssbssa@yahoo.de>
Date: Fri, 1 Mar 2019 14:07:47 +0100
Subject: [PATCH 2/2] Don't expand symtabs when looking for a filename.

---
 gdb/linespec.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index e902b11c8e..0f2fcfdff0 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1143,7 +1143,8 @@ iterate_over_all_matching_symtabs
 
     for (objfile *objfile : current_program_space->objfiles ())
       {
-	if (objfile->sf)
+	if (objfile->sf
+	    && lookup_name.name ().find ('.') == std::string::npos)
 	  objfile->sf->qf->expand_symtabs_matching (objfile,
 						    NULL,
 						    lookup_name,
-- 
2.15.1.windows.2

Re: question about expand_symtabs_matching()

[Tom Tromey]

Hannes> This is probably a stupid question.

Not at all.

Hannes> In gdb/linespec.c:1146 is this call:
Hannes>       if (objfile->sf)
Hannes>         objfile->sf->qf->expand_symtabs_matching (objfile,
Hannes>                                                   NULL,
Hannes>                                                   lookup_name,
Hannes>                                                   NULL, NULL,
Hannes>                                                   search_domain);
Hannes> What exactly is this call doing, since none of the 3 callback functions is used?

gdb first scans debug symbols to create "partial symbol tables" (or in
some cases it uses the "gdb index").

These don't contain much information, so in order to actually do much
with the symbols, the relevant psymtabs must be expanded into full
symbol tables.

This call is expanding all the partial symbol tables that match
lookup_name and search_domain.  The loop that comes after this actually
does the work of the function, by looking only at expanded symbol
tables.

Hannes> This is slowing gdb down quite a lot if you have pending breakpoints, and are 
Hannes> loading/unloading many shared libraries.

Yeah.  gdb does too much work when re-evaluating breakpoint locations.

For example, it seems to me that when re-evaluating a breakpoint
location, there's no reason to search an objfile again -- the results
can't have changed.  There are various ways this could be tackled but I
suppose the basic idea is to cache the results somewhere.

Hannes> When I removed it, I couldn't see any different behavior (other than being faster).

I think it would make a difference if the inferior dlopen()d a library
and the library had a new location for an existing breakpoint.

Tom

question about expand_symtabs_matching()

[Hannes Domani via gdb]

Hello


This is probably a stupid question.

In gdb/linespec.c:1146 is this call:

      if (objfile->sf)
        objfile->sf->qf->expand_symtabs_matching (objfile,
                                                  NULL,
                                                  lookup_name,
                                                  NULL, NULL,
                                                  search_domain);

What exactly is this call doing, since none of the 3 callback functions is used?

This is slowing gdb down quite a lot if you have pending breakpoints, and are 
loading/unloading many shared libraries.

When I removed it, I couldn't see any different behavior (other than being faster).

I'm on Windows, specifically x86_64-w64-mingw32.


Regards
Hannes Domani