March 2017 - core-libs-dev

RFR: 8177136: Caller sensitive methods Logger.getLogger, Logger.getAnonymousLogger, and System.getLogger should throw IllegalCallerException if there is no caller on the stack.
by Daniel Fuchs 21 Mar '17

21 Mar '17

Hi, Please find below a patch for: 8177136: Caller sensitive methods Logger.getLogger, Logger.getAnonymousLogger, and System.getLogger should throw IllegalCallerException if there is no caller on the stack. https://bugs.openjdk.java.net/browse/JDK-8177136 Caller sensitive methods Logger.getLogger, Logger.getAnonymousLogger, and System.getLogger currently throw an undocumented NullPointerException if they are called from JNI and there is no Java frame on the stack. Throwing NullPointerException is confusing and makes it look like there is a bug in the implementation. In truth, these method are @CallerSensitive, and therefore must not be called in a context where the caller cannot be determined. Therefore, the right thing to do is to throw IllegalCallerException and document this. webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8177136/webrev.00/ As per Rampdown Phase 2 Process [1] I'd also like to get confirmation that this is a reasonable proposal to fix in 9. This fix just transmutes a NullPointerException (which should never happen at this point in regular usage of the API) into an IllegalCallerException which will help diagnosing the fact that the API is called from a context where it's not supposed to be used. The risk of fixing should therefore be very limited. best regards, -- daniel [1] Rampdown Phase 2 Process http://mail.openjdk.java.net/pipermail/jdk9-dev/2017-March/005666.html

4 11

Compare a Path element to a string
by Weijun Wang 21 Mar '17

21 Mar '17

Hi Alan I'm profiling the new FilePermission implementation and found a lot of time spending on comparing a Path to a string, and the path has only one element. Here are them: 1. Comparing with "-": Path lastName = npath.getFileName(); if (lastName != null && lastName.toString().equals("-")) { ... } 2. Comparing with "": p = p2.relativize(p1).normalize(); if (p.getName(0).toString().isEmpty()) { ... } 3. Comparing with "..": for (Path item: p) { String s = item.toString(); if (!s.equals("..")) { ... } } I can think of comparing with static final Path fields, but is there a even faster way? Thanks Max

2 1

RFR [9]: 8177036: Class.checkMemberAccess throws NPE when calling Class methods via JNI
by Claes Redestad 20 Mar '17

20 Mar '17

Hi, please review this fix to avoid NPEs when calling certain Class methods via JNI: Bug: https://bugs.openjdk.java.net/browse/JDK-8177036 Webrev: http://cr.openjdk.java.net/~redestad/8177036/jdk.01/ Thanks! /Claes

6 10

java.lang.ref.Finalizer locks
by Milan Mimica 20 Mar '17

20 Mar '17

Hello Hope you can help me with an advice. I'm using a library, namely spring-jdbc, which implements java.lang.Object.finalize() in one static inner class which does get instantiated *a lot*. It creates significant contention on synchronized block in java.lang.ref.Finalizer from Object's constructor. Are there any plans to improve this (except from removing it)? I can see it's not trivial, but maybe it could be reimplemented lock-free? Milan Mimica, Senior Software Engineer / Division Lead Office: Mletacka 12/III, 52100 Pula, Croatia | Fax: +38552210979 | Mobile: +385993061692 Email: Milan.Mimica(a)infobip.com | Skype: mmimicaib www.infobip.com / GSMA Associate Member / Mobey Forum Member This message is private and confidential. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Infobip d.o.o. If you have received this message in error, please notify us immediately via email to customer.support(a)infobip.com or telephone +442032864231.

3 3

RFR: 8177086: java.lang.reflect.AccessibleObject::canAccess should share access cache with internal method ::checkAccess
by Peter Levart 20 Mar '17

20 Mar '17

Hi, Module system implementation refresh 2017/2 (JDK-8173393) introduced new API method AccessibleObject::canAccess which can be used to test if the caller has access to the reflected member (with possible target object argument for instance members). The implementation of this method, after some parameter validation checks, delegates directly to jdk.internal.reflect.Reflection::verifyMemberAccess. This is sub-optimal. Co-located internal method AccessibleObject::checkAccess also delegates to Reflection::verifyMemberAccess, but it also uses a one-element cache of access-check decision, which greatly speeds up repeated calls by the same caller to the same reflected member. The cache could be shared between those two methods which would improve performance of code idioms like this: if (member.canAccess(target) || member.trySetAccessible()) { member.invoke(....); ... } else { ... } Here's a proposed patch to improve this new method's performance: http://cr.openjdk.java.net/~plevart/jdk9-dev/8177086_AccessibleObject.canAc… The corresponding jira issue is: https://bugs.openjdk.java.net/browse/JDK-8177086 I don't know whether this should go to jdk9 or perhaps later to jdk9u, since this is new API that only came into existence 1 month ago and this patch improves its implementation. Should It perhaps go to jake repo 1st? Presented patch is made against current jdk9/jdk repo. Regards, Peter

