This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 0/5] cp-namespace.c cleanup pass
- From: Doug Evans <xdje42 at gmail dot com>
- To: gdb-patches at sourceware dot org
- Date: Mon, 22 Dec 2014 09:32:19 -0800
- Subject: Re: [PATCH 0/5] cp-namespace.c cleanup pass
- Authentication-results: sourceware.org; auth=none
- References: <m37fxtihsy dot fsf at seba dot sebabeach dot org>
Doug Evans <xdje42@gmail.com> writes:
> Hi.
>
> This patch set is a cleanup pass over cp-namespace.c.
>
> There are two things I wish to achieve here:
>
> 1a) Remove redundancy to make the code more readable.
> There are several redundant lookups here, and it's hard to reason
> about the code because one has to ask if there's something one is
> missing or whether the lookup really is redundant. Plus it's more
> code to have to follow while studying the flow of any particular
> function.
>
> 1b) Performance.
> While redundant lookups don't always hurt performance if a symbol is
> found, efficient handling of the null case (symbol not found) is at
> least as important.
>
> 2) Lay the groundwork so that I can finish this patch, that adds support
> for looking up primitive types as symbols:
> https://sourceware.org/ml/gdb-patches/2014-12/msg00169.html
>
> 1/5: whitespace
> 2/5: simplify cp_lookup_symbol_in_namespace
> 3/5: introduce cp_lookup_nested_symbol_1
> 4/5: remove redundant calls to cp_lookup_symbol_in_namespace
> 5/5: remove redundancies in cp_lookup_symbol_nonlocal
>
> Appended is a patch I wrote to log symbol lookup calls while
> running the testsuite, and here are the before/after numbers:
>
> [dje@seba gdb]$ sed -e "s/ *(.*$//" </tmp/gdb-call.log >before.tmp
> [dje@seba gdb]$ sort before.tmp | uniq -c
> 40230 basic_lookup_symbol_nonlocal
> 74647 lookup_global_symbol
> 18834 lookup_language_this
> 73063 lookup_local_symbol
> 30849 lookup_static_symbol
> 73063 lookup_symbol_aux
> 59411 lookup_symbol_in_block
> 656240 lookup_symbol_in_objfile
> 23 lookup_symbol_in_objfile_from_linkage_name
> 656298 lookup_symbol_in_objfile_symtabs
> 74075 lookup_symbol_in_static_block
> 614899 lookup_symbol_via_quick_fns
>
> [dje@seba gdb]$ sed -e "s/ *(.*$//" </tmp/gdb-call.log >after.tmp
> [dje@seba gdb]$ sort after.tmp | uniq -c
> 39019 basic_lookup_symbol_nonlocal
> 61959 lookup_global_symbol
> 9614 lookup_language_this
> 71826 lookup_local_symbol
> 29565 lookup_static_symbol
> 71826 lookup_symbol_aux
> 55214 lookup_symbol_in_block
> 504688 lookup_symbol_in_objfile
> 23 lookup_symbol_in_objfile_from_linkage_name
> 504746 lookup_symbol_in_objfile_symtabs
> 63217 lookup_symbol_in_static_block
> 463884 lookup_symbol_via_quick_fns
>
> There's a clear reduction in the number of symbol looks being done here.
> A later patch will augment our performance testsuite, but my main focus
> at the moment is getting to the "use the index better" speedup patch.
> [And along the way cleaning things up so that I understand the code
> better, and *hopefully* making it so that others will too.]
Hi. fyi,
I committed this series.