June 2015 - core-libs-dev

String.contains(CharSequence) calls toString on argument
by Tomasz Kowalczewski 03 Jun '15

03 Jun '15

Hello! Current implementation of String.contains that accepts CharSequence calls toString on it and passes resulting string to indexOf(String). This IMO defeats the purpose of using CharSequences (that is to have a mutable character buffer and not allocate unnecessary objects). Is changing this a desirable development? It seems pretty straightforward to port indexOf(String) to use CharSequence. If all you need is patch then I can work on it (I have signed OCA) just wanted to make sure it is not a futile work. -- Regards, Tomasz Kowalczewski

6 15

RFR: 8081824: Remove dead code GetPublicJREHome in the launcher
by Kumar Srinivasan 03 Jun '15

03 Jun '15

Hello, Please review the removal of launcher/windows/registry dead code, the details of which are in the bug report. https://bugs.openjdk.java.net/browse/JDK-8081824 The webrev: http://cr.openjdk.java.net/~ksrini/8081824/webrev.00/ Thanks Kumar

2 1

RFR 9: 8067808 : java/lang/ProcessBuilder/Basic.java failed on Assertion
by Roger Riggs 03 Jun '15

03 Jun '15

Please review test cleanup to make process id testing more reliable. It now spawns javaChild to report the child pid instead of parsing the output of OS specific commands. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-builder-8067808/ Issue: http://cr.openjdk.java.net/~rriggs/webrev-builder-8067808/ Thanks, Roger p.s. I'm looking for a review also for: 8081567 : java/lang/ProcessHandle/InfoTest.java failed Cannot run program "whoami"

2 2

RFR: JDK-8079063 ZoneOffsetTransition constructor should ignore nanoseconds
by Peter Levart 03 Jun '15

03 Jun '15

