Re: [PATCH] gdb: Skip useless minsym scan in variable-only symbol searches

[Thiago Jung Bauermann <thiago.bauermann@linaro.org>]

Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:

> The symbol-listing command -symbol-info-* family goes through
> global_symbol_searcher::search, which has 3 steps:
>
> 1. expand_symtabs (objfile, preg): make sure every compunit that could
>    contain a match is expanded.  Returns found_msymbol = true if it
>    saw a matching msymbol that has no debug info.
>
> 2. add_matching_symbols (...): walk the compunits and collect the real
>    matches.
>
> 3. After the loop, a minsym fallback: if found_msymbol was set, or
>    unconditionally for any variable search with no file filter, re-scan
>    msymbols and append the ones without debug info (unless the searcher
>    set exclude_minsyms).
>
> Step 1 had two phases, first objfile::search, which uses the
> quick-symbol index to expand candidate compunits; then the pre-pass
> for every msymbol in the objfile whose name matches the regexp:
>
> 1.1. function search: call find_compunit_symtab_for_pc (address).  This
>      consults the quick functions and can expand the compunit containing
>      that address.
>
> 1.2. variable search: call lookup_symbol_in_objfile_from_linkage_name,
>      which does a linear scan of objfile->compunits () per lookup.  It
>      can not expand anything; its only output is setting found_msymbol
>      when the lookup fails.
>
> So for variables the pre-pass costs 'msymbols * expanded-compunits'
> block lookups and produces exactly one bit of information (found_msymbol)
> and this dominates the command execution time.
>
> And in step 3. these MI3 commands (search_module_symbols) calls
> spec2.set_exclude_minsyms (true), which sets !m_exclude_minsyms to false.
> The fallback never runs (found_msymbol is ignored), making the pre-pass
> calculation not required.
>
> The fix is make SEARCH_VAR_DOMAIN skip the pre-pass.  For variable
> searches with no filenames the fallback condition already contains
> '(m_kind & SEARCH_VAR_DOMAIN) != 0' as an unconditional alternative.
>
> Since the pre-pass now only runs for searches that include
> SEARCH_FUNCTION_DOMAIN, the lookup_symbol_in_objfile_from_linkage_name
> branch of the conditional inside the loop is unreachable.
>
> Also update the comments, which still described the variable lookup and
> claimed it forces symtabs to be read.  That was true when
> lookup_symbol_in_objfile_from_linkage_name was added in commit
> 422d65e705c7, but it no longer expands any symtab.
>
> -symbol-info-module-variables --module <M> on the 2000-module benchmark
> drops from 90 s to 0.56 s cold.
>
> Change-Id: I6019bcf80629f21cf6dee74d0e96fa788a70e220
> ---
>  gdb/symtab.c | 35 ++++++++++++++---------------------
>  1 file changed, 14 insertions(+), 21 deletions(-)

I reviewed this patch and it looks good to me, but I'm not knowledgeable
enough on GDB symbol handling for a Reviewed-by.

I did check that it doesn't cause any testsuite regression so:

Tested-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>

-- 
Thiago
(he/him)