April 2016 - core-libs-dev

RFR 7183985: Class.getAnnotation() throws an ArrayStoreException when the annotation class not present
by Liam Miller-Cushon 27 Apr '16

27 Apr '16

If an annotation value is an array of class literals or enum constants, calling Class.getAnnotation() fails with ArrayStoreException if the element type is not found. This patch implements the proposed fix from the bug thread, which allows getAnnotation() to succeed and a TypeNotPresentException to be thrown from Annotation.value(). bug: https://bugs.openjdk.java.net/browse/JDK-7183985 The patch is attached.

2 6

RFR(m): 8140281 deprecate Optional.get()
by Stuart Marks 27 Apr '16

27 Apr '16

Hi all, Please review these webrevs that deprecate Optional.get() and to replace it with Optional.getWhenPresent(). The corresponding changes are also applied to OptionalDouble.getAsDouble(), OptionalInt.getAsInt(), and OptionalLong.getAsLong(). Unlike most deprecations, this isn't about the function or the utility of some API, it's about the name. The solution is basically to rename the API. The problem is that "get" shows up as the "obvious" choice in things like IDE code completion, leading to code that mishandles empty Optionals. Typical Stack Overflow discourse runs something like this: Q: what do I do with this Optional thing A: just call get() Q: thanks, it works! Of course, it works until it doesn't. Examining the JDK's use of Optional.get(), I didn't see very many cases that called get() without first checking for the presence of a value. But I did see quite a number of cases like this: if (opt.isPresent()) { doSomething(opt.get()); } else { doSomethingElse(); } In many of these cases, the code could be refactored to use other Optional methods such as filter(), map(), or ifPresent(). In any case this reinforces the contention that use of get() leads to poor code. For this changeset, in just about all cases I've simply replaced the call to get() with a call to getWhenPresent(). In a couple cases I replaced the stream calls .filter(Optional::isPresent).map(Optional::get) with .flatMap(Optional::stream) which I hope will become the new idiom for unwrapping a stream of Optionals. While many cases could be cleaned up further, I didn't change them. The reasons are that I didn't want to spend too much time putting code cleanup into the critical path of this changeset (I'd be happy to help later); doing so would create potential conflicts with code coming in from the Jigsaw forest; and there are non-obvious places where converting from a conditional to one of the lambda-based methods could cause performance problems at startup. There are also a few cases where simplification is prevented because it would end up causing the resulting lambda expressions to throw checked exceptions. :-( Webrevs here: http://cr.openjdk.java.net/~smarks/reviews/8140281/webrev.0.langtools/ http://cr.openjdk.java.net/~smarks/reviews/8140281/webrev.0.jdk/ Thanks, s'marks

13 22

JDK-8155102: Process.toString could include pid, isAlive, exitStatus
by Andrey Dyachkov 27 Apr '16

27 Apr '16

Hi, I would like to familiarise myself with contributing process. Can I take this bug to fix? -- Thanks, Andrey

2 1

RFR(XS): 8155214: java/lang/invoke/PermuteArgsTest.java fails due to exhausted code cache
by Michael Haupt 27 Apr '16

27 Apr '16

Dear all, please review this change. Bug: https://bugs.openjdk.java.net/browse/JDK-8155214 Webrev: http://cr.openjdk.java.net/~mhaupt/8155214/webrev.00/ The failing test, unlike several other tests in the method handles area, was not safeguarded against code cache overflows. Thanks, Michael -- <http://www.oracle.com/> Dr. Michael Haupt | Principal Member of Technical Staff Phone: +49 331 200 7277 | Fax: +49 331 200 7561 Oracle Java Platform Group | LangTools Team | Nashorn Oracle Deutschland B.V. & Co. KG | Schiffbauergasse 14 | 14467 Potsdam, Germany ORACLE Deutschland B.V. & Co. KG | Hauptverwaltung: Riesstraße 25, D-80992 München Registergericht: Amtsgericht München, HRA 95603 Komplementärin: ORACLE Deutschland Verwaltung B.V. | Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697 Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher <http://www.oracle.com/commitment> Oracle is committed to developing practices and products that help protect the environment

2 1

RFR(S): 8155106: MHs.Lookup.findConstructor returns handles for array classes
by Michael Haupt 27 Apr '16

27 Apr '16

Dear all, please review this change. Bug: https://bugs.openjdk.java.net/browse/JDK-8155106 Webrev: http://cr.openjdk.java.net/~mhaupt/8155106/webrev.00/ Thanks, Michael -- <http://www.oracle.com/> Dr. Michael Haupt | Principal Member of Technical Staff Phone: +49 331 200 7277 | Fax: +49 331 200 7561 Oracle Java Platform Group | LangTools Team | Nashorn Oracle Deutschland B.V. & Co. KG | Schiffbauergasse 14 | 14467 Potsdam, Germany ORACLE Deutschland B.V. & Co. KG | Hauptverwaltung: Riesstraße 25, D-80992 München Registergericht: Amtsgericht München, HRA 95603 Komplementärin: ORACLE Deutschland Verwaltung B.V. | Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697 Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher <http://www.oracle.com/commitment> Oracle is committed to developing practices and products that help protect the environment

3 2

RFR 8154733, Fix module dependencies missed in java.rmi tests
by Felix Yang 27 Apr '16

27 Apr '16

Hi all, please review the fix to explicitly declare module dependencies for rmi tests. Bug: https://bugs.openjdk.java.net/browse/JDK-8154733 Webrev: http://cr.openjdk.java.net/~xiaofeya/8154733/webrev.00/ Thanks, Felix

3 6

RFR 9: 8066750 broke jdk9 builds; backup the changeset
by Roger Riggs 27 Apr '16

27 Apr '16

Please review the backout of the change for 8806750 which broke some builds that have not yet removed a dependency. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-backout-8155182/ After the dependencies are cleared, I'll send a new review for the original removal. Sorry for the hassle, Roger [1] 88066750 webrev: http://cr.openjdk.java.net/~rriggs/webrev-httpproxy-8066750/

3 3

RFR 8154447: Exempt classes under java.util.concurrent from MH.Lookup restrictions
by Paul Sandoz 27 Apr '16

27 Apr '16

Hi, Please review: http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8154447-mh-lookup-restricted-p… <http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8154447-mh-lookup-restricted-p…> https://bugs.openjdk.java.net/browse/JDK-8154447 <https://bugs.openjdk.java.net/browse/JDK-8154447> This is a quick fix to allow classes under java.util.concurrent to call MethodHandles.lookup(), and thus unblock the use of such classes using VarHandles. To verify and ensure we don’t regress i included a simple test. These fragile checks will be revisited later under another issue. Paul.

3 3

Re: RFR: JDK-8152690: main thread does not have native thread name
by Yasumasa Suenaga 26 Apr '16

26 Apr '16

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" <yasuenag(a)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(a)oracle.com >>> <mailto:gerard.ziemski@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(a)gmail.com >>> <mailto:yasuenag@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/01903… >>> > >>> >>> > >>> >>> > >>> >>> > >>> 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&… >>> > >>>>>>>>>>>>> >>> > >>>>>>>>>>>>> >>> > >>>>>>>>>>>>> >>> > >>>>>>>>>>>>> >>> > >>>>>>>>>>>>> >>> > >>>>>>>>>>>>> >>> > >>>>>>>>>>>>>> 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(a)oracle.com >>> <mailto:robbin.ehn@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(a)oracle.com <mailto:robbin.ehn@oracle.com> >>> > >>>>>>>>>>>>>>>>>>>>>> <mailto:robbin.ehn@oracle.com >>> <mailto:robbin.ehn@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(a)oracle.com >>> <mailto:robbin.ehn@oracle.com> >>> > >>>>>>>>>>>>>>>>>>>>>> <mailto:robbin.ehn@oracle.com >>> <mailto:robbin.ehn@oracle.com>> >>> > >>>>>>>>>>>>>>>>>>>>>> >> <mailto:robbin.ehn@oracle.com >>> <mailto:robbin.ehn@oracle.com> >>> > >>>>>>>>>>>>>>>>>>>>>> <mailto:robbin.ehn@oracle.com >>> <mailto:robbin.ehn@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 >>> > >>>>>>>>>>>>>>>>>>>>>> >> > > >>> > >>>>>>>>>>>>>>>>>>>>>> >> >>> > >>>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>>>>>>>>>>>>>> >>> > >>>>>>>>>> >>> > >>>

3 18

Re: Fwd: RFR(m): 8140281 deprecate Optional.get()
by Ali Ebrahimi 26 Apr '16

26 Apr '16

Hi, I agree with Jon, Vitaly, I think the name Optional completely describe its intents and semantics so I don't think we need more descriptive method name here. Optional === may be have value So Optional.get === may be return value Therefore, I think this all effort doesn't add any (or enough) value, So isn't worth of it (from resource POV). Done side: The all current source codes that uses this method when compiling with java9 get dozen of warnings. On Tue, Apr 26, 2016 at 4:13 AM, Jonathan Gibbons < jonathan.gibbons(a)oracle.com> wrote: > OK. > > The best I can say is that I disagree with the underlying change (to > deprecate Optional.get) but that if the API change is generally agreed to, > then I reluctantly agree with the edit here. > > The bug needs a noreg-label. > > -- jon > > > > > On 04/25/2016 04:37 PM, Stuart Marks wrote: > >> >> >> On 4/25/16 4:27 PM, Jonathan Gibbons wrote: >> >>> Enter.java: >>> It's pretty ugly that the recommended usage is to have to add a >>> SuppressWarnings >>> for a reasonable and valid use of Optional.get. >>> >>> Are you sure this is the API you want? >>> >> >> Ah, I had neglected to explain this one, sorry. >> >> In the majority cases, uses of get() can be refactored away to use one of >> the other Optional methods. >> >> In just about all the remaining cases, the code should use the >> replacement method getWhenPresent(), which has the same semantics as the >> current get() call. This is called for if lambdas should be avoided because >> of startup performance, or because of checked exceptions, or if it's >> provable from context that a value is always present. >> >> The case of Enter.java is a special one; it gets compiled with the boot >> libraries (JDK 8) and getWhenPresent() doesn't exist in those. It doesn't >> generate a warning though. But as you explained to me the other day, this >> code is later recompiled against the JDK 9 libraries, and in that case it >> does generate the warning. >> >> So for this case I think calling get() with @SuppressWarnings is the way >> to proceed. I opted to extract a local variable and put @SW on it, in order >> to minimize its scope, but if you prefer an alternative I'd be happy to >> change it. >> >> s'marks >> > > -- Best Regards, Ali Ebrahimi

3 2