On 23/07/2015 4:11 PM, Jeremy Manson wrote:
Okay. TBH, thinking about it more, it's a little weird for something named os::current_thread_id() to be different from Thread::current()->osThread->thread_id(), or for either of them to be different from "what the OS thinks the thread id is", so it seems like the Right Thing to Do (in the absence of any platform-specific issues).
When there are two different levels of thread ID it isn't that simple - especially as, like you say, one can be used to look in /proc while the other can't. So perhaps better naming is in order.
Cheers, David
Jeremy On Wed, Jul 22, 2015 at 10:57 PM, David Holmes <david.hol...@oracle.com <mailto:david.hol...@oracle.com>> wrote: On 23/07/2015 3:15 PM, Jeremy Manson wrote: Hey David, Thanks for the offer of sponsorship. My goal here is really to make the log output on Linux usable. I want to be able to map the log output back to an actual thread. I don't think it really matters to users if the API consistently means "kernel thread ID" or "threading API thread ID", as long as they can figure out what the output means. I think consistency is important else developers don't know what they should be using where - which is the current situation. Since what I am doing (in effect) to accomplish my goal is to ensure that the API returns the same value as osthread()->thread_id() does for the current thread, I could just... do precisely that. os::current_thread_id could just return osthread()->thread_id() for the current thread. I don't have easy access to anything for testing other than Linux, though, so whether it worked (or even compiled) on the other platforms would be a bit of a guess (per the last time we did this dance). Defining os::current_thread_id() to be Thread::current()->osThread()->thread_id() assumes you can make those calls in the context in which os::current_thread_id() is used - if we crash during thread startup then some of those may be null and the id not even set. The current implementation is independent of the state of thread within the VM. So its okay to return the same thing as Thread::current()->osThread()->thread_id() but it needs to be done directly. Again any platform for which this would cause a change in behaviour needs to be examined. It may be other platforms have the same problem you are trying to fix for linux. If I get time later I'll try to check what each platform does. Thanks, David Seem reasonable? Jeremy On Wed, Jul 22, 2015 at 7:08 PM, David Holmes <david.hol...@oracle.com <mailto:david.hol...@oracle.com> <mailto:david.hol...@oracle.com <mailto:david.hol...@oracle.com>>> wrote: On 23/07/2015 8:01 AM, Jeremy Manson wrote: Based on the feedback, this seems to be a good idea, approximately. Coleen would have sponsored, but she's going on vacation. Anyone else feel like sponsoring? Hold up a minute! :) There are different notions of "native thread id" that exist. First we have the "user level thread id" - this is what is reported by pthread_self in POSIX and thr_self in UI. Then we also have the OS specific "thread" id, also referred to as a LWP or "kernel scheduling entity" or "kernel thread" - the id for this is what gettid() maps back to on Linux. This distinction may not exist on all platforms. Unfortunately os::current_thread_id does not define which of these it represents: // thread id on Linux/64bit is 64bit, on Windows and Solaris, it's 32bit static intx current_thread_id(); and so on some platforms it returns the "user thread id" (eg pthread_self()), and on some it returns the same as gettid (ie OSX - but I don't know if the mach thread id is truly a "LWP" id ?). Also note that on some platforms the osThread stores the id of the "user-level thread" and on some the "kernel thread". Hence another source of confusion. :( So if you want to enforce that os::current_thread_id() represents the "kernel thread" then that should be applied consistently across all platforms**, and for platforms for which there is a change to make you have to ensure the usage of os::current_thread_id() is not semantically altered by the change. ** Of course a platform may only have a single notion of "thread" I'm happy to sponsor such a proposal. And don't worry about maintaining compatibility with archaic Linux versions for JDK9 (less cleanup to do later). Thanks, David Jeremy On Wed, Jul 22, 2015 at 11:22 AM, Jeremy Manson <jeremyman...@google.com <mailto:jeremyman...@google.com> <mailto:jeremyman...@google.com <mailto:jeremyman...@google.com>> <mailto:jeremyman...@google.com <mailto:jeremyman...@google.com> <mailto:jeremyman...@google.com <mailto:jeremyman...@google.com>>>> wrote: Hey folks, os::current_thread_id on Linux now maps to pthread_self. The problem with pthread_self is that it only makes sense in the context of the running process. When it is written out to the log (as it is in several places), there really isn't a way (AFAICT) for the user to map it back to anything useful. As it happens, that value is mostly used to write to the log. The places where it doesn't do so don't seem to need to use pthread_self for any particular reason. Meanwhile, the SIGQUIT stack dump uses java_thread->osthread()->thread_id() as the nid. On Linux, that maps back to os::Linux::gettid(), whish is also what gets exposed in /proc. That makes it much easier to see what threads might be doing the log write. Would it be okay to change os::current_thread_id to point to os::Linux::gettid()? That way, it can be mapped back to the output of a SIGQUIT dump. The downside of gettid() is that it is only available on Linux>2.4.11, but that dates from 2001. If we really still want to support it, we could check for that. Thoughts? Jeremy
