April 2017 - core-libs-dev

RFR: 8178910 Problemlist sample tests
by Alexandre (Shura) Iline 18 Apr '17

18 Apr '17

Hi. Could you please review a quick fix to temporarily disable a couple of tests: $ hg diff ProblemList.txt diff --git a/test/ProblemList.txt b/test/ProblemList.txt --- a/test/ProblemList.txt +++ b/test/ProblemList.txt @@ -308,4 +308,7 @@ org/omg/CORBA/OrbPropertiesTest.java 8175177 generic-all +sample/mergesort/MergeSortTest.java 8178912 generic-all +sample/chatserver/ChatTest.java 8178912 generic-all + ############################################################################ $ Thank you. Shura

3 2

RFR [9] 8177738: Runtime.Version must be a value-based class
by Pavel Rappo 18 Apr '17

18 Apr '17

Hello, Please review the following change: http://cr.openjdk.java.net/~prappo/8177738/webrev.00/ This is an attempt to resolve a group of issues related to Version-String API and its implementation: 8177738: Runtime.Version must be a value-based class 8148822: (spec) Regex in Runtime.Version and JEP 223 should match 8160954: (spec) Runtime.Version regex and $PRE/$OPT issues 8148877: (spec) Specify when an empty '+' is required in a version string A good amount of work has been already done here and we just need to finish it. Let me explain what this patch does starting from the most trivial changes. - Fixes certain javadoc issues [a] (e.g. dangling html tags, overly qualified symbol references in @link tags, etc.) - Refactors Basic.java test, making it a little bit easier to read/modify this test in the future. - Addresses [2] in Runtime.java:963,1050. In addition I've rephrased Runtime.java:951 to reflect the fact a version string contains only the mentioned components and nothing else. In other words, $VSTR *matches* a valid version-string rather than *finds* itself somewhere in this version-string. - Makes Runtime.Version a value-based class [1][5]. The class has gotten `final` modifier, the only constructor has become private and the usual (in this case) verbiage has been added to the javadoc. Before that, Runtime.Version had ticked all other the boxes in the value-based class checklist already. - Improves $VNUM list mechanics in VersionProps.java.template and Runtime.java:1149-1153 by providing immutable list instead of unmodifiable. This change has become possible due to the fact Runtime.Version constructor has become private and, thus, all the callers are known and trusted. - Removes redundant/explicit javadoc declarations in `hashCode`, `equals` and `compareTo` methods. These methods are documented needlessly explicit. The only truly justified comment sits inside `compareToIgnoreOptional` method. Since `compareToIgnoreOptional` and `equalsIgnoreOptional` are not inherited, I marked the "consistent-with" relationship between them explicitly [c]. - Now a few words about a not-so-pleasant part which addresses [3][4]. In a nutshell the regex-style format ($VSTR) which is used in the spec to define the version-string, matches all valid version strings and *some more*. Here's the format: $VNUM(-$PRE)?(\+($BUILD)?(-$OPT)?)? If we expand all optional elements we will get the following 8 possibilities: ## $VSTR $PRE $BUILD $OPT EXAMPLE -------------------------------------------------------------------------------- 1 $VNUM 0 0 0 9.0.1 2 $VNUM+-$OPT 0 0 1 9.0.1+-longcat.dev 3 $VNUM+$BUILD 0 1 0 9.0.1+256 4 $VNUM+$BUILD-$OPT 0 1 1 9.0.1+256-longcat.dev 5 $VNUM-$PRE 1 0 0 9.0.1-ea * 6 $VNUM-$PRE+-$OPT 1 0 1 9.0.1-ea+-longcat.dev 7 $VNUM-$PRE+$BUILD 1 1 0 9.0.1-ea+256 8 $VNUM-$PRE+$BUILD-$OPT 1 1 1 9.0.1-ea+256-longcat.dev Now, ##6 was disallowed by design. In the case when $BUILD is not present, but both $PRE and $OPT are, the $VSTR will instead look like this: 6 $VNUM-$PRE-$OPT 1 0 1 9.0.1-ea-longcat.dev (i.e. without the "+" separator). This is a problem because it means that in addition to the format it needs a clear description of exceptional cases which are to be subtracted from the set of matched version-strings. Such a description has been proposed in [6] (you can have a look at the webrev that thread refers to): * <p> If the build number is unset, then {@code '+'} may only be present * when {@code $OPT} is present and {@code $PRE} is not. It indeed may be a satisfactory solution. However I would argue that it's probably not the best one. I suspect spec readers will jump straight to the format completely missing "the special case" paragraph simply because the format attracts attention and neatly defines the version-string. Neatly but wrongly. Another option would be to split the $VSTR format into a minimal number of sub-cases (e.g. Runtime:1032-1034). This will require no extra "special case" paragraph and will draw reader's attention to all the subtleties on the spot. # ## -------------------------------------------------------------------------------- 1 $VNUM(-$PRE)?\+$BUILD(-$OPT)? 3, 4, 7, 8 2 $VNUM-$PRE(-$OPT)? 5, 6 3 $VNUM(+-$OPT)? 1, 2 One more option would be to require "+" to be there in the case where any component to the right from $PRE is present (i.e. $BUILD or $OPT or both). This will greatly simplify the rules and allow the $VSTR to stay as it is today. This option would affect only the case ##6: 6 $VNUM-$PRE+-$OPT 1 0 1 9.0.1-ea+-longcat.dev ^^ Yes, it looks a bit ugly, but we already have this ugliness in case 2 (and we can't get rid of it because of [3]): 2 $VNUM+-$OPT 0 0 1 9.0.1+-longcat.dev ^^ I suspect both 2 and 6 will be pretty rare anyway as it would mean optional build information is there, but the build number is not. Sounds like a corner case to me. Thanks, -Pavel -------------------------------------------------------------------------------- [1] https://bugs.openjdk.java.net/browse/JDK-8177738 [2] https://bugs.openjdk.java.net/browse/JDK-8148822 [3] https://bugs.openjdk.java.net/browse/JDK-8148877 [4] https://bugs.openjdk.java.net/browse/JDK-8160954 [5] http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-March/046813.html [6] http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-October/044277.ht… [a] If you build locally or use promoted builds, please keep in mind there's a known issue with rendering documentation for Runtime.Version in some browsers: https://bugs.openjdk.java.net/browse/JDK-8177727 [b] Startup time/memory footprint might benefit as well (it will definitely require some benchmarking to determine if it is the case) [c] Even though I'm fully aware I do not have the full knowledge the design decisions were based upon, I must say I fail to see why equalsIgnoreOptional and compareToIgnoreOptional methods have been introduced from the day 1. It looks like something that could be neatly solved by providing a custom Comparator if needed.