4 8

RFR: 8159523. Fix tests depending on absence of -limitmods in VM arguments.
by Alexandre (Shura) Iline 20 Mar '17

20 Mar '17

Hi, Could you please review the suggested fox for: https://bugs.openjdk.java.net/browse/JDK-8159523 There has been a bit of discussion on jigsaw-dev, but it perhaps make sense to include core-libs-dev. There have been some fixes since the review was published, so we are now at revision #4: http://cr.openjdk.java.net/~shurailine/8159523/webrev.04/ The background for this fix is sufficiently captured in the original e-mail: > 1. An attempt was made to enhance jdk.testlibrary.ProcessTools class to support some filtering java and VM options. That implementation came out really overloaded (check executeModularTest(...) in [1]). > 2. It was suggested that a builder API could be introduced instead. “toolbox” classes from langtools test library were suggested as a start [2]. > 3. Further on, it was agreed that the classes need to be copied into the JDK test library rather than updated in place or somehow else shared between langtools and jdk test libraries. Quite recently the suggested improvement was discussed with a few folks from hotspot team where the same or a similar implementation could be reused. I am CCing Igor and Dima to comment. Thank you Shura > Hi, > > Please review a fix for https://bugs.openjdk.java.net/browse/JDK-8159523 > > To recap what has happened in the past. > > 1. An attempt was made to enhance jdk.testlibrary.ProcessTools class to support some filtering java and VM options. That implementation came out really overloaded (check executeModularTest(...) in [1]). > 2. It was suggested that a builder API could be introduced instead. “toolbox” classes from langtools test library were suggested as a start [2]. > 3. Further on, it was agreed that the classes need to be copied into the JDK test library rather than updated in place or somehow else shared between langtools and jdk test libraries. > > I have started porting a few tasks (JavaTask, JavacTask and JarTask) only to realize that there are many questions which may cause design discussions. So, instead, I have rolled back to one class (JavaTask) and the superclasses. If the design is acceptable, more tasks could be added as needed. > > The webrev: http://cr.openjdk.java.net/~shurailine/8159523/webrev.01 > > Thank you. > > Shura > > [1] http://cr.openjdk.java.net/~shurailine/8159523/webrev.00/test/lib/testlibra… > > [2] http://hg.openjdk.java.net/jdk9/jdk9/langtools/file/8e9e1a2373a4/test/tools…

4 6

Runtime.Version spec
by Pavel Rappo 20 Mar '17

20 Mar '17

