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

[Simon Marchi <simark@simark.ca>]

On 2020-07-24 12:59 p.m., John Baldwin wrote:
> 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.

Ok, thanks for the tips.  So I'll probably keep it like this for now then.

I'll still take a look at the kernel implementations like Pedro suggested.

Simon