Hi all, I have uploaded webrev.04 as below. Could you review again?
> - hotspot: > http://cr.openjdk.java.net/~ysuenaga/JDK-8152690/webrev.04/hotspot/ > > - jdk: > http://cr.openjdk.java.net/~ysuenaga/JDK-8152690/webrev.04/jdk/ Thanks, Yasumasa 2016/04/19 22:43 "Yasumasa Suenaga" <yasue...@gmail.com>: > > Hi David, > > Thank you for your comment. > I uploaded new webrev. Could you review again? > > - hotspot: > http://cr.openjdk.java.net/~ysuenaga/JDK-8152690/webrev.04/hotspot/ > > - jdk: > http://cr.openjdk.java.net/~ysuenaga/JDK-8152690/webrev.04/jdk/ > > >> That aside I'm not sure why you do this so late in the process, I would have done it immediately after here: > > > I think that native thread name ("main") should be set just before > main method call. > However, main thread is already started, so I moved it as you suggested. > > >> One thing I dislike about the current structure is that we have to go from char* to java.lang.String to call setNativeName which then calls JVM_SetNativeThreadName which converts the j.l.String back to a char* ! > > > SoI proposed to export new JVM function to set native thread name with > const char *. > > > Thanks, > > Yasumasa > > > > On 2016/04/19 14:04, David Holmes wrote: >> >> Hi Yasumasa, >> >> Thanks for persevering with this to get it into the current form. Sorry I haven't been able to do a detailed review until now. >> >> On 19/04/2016 9:28 AM, Yasumasa Suenaga wrote: >>> >>> Hi Gerard, >>> >>> 2016/04/19 3:14 "Gerard Ziemski" <gerard.ziem...@oracle.com >>> <mailto:gerard.ziem...@oracle.com>>: >>> > >>> > hi Yasumasa, >>> > >>> > Nice work. I have 2 questions: >>> > >>> > ======== >>> > File: java.c >>> > >>> > #1 Shouldn’t we be checking for “(*env)->ExceptionOccurred(env)” >>> after every single JNI call? In this example instead of NULL_CHECK, >>> should we be using CHECK_EXCEPTION_NULL_LEAVE macro? >>> >>> It is not critical if we encounter error at JNI function call because >>> we cannot set native thread name only. >>> So I think that we do not need to leave from launcher process. >> >> >> I agree we do not need to abort if an exception occurs (and in fact I don't think an exception is even possible from this code), but we should ensure any pending exception is cleared before any futher JNI calls might be made. Note that NULL_CHECK is already used extensively throughout the launcher code - so if this usage is wrong then it is all wrong! More on this code below ... >> >> Other comments: >> >> hotspot/src/share/vm/prims/jvm.cpp >> >> Please add a comment to the method now that you removed the internal comment: >> >> // Sets the native thread name for a JavaThread. If specifically >> // requested JNI-attached threads can also have their native name set; >> // otherwise we do not modify JNI-attached threads as it may interfere >> // with the application that created them. >> >> --- >> >> jdk/src/java.base/share/classes/java/lang/Thread.java >> >> Please add the following comments: >> >> + // Don't modify JNI-attached threads >> setNativeName(name, false); >> >> + // May be called directly via JNI or reflection (when permitted) to >> + // allow JNI-attached threads to set their native name >> private native void setNativeName(String name, boolean allowAttachedThread); >> >> --- >> >> jd/src/java.base/share/native/libjli/java.c >> >> 328 #define LEAVE() \ >> 329 SetNativeThreadName(env, "DestroyJavaVM"); \ >> >> I was going to suggest this be set later, but realized we have to be attached to do this and that happens inside DestroyJavaVM. :) >> >> + /* Set native thread name. */ >> + SetNativeThreadName(env, "main"); >> >> The comment is redundant given the name of the method. That aside I'm not sure why you do this so late in the process, I would have done it immediately after here: >> >> 386 if (!InitializeJVM(&vm, &env, &ifn)) { >> 387 JLI_ReportErrorMessage(JVM_ERROR1); >> 388 exit(1); >> 389 } >> + SetNativeThreadName(env, "main"); >> >> >> + /** >> + * Set native thread name as possible. >> + */ >> >> Other than the as->if change I'm unclear where the "possible" bit comes into play - why would it not be possible? >> >> 1705 NULL_CHECK(cls = FindBootStrapClass(env, "java/lang/Thread")); >> 1706 NULL_CHECK(currentThreadID = (*env)->GetStaticMethodID(env, cls, >> 1707 "currentThread", "()Ljava/lang/Thread;")); >> 1708 NULL_CHECK(currentThread = (*env)->CallStaticObjectMethod(env, cls, >> 1709 currentThreadID)); >> 1710 NULL_CHECK(setNativeNameID = (*env)->GetMethodID(env, cls, >> 1711 "setNativeName", "(Ljava/lang/String;Z)V")); >> 1712 NULL_CHECK(nameString = (*env)->NewStringUTF(env, name)); >> 1713 (*env)->CallVoidMethod(env, currentThread, setNativeNameID, >> 1714 nameString, JNI_TRUE); >> >> As above NULL_CHECK is fine here, but we should check for and clear any pending exception after CallVoidMethod. >> >> One thing I dislike about the current structure is that we have to go from char* to java.lang.String to call setNativeName which then calls JVM_SetNativeThreadName which converts the j.l.String back to a char* ! Overall I wonder about the affect on startup cost. But if there is an issue we can revisit this. >> >> Thanks, >> David >> ----- >> >> >>> > #2 Should the comment for “SetNativeThreadName” be “Set native thread >>> name if possible.” not "Set native thread name as possible.”? >>> >>> Sorry for my bad English :-) >>> >>> Thanks, >>> >>> Yasumasa >>> >>> > cheers >>> > >>> > > On Apr 16, 2016, at 4:29 AM, Yasumasa Suenaga <yasue...@gmail.com >>> <mailto:yasue...@gmail.com>> wrote: >>> > > >>> > > Hi David, >>> > > >>> > > I uploaded new webrev: >>> > > >>> > > - hotspot: >>> > > http://cr.openjdk.java.net/~ysuenaga/JDK-8152690/webrev.03/hotspot/ >>> > > >>> > > - jdk: >>> > > http://cr.openjdk.java.net/~ysuenaga/JDK-8152690/webrev.03/jdk/ >>> > > >>> > > >>> > >> it won't work unless you change the semantics of setName so I'm >>> not sure what you were thinking here. To take advantage of an arg taking >>> JVM_SetNativThreadName you would need to call it directly as no Java >>> code will call it . ??? >>> > > >>> > > I added a flag for setting native thread name to JNI-attached thread. >>> > > This change can set native thread name if main thread changes to >>> JNI-attached thread. >>> > > >>> > > >>> > > Thanks, >>> > > >>> > > Yasumasa >>> > > >>> > > >>> > > On 2016/04/16 16:11, David Holmes wrote: >>> > >> On 16/04/2016 3:27 PM, Yasumasa Suenaga wrote: >>> > >>> Hi David, >>> > >>> >>> > >>>> That change in behaviour may be a problem, it could be considered a >>> > >>>> regression that setName stops setting the native thread main, even >>> > >>>> though we never really intended it to work in the first place. >>> :( Such >>> > >>>> a change needs to go through CCC. >>> > >>> >>> > >>> I understood. >>> > >>> Can I send CCC request? >>> > >>> (I'm jdk 9 commiter, but I'm not employee at Oracle.) >>> > >> >>> > >> Sorry you can't file a CCC request, you would need a sponsor for >>> that. But at this stage I don't think I agree with the proposed change >>> because of the change in behaviour - there's no way to restore the >>> "broken" behaviour. >>> > >> >>> > >>> I want to continue to discuss about it on JDK-8154331 [1]. >>> > >> >>> > >> Okay we can do that. >>> > >> >>> > >>> >>> > >>>> Further, we expect the launcher to use the supported JNI >>> interface (as >>> > >>>> other processes would), not the internal JVM interface that >>> exists for >>> > >>>> the JDK sources to communicate with the JVM. >>> > >>> >>> > >>> I think that we do not use JVM interface if we add new method in >>> > >>> LauncherHelper as below: >>> > >>> ---------------- >>> > >>> diff -r f02139a1ac84 >>> > >>> src/java.base/share/classes/sun/launcher/LauncherHelper.java >>> > >>> --- a/src/java.base/share/classes/sun/launcher/LauncherHelper.java >>> > >>> Wed Apr 13 14:19:30 2016 +0000 >>> > >>> +++ b/src/java.base/share/classes/sun/launcher/LauncherHelper.java >>> > >>> Sat Apr 16 11:25:53 2016 +0900 >>> > >>> @@ -960,4 +960,8 @@ >>> > >>> else >>> > >>> return md.toNameAndVersion() + " (" + loc + ")"; >>> > >>> } >>> > >>> + >>> > >>> + static void setNativeThreadName(String name) { >>> > >>> + Thread.currentThread().setName(name); >>> > >>> + } >>> > >> >>> > >> You could also make that call via JNI directly, so not sure the >>> helper adds much here. But it won't work unless you change the semantics >>> of setName so I'm not sure what you were thinking here. To take >>> advantage of an arg taking JVM_SetNativThreadName you would need to call >>> it directly as no Java code will call it . ??? >>> > >> >>> > >> David >>> > >> ----- >>> > >> >>> > >>> } >>> > >>> diff -r f02139a1ac84 src/java.base/share/native/libjli/java.c >>> > >>> --- a/src/java.base/share/native/libjli/java.c Wed Apr 13 14:19:30 >>> > >>> 2016 +0000 >>> > >>> +++ b/src/java.base/share/native/libjli/java.c Sat Apr 16 11:25:53 >>> > >>> 2016 +0900 >>> > >>> @@ -125,6 +125,7 @@ >>> > >>> static void PrintUsage(JNIEnv* env, jboolean doXUsage); >>> > >>> static void ShowSettings(JNIEnv* env, char *optString); >>> > >>> static void ListModules(JNIEnv* env, char *optString); >>> > >>> +static void SetNativeThreadName(JNIEnv* env, char *name); >>> > >>> >>> > >>> static void SetPaths(int argc, char **argv); >>> > >>> >>> > >>> @@ -325,6 +326,7 @@ >>> > >>> * mainThread.isAlive() to work as expected. >>> > >>> */ >>> > >>> #define LEAVE() \ >>> > >>> + SetNativeThreadName(env, "DestroyJavaVM"); \ >>> > >>> do { \ >>> > >>> if ((*vm)->DetachCurrentThread(vm) != JNI_OK) { \ >>> > >>> JLI_ReportErrorMessage(JVM_ERROR2); \ >>> > >>> @@ -488,6 +490,9 @@ >>> > >>> mainArgs = CreateApplicationArgs(env, argv, argc); >>> > >>> CHECK_EXCEPTION_NULL_LEAVE(mainArgs); >>> > >>> >>> > >>> + /* Set native thread name. */ >>> > >>> + SetNativeThreadName(env, "main"); >>> > >>> + >>> > >>> /* Invoke main method. */ >>> > >>> (*env)->CallStaticVoidMethod(env, mainClass, mainID, mainArgs); >>> > >>> >>> > >>> @@ -1686,6 +1691,22 @@ >>> > >>> joptString); >>> > >>> } >>> > >>> >>> > >>> +/** >>> > >>> + * Set native thread name as possible. >>> > >>> + */ >>> > >>> +static void >>> > >>> +SetNativeThreadName(JNIEnv *env, char *name) >>> > >>> +{ >>> > >>> + jmethodID setNativeThreadNameID; >>> > >>> + jstring nameString; >>> > >>> + jclass cls = GetLauncherHelperClass(env); >>> > >>> + NULL_CHECK(cls); >>> > >>> + NULL_CHECK(setNativeThreadNameID = >>> (*env)->GetStaticMethodID(env, cls, >>> > >>> + "setNativeThreadName", "(Ljava/lang/String;)V")); >>> > >>> + NULL_CHECK(nameString = (*env)->NewStringUTF(env, name)); >>> > >>> + (*env)->CallStaticVoidMethod(env, cls, setNativeThreadNameID, >>> > >>> nameString); >>> > >>> +} >>> > >>> + >>> > >>> /* >>> > >>> * Prints default usage or the Xusage message, see >>> > >>> sun.launcher.LauncherHelper.java >>> > >>> */ >>> > >>> ---------------- >>> > >>> >>> > >>> So I want to add new arg to JVM_SetNativeThreadName(). >>> > >>> >>> > >>>> However this is still a change to the exported JVM interface and so >>> > >>>> has to be approved. >>> > >>> >>> > >>> Do you mean that this change needs CCC? >>> > >>> >>> > >>> >>> > >>> Thanks, >>> > >>> >>> > >>> Yasumasa >>> > >>> >>> > >>> >>> > >>> [1] >>> > >>> >>> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-April/019034.html >>> > >>> >>> > >>> >>> > >>> >>> > >>> On 2016/04/16 7:26, David Holmes wrote: >>> > >>>> On 15/04/2016 11:20 PM, Yasumasa Suenaga wrote: >>> > >>>>> Hi David, >>> > >>>>> >>> > >>>>>> I think it is a bug based on the comment here: >>> > >>>>>> >>> > >>>>>> JavaThread(bool is_attaching_via_jni = false); // for main >>> thread and >>> > >>>>>> JNI attached threads >>> > >>>>> >>> > >>>>> I filed it to JBS as JDK-8154331. >>> > >>>>> I will send review request to hotspot-runtime-dev. >>> > >>>>> >>> > >>>>> >>> > >>>>>> Though that will introduce a change in behaviour by itself as >>> setName >>> > >>>>>> will no longer set the native name for the main thread. >>> > >>>>> >>> > >>>>> I know. >>> > >>>> >>> > >>>> That change in behaviour may be a problem, it could be considered a >>> > >>>> regression that setName stops setting the native thread main, even >>> > >>>> though we never really intended it to work in the first place. >>> :( Such >>> > >>>> a change needs to go through CCC. >>> > >>>> >>> > >>>>> I checked changeset history. >>> > >>>>> JVM_SetNativeThreadName() was introduced in JDK-7098194, and it is >>> > >>>>> backported JDK 8. >>> > >>>> >>> > >>>> Yes this all came in as part of the OSX port in 7u2. >>> > >>>> >>> > >>>>> However, this function seems to be called from >>> Thread#setNativeName() >>> > >>>>> only. >>> > >>>>> In addition, Thread#setNativeName() is private method. >>> > >>>>> >>> > >>>>> Thus I think that we can add an argument to >>> JVM_SetNativeThreadName() >>> > >>>>> for force setting. >>> > >>>>> (e.g. "bool forced") >>> > >>>>> >>> > >>>>> It makes a change of JVM API. >>> > >>>>> However, this function is NOT public, so I think we can add one >>> more >>> > >>>>> argument. >>> > >>>>> >>> > >>>>> What do you think about this? >>> > >>>>> If it is accepted, we can set native thread name from Java >>> launcher. >>> > >>>> >>> > >>>> The private/public aspect of the Java API is not really at >>> issue. Yes >>> > >>>> we would add another arg to the JVM function to allow it to apply to >>> > >>>> JNI-attached threads as well (I'd prefer the arg reflect that >>> not just >>> > >>>> "force"). However this is still a change to the exported JVM >>> interface >>> > >>>> and so has to be approved. Further, we expect the launcher to >>> use the >>> > >>>> supported JNI interface (as other processes would), not the internal >>> > >>>> JVM interface that exists for the JDK sources to communicate >>> with the >>> > >>>> JVM. >>> > >>>> >>> > >>>> David >>> > >>>> ----- >>> > >>>> >>> > >>>>> >>> > >>>>> Thanks, >>> > >>>>> >>> > >>>>> Yasumasa >>> > >>>>> >>> > >>>>> >>> > >>>>> On 2016/04/15 19:16, David Holmes wrote: >>> > >>>>>> Hi Yasumasa, >>> > >>>>>> >>> > >>>>>> On 15/04/2016 6:53 PM, Yasumasa Suenaga wrote: >>> > >>>>>>> Hi David, >>> > >>>>>>> >>> > >>>>>>>> The fact that the "main" thread is not tagged as being a >>> JNI-attached >>> > >>>>>>>> thread seems accidental to me >>> > >>>>>>> >>> > >>>>>>> Should I file it to JBS? >>> > >>>>>> >>> > >>>>>> I think it is a bug based on the comment here: >>> > >>>>>> >>> > >>>>>> JavaThread(bool is_attaching_via_jni = false); // for main >>> thread and >>> > >>>>>> JNI attached threads >>> > >>>>>> >>> > >>>>>> Though that will introduce a change in behaviour by itself as >>> setName >>> > >>>>>> will no longer set the native name for the main thread. >>> > >>>>>> >>> > >>>>>>> I think that we can fix as below: >>> > >>>>>>> --------------- >>> > >>>>>>> diff -r 52aa0ee93b32 src/share/vm/runtime/thread.cpp >>> > >>>>>>> --- a/src/share/vm/runtime/thread.cpp Thu Apr 14 13:31:11 >>> 2016 +0200 >>> > >>>>>>> +++ b/src/share/vm/runtime/thread.cpp Fri Apr 15 17:50:10 >>> 2016 +0900 >>> > >>>>>>> @@ -3592,7 +3592,7 @@ >>> > >>>>>>> #endif // INCLUDE_JVMCI >>> > >>>>>>> >>> > >>>>>>> // Attach the main thread to this os thread >>> > >>>>>>> - JavaThread* main_thread = new JavaThread(); >>> > >>>>>>> + JavaThread* main_thread = new JavaThread(true); >>> > >>>>>>> main_thread->set_thread_state(_thread_in_vm); >>> > >>>>>>> main_thread->initialize_thread_current(); >>> > >>>>>>> // must do this before set_active_handles >>> > >>>>>>> @@ -3776,6 +3776,9 @@ >>> > >>>>>>> // Notify JVMTI agents that VM initialization is complete >>> - nop if >>> > >>>>>>> no agents. >>> > >>>>>>> JvmtiExport::post_vm_initialized(); >>> > >>>>>>> >>> > >>>>>>> + // Change attach status to "attached" >>> > >>>>>>> + main_thread->set_done_attaching_via_jni(); >>> > >>>>>>> + >>> > >>>>>> >>> > >>>>>> I think we can do this straight after the JavaThread constructor. >>> > >>>>>> >>> > >>>>>>> if (TRACE_START() != JNI_OK) { >>> > >>>>>>> vm_exit_during_initialization("Failed to start tracing >>> > >>>>>>> backend."); >>> > >>>>>>> } >>> > >>>>>>> --------------- >>> > >>>>>>> >>> > >>>>>>> >>> > >>>>>>>> If it wants to name its native threads then it is free to do so, >>> > >>>>>>> >>> > >>>>>>> Currently, JVM_SetNativeThreadName() cannot change native >>> thread name >>> > >>>>>>> when the caller thread is JNI-attached thread. >>> > >>>>>>> However, I think that it should be changed if Java developer >>> calls >>> > >>>>>>> Thread#setName() explicitly. >>> > >>>>>>> It is not the same of changing native thread name at >>> > >>>>>>> Threads::create_vm(). >>> > >>>>>>> >>> > >>>>>>> If it is allowed, I want to fix SetNativeThreadName() as below. >>> > >>>>>>> What do you think about this? >>> > >>>>>> >>> > >>>>>> The decision to not change the name of JNI-attached threads was a >>> > >>>>>> deliberate one** - this functionality originated with the OSX >>> port and >>> > >>>>>> it was reported that the initial feedback with this feature was to >>> > >>>>>> ensure it didn't mess with thread names that had been set by >>> the host >>> > >>>>>> process. If we do as you propose then we will just have an >>> > >>>>>> inconsistency for people to complain about: "why does my >>> native thread >>> > >>>>>> only have a name if I call cur.setName(cur.getName()) ?" >>> > >>>>>> >>> > >>>>>> ** If you follow the bugs and related discussions on this, the >>> > >>>>>> semantics and limitations (truncation, current thread only, >>> non-JNI >>> > >>>>>> threads only) of setting the native thread name were supposed >>> to be >>> > >>>>>> documented in the release notes - but as far as I can see that >>> never >>> > >>>>>> happened. :( >>> > >>>>>> >>> > >>>>>> My position on this remains that if it is desirable for the main >>> > >>>>>> thread (and DestroyJavaVM thread) to have native names then the >>> > >>>>>> launcher needs to be setting them using the available platform >>> APIs. >>> > >>>>>> Unfortunately this is complicated - as evidenced by the VM >>> code for >>> > >>>>>> this - due to the need to verify API availability. >>> > >>>>>> >>> > >>>>>> Any change in behaviour in relation to Thread.setName would >>> have to go >>> > >>>>>> through our CCC process I think. But a change in the launcher >>> would >>> > >>>>>> not. >>> > >>>>>> >>> > >>>>>> Sorry. >>> > >>>>>> >>> > >>>>>> David >>> > >>>>>> ----- >>> > >>>>>> >>> > >>>>>>> --------------- >>> > >>>>>>> --- a/src/share/vm/prims/jvm.cpp Thu Apr 14 13:31:11 >>> 2016 +0200 >>> > >>>>>>> +++ b/src/share/vm/prims/jvm.cpp Fri Apr 15 17:50:10 >>> 2016 +0900 >>> > >>>>>>> @@ -3187,7 +3187,7 @@ >>> > >>>>>>> JavaThread* thr = java_lang_Thread::thread(java_thread); >>> > >>>>>>> // Thread naming only supported for the current thread, >>> doesn't >>> > >>>>>>> work >>> > >>>>>>> for >>> > >>>>>>> // target threads. >>> > >>>>>>> - if (Thread::current() == thr && >>> !thr->has_attached_via_jni()) { >>> > >>>>>>> + if (Thread::current() == thr) { >>> > >>>>>>> // we don't set the name of an attached thread to avoid >>> stepping >>> > >>>>>>> // on other programs >>> > >>>>>>> const char *thread_name = >>> > >>>>>>> >>> java_lang_String::as_utf8_string(JNIHandles::resolve_non_null(name)); >>> > >>>>>>> --------------- >>> > >>>>>>> >>> > >>>>>>> >>> > >>>>>>> Thanks, >>> > >>>>>>> >>> > >>>>>>> Yasumasa >>> > >>>>>>> >>> > >>>>>>> >>> > >>>>>>> On 2016/04/15 13:32, David Holmes wrote: >>> > >>>>>>>> >>> > >>>>>>>> >>> > >>>>>>>> On 15/04/2016 1:11 AM, Yasumasa Suenaga wrote: >>> > >>>>>>>>> Roger, >>> > >>>>>>>>> Thanks for your comment! >>> > >>>>>>>>> >>> > >>>>>>>>> David, >>> > >>>>>>>>> >>> > >>>>>>>>>>>> I'll wait to see what Kumar thinks about this. I don't like >>> > >>>>>>>>>>>> exposing >>> > >>>>>>>>>>>> a new JVM function this way. >>> > >>>>>>>>> >>> > >>>>>>>>> I tried to call Thread#setName() after initializing VM (before >>> > >>>>>>>>> calling >>> > >>>>>>>>> main method), >>> > >>>>>>>>> I could set native thread name. >>> > >>>>>>>>> However, DestroyJavaVM() calls AttachCurrentThread(). So we >>> can't >>> > >>>>>>>>> set >>> > >>>>>>>>> native thread name for DestroyJavaVM. >>> > >>>>>>>> >>> > >>>>>>>> Right - I came to the same realization earlier this morning. >>> Which, >>> > >>>>>>>> unfortunately, takes me back to the basic premise here that >>> we don't >>> > >>>>>>>> set the name of threads not created by the JVM. The fact >>> that the >>> > >>>>>>>> "main" thread is not tagged as being a JNI-attached thread seems >>> > >>>>>>>> accidental to me - so JVM_SetNativeThreadName is only working by >>> > >>>>>>>> accident for the initial attach, and can't be used for the >>> > >>>>>>>> DestroyJavaVM part - which leaves the thread inconsistently >>> named at >>> > >>>>>>>> the native level. >>> > >>>>>>>> >>> > >>>>>>>> I'm afraid my view here is that the launcher has to be >>> treated like >>> > >>>>>>>> any other process that might host a JVM. If it wants to name its >>> > >>>>>>>> native threads then it is free to do so, but I would not be >>> exporting >>> > >>>>>>>> a function from the JVM to do that - it would have to use the OS >>> > >>>>>>>> specific API's for that on a platform-by-platform basis. >>> > >>>>>>>> >>> > >>>>>>>> Sorry. >>> > >>>>>>>> >>> > >>>>>>>> David >>> > >>>>>>>> ----- >>> > >>>>>>>> >>> > >>>>>>>> >>> > >>>>>>>>> >>> > >>>>>>>>> >>> > >>>>>>>>> Thanks, >>> > >>>>>>>>> >>> > >>>>>>>>> Yasumasa >>> > >>>>>>>>> >>> > >>>>>>>>> >>> > >>>>>>>>> On 2016/04/14 23:24, Roger Riggs wrote: >>> > >>>>>>>>>> Hi, >>> > >>>>>>>>>> >>> > >>>>>>>>>> Comments: >>> > >>>>>>>>>> >>> > >>>>>>>>>> jvm.h: The function names are too similar but perform >>> different >>> > >>>>>>>>>> functions: >>> > >>>>>>>>>> >>> > >>>>>>>>>> -JVM_SetNativeThreadName0 vs JVM_SetNativeThreadName >>> > >>>>>>>>>> >>> > >>>>>>>>>> - The first function applies to the current thread, the >>> second >>> > >>>>>>>>>> one a >>> > >>>>>>>>>> specific java thread. >>> > >>>>>>>>>> It would seem useful for there to be a comment somewhere >>> about >>> > >>>>>>>>>> what >>> > >>>>>>>>>> the new function does. >>> > >>>>>>>>>> >>> > >>>>>>>>>> windows/native/libjli/java_md.c: line 408 casts to (void*) >>> > >>>>>>>>>> instead of >>> > >>>>>>>>>> (SetNativeThreadName0_t) >>> > >>>>>>>>>> as is done on unix and mac. >>> > >>>>>>>>>> >>> > >>>>>>>>>> - macosx/native/libjli/java_md_macosx.c: >>> > >>>>>>>>>> - 737: looks wrong to overwriteifn->GetCreatedJavaVMs >>> used at >>> > >>>>>>>>>> line 730 >>> > >>>>>>>>>> - 738 Incorrect indentation; if possible keep the cast >>> on the >>> > >>>>>>>>>> same >>> > >>>>>>>>>> line as dlsym... >>> > >>>>>>>>>> >>> > >>>>>>>>>> $.02, Roger >>> > >>>>>>>>>> >>> > >>>>>>>>>> >>> > >>>>>>>>>> On 4/14/2016 9:32 AM, Yasumasa Suenaga wrote: >>> > >>>>>>>>>>>> That is an interesting question which I haven't had time to >>> > >>>>>>>>>>>> check - >>> > >>>>>>>>>>>> sorry. If the main thread is considered a JNI-attached >>> thread >>> > >>>>>>>>>>>> then >>> > >>>>>>>>>>>> my suggestion wont work. If it isn't then my suggestion >>> should >>> > >>>>>>>>>>>> work >>> > >>>>>>>>>>>> (but it means we have an inconsistency in our treatment of >>> > >>>>>>>>>>>> JNI-attached threads :( ) >>> > >>>>>>>>>>> >>> > >>>>>>>>>>> I ran following program on JDK 9 EA b112, and I confirmed >>> native >>> > >>>>>>>>>>> thread name (test) was set. >>> > >>>>>>>>>>> --------- >>> > >>>>>>>>>>> public class Sleep{ >>> > >>>>>>>>>>> public static void main(String[] args) throws Exception{ >>> > >>>>>>>>>>> Thread.currentThread().setName("test"); >>> > >>>>>>>>>>> Thread.sleep(3600000); >>> > >>>>>>>>>>> } >>> > >>>>>>>>>>> } >>> > >>>>>>>>>>> --------- >>> > >>>>>>>>>>> >>> > >>>>>>>>>>> >>> > >>>>>>>>>>>> I'll wait to see what Kumar thinks about this. I don't like >>> > >>>>>>>>>>>> exposing >>> > >>>>>>>>>>>> a new JVM function this way. >>> > >>>>>>>>>>> >>> > >>>>>>>>>>> I will update webrev after hearing Kumar's comment. >>> > >>>>>>>>>>> >>> > >>>>>>>>>>> >>> > >>>>>>>>>>> Thanks, >>> > >>>>>>>>>>> >>> > >>>>>>>>>>> Yasumasa >>> > >>>>>>>>>>> >>> > >>>>>>>>>>> >>> > >>>>>>>>>>> On 2016/04/14 21:32, David Holmes wrote: >>> > >>>>>>>>>>>> On 14/04/2016 1:52 PM, Yasumasa Suenaga wrote: >>> > >>>>>>>>>>>>> Hi, >>> > >>>>>>>>>>>>> >>> > >>>>>>>>>>>>> On 2016/04/14 9:34, David Holmes wrote: >>> > >>>>>>>>>>>>>> Hi, >>> > >>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>> On 14/04/2016 1:28 AM, Yasumasa Suenaga wrote: >>> > >>>>>>>>>>>>>>> Hi David, >>> > >>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>> Thanks for your comment. >>> > >>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>> I exported new JVM function to set native thread >>> name, and JLI >>> > >>>>>>>>>>>>>>> uses it >>> > >>>>>>>>>>>>>>> in new webrev. >>> > >>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>> First the launcher belongs to another team so >>> core-libs will >>> > >>>>>>>>>>>>>> need to >>> > >>>>>>>>>>>>>> review and approve this (in particular Kumar) - now cc'd. >>> > >>>>>>>>>>>>> >>> > >>>>>>>>>>>>> Thanks! >>> > >>>>>>>>>>>>> I'm waiting to review :-) >>> > >>>>>>>>>>>>> >>> > >>>>>>>>>>>>> >>> > >>>>>>>>>>>>>> Personally I would have used a Java upcall to >>> Thread.setName >>> > >>>>>>>>>>>>>> rather >>> > >>>>>>>>>>>>>> than exporting JVM_SetNativeThreadName. No hotspot changes >>> > >>>>>>>>>>>>>> needed in >>> > >>>>>>>>>>>>>> that case. >>> > >>>>>>>>>>>>> >>> > >>>>>>>>>>>>> As I wrote [1] in JBS, I changed to use Thread#setName() in >>> > >>>>>>>>>>>>> Thread#init(), >>> > >>>>>>>>>>>>> but I could not change native thread name. >>> > >>>>>>>>>>>> >>> > >>>>>>>>>>>> At Thread.init time the thread is not alive, which is >>> why the >>> > >>>>>>>>>>>> native >>> > >>>>>>>>>>>> name is not set. >>> > >>>>>>>>>>>> >>> > >>>>>>>>>>>>> I guess that caller of main() is JNI attached thread. >>> > >>>>>>>>>>>> >>> > >>>>>>>>>>>> That is an interesting question which I haven't had time to >>> > >>>>>>>>>>>> check - >>> > >>>>>>>>>>>> sorry. If the main thread is considered a JNI-attached >>> thread >>> > >>>>>>>>>>>> then >>> > >>>>>>>>>>>> my suggestion wont work. If it isn't then my suggestion >>> should >>> > >>>>>>>>>>>> work >>> > >>>>>>>>>>>> (but it means we have an inconsistency in our treatment of >>> > >>>>>>>>>>>> JNI-attached threads :( ) >>> > >>>>>>>>>>>> >>> > >>>>>>>>>>>> I'll wait to see what Kumar thinks about this. I don't like >>> > >>>>>>>>>>>> exposing >>> > >>>>>>>>>>>> a new JVM function this way. >>> > >>>>>>>>>>>> >>> > >>>>>>>>>>>> Thanks, >>> > >>>>>>>>>>>> David >>> > >>>>>>>>>>>> ----- >>> > >>>>>>>>>>>> >>> > >>>>>>>>>>>>> Thus I think that we have to provide a function to set >>> native >>> > >>>>>>>>>>>>> thread name. >>> > >>>>>>>>>>>>> >>> > >>>>>>>>>>>>> >>> > >>>>>>>>>>>>> Thanks, >>> > >>>>>>>>>>>>> >>> > >>>>>>>>>>>>> Yasumasa >>> > >>>>>>>>>>>>> >>> > >>>>>>>>>>>>> >>> > >>>>>>>>>>>>> [1] >>> > >>>>>>>>>>>>> >>> https://bugs.openjdk.java.net/browse/JDK-8152690?focusedCommentId=13926851&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13926851 >>> > >>>>>>>>>>>>> >>> > >>>>>>>>>>>>> >>> > >>>>>>>>>>>>> >>> > >>>>>>>>>>>>> >>> > >>>>>>>>>>>>> >>> > >>>>>>>>>>>>> >>> > >>>>>>>>>>>>>> Thanks, >>> > >>>>>>>>>>>>>> David >>> > >>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>> Could you review again? >>> > >>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>> - hotspot: >>> > >>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>> >>> http://cr.openjdk.java.net/~ysuenaga/JDK-8152690/webrev.02/hotspot/ >>> > >>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>> - jdk: >>> > >>>>>>>>>>>>>>> >>> http://cr.openjdk.java.net/~ysuenaga/JDK-8152690/webrev.02/jdk/ >>> > >>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>> Thanks, >>> > >>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>> Yasumasa >>> > >>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>> On 2016/04/13 22:00, David Holmes wrote: >>> > >>>>>>>>>>>>>>>> I'll answer on this original thread as well ... >>> > >>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>> Hi Yasumasa, >>> > >>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>> Please see my updates to the bug (sorry have been on >>> > >>>>>>>>>>>>>>>> vacation). >>> > >>>>>>>>>>>>>>>> This >>> > >>>>>>>>>>>>>>>> needs to be done in the launcher to be correct as we >>> do not >>> > >>>>>>>>>>>>>>>> set the >>> > >>>>>>>>>>>>>>>> name of threads that attach via JNI, which includes the >>> > >>>>>>>>>>>>>>>> "main" >>> > >>>>>>>>>>>>>>>> thread. >>> > >>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>> David >>> > >>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>> On 31/03/2016 9:49 AM, Yasumasa Suenaga wrote: >>> > >>>>>>>>>>>>>>>>> Thanks Robbin, >>> > >>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>> I'm waiting a sponsor and more reviewer :-) >>> > >>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>> Yasumasa >>> > >>>>>>>>>>>>>>>>> 2016/03/31 5:58 "Robbin Ehn" <robbin....@oracle.com >>> <mailto:robbin....@oracle.com>>: >>> > >>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>> FYI: I'm not a Reviewer. >>> > >>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>> /Robbin >>> > >>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>> On 03/30/2016 10:55 PM, Robbin Ehn wrote: >>> > >>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>> Thanks, looks good. >>> > >>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>> /Robbin >>> > >>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>> On 03/30/2016 03:47 PM, Yasumasa Suenaga wrote: >>> > >>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>> Hi, >>> > >>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>> I uploaded new webrev. >>> > >>>>>>>>>>>>>>>>>>>> Could you review it? >>> > >>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>> >>> http://cr.openjdk.java.net/~ysuenaga/JDK-8152690/webrev.01/ >>> > >>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>> Thanks, >>> > >>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>> Yasumasa >>> > >>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>> On 2016/03/30 19:10, Robbin Ehn wrote: >>> > >>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>> Hi, >>> > >>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>> On 03/30/2016 11:41 AM, Yasumasa Suenaga wrote: >>> > >>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>>> Hi Robbin, >>> > >>>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>>> 2016/03/30 18:22 "Robbin Ehn" >>> <robbin....@oracle.com <mailto:robbin....@oracle.com> >>> > >>>>>>>>>>>>>>>>>>>>>> <mailto:robbin....@oracle.com >>> <mailto:robbin....@oracle.com>>>: >>> > >>>>>>>>>>>>>>>>>>>>>> > >>> > >>>>>>>>>>>>>>>>>>>>>> > Hi Yasumasa, >>> > >>>>>>>>>>>>>>>>>>>>>> > >>> > >>>>>>>>>>>>>>>>>>>>>> > >>> > >>>>>>>>>>>>>>>>>>>>>> > On 03/25/2016 12:48 AM, Yasumasa Suenaga wrote: >>> > >>>>>>>>>>>>>>>>>>>>>> >> >>> > >>>>>>>>>>>>>>>>>>>>>> >> Hi Robbin, >>> > >>>>>>>>>>>>>>>>>>>>>> >> 2016/03/25 1:51 "Robbin Ehn" >>> > >>>>>>>>>>>>>>>>>>>>>> <robbin....@oracle.com >>> <mailto:robbin....@oracle.com> >>> > >>>>>>>>>>>>>>>>>>>>>> <mailto:robbin....@oracle.com >>> <mailto:robbin....@oracle.com>> >>> > >>>>>>>>>>>>>>>>>>>>>> >> <mailto:robbin....@oracle.com >>> <mailto:robbin....@oracle.com> >>> > >>>>>>>>>>>>>>>>>>>>>> <mailto:robbin....@oracle.com >>> <mailto:robbin....@oracle.com>>>>: >>> > >>>>>>>>>>>>>>>>>>>>>> >> >>> > >>>>>>>>>>>>>>>>>>>>>> >> > >>> > >>>>>>>>>>>>>>>>>>>>>> >> > Hi Yasumasa, >>> > >>>>>>>>>>>>>>>>>>>>>> >> > >>> > >>>>>>>>>>>>>>>>>>>>>> >> > I'm not sure why you don't set it: >>> > >>>>>>>>>>>>>>>>>>>>>> >> > >>> > >>>>>>>>>>>>>>>>>>>>>> >> > diff -r ded6ef79c770 >>> > >>>>>>>>>>>>>>>>>>>>>> src/share/vm/runtime/thread.cpp >>> > >>>>>>>>>>>>>>>>>>>>>> >> > --- a/src/share/vm/runtime/thread.cpp Thu >>> > >>>>>>>>>>>>>>>>>>>>>> Mar 24 >>> > >>>>>>>>>>>>>>>>>>>>>> 13:09:16 2016 >>> > >>>>>>>>>>>>>>>>>>>>>> +0000 >>> > >>>>>>>>>>>>>>>>>>>>>> >> > +++ b/src/share/vm/runtime/thread.cpp Thu >>> > >>>>>>>>>>>>>>>>>>>>>> Mar 24 >>> > >>>>>>>>>>>>>>>>>>>>>> 17:40:09 2016 >>> > >>>>>>>>>>>>>>>>>>>>>> +0100 >>> > >>>>>>>>>>>>>>>>>>>>>> >> > @@ -3584,6 +3584,7 @@ >>> > >>>>>>>>>>>>>>>>>>>>>> >> > JavaThread* main_thread = new >>> JavaThread(); >>> > >>>>>>>>>>>>>>>>>>>>>> >> > >>> main_thread->set_thread_state(_thread_in_vm); >>> > >>>>>>>>>>>>>>>>>>>>>> >> > main_thread->initialize_thread_current(); >>> > >>>>>>>>>>>>>>>>>>>>>> >> > + >>> main_thread->set_native_thread_name("main"); >>> > >>>>>>>>>>>>>>>>>>>>>> >> > // must do this before >>> set_active_handles >>> > >>>>>>>>>>>>>>>>>>>>>> >> > main_thread->record_stack_base_and_size(); >>> > >>>>>>>>>>>>>>>>>>>>>> >> > >>> > >>>>>>>>>>>>>>>>>>>>>> >>> main_thread->set_active_handles(JNIHandleBlock::allocate_block()); >>> > >>>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>>> >> > >>> > >>>>>>>>>>>>>>>>>>>>>> >> > here instead? Am I missing something? >>> > >>>>>>>>>>>>>>>>>>>>>> >> >>> > >>>>>>>>>>>>>>>>>>>>>> >> Native thread name is the same to thread >>> name in >>> > >>>>>>>>>>>>>>>>>>>>>> Thread >>> > >>>>>>>>>>>>>>>>>>>>>> class. >>> > >>>>>>>>>>>>>>>>>>>>>> >> It is set in c'tor in Thread or setName(). >>> > >>>>>>>>>>>>>>>>>>>>>> >> If you create new thread in Java app, native >>> > >>>>>>>>>>>>>>>>>>>>>> thread >>> > >>>>>>>>>>>>>>>>>>>>>> name >>> > >>>>>>>>>>>>>>>>>>>>>> will be >>> > >>>>>>>>>>>>>>>>>>>>>> set at >>> > >>>>>>>>>>>>>>>>>>>>>> >> startup. However, main thread is already >>> starte >>> > >>>>>>>>>>>>>>>>>>>>>> in VM. >>> > >>>>>>>>>>>>>>>>>>>>>> >> Thread name for "main" is set in >>> > >>>>>>>>>>>>>>>>>>>>>> create_initial_thread(). >>> > >>>>>>>>>>>>>>>>>>>>>> >> I think that the place of setting thrrad name >>> > >>>>>>>>>>>>>>>>>>>>>> should >>> > >>>>>>>>>>>>>>>>>>>>>> be the >>> > >>>>>>>>>>>>>>>>>>>>>> same. >>> > >>>>>>>>>>>>>>>>>>>>>> > >>> > >>>>>>>>>>>>>>>>>>>>>> > >>> > >>>>>>>>>>>>>>>>>>>>>> > Yes, I see your point. But then something like >>> > >>>>>>>>>>>>>>>>>>>>>> this is >>> > >>>>>>>>>>>>>>>>>>>>>> nicer, no? >>> > >>>>>>>>>>>>>>>>>>>>>> > >>> > >>>>>>>>>>>>>>>>>>>>>> > --- a/src/share/vm/runtime/thread.cpp Tue >>> Mar 29 >>> > >>>>>>>>>>>>>>>>>>>>>> 09:43:05 >>> > >>>>>>>>>>>>>>>>>>>>>> 2016 >>> > >>>>>>>>>>>>>>>>>>>>>> +0200 >>> > >>>>>>>>>>>>>>>>>>>>>> > +++ b/src/share/vm/runtime/thread.cpp Wed >>> Mar 30 >>> > >>>>>>>>>>>>>>>>>>>>>> 10:51:12 >>> > >>>>>>>>>>>>>>>>>>>>>> 2016 >>> > >>>>>>>>>>>>>>>>>>>>>> +0200 >>> > >>>>>>>>>>>>>>>>>>>>>> > @@ -981,6 +981,7 @@ >>> > >>>>>>>>>>>>>>>>>>>>>> > // Creates the initial Thread >>> > >>>>>>>>>>>>>>>>>>>>>> > static oop create_initial_thread(Handle >>> > >>>>>>>>>>>>>>>>>>>>>> thread_group, >>> > >>>>>>>>>>>>>>>>>>>>>> JavaThread* >>> > >>>>>>>>>>>>>>>>>>>>>> thread, >>> > >>>>>>>>>>>>>>>>>>>>>> > TRAPS) { >>> > >>>>>>>>>>>>>>>>>>>>>> > + static const char* initial_thread_name = >>> "main"; >>> > >>>>>>>>>>>>>>>>>>>>>> > Klass* k = >>> > >>>>>>>>>>>>>>>>>>>>>> >>> SystemDictionary::resolve_or_fail(vmSymbols::java_lang_Thread(), >>> > >>>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>>> true, >>> > >>>>>>>>>>>>>>>>>>>>>> CHECK_NULL); >>> > >>>>>>>>>>>>>>>>>>>>>> > instanceKlassHandle klass (THREAD, k); >>> > >>>>>>>>>>>>>>>>>>>>>> > instanceHandle thread_oop = >>> > >>>>>>>>>>>>>>>>>>>>>> klass->allocate_instance_handle(CHECK_NULL); >>> > >>>>>>>>>>>>>>>>>>>>>> > @@ -988,8 +989,10 @@ >>> > >>>>>>>>>>>>>>>>>>>>>> > java_lang_Thread::set_thread(thread_oop(), >>> thread); >>> > >>>>>>>>>>>>>>>>>>>>>> > java_lang_Thread::set_priority(thread_oop(), >>> > >>>>>>>>>>>>>>>>>>>>>> NormPriority); >>> > >>>>>>>>>>>>>>>>>>>>>> > thread->set_threadObj(thread_oop()); >>> > >>>>>>>>>>>>>>>>>>>>>> > - >>> > >>>>>>>>>>>>>>>>>>>>>> > - Handle string = >>> > >>>>>>>>>>>>>>>>>>>>>> java_lang_String::create_from_str("main", >>> > >>>>>>>>>>>>>>>>>>>>>> CHECK_NULL); >>> > >>>>>>>>>>>>>>>>>>>>>> > + >>> > >>>>>>>>>>>>>>>>>>>>>> > + >>> > >>>>>>>>>>>>>>>>>>>>>> >>> thread->set_native_thread_name(initial_thread_name); >>> > >>>>>>>>>>>>>>>>>>>>>> > + >>> > >>>>>>>>>>>>>>>>>>>>>> > + Handle string = >>> > >>>>>>>>>>>>>>>>>>>>>> >>> java_lang_String::create_from_str(initial_thread_name, >>> > >>>>>>>>>>>>>>>>>>>>>> CHECK_NULL); >>> > >>>>>>>>>>>>>>>>>>>>>> > >>> > >>>>>>>>>>>>>>>>>>>>>> > JavaValue result(T_VOID); >>> > >>>>>>>>>>>>>>>>>>>>>> > JavaCalls::call_special(&result, thread_oop, >>> > >>>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>>> Okay, I will upload new webrev later. >>> > >>>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>> Thanks! >>> > >>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>>> >> > The launcher seem to name itself 'java' and >>> > >>>>>>>>>>>>>>>>>>>>>> naming >>> > >>>>>>>>>>>>>>>>>>>>>> this >>> > >>>>>>>>>>>>>>>>>>>>>> thread >>> > >>>>>>>>>>>>>>>>>>>>>> just >>> > >>>>>>>>>>>>>>>>>>>>>> >> > 'main' is confusing to me. >>> > >>>>>>>>>>>>>>>>>>>>>> >> > >>> > >>>>>>>>>>>>>>>>>>>>>> >> > E.g. so main thread of the process (and >>> thus >>> > >>>>>>>>>>>>>>>>>>>>>> the >>> > >>>>>>>>>>>>>>>>>>>>>> process) is >>> > >>>>>>>>>>>>>>>>>>>>>> 'java' but >>> > >>>>>>>>>>>>>>>>>>>>>> >> > first JavaThread is 'main'. >>> > >>>>>>>>>>>>>>>>>>>>>> >> >>> > >>>>>>>>>>>>>>>>>>>>>> >> The native main thread in the process is not >>> > >>>>>>>>>>>>>>>>>>>>>> JavaThread. >>> > >>>>>>>>>>>>>>>>>>>>>> It is >>> > >>>>>>>>>>>>>>>>>>>>>> waiting >>> > >>>>>>>>>>>>>>>>>>>>>> >> for ending of Java main thread with >>> > >>>>>>>>>>>>>>>>>>>>>> pthread_join(). >>> > >>>>>>>>>>>>>>>>>>>>>> >> set_native_thread_name() is for >>> JavaThread. So I >>> > >>>>>>>>>>>>>>>>>>>>>> think that >>> > >>>>>>>>>>>>>>>>>>>>>> we do >>> > >>>>>>>>>>>>>>>>>>>>>> not >>> > >>>>>>>>>>>>>>>>>>>>>> >> need to call it for native main thread. >>> > >>>>>>>>>>>>>>>>>>>>>> > >>> > >>>>>>>>>>>>>>>>>>>>>> > >>> > >>>>>>>>>>>>>>>>>>>>>> > Not sure if we can change it anyhow, since >>> we want >>> > >>>>>>>>>>>>>>>>>>>>>> java and >>> > >>>>>>>>>>>>>>>>>>>>>> native >>> > >>>>>>>>>>>>>>>>>>>>>> name to be the same and java thread name might >>> have >>> > >>>>>>>>>>>>>>>>>>>>>> some >>> > >>>>>>>>>>>>>>>>>>>>>> dependents. >>> > >>>>>>>>>>>>>>>>>>>>>> > >>> > >>>>>>>>>>>>>>>>>>>>>> > The name is visible in e.g. /proc. >>> > >>>>>>>>>>>>>>>>>>>>>> > >>> > >>>>>>>>>>>>>>>>>>>>>> > $ ps H -C java -o 'pid tid comm' | head -4 >>> > >>>>>>>>>>>>>>>>>>>>>> > PID TID COMMAND >>> > >>>>>>>>>>>>>>>>>>>>>> > 6423 6423 java >>> > >>>>>>>>>>>>>>>>>>>>>> > 6423 6424 main >>> > >>>>>>>>>>>>>>>>>>>>>> > 6423 6425 GC Thread#0 >>> > >>>>>>>>>>>>>>>>>>>>>> > >>> > >>>>>>>>>>>>>>>>>>>>>> > It would be nice with something like 'Java Main >>> > >>>>>>>>>>>>>>>>>>>>>> Thread'. >>> > >>>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>>> I do not think so. >>> > >>>>>>>>>>>>>>>>>>>>>> Native main thread might not be a Java >>> launcher - e.g. >>> > >>>>>>>>>>>>>>>>>>>>>> Apache >>> > >>>>>>>>>>>>>>>>>>>>>> commons-daemon, JNI application, etc. >>> > >>>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>>> If you want to change native main thread name, >>> I think >>> > >>>>>>>>>>>>>>>>>>>>>> that we >>> > >>>>>>>>>>>>>>>>>>>>>> have to >>> > >>>>>>>>>>>>>>>>>>>>>> change Java launcher code. >>> > >>>>>>>>>>>>>>>>>>>>>> Should I include it in new webrev? >>> > >>>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>> No >>> > >>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>> Thanks again! >>> > >>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>> /Robbin >>> > >>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>>> Thanks, >>> > >>>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>>> Yasumasa >>> > >>>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>>> > Thanks >>> > >>>>>>>>>>>>>>>>>>>>>> > >>> > >>>>>>>>>>>>>>>>>>>>>> > /Robbin >>> > >>>>>>>>>>>>>>>>>>>>>> > >>> > >>>>>>>>>>>>>>>>>>>>>> > >>> > >>>>>>>>>>>>>>>>>>>>>> >> >>> > >>>>>>>>>>>>>>>>>>>>>> >> Thanks, >>> > >>>>>>>>>>>>>>>>>>>>>> >> >>> > >>>>>>>>>>>>>>>>>>>>>> >> Yasumasa >>> > >>>>>>>>>>>>>>>>>>>>>> >> >>> > >>>>>>>>>>>>>>>>>>>>>> >> > Thanks! >>> > >>>>>>>>>>>>>>>>>>>>>> >> > >>> > >>>>>>>>>>>>>>>>>>>>>> >> > /Robbin >>> > >>>>>>>>>>>>>>>>>>>>>> >> > >>> > >>>>>>>>>>>>>>>>>>>>>> >> > On 03/24/2016 03:26 PM, Yasumasa >>> Suenaga wrote: >>> > >>>>>>>>>>>>>>>>>>>>>> >> > > Hi all, >>> > >>>>>>>>>>>>>>>>>>>>>> >> > > >>> > >>>>>>>>>>>>>>>>>>>>>> >> > > HotSpot for Linux will set thread >>> name via >>> > >>>>>>>>>>>>>>>>>>>>>> pthread_setname_np(). >>> > >>>>>>>>>>>>>>>>>>>>>> >> > > However, main thread does not have it. >>> > >>>>>>>>>>>>>>>>>>>>>> >> > > >>> > >>>>>>>>>>>>>>>>>>>>>> >> > > All JavaThread have native name, and main >>> > >>>>>>>>>>>>>>>>>>>>>> thread is >>> > >>>>>>>>>>>>>>>>>>>>>> JavaThread. >>> > >>>>>>>>>>>>>>>>>>>>>> >> > > For consistency, main thread should have >>> > >>>>>>>>>>>>>>>>>>>>>> native >>> > >>>>>>>>>>>>>>>>>>>>>> thread >>> > >>>>>>>>>>>>>>>>>>>>>> name. >>> > >>>>>>>>>>>>>>>>>>>>>> >> > > >>> > >>>>>>>>>>>>>>>>>>>>>> >> > > I uploaded a webrev. Could you review it? >>> > >>>>>>>>>>>>>>>>>>>>>> >> > > >>> > >>>>>>>>>>>>>>>>>>>>>> >> > > >>> > >>>>>>>>>>>>>>>>>>>>>> >>> http://cr.openjdk.java.net/~ysuenaga/JDK-8152690/webrev.00/ >>> > >>>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>>> >> > > >>> > >>>>>>>>>>>>>>>>>>>>>> >> > > I cannot access JPRT. >>> > >>>>>>>>>>>>>>>>>>>>>> >> > > So I need a sponsor. >>> > >>>>>>>>>>>>>>>>>>>>>> >> > > >>> > >>>>>>>>>>>>>>>>>>>>>> >> > > >>> > >>>>>>>>>>>>>>>>>>>>>> >> > > Thanks, >>> > >>>>>>>>>>>>>>>>>>>>>> >> > > >>> > >>>>>>>>>>>>>>>>>>>>>> >> > > Yasumasa >>> > >>>>>>>>>>>>>>>>>>>>>> >> > > >>> > >>>>>>>>>>>>>>>>>>>>>> >> >>> > >>>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>> >>> > >>>
