Re: RFA [threads]: Thread cache

[Mark Kettenis <kettenis at chello dot nl>]

Daniel Jacobowitz <drow@mvista.com> writes:

> I've figured out how to fix print-threads.exp (see my ramblings on gdb@
> yesterday for a bad description of the problem; better coming soon). 
> However, to do it, I discovered that it was actually _required_ that we
> cache certain information from libthread_db, instead of merely beneficial.
>
> So I implemented the cache.  This patch is the entire cache mechanism,
> except for updating the comment at the top of the file saying we need one.
> Before I get to the patch itself, some numbers:

This looks good!  Please check it in, regardless of the things I say
further on in this message.

> Now, on to the patch itself.  I replace all calls to td_ta_map_id2thr_p
> and most calls to td_thr_get_info_p [Hmm, I don't see any reason not to
> convert the others too; I will do that in a separate patch if this one is
> approved, and see how much more it takes off the runtime] with calls to
> wrapper functions which cache the data in the struct private_thread_info.
> The cache is invalidated at every resume(); there's some information that we
> could keep if we are guaranteed a 1-to-1 threads implementation with no
> migration, like LinuxThreads or NPTL, but I'm being conservative for now.

Note that the thread_db.h interface provides TD_SWITCHTO and
TD_SWITCHFROM events.  I'd be perfectly happy if you'd cache info
about the LWP a particular user-level thread is bound to if you'd
invalidate this info upon receiving those events (which should never
happen in a 1-to-1 threads implementation.

That said, would re-enabling TD_DEATH events somehow make things more
robust for you?  TD_DEATH was broken in glibc 2.1.3, but anybody who's
doing any serious threads development should be using a more recent
glibc.

> The call to target_pid_to_str is moved below the call to add_thread in
> attach_thread(), since the cache requires that a struct thread_info * exist
> for the ptid being printed.

Makes sense.

> Oh, and in thread_db_pid_to_str I replace an error () with putting "Missing"
> into the string; there's no point in target_pid_to_str failing, since it's
> only used for display.

Fine with me.

Thanks!

Mark