Hi, Now that JDK-8074003 is in, I'd like to discuss how to tackle JDK-8079063. Package-private ZoneOffsetTransition constructor that takes LocalDateTime transition is invoked from the following 4 places: 1 - the public static factory method: /** * Obtains an instance defining a transition between two offsets. * <p> * Applications should normally obtain an instance from {@link ZoneRules}. * This factory is only intended for use when creating {@link ZoneRules}. * * @param transition the transition date-time at the transition, which never * actually occurs, expressed local to the before offset, not null * @param offsetBefore the offset before the transition, not null * @param offsetAfter the offset at and after the transition, not null * @return the transition, not null * @throws IllegalArgumentException if {@code offsetBefore} and {@code offsetAfter} * are equal, or {@code transition.getNano()} returns non-zero value */ public static ZoneOffsetTransition of(LocalDateTime transition, ZoneOffset offsetBefore, ZoneOffset offsetAfter) { ...this one already disallows transition parameters that have transition.getNano() != 0. 2 - Lines 498..500 of ZoneOffsetTransitionRule: LocalDateTime localDT = LocalDateTime.of(date, time); LocalDateTime transition = timeDefinition.createDateTime(localDT, standardOffset, offsetBefore); return new ZoneOffsetTransition(transition, offsetBefore, offsetAfter); ...where 'time' is an instance field of ZoneOffsetTransitionRule. The ZoneOffsetTransitionRule public static factory method has the following definition: /** * Obtains an instance defining the yearly rule to create transitions between two offsets. * <p> * Applications should normally obtain an instance from {@link ZoneRules}. * This factory is only intended for use when creating {@link ZoneRules}. * * @param month the month of the month-day of the first day of the cutover week, not null * @param dayOfMonthIndicator the day of the month-day of the cutover week, positive if the week is that * day or later, negative if the week is that day or earlier, counting from the last day of the month, * from -28 to 31 excluding 0 * @param dayOfWeek the required day-of-week, null if the month-day should not be changed * @param time the cutover time in the 'before' offset, not null * @param timeEndOfDay whether the time is midnight at the end of day * @param timeDefnition how to interpret the cutover * @param standardOffset the standard offset in force at the cutover, not null * @param offsetBefore the offset before the cutover, not null * @param offsetAfter the offset after the cutover, not null * @return the rule, not null * @throws IllegalArgumentException if the day of month indicator is invalid * @throws IllegalArgumentException if the end of day flag is true when the time is not midnight */ public static ZoneOffsetTransitionRule of( Month month, int dayOfMonthIndicator, DayOfWeek dayOfWeek, LocalTime time, boolean timeEndOfDay, TimeDefinition timeDefnition, ZoneOffset standardOffset, ZoneOffset offsetBefore, ZoneOffset offsetAfter) { Objects.requireNonNull(month, "month"); Objects.requireNonNull(time, "time"); Objects.requireNonNull(timeDefnition, "timeDefnition"); Objects.requireNonNull(standardOffset, "standardOffset"); Objects.requireNonNull(offsetBefore, "offsetBefore"); Objects.requireNonNull(offsetAfter, "offsetAfter"); if (dayOfMonthIndicator < -28 || dayOfMonthIndicator > 31 || dayOfMonthIndicator == 0) { throw new IllegalArgumentException("Day of month indicator must be between -28 and 31 inclusive excluding zero"); } if (timeEndOfDay && time.equals(LocalTime.MIDNIGHT) == false) { throw new IllegalArgumentException("Time must be midnight when end of day flag is true"); } return new ZoneOffsetTransitionRule(month, dayOfMonthIndicator, dayOfWeek, time, timeEndOfDay, timeDefnition, standardOffset, offsetBefore, offsetAfter); } ...which allows 'time' parameter (that becomes ZoneOffsetTransitionRule's 'time' field) with no restriction as to whether it can contain nanos != 0 or not. 3, 4 - Lines 668..678 of ZoneRules private getOffsetInfo method: LocalDateTime dtBefore = savingsLocalTransitions[index]; LocalDateTime dtAfter = savingsLocalTransitions[index + 1]; ZoneOffset offsetBefore = wallOffsets[index / 2]; ZoneOffset offsetAfter = wallOffsets[index / 2 + 1]; if (offsetAfter.getTotalSeconds() > offsetBefore.getTotalSeconds()) { // gap return new ZoneOffsetTransition(dtBefore, offsetBefore, offsetAfter); } else { // overlap return new ZoneOffsetTransition(dtAfter, offsetBefore, offsetAfter); } ...where dtBefore/dtAfter "transition" parameters are taken from savingsLocalTransitions[] array that is filled-in in ZoneRules constructors from passed-in ZoneOffsetTransition objects. So here no nanos != 0 can sneak in if ZoneOffsetTransition invariant holds. The only place where nanos can sneak-in therefore seems to be the public ZoneOffsetTransitionRule.of() factory method. The question is whether the spec. could be changed so that ZoneOffsetTransitionRule.of() factory method would add another @throws definition: * @throws IllegalArgumentException if {@code time.getNano()} returns non-zero value This, I think, would be consistent with ZoneOffsetTransition.of() factory method and I believe early enough in the live of the API so that no custom software would be affected: http://cr.openjdk.java.net/~plevart/jdk9-dev/ZoneOffsetTransitionRule.time/… What do you think? Regards, Peter

3 6

RFR [9] 8072839: JAX-B Plugability Layer: using java.util.ServiceLoader
by Miroslav Kos 03 Jun '15

03 Jun '15

Hi everybody, this is review request for: 8072839: JAX-B Plugability Layer: using java.util.ServiceLoader The JAX-B API changed a little bit - proprietary ServiceLoader-like code has been replaced by java.util.ServiceLoader. This change is required by Jigsaw, old configuration way still supported. JBS:https://bugs.openjdk.java.net/browse/JDK-8072839 webrev: http://cr.openjdk.java.net/~mkos/8072839/jaxws.02/index.html CCC: http://ccc.us.oracle.com/8072839 Testing: JAX-WS (standalone) unit tests, no, jtreg tests available now, TCK/JCK tests will require new tests. Thanks Miran

3 6

JDK 9 RFR of JDK-8081773: sun/net/www/protocol/https/ChunkedOutputStream.java references library that doesn't exist
by Amy Lu 03 Jun '15

03 Jun '15

sun/net/www/protocol/https/ChunkedOutputStream.java This test fails with jtreg4.1/b12 because jtreg4.1/b12 adds stricter checking of @library tags and the library directory defined in the test doesn't exist. Please help to review and sponsor this fix, removed the unneeded @library. bug: https://bugs.openjdk.java.net/browse/JDK-8081773 webrev: http://cr.openjdk.java.net/~amlu/8081773/webrev.00/ --- old/test/sun/net/www/protocol/https/ChunkedOutputStream.java 2015-06-03 11:48:01.000000000 +0800 +++ new/test/sun/net/www/protocol/https/ChunkedOutputStream.java 2015-06-03 11:48:01.000000000 +0800 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2004, 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2004, 2015, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -25,7 +25,6 @@ * @test * @bug 5026745 * @modules java.base/sun.net.www - * @library ../../httpstest/ * @build TestHttpsServer HttpCallback * @run main/othervm ChunkedOutputStream * Thanks, Amy

2 2

JDK 9 RFR of JDK-8081775: two lib/testlibrary tests are failing with "Error. failed to clean up files after test" with jtreg 4.1 b12
by Amy Lu 03 Jun '15

03 Jun '15

lib/testlibrary/OutputAnalyzerReportingTest.java lib/testlibrary/OutputAnalyzerTest.java These tests fail with jtreg4.1/b12 because jtreg4.1/b12 adds stricter checking of @library tags and the library directory defined in the test doesn't exist. Please review the fix, removed the unneeded @library. bug: https://bugs.openjdk.java.net/browse/JDK-8081775 webrev: http://cr.openjdk.java.net/~amlu/8081775/webrev.00/ --- old/test/lib/testlibrary/OutputAnalyzerReportingTest.java 2015-06-03 10:48:57.000000000 +0800 +++ new/test/lib/testlibrary/OutputAnalyzerReportingTest.java 2015-06-03 10:48:57.000000000 +0800 @@ -28,7 +28,6 @@ * @summary Test the OutputAnalyzer reporting functionality, * such as printing additional diagnostic info * (exit code, stdout, stderr, command line, etc.) - * @library /testlibrary * @modules java.management * @build jdk.testlibrary.* * @run main jdk.testlibrary.OutputAnalyzerReportingTest --- old/test/lib/testlibrary/OutputAnalyzerTest.java 2015-06-03 10:48:58.000000000 +0800 +++ new/test/lib/testlibrary/OutputAnalyzerTest.java 2015-06-03 10:48:58.000000000 +0800 @@ -25,7 +25,6 @@ /* * @test * @summary Test the OutputAnalyzer utility class - * @library /testlibrary * @modules java.management * @build jdk.testlibrary.* * @run main jdk.testlibrary.OutputAnalyzerTest Thanks, Amy

2 1

8058779: Faster implementation of String.replace(CharSequence, CharSequence)
by Ivan Gerasimov 02 Jun '15

02 Jun '15

Hello everybody! I know many people here like it when the performance is getting better. It was suggested to make the literal variant of String.replace() faster. Currently, this method is implemented as a few calls to regexp API, so that the whole implementation takes only two lines of code. I've created two versions of the fix. In the first one, we scan the string and store indices of the found substrings in an array. Then, we allocate the precisely sized char array and fill it it. The case with the empty target has to be handled separately. BUGURL: https://bugs.openjdk.java.net/browse/JDK-8058779 WEBREV: http://cr.openjdk.java.net/~igerasim/8058779/00/webrev/ The second variant is much less verbose, however it's less efficient too. Here the StringJoiner is used as an intermediate storage. WEBREV: http://cr.openjdk.java.net/~igerasim/8058779/01/webrev/ Here are the micro-benchmark results (in a string of ~300 chars do ~15 replacements). 0) Baseline MyBenchmark.test thrpt 40 257'051.948 ± 4537.484 ops/s 1) Heavy-duty +308% MyBenchmark.test thrpt 40 1'049'235.602 ± 15501.803 ops/s 2) StringJoiner +190% MyBenchmark.test thrpt 40 746'000.629 ± 15387.036 ops/s Personally, I like my first variant better, even though it adds almost 300 lines of code. But I'd like to hear what people think of it. Sincerely yours, Ivan

9 48

JEP 254: Compact Strings
by mark.reinhold＠oracle.com 02 Jun '15

02 Jun '15

New JEP Candidate: http://openjdk.java.net/jeps/254 - Mark

5 9

RFR 9: 8081566: java/lang/ProcessHandle/InfoTest.java failed on case sensitive command
by Roger Riggs 01 Jun '15

01 Jun '15

Please review this testbug fix to correctly check the reported command name. On Windows the command filename may differ only by case and should be checked to see if they are the same file, not just the same string. webrev: http://cr.openjdk.java.net/~rriggs/webrev-infotest-8081566/ Issue: https://bugs.openjdk.java.net/browse/JDK-8081566 Thanks, Roger

3 2