RFR: JDK-8152690: main thread does not have native thread name
Kumar Srinivasan
kumar.x.srinivasan at oracle.com
Fri Apr 22 14:41:34 UTC 2016
Hi,
This is in the wrong place:
1715 if ((*env)->ExceptionOccurred(env)) {
1716 /*
1717 * We can clear pending exception because exception at this point
1718 * is not critical.
1719 */
1720 (*env)->ExceptionClear(env);
1721 }
This above needs to be after
391 SetNativeThreadName(env, "main");
392
Here is why, supposing 1704 through 1711, returns a NULL,
but have also encountered an exception. In which case
the method SetNativeThreadName will return to the caller, as
if nothing has happened, because NULL_CHECK simply checks for NULL
and returns to the caller. This will cause the caller to enter
the VM with exceptions.
Kumar
On 4/22/2016 5:11 AM, Yasumasa Suenaga wrote:
> Hi David,
>
>> I don't think we need to report the exception, but can just ignore
>> it. Either way we have to clear the exception before continuing.
>
> I've fixed it in new webrev:
>
> http://cr.openjdk.java.net/~ysuenaga/JDK-8152690/webrev.05/hotspot/
> http://cr.openjdk.java.net/~ysuenaga/JDK-8152690/webrev.05/jdk/
>
>
> Thanks,
>
> Yasumasa
>
>
> On 2016/04/22 15:33, David Holmes wrote:
>> On 22/04/2016 1:36 PM, Yasumasa Suenaga wrote:
>>> 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/
>>
>> Looks fine.
>>
>>> >
>>> > - jdk:
>>> > http://cr.openjdk.java.net/~ysuenaga/JDK-8152690/webrev.04/jdk/
>>
>> As per private email (but repeated here on the record) in java.c:
>>
>> 715 if ((*env)->ExceptionOccurred(env)) {
>> 1716 JLI_ReportExceptionDescription(env);
>> 1717 }
>>
>> I don't think we need to report the exception, but can just ignore
>> it. Either way we have to clear the exception before continuing.
>>
>> Thanks,
>> David
>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>> 2016/04/19 22:43 "Yasumasa Suenaga" <yasuenag at gmail.com
>>> <mailto:yasuenag at 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.ziemski at oracle.com
>>> <mailto:gerard.ziemski at oracle.com>
>>> >>> <mailto:gerard.ziemski at oracle.com
>>> <mailto:gerard.ziemski at 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
>>> <yasuenag at gmail.com <mailto:yasuenag at gmail.com>
>>> >>> <mailto:yasuenag at gmail.com <mailto:yasuenag at 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.ehn at oracle.com <mailto:robbin.ehn at oracle.com>
>>> >>> <mailto:robbin.ehn at oracle.com <mailto:robbin.ehn at 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.ehn at oracle.com <mailto:robbin.ehn at oracle.com>
>>> <mailto:robbin.ehn at oracle.com <mailto:robbin.ehn at oracle.com>>
>>> >>> > >>>>>>>>>>>>>>>>>>>>>> <mailto:robbin.ehn at oracle.com
>>> <mailto:robbin.ehn at oracle.com>
>>> >>> <mailto:robbin.ehn at oracle.com <mailto:robbin.ehn at oracle.com>>>>:
>>> >>> > >>>>>>>>>>>>>>>>>>>>>> >
>>> >>> > >>>>>>>>>>>>>>>>>>>>>> > Hi Yasumasa,
>>> >>> > >>>>>>>>>>>>>>>>>>>>>> >
>>> >>> > >>>>>>>>>>>>>>>>>>>>>> >
>>> >>> > >>>>>>>>>>>>>>>>>>>>>> > On 03/25/2016 12:48 AM, Yasumasa
>>> Suenaga wrote:
>>> >>> > >>>>>>>>>>>>>>>>>>>>>> >>
>>> >>> > >>>>>>>>>>>>>>>>>>>>>> >> Hi Robbin,
>>> >>> > >>>>>>>>>>>>>>>>>>>>>> >> 2016/03/25 1:51 "Robbin Ehn"
>>> >>> > >>>>>>>>>>>>>>>>>>>>>> <robbin.ehn at oracle.com
>>> <mailto:robbin.ehn at oracle.com>
>>> >>> <mailto:robbin.ehn at oracle.com <mailto:robbin.ehn at oracle.com>>
>>> >>> > >>>>>>>>>>>>>>>>>>>>>> <mailto:robbin.ehn at oracle.com
>>> <mailto:robbin.ehn at oracle.com>
>>> >>> <mailto:robbin.ehn at oracle.com <mailto:robbin.ehn at oracle.com>>>
>>> >>> > >>>>>>>>>>>>>>>>>>>>>> >> <mailto:robbin.ehn at oracle.com
>>> <mailto:robbin.ehn at oracle.com>
>>> >>> <mailto:robbin.ehn at oracle.com <mailto:robbin.ehn at oracle.com>>
>>> >>> > >>>>>>>>>>>>>>>>>>>>>> <mailto:robbin.ehn at oracle.com
>>> <mailto:robbin.ehn at oracle.com>
>>> >>> <mailto:robbin.ehn at oracle.com <mailto:robbin.ehn at 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
>>> >>> > >>>>>>>>>>>>>>>>>>>>>> >> > >
>>> >>> > >>>>>>>>>>>>>>>>>>>>>> >>
>>> >>> > >>>>>>>>>>>>>>>>>>>>>>
>>> >>> > >>>>>>>>>>>>>>>>>>>>>>
>>> >>> > >>>>>>>>>>
>>> >>> >
>>> >>>
>>>
More information about the core-libs-dev
mailing list