Re: [PATCH 4/4] gdb: change regcache list to be a map

John Baldwin
2020-07-24T16:59:54Z

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