From: Doug Evans <xdje42@gmail.com>
To: gdb-patches@sourceware.org
Subject: Re: [PATCH 0/5] cp-namespace.c cleanup pass
Date: Mon, 22 Dec 2014 17:33:00 -0000 [thread overview]
Message-ID: <m3sig7oajg.fsf@sspiff.org> (raw)
In-Reply-To: <m37fxtihsy.fsf@seba.sebabeach.org> (Doug Evans's message of "Sun, 14 Dec 2014 21:49:17 -0800")
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.
prev parent reply other threads:[~2014-12-22 17:33 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-12-15 5:50 Doug Evans
2014-12-22 17:33 ` Doug Evans [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m3sig7oajg.fsf@sspiff.org \
--to=xdje42@gmail.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).