This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patchv2 2/2] Accelerate lookup_symbol_aux_objfile 14.5x [Re: [patch 0/2] Accelerate symbol lookups 15x]
- From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- To: Doug Evans <xdje42 at gmail dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Mon, 27 Oct 2014 09:54:37 +0100
- Subject: Re: [patchv2 2/2] Accelerate lookup_symbol_aux_objfile 14.5x [Re: [patch 0/2] Accelerate symbol lookups 15x]
- Authentication-results: sourceware.org; auth=none
- References: <20141020214410 dot GA22011 at host2 dot jankratochvil dot net> <CAP9bCMQ7EXyXJiqK4j2UA9YgxkQiNFFqJPOpbPXH8-YZMRLh2w at mail dot gmail dot com> <20141023182434 dot GA31412 at host2 dot jankratochvil dot net> <CAP9bCMTrzU7srWLfiS2814nfBEySJ-i6yKy7AKHBvXbUoLa-rQ at mail dot gmail dot com> <20141024073308 dot GA20087 at host2 dot jankratochvil dot net> <CAP9bCMSrz7HswXRckCvTy7tcWrNktbbYm6hXcT3GxLCZ1zNTwA at mail dot gmail dot com> <m3k33mjbw8 dot fsf at sspiff dot org>
On Mon, 27 Oct 2014 06:54:15 +0100, Doug Evans wrote:
> I'd be grateful if you could replace your 1/2 and 2/2 with this experiment
> and see what numbers you get. It's good to have data.
Benchmark on non-trivial application with 'p <tab><tab>':
Command execution time: 0.091000 (cpu), 0.092709 (wall) --- Doug's fix
Command execution time: 4.215000 (cpu), 4.241466 (wall) --- both fixes with new [patch 2/2]
Command execution time: 7.373000 (cpu), 7.395095 (wall) --- both fixes
Command execution time: 13.572000 (cpu), 13.592689 (wall) --- just lookup_symbol_aux_objfile fix
Command execution time: 113.036000 (cpu), 113.067995 (wall) --- FSF GDB HEAD
That is 113.067995/0.092709 = 1219x improvement.
> Alas, the experiment is just that because gdb only looks up
> some symbols from expanded symtabs and not partial symtabs/gdb_index,
> because neither partial syms nor the index record all symbols,
> and thus there are several testsuite regressions.
> We would have to fix this.
OK, so running a regression testsuite with your patch is pointless now.
> However, for basic symbol lookup, only searching the index, and never
> searching already expanded symtabs, makes sense: the index knows
> where the symbol lives, so why search anywhere else?
What about inlined function instances? Are they in .gdb_index? And if they are
we need all their instances while .gdb_index always points to only one
instance. I did not check/test it, just an idea now.
> And in the null case, which is what is killing performance in your example,
> we certainly want to go to the index first, not second.
I was looking if Tom Tromey justified why
quick_symbol_functions::lookup_symbol returns NULL on already expanded symtabs
- this was introduced by:
Subject: [0/4] RFC: refactor partial symbol tables
Message-ID: <m38wbyc31o.fsf@fleche.redhat.com>
But I haven't found anything, probably just to make the implementation
safer/easier.
> So the question is, what to do in the meantime.
>
> I'm ok with your 2/2 patch (with the changes I've requested) since I think
> it's reasonable regardless of anything else.
> [btw, I've submitted a patch to move lookup_block_symbol to block.c:
> https://sourceware.org/ml/gdb-patches/2014-10/msg00720.html]
OK, I will rebase the patch 2/2 after it gets checked in.
> Your 1/2 patch (dictionary hash caching) still gives me pause.
> I didn't have time to collect more timing data this weekend.
> I might be ok with it going in provided it can be removed without
> effort if/when the above improvements are applied.
The improvements above IIUC apply only for objfiles with .gdb_index.
That patch 1/2 applied even for non-.gdb_index objfiles.
> Before this patch with plain FSF GDB I get 7.5 seconds for "p/r var".
> With this patch it's 0.005.
It matches my benchmark above.
Thanks,
Jan