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

Pedro Alves
2020-04-16T20:12:52Z

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.

>
> 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?

Thanks,
Pedro Alves