mirror of
https://sourceware.org/git/binutils-gdb.git
synced 2026-08-27 00:26:02 -04:00
No description
c019c04618
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>
|
||
|---|---|---|
| bfd | ||
| binutils | ||
| config | ||
| contrib | ||
| cpu | ||
| elfcpp | ||
| etc | ||
| gas | ||
| gdb | ||
| gdbserver | ||
| gdbsupport | ||
| gnulib | ||
| gold | ||
| gprof | ||
| gprofng | ||
| include | ||
| ld | ||
| libbacktrace | ||
| libctf | ||
| libdecnumber | ||
| libiberty | ||
| libsframe | ||
| opcodes | ||
| readline | ||
| sim | ||
| texinfo | ||
| zlib | ||
| .cvsignore | ||
| .editorconfig | ||
| .gitattributes | ||
| .gitignore | ||
| .pre-commit-config.yaml | ||
| ar-lib | ||
| ChangeLog | ||
| compile | ||
| config-ml.in | ||
| config.guess | ||
| config.rpath | ||
| config.sub | ||
| configure | ||
| configure.ac | ||
| COPYING | ||
| COPYING.LIB | ||
| COPYING.LIBGLOSS | ||
| COPYING.NEWLIB | ||
| COPYING3 | ||
| COPYING3.LIB | ||
| depcomp | ||
| djunpack.bat | ||
| install-sh | ||
| libtool.m4 | ||
| ltgcc.m4 | ||
| ltmain.sh | ||
| ltoptions.m4 | ||
| ltsugar.m4 | ||
| ltversion.m4 | ||
| lt~obsolete.m4 | ||
| MAINTAINERS | ||
| Makefile.def | ||
| Makefile.in | ||
| Makefile.tpl | ||
| makefile.vms | ||
| missing | ||
| mkdep | ||
| mkinstalldirs | ||
| move-if-change | ||
| multilib.am | ||
| README | ||
| README-maintainer-mode | ||
| SECURITY.txt | ||
| setup.com | ||
| src-release.sh | ||
| symlink-tree | ||
| test-driver | ||
| ylwrap | ||
README for GNU development tools This directory contains various GNU compilers, assemblers, linkers, debuggers, etc., plus their support routines, definitions, and documentation. If you are receiving this as part of a GDB release, see the file gdb/README. If with a binutils release, see binutils/README, and so on. That'll give you info about this package -- supported targets, how to use it, how to report bugs, etc. It is now possible to automatically configure and build a variety of tools with one command. To build all of the tools contained herein, run the ``configure'' script here, e.g.: ./configure make To install them (by default in /usr/local/bin, /usr/local/lib, etc), then do: make install (If the configure script can't determine your type of computer, give it the name as an argument, for instance ``./configure sun4''. You can use the script ``config.sub'' to test whether a name is recognized; if it is, config.sub translates it to a triplet specifying CPU, vendor, and OS.) If you have more than one compiler on your system, it is often best to explicitly set CC in the environment before running configure, and to also set CC when running make. For example (assuming sh/bash/ksh): CC=gcc ./configure make A similar example using csh: setenv CC gcc ./configure make Much of the code and documentation enclosed is copyright by the Free Software Foundation, Inc. See the file COPYING or COPYING.LIB in the various directories, for a description of the GNU General Public License terms under which you can copy the files. REPORTING BUGS: Again, see gdb/README, binutils/README, etc., for info on where and how to report problems.