Re: [PATCH 28/28] Decouple inferior_ptid/inferior_thread(); dup ptids in thread list (PR/25412)

[Simon Marchi <simark@simark.ca>]

On 2020-04-16 4:12 p.m., Pedro Alves via Gdb-patches wrote:
> On 4/16/20 8:39 PM, Simon Marchi wrote:
> 
>> I checked the rest of the series, I didn't find any obvious problem.
> 
> Thanks.
> 
>>
>> I have a patch [1] that I might consider sending upstream some day, which replaces the
>> inferior thread lists with maps, using the ptid as the key.  This was made for targets
>> that have a lot of threads (like, thousands), where looking up a thread object from a
>> ptid would take a bit of time, and that would add up.  If there can be two threads in
>> the list with the same ptid, that won't work with a map using the ptid as the key.
>>
>> So I was wondering, when we want to delete a thread but its refcount is not 0, does it
>> need to stay in the thread list?  What if we remove it from the list, and whatever
>> decrements its refcount to 0 deletes it?  Do you think that could work?
> 
> I think it could.  That's something that I considered, as something that is made
> possible with this patchset, because, before we needed it to stay in the list so
> that the lookup inside inferior_thread(), but after the series the lookup is gone.
> We'd also need to look out for places that want to walk all threads, instead of
> just non-exited ones, but on a quick look, I didn't find any left that couldn't
> be converted to walk to non-exited threads only.
> 
> Another thing that makes it possible is that "info threads" doesn't show the
> exited threads in the list.  Even if we wanted to show the current thread
> in the list if it's exited (which is something I considered), we could still
> easily do that, though.  Currently, we just show the
> "The current thread <Thread ID %s> has terminated.  See `help thread'"
> message at the end of the thread list.

Indeed, I checked "info threads" before posting this, because it's one case I
know the deleted thread was referred to.

>> It's possible, however, that with your series, the number of times we look up a thread
>> from its ptid is reduced enough that it makes my patch not really useful.
>>
>> Simon
>>
>> [1] https://github.com/simark/binutils-gdb/commits/inferior-thread-map
> 
> How are you making sure that iterating the threads walks them in
> creation/insertion order instead of ptid_t order?

The only spot that I found the order mattered was precisely for "info threads".
There, I gather the list of threads to display, sort them by per_inf_num, and
print them.

https://github.com/simark/binutils-gdb/commit/25c67729996dc83912d34a5901016404b3638bc1#diff-5479a5443dca2232f8552dfb5b30ac3dR1104

Simon