public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Tom Tromey <tom@tromey.com>, gdb-patches@sourceware.org
Subject: Re: [RFA 2/2] Simplify the psymbol hash function
Date: Wed, 08 Nov 2017 11:12:00 -0000	[thread overview]
Message-ID: <92a5383c-5fd0-baee-fbaa-977669a3c613@redhat.com> (raw)
In-Reply-To: <20171104161356.17565-3-tom@tromey.com>

On 11/04/2017 04:13 PM, Tom Tromey wrote:
> This patch simplifies the psymbol_hash function, by changing it not to
> examine the contents of the symbol's name.  This change just mirrors
> what psymbol_compare already does -- it is checking for name equality,
> which is presumably ok because symbol names are generally interned.

Can you expand a bit more on the "presumably ok" part?  I think
it'd be nice to mention/explain this assumption in a comment
in the code.

> This change speeds up psymbol reading.  "gdb -nx -batch gdb"
> previously took ~1.8 seconds on my machine, and with this patch it now
> takes ~1.7 seconds.

That sounds great however I do wonder whether the bug is the other 
way around though.  What do the statistics say, e.g., debugging
gdb and firefox?  Do we end up deduping more or fewer
symbols in the bcache?

I read the original thread that added these custom functions,
and the original patch used strcmp in both hash and compare,
and then somehow the end result was what we have today.

See:
 https://sourceware.org/ml/gdb-patches/2010-08/msg00218.html
and:
 https://sourceware.org/ml/gdb-patches/2010-08/msg00242.html

So I'd love it that your patch is correct.  I'd just appreciate
a bit more detail since I'm not awfully familiar with this area.

Thanks,
Pedro Alves

  reply	other threads:[~2017-11-08 11:12 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-04 16:14 [RFA 0/2] some minor symbol-reading performance improvements Tom Tromey
2017-11-04 16:14 ` [RFA 2/2] Simplify the psymbol hash function Tom Tromey
2017-11-08 11:12   ` Pedro Alves [this message]
2017-11-08 11:42     ` Pedro Alves
2017-11-08 19:08       ` Tom Tromey
2017-11-09 14:09         ` Tom Tromey
2017-11-09 14:10           ` Pedro Alves
2017-11-04 16:14 ` [RFA 1/2] Speed up dict_hash Tom Tromey
2017-11-08 10:46   ` Pedro Alves
2017-11-08 18:44     ` Tom Tromey
2017-11-08 22:41       ` Pedro Alves
2017-11-08 23:01         ` Tom Tromey
2017-11-08 23:51           ` Pedro Alves

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=92a5383c-5fd0-baee-fbaa-977669a3c613@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=tom@tromey.com \
    /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).