Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: John Baldwin <jhb@FreeBSD.org>
To: Pedro Alves <pedro@palves.net>,
	Simon Marchi <simon.marchi@efficios.com>,
	 gdb-patches@sourceware.org
Cc: Laurent <Laurent.Morichetti@amd.com>
Subject: Re: [PATCH 4/4] gdb: change regcache list to be a map
Date: Fri, 24 Jul 2020 09:59:50 -0700	[thread overview]
Message-ID: <0357b2ae-cf42-3743-2c9d-ad116546e747@FreeBSD.org> (raw)
In-Reply-To: <5b521483-2f09-3424-943d-40f83b3080af@palves.net>

On 7/23/20 6:53 PM, Pedro Alves wrote:
> On 7/20/20 9:41 PM, Simon Marchi via Gdb-patches wrote:
>> The function regcache_thread_ptid_changed is called when a thread
>> changes ptid.  It is implemented efficiently using the map, although
>> that's not very important: it is not called often, mostly when creating
>> an inferior, on some specific platforms.
>>
>> Note: In hash_target_ptid, I am combining hash values from std::hash by
>> summing them.  I don't think it's ideal, since std::hash is just the
>> identity function for base types.  But I don't know what would be better
>> to reduce the change of collisions.  If anybody has a better idea, I'd
>> be interested.
> 
> I'd maybe look in some kernel sources, e.g., Linux or BSD, what they
> use as hash function for pids.

FreeBSD just does a very simple and with a power of 2 based on the
maximum number of processes (set via a boot-time tunable).

However, it has separate hash tables for pids and thread ids (tids).
It doesn't have a single hash table of threads indexed by (pid, tid)
because the tid namespace is independent and a tid is a fully unique
name to a kernel thread (lwp).

>> +/* Functor to hash a ptid.  */
>> +
>> +struct hash_ptid
>> +{
>> +  size_t operator() (const ptid_t &ptid) const
>> +  {
>> +    std::hash<long> long_hash;
>> +
>> +    return (long_hash (ptid.pid ())
>> +	    + long_hash (ptid.lwp ())
>> +	    + long_hash (ptid.tid ()));
>> +  }
>> +};

I think summing the three components might be the best option.
Presumably the low bits of the hash are what actually get used and
so the goal would be to have more entropy there.  This means you
probably would _not_ want to do something like:

pid << 32 | lwp << 16 | tid

since many native backends will have tid of all zeroes.  It would
also not be ideal for backends that only do processes (so only
pid is non-zero).  The sum approach degrades to the right thing
for those targets without needing extra complication or conditionals.

-- 
John Baldwin


  reply	other threads:[~2020-07-24 16:59 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-20 20:40 [PATCH 0/4] Regcache fix and optimization Simon Marchi
2020-07-20 20:40 ` [PATCH 1/4] gdb: rename regcache::current_regcache to regcache::regcaches Simon Marchi
2020-07-23 20:01   ` Pedro Alves
2020-07-20 20:40 ` [PATCH 2/4] gdb: move regcache::regcaches to regcache.c Simon Marchi
2020-07-23 20:03   ` Pedro Alves
2020-07-20 20:41 ` [PATCH 3/4] gdb: pass target to thread_ptid_changed observable Simon Marchi
2020-07-23 20:42   ` Pedro Alves
2020-07-30 15:27     ` Simon Marchi
2020-08-05 14:50       ` Pedro Alves
2020-08-05 19:08         ` Simon Marchi
2020-08-05 22:29           ` Pedro Alves
2020-07-20 20:41 ` [PATCH 4/4] gdb: change regcache list to be a map Simon Marchi
2020-07-24  1:53   ` Pedro Alves
2020-07-24 16:59     ` John Baldwin [this message]
2020-07-30 16:26       ` Simon Marchi
2020-07-30 16:58     ` Simon Marchi
2020-07-30 17:03       ` Simon Marchi
2020-08-05 18:02         ` Pedro Alves
2020-08-05 20:25           ` Simon Marchi
2020-07-30 17:07       ` Simon Marchi
2020-07-30 18:17     ` Simon Marchi
2020-08-05 18:14       ` Pedro Alves
2020-08-10 19:15   ` Tom Tromey
2020-08-10 19:25     ` Simon Marchi
2020-08-12 12:52   ` Tom Tromey
2020-08-12 15:17     ` Tom Tromey
2020-08-06 20:27 ` [PATCH 0/4] Regcache fix and optimization Simon Marchi

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=0357b2ae-cf42-3743-2c9d-ad116546e747@FreeBSD.org \
    --to=jhb@freebsd.org \
    --cc=Laurent.Morichetti@amd.com \
    --cc=gdb-patches@sourceware.org \
    --cc=pedro@palves.net \
    --cc=simon.marchi@efficios.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