gdb/dwarf: make abbrev_table_cache read-through, make it mandatoryHEADmaster

This patch implements some performance improvements initially sent as
part of this series [1], but now as a single patch.  There was some push
back from Tom regarding the increased memory usage, but I think that my
solution is still desirable: it makes the code simpler, and I think that
the memory usage it not an issue (the usage is transient and the amount
of memory used by the abbrev tables is relatively low).

As a reminder, here is the problem I'm looking to solve: when using
split DWARF (.dwo files) plus type units, all units inside a .dwo file
(generally one compile unit plus many type units) share the same abbrev
table.  So we end up re-reading that same abbrev tables many times, and
its gets very noticeable in function process_skeletonless_type_units.

cooked_index_worker_debug_info::process_type_units does some work to
avoid this problem, but it only works when everything is in the main
file (not using split DWARF).

Right now, we cache abbrev tables only in specific cases during
indexing, like when one CU imports things from another CU.  My previous
series changed cutu_reader so that it would add any abbrev table it
would read to the abbrev table cache (if it was passed one as a
parameter).  This allowed using a cache in
process_skeletonless_type_units and cut the time down significantly.

This patch goes a bit further in order to simplify cutu_reader even
further:

 - It makes the abbrev table cache read-through, meaning that when you
   request an abbrev table and it's not in the cache, it will go read
   it (and cache it).

 - It makes passing an abbrev table cache to the constructors mandatory
   (even when we wouldn't benefit from using a cache).

The result is that cutu_reader doesn't need to manage multiple cases of
how to obtain an abbrev table, and it doesn't need to manage adding
abbrev tables to the cache.  It no longer needs to manage the ownership
of the abbrev tables either: the abbrev tables are always owned by the
cache.  And the cases of abbrev table sharing are well handled
transparently.

This means that we pay a small price when we don't necessarily need to
(sometimes building and destroying an abbrev_table_cache for just one
cutu_reader), but I think that this price is not significant and the
code simplification is welcome.

In concrete terms, this patch:

 - changes abbrev_table_cache::find to become abbrev_table_cache::get,
   making it read-through
 - removes abbrev_table_cache::add
 - removes the abbrev_table parameter from the main cutu_reader
   constructor
 - makes the abbrev_table_cache parameter mandatory (a reference) in the
   main cutu_reader constructor, adds it to the alternative constructor,
   and passes it down to a few methods
 - adjusts the cutu_reader code obtaining an abbrev table to just call
   abbrev_table_cache::get
 - adjusts all the cutu_reader users to pass an abbrev_table_cache (if
   not already)
 - removes the code in
   cooked_index_worker_debug_info::process_type_units meant to
   efficiently handle TUs that share abbrev tables - the cache now does
   this

The specific split DWARF + type units performance problem at the origin
of this work gets fixed by the fact that
cooked_index_worker_debug_info::process_skeletonless_type_unit now
passes an abbrev table cache to cutu_reader, and
cutu_reader::read_cutu_die_from_dwo uses it.

As a test, I'm using a build of Blender compiled with -gsplit-dwarf and
-fdebug-types-section.  I run this:

    $ ./gdb -nx -q --data-directory=data-directory -ex 'maint set dwarf sync on' -ex 'maintenance set per-command time on' -ex "file /data1/smarchi/blender-build/relwithdebinfo-clang-debugtypes-splitdwarf/bin/blender" -batch

and look at the time taken by the "DWARF skeletonless type units"
step.  Before looks like:

    Time for "DWARF skeletonless type units": wall 11.131, user 10.699, sys 0.431, user+sys 11.130, 100.0 % CPU

and after looks like:

    Time for "DWARF skeletonless type units": wall 1.751, user 1.221, sys 0.518, user+sys 1.739, 99.3 % CPU

The total run time (wall clock time) of the command goes from about 18.5
seconds to about 9.5 seconds.

I removed this assert in cutu_reader, because it relies on abbrev_cache
as a flag for whether we're in the indexer:

      /* If an existing_cu is provided, a dwarf2_cu must not exist for
	 this_cu in per_objfile yet.  Here, CACHE doubles as a flag to
	 let us know that the CU is being scanned using the parallel
	 indexer.  This assert is avoided in this case because (1) it
	 is irrelevant, and (2) the get_cu method is not
	 thread-safe.  */
      gdb_assert (abbrev_cache != nullptr
		  || per_objfile.get_cu (&this_cu) == nullptr);

It's not clear to me if this assert is important or how to implement it
differently.

[1] https://inbox.sourceware.org/gdb-patches/20250326200002.136200-4-simon.marchi@efficios.com/

Change-Id: Idf8a514326fb35be8bda0d7ebe3cee4b7304941a
Approved-By: Tom Tromey <tom@tromey.com>

0 files changed, 0 insertions, 0 deletions