3 4

[9] RFR: 8177314: java VM fails to start with a Japanese ShiftJIS locale
by Naoto Sato 18 Apr '17

18 Apr '17

Hi, Please review the fix for the following issue: https://bugs.openjdk.java.net/browse/JDK-8177314 The proposed change is located at: http://cr.openjdk.java.net/~naoto/8177314/webrev.00/ The change is to move SJIS based encodings into java.base module on Linux so that SJIS locale can be used as the default locale to start the JVM. Although they are not the default locale in Linux distributions, they still seem to be widely used. Naoto

4 6

RFR: 8178889: Move creation of AbstractChronology comparators to call sites
by Claes Redestad 18 Apr '17

18 Apr '17

Hi, in AbstractChronology we define a number of comparators which could be moved to their respective call site and thus be lazily initialized when needed with no performance penalty. Bug: https://bugs.openjdk.java.net/browse/JDK-8178889 Webrev: http://cr.openjdk.java.net/~redestad/8178889/jdk.01/ Testing/analysis: - naive micro to show this doesn't cause a regression in steady state[1], for those in doubt - reduces executed bytecode by ~127K and drops 5 generated classes from startup of applications that depend on AbstractChronology but don't use any of these comparators - pass all java/time tests Thanks! /Claes [1] package org.openjdk; import org.openjdk.jmh.annotations.Benchmark; import java.time.chrono.*; import java.lang.reflect.*; public class ChronoComparatorBench { public static Method timeLineOrder; static { try { timeLineOrder = ChronoLocalDate.class.getDeclaredMethod("timeLineOrder"); } catch (Exception e) { throw new RuntimeException(e); } } @Benchmark public Object getComparator() throws Exception { return timeLineOrder.invoke(null); } } -f 1 -tu us -bm avgt Benchmark Mode Cnt Score Error Units ChronoComparatorBench.getComparator avgt 20 5.979 ± 0.067 ns/op # before ChronoComparatorBench.getComparator avgt 20 5.975 ± 0.054 ns/op # after

2 1

JDK 9 RFR(s): 8177789 fix collections framework links to point to java.util package doc
by Stuart Marks 17 Apr '17

17 Apr '17

Hi all, Please review this changeset that fixes up links around the JDK that point to the old Collections technotes/guides area. This changeset points them to the java.util package summary instead. Most of the changes here simply adjust a link location. The file that's different is java/util/package-info.java, which adds the anchor to which all the other links point. Martin, This changeset includes a bunch of changes to java.util.concurrent classes, as well as to files like ArrayDeque which straddle the border between the JDK and the JSR166 repo. How did you want to handle these changes? I could just push this and you could merge upstream, or I could slice off the diffs from selected files and have you put them into JSR166 and then merge into the JDK. In any case, the changes are all one-liners, so I don't expect any conflicts. Let me know. Webrev: http://cr.openjdk.java.net/~smarks/reviews/8177789/webrev.0/ Thanks, s'marks