I have a couple of questions on the spec wording. 1. * <li> <a name="build">{@code $BUILD}</a>, matching {@code * (0|[1-9][0-9]*)} --- The build number, incremented for each promoted * build. {@code $BUILD} is reset to {@code 1} when any portion of {@code * $VNUM} is incremented. If the build number is reset to 1 each time $VNUM is changed, then what's the purpose of build number 0? The spec allows it, but doesn't mention how it is used. 2. * comparison method. The comparison methods {@link #compareTo(Version) * compareTo()} and {@link #compareToIgnoreOptional(Version) * compareToIgnoreOptional()} should be used consistently with the * corresponding methods {@link #equals(Object) equals()} and {@link * #equalsIgnoreOptional(Object) equalsIgnoreOptional()}. The phrase "should be used consistently with" reads a bit strange to me. Can it be the case that what was actually meant is that Version class guarantees `compareTo` method is consistent with `equals` and `compareToIgnoreOptional` is consistent with `equalsIgnoreOptional`. If so, then this is not the question of how they are used (which is up to a caller), but of how they are implemented. Here's what java.lang.Comparable has to say about it: * The natural ordering for a class {@code C} is said to be consistent * with equals if and only if {@code e1.compareTo(e2) == 0} has * the same boolean value as {@code e1.equals(e2)} for every * {@code e1} and {@code e2} of class {@code C}. ... * It is strongly recommended (though not required) that natural orderings be * consistent with equals. This is so because sorted sets (and sorted maps) * without explicit comparators behave "strangely" when they are used with * elements (or keys) whose natural ordering is inconsistent with equals. ... * It is strongly recommended, but not strictly required that * {@code (x.compareTo(y)==0) == (x.equals(y))}. Generally speaking, any * class that implements the {@code Comparable} interface and violates * this condition should clearly indicate this fact. The recommended * language is "Note: this class has a natural ordering that is * inconsistent with equals." Maybe an explicit note could be moved to each of the comparison methods and/or the top-level paragraph is rephrased a bit? * This method provides ordering which is consistent with equals() and * This method provides ordering which is consistent with equalsIgnoreOptional() 3. The following explicit wordings do no harm, of course, but in my opinion they bloat the spec. In comparison methods: * A version is not comparable to any other type of object. If we make this class `final` as per [1], this might be removed. In `equals` and `hashCode` methods: * This method satisfies the general contract of the ... method. Well, once again, no harm obviously, but being a subtype is kind of all information we really need to know to infer that. *. Now an API question. Does it make any sense to make a stronger guarantee for Version.version method to return an immutable list (as in List.of) rather than just an unmodifiable one? Thanks. -------------------------------------------------------------------------------- [1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-March/046814.html

1 0

How to profile in jdk9?
by Weijun Wang 20 Mar '17

20 Mar '17

Sorry if the question reveals my ignorance. I need to profile with the latest jdk9 but everything I tried does not work: - hprof gone - try jvisualvm 1.3.9 but does not work with jdk9+161 on my Ubuntu. Even if I added 2 --add-opens still got a blank window because "cannot load even default layout". - netbeans 20170316 always shows "java.lang.IllegalArgumentException: Root cannot be null" whenever I want to run/debug/profile a program. What shall I do? Thanks Max

1 1

RFR(XS)(10): 8175342: assert(InstanceKlass::cast(k)->is_initialized()) failed: need to increase java_thread_min_stack_allowed calculation
by Chris Plummer 17 Mar '17

17 Mar '17

Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8175342 http://cr.openjdk.java.net/~cjplummer/8175342/webrev.00/webrev.jdk The assert is somewhat misleading, although it did properly detect a "too small" stack issue. The test was run with -Xss256k on a system with 64k pages. On this system 256k is suppose to be the smallest allowed stack size, so -Xss256k should work. The thread that the assert happens on is the main thread created by ContinueInNewThread0(). By default pthread gives new threads a guard page the size of an OS page. pthreads is suppose to add additional stack space for the guard page, but it doesn't. Later we call current_stack_region(), which among other things, computes the size of the stack. It has the following code to deal with the pthread guard page bug: // Work around NPTL stack guard error. size_t guard_size = 0; rslt = pthread_attr_getguardsize(&attr, &guard_size); *bottom += guard_size; *size -= guard_size; So the net effect is hotspot treats the stack as only being 192k, not 256k. However, in terms of usable stack space, hotspot then also subtracts the red, yellow, and shadow zones. Each of these is one OS page. So that subtracts another 192k, leaving us with 0k. The assert is a by product of this. It turns out this pthread guard page problem is already fixed for all java threads except the main thread. We do the following in os::create_thread(): pthread_attr_setguardsize(&attr, os::Linux::default_guard_size(thr_type)); For java threads, os::Linux::default_guard_size() returns 0, so the above code removes the guard page for java threads. The fix I'm proposing for the main thread does the same. Tested by running the test in question dozens of times on all supported platforms. Also ran most tests we do for nightlies except for longer running ones. thanks, Chris

3 6

RFR 8175013: Add @Generated Annotation
by Lance Andersen 17 Mar '17

17 Mar '17

Hi all, Per the discussion [1], please find the webrev for adding a replacement annotation for javax.annotation.Generated . The CCC has been approved. The webrev can be found at: http://cr.openjdk.java.net/~lancea/8175013/webrev.00/ <http://cr.openjdk.java.net/~lancea/8175013/webrev.00/> Best Lance [1] http://mail.openjdk.java.net/pipermail/jigsaw-dev/2017-February/011320.html <http://mail.openjdk.java.net/pipermail/jigsaw-dev/2017-February/011320.html> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 Lance.Andersen(a)oracle.com <mailto:Lance.Andersen@oracle.com>

5 8