3 4

RFR 9: 8165641 : Deprecate Object.finalize
by Roger Riggs 17 Apr '17

17 Apr '17

Finalizers are inherently problematic and their use can lead to performance issues, deadlocks, hangs, and other problematic behavior. The problems have been accumulating for many years and the first step to deprecate Object.finalize and the overrides in the JDK to communicate the issues, recommend alternatives, and motivate changes where finalization is currently used. The behavior of finalization nor any uses of finalize are not modified by this change. Most of the changes are to suppress compilation warnings within the JDK. Please review and comment. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-finalize-deprecate-8165641/ Issue: https://bugs.openjdk.java.net/browse/JDK-8165641 Thanks, Roger

20 71

RFR[9] JDK-8178840: Adds FieldSetAccessibleTest.java and VerifyJimage.java to ProblemList
by sha.jiang＠oracle.com 17 Apr '17

17 Apr '17

Hi, Because of JDK-8178776, java/lang/Class/getDeclaredField/FieldSetAccessibleTest.java and tools/jimage/VerifyJimage.java should be added to ProblemList. diff -r 9ba6760481a8 test/ProblemList.txt --- a/test/ProblemList.txt Sat Apr 15 16:54:33 2017 -0400 +++ b/test/ProblemList.txt Sun Apr 16 19:30:49 2017 -0700 @@ -124,6 +124,7 @@ # jdk_lang java/lang/StringCoding/CheckEncodings.sh 7008363 generic-all +java/lang/Class/getDeclaredField/FieldSetAccessibleTest.java 8178776 generic-all jdk/internal/misc/JavaLangAccess/NewUnsafeString.java 8176188 generic-all @@ -255,6 +256,8 @@ tools/jimage/JImageListTest.java 8170120 generic-all tools/jimage/JImageVerifyTest.java 8170120 generic-all +tools/jimage/VerifyJimage.java 8178776 generic-all + ############################################################################ # jdk_jdi Best regards, John Jiang

3 2

RFR of JDK-8145163: Test Task for Platform Logging API and Service -- for moduralization
by Hamlin Li 14 Apr '17

14 Apr '17

Would you please review the below patch? bug: https://bugs.openjdk.java.net/browse/JDK-8145163 webrev: http://cr.openjdk.java.net/~mli/8145163/webrev.00/ Thank you -Hamlin

2 4

JDK 9 RFR of JDK-8169971: tools/jlink/multireleasejar/JLinkMultiReleaseJarTest.java fails intermittently
by Amy Lu 13 Apr '17

13 Apr '17

Please review this test-only change. tools/jlink/multireleasejar/JLinkMultiReleaseJarTest.java This test fails intermittently on Windows platform in the final cleanup step, @AfterClass, in which test tries to delete all dirs and files it created. Though it’s okay to leave this cleanup work to jtreg, I still make it done in test by making test to create everything under ./testoutput (testdir), and delete this testdir directly with FileUtils.deleteFileTreeWithRetry(testdir). Moreover, in FileUtils, as it tries to delete path with retry, we might want to give it a chance to "retry" in case of DirectoryNotEmptyException. bug: https://bugs.openjdk.java.net/browse/JDK-8169971 webrev: http://cr.openjdk.java.net/~amlu/8169971/webrev.00/ Thanks, Amy

4 6

RFR(S) : 8178415: remove ProcessTools::getPlatformSpecificVMArgs from testlibary
by Igor Ignatyev 12 Apr '17

12 Apr '17

http://cr.openjdk.java.net/~iignatyev/8178415/webrev.00/ > 59 lines changed: 0 ins; 59 del; 0 mod; Hi all, could you please review this small clean up of testlibaries? the only platform specific vm arguments which we had were -d64/32. they were deprecated in jdk9 by 8168010 and will be removed in jdk10 by 8169646. therefore ProcessTools::getPlatformSpecificVMArgs method is not needed anymore and should be removed. JBS: https://bugs.openjdk.java.net/browse/JDK-8178415 webrev: http://cr.openjdk.java.net/~iignatyev/8178415/webrev.00/ testing: :jdk_tier{1-3}, :hotspot_all 8168010: https://bugs.openjdk.java.net/browse/JDK-8168010 8169646: https://bugs.openjdk.java.net/browse/JDK-8169646 Thanks, -- Igor

3 3