December 2017 - core-libs-dev

Draft JEP: Enhanced pseudo-random number generators
by Guy Steele 23 May '19

23 May '19

Please check out a new draft JEP https://bugs.openjdk.java.net/browse/JDK-8193209 <https://bugs.openjdk.java.net/browse/JDK-8193209> that proposes new interface types and implementations for pseudo-random number generators. —Guy

2 3

[10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()
by Claes Redestad 26 Mar '18

26 Mar '18

Hi, please help review this patch to consolidate the number of implementation classes returned by the static collection factories: http://cr.openjdk.java.net/~redestad/8193128/open.00/ I set out to explore options for addressing small inefficiencies we've been running into, the latest after replacing use of @Stable arrays in java.lang.invoke with List.of() (JDK-8184777): - List.indexOf deferred to the iterator in AbstractList, which check for concurrent modification - Warmup takes a bit longer due to having to warm up four different classes and associated methods - Slowdowns in certain code paths attributable to certain calls becoming megamorphic Microbenchmark: http://cr.openjdk.java.net/~redestad/scratch/less_immutables/ListMorphism.j… http://cr.openjdk.java.net/~redestad/scratch/less_immutables/benchmarks.jar The benchmark explores how call-sites behave when performing some naive operations on a few different Lists. For every benchmark using List.of() there's a variant using ArrayList for comparison: Baseline: Benchmark Mode Cnt Score Error Units ListMorphism.finalGetFromArrayList thrpt 25 92.659 ± 3.058 ops/us ListMorphism.finalGetFromList thrpt 25 335.245 ± 0.244 ops/us # 3.6x ListMorphism.finalSumSizesArrayList thrpt 25 245.020 ± 0.106 ops/us ListMorphism.finalSumSizesList thrpt 25 335.107 ± 0.439 ops/us # 1.4x ListMorphism.getFromArrayList thrpt 25 70.343 ± 0.972 ops/us ListMorphism.getFromList thrpt 25 37.121 ± 0.135 ops/us # 0.53x ListMorphism.getFromArrayList12 thrpt 25 109.890 ± 0.286 ops/us ListMorphism.getFromList12 thrpt 25 109.552 ± 6.972 ops/us # 1.0x ListMorphism.sumSizesArrayList thrpt 25 131.467 ± 4.672 ops/us ListMorphism.sumSizesList thrpt 25 57.877 ± 0.102 ops/us # 0.45x ListMorphism.sumSizesArrayList12 thrpt 25 208.652 ± 11.294 ops/us ListMorphism.sumSizesList12 thrpt 25 227.269 ± 0.961 ops/us # 1.1x The good: When dealing with List literals (the final* benchmarks), List.of() allows really nice speed-ups compared to ArrayList. The bad: When not used as a literal, List.of() implementations at call-sites can cause a substantial slowdown (megamorphic dispatch) The ugly: After some explorations[1], I narrowed in on the following experiment: http://cr.openjdk.java.net/~redestad/scratch/less_immutables/webrev/ Basically: Merge List1 and List2 into a single class, List12, merge List0 into ListN (List0 has a singleton instance, so might as well be an instance of ListN). Same for Set0,1,2,N. Map0 is merged into MapN. This strikes a balance between throughput, footprint and slightly better startup/warmup behavior. According to jol estimates[2] the size of List12 is the same as both List1 and List2 in the current JDK implementation. Set12 is footprint neutral compared to Set2 on all platforms but lose a few bytes on 64-bit VMs compared to Set1. Benchmark Mode Cnt Score Error Units ListMorphism.finalGetFromArrayList thrpt 25 93.046 ± 0.569 ops/us ListMorphism.finalGetFromList thrpt 25 335.280 ± 0.154 ops/us # 3.6x ListMorphism.finalSumSizesArrayList thrpt 25 244.595 ± 1.085 ops/us ListMorphism.finalSumSizesList thrpt 25 303.351 ± 0.182 ops/us # 1.2x ListMorphism.getFromArrayList thrpt 25 70.631 ± 0.070 ops/us ListMorphism.getFromList thrpt 25 73.258 ± 2.955 ops/us # 1.04x ListMorphism.getFromArrayList12 thrpt 25 109.921 ± 0.096 ops/us ListMorphism.getFromList12 thrpt 25 127.392 ± 0.088 ops/us # 1.16x ListMorphism.sumSizesArrayList thrpt 25 131.393 ± 4.882 ops/us ListMorphism.sumSizesList thrpt 25 107.686 ± 5.286 ops/us # 0.82x ListMorphism.sumSizesArrayList12 thrpt 25 212.350 ± 0.134 ops/us ListMorphism.sumSizesList12 thrpt 25 198.778 ± 0.479 ops/us # 0.94x The experiment has a flag to change number of specialized List/Set/Map classes (-Djdk.ImmutableCollections.specializations=0|1|2, default=2). 1 specialization (List1 + ListN, Set1 + SetN) is more or less the same as 2, some ~1-2% improvements, mainly in sumSizes micros. 0 specializations (List-, Set, MapN only) achieves a small increase in throughput on some micros by ensuring callsites stay monomorphic, but it's not very substantial by my measures (~5%, but mostly in sumSizes micros). Keeping the footprint more or less the same, while evening out a few rough edges and improving startup and static footprint seems like the overall best option. An alternative would be to keep the experimental flag, but I don't think a 5% gain on micros warrants the extra maintenance burden and testing that entails. The proposed patch is more or less this experiment with 2 specializations, but having removed the flag and code movement needed to support it removed (along with a fix in the writeReplace methods of List12/Set12) Thanks! /Claes [1] Older experiments: http://cr.openjdk.java.net/~redestad/immutable/list12N.0/ -- simply merge L0 into LN (still have L1, L2 and LN) - nothing really changed, though http://cr.openjdk.java.net/~redestad/immutable/list1N.0/ -- L0 merged into LN, drop L2. Substantial performance boost on micros. Footprint drop for 2-element lists. http://cr.openjdk.java.net/~redestad/immutable/listNdouble.0/ -- L0+L1+L2+LN merged into one implementation. Same footprint with a single class, but loses a lot on throughput in micros. http://cr.openjdk.java.net/~redestad/immutable/listNsingle.0/ -- L0+L1+LN merged, drop L2. Simplification of the previous. Like the list1N.0 experiment in footprint, but a loss in throughput on all measures. No approach seemed a win across the board here: we either had to accept a footprint reduction, or see throughput suffer dramatically. [2] http://openjdk.java.net/projects/code-tools/jol/

11 51

RFR of 8180451: ByteArrayInputStream should override readAllBytes, readNBytes, and transferTo
by Brian Burkhalter 23 Mar '18

23 Mar '18

https://bugs.openjdk.java.net/browse/JDK-8180451 http://cr.openjdk.java.net/~bpb/8180451/webrev.00 1. Add overrides of readAllBytes(), readNBytes(), and transferTo(). 2. Simplify parameter checks in read(byte[],int,int). 3. Change <code>foo</code> to {@code foo}. Thanks, Brian

5 11

Re: Manifest Related Bugs JDK-6372077, JDK-6202130, JDK-8176843, JDK-4842483, JDK-6443578, JDK-6910466, JDK-4935610, and JDK-4271239
by Sean Mullan 01 Mar '18

01 Mar '18

Hi Phillip, All of these bugs are in the core-libs area, so I am copying the core-libs-dev list since that is where they should be discussed and reviewed. I have -bcc-ed security-dev (where this was originally posted). --Sean On 10/2/17 1:24 PM, Philipp Kunz wrote: > Hi, > > While fixing JDK-6695402 I came across other related bugs to manifests > such as JDK-6372077, JDK-6202130, JDK-8176843, JDK-4842483, and > JDK-6443578 which all relate to manifest reading and writing. Somewhere > bug 7071055 is mentioned but I cannot find anywhere. Another group of > bugs, JDK-6910466, JDK-4935610, and JDK-4271239 concern the mandatory > manifest main attributes Manifest-Version or Signature-Version and at > first glance are duplicates. If you know of more known bugs, not > necessarily present in jira, I'd be glad to get notified. > > There are also some comments about utf handling and line breaking in the > code of Manifest: > http://hg.openjdk.java.net/jdk10/master/file/a0116bcc65b7/src/java.base/sha… > http://hg.openjdk.java.net/jdk10/master/file/a0116bcc65b7/src/java.base/sha… > http://hg.openjdk.java.net/jdk10/master/file/a0116bcc65b7/src/java.base/sha… > > Furthermore, Attributes.map should declare appropriate type parameters: > http://hg.openjdk.java.net/jdk10/master/file/a0116bcc65b7/src/java.base/sha… > The specification would also require that `header names must not start > with _, - or "From"` > (http://docs.oracle.com/javase/7/docs/technotes/guides/jar/jar.html#Section-…) > but I would opt not to add this as a hard restriction because I guess it > can be assumed that such names are in use now after having been working > for years. A warning to a logger might be conceivable such as in > http://hg.openjdk.java.net/jdk10/master/file/a0116bcc65b7/src/java.base/sha… > Attribute values are never checked at all and invalid characters such as > line breaks are never detected except that when reading the manifest > again the values are cut off. > The tab character (U+0008) does not work in manifest values. > I suspect that there are also issues regarding the iteration order but I > haven't got a prove yet unlike for the other points above: > http://hg.openjdk.java.net/jdk10/master/file/a0116bcc65b7/src/java.base/sha… > There is duplicated or very similar code in Attributes and Manifest: > Attributes.write-Manifest.write-Attributes.writeMain and > Attributes.read-Manifest.read. > Resolving JDK-6202130 would have the additional benefit to be able to > view manifests with any utf-8 capable editor even if multi-byte > characters break across lines. > > Fixing these issues individually looks like more complicated work than > fixing them all at once, I guess, also because of a very low current > test coverage. So I'd suggest to add some thorough tests along with > fixing these issues. But before I start I would like to get some > guidance, assistance or at least confirmation on how to proceed. I'm new > to open jdk and have submitted only one patch so far. > > Is it ok to add tests for things that have worked before? > Is it ok to refactor duplicated code just for the added value to reduce > effort for testing? > I assume compatibility to and from existing manifests is the highest > priority, correct? This would also be the hard part in adding as > complete test coverage as possible. What would be acceptable criteria to > meet? > Why does Attributes not extend LinkedHashMap and why does Manifest not > extend HashMap? Any objection? > While I would not want to write code that looks slow or change more than > necessary one can never know before having performance actually > measured. Is there some way this is dealt with or should I wait for such > feedback until after patch submission? > > Would someone sponsor? > > Regards, > Philipp > > > > > ------------------------------------------------------------------------ > > > > Paratix GmbH > St Peterhofstatt 11 > 8001 Zürich > > +41 (0)76 397 79 35 > philipp.kunz(a)paratix.ch <mailto:philipp.kunz@paratix.ch>

5 12

Thread.single_step field, anything using it?
by Alan Bateman 15 Feb '18

15 Feb '18

I've been going through the fields in java.lang.Thread and I'm wondering if this field can be removed: /* Whether or not to single_step this thread. */ private boolean single_step; This field was used in the original Classic VM (pre-OpenJDK history). It doesn't appear to be used in the HotSpot VM. Does anyone know of any reason to keep it? Are there other VMs using it by any chance? -Alan

3 3

RFR: 8194154 patch for crash at File.getCanonicalPath()
by Wenqian Pei 14 Feb '18

14 Feb '18

Hi: Bug: https://bugs.openjdk.java.net/browse/JDK-8194154 We found that if user defines -Duser.dir like "/home/a/b/c/", jvm will crash at File.getCanonicalPath() in java process bootstrap (before invoking user's java code). The native implematation of canonicalize_md.c:collapsible(char *names) has problem in processing double '/', parameter 'names' need normalized before JNI_CALL. This patch normalize parameters before call canonicalize0() in this call path Patch and test are in mailbox attachments. Can I please have a review for this patch? Thanks Wenqian Pei

6 11

RFR [11] (JAXP): 6857903: SAXException.initCause() does not correctly set Exception
by Aleks Efimov 08 Feb '18

08 Feb '18

Hello, Please, help to review the fix for jaxp bug that fixes SAXException to correctly set exception cause with 'setCause' method: http://cr.openjdk.java.net/~aefimov/6857903/dev/00/ I've tried to keep the fix miminal with respect to serial form of this API class, i.e. kept private 'exception' field. The new test case was added to IssueTracker56Test unit test. Testing showed no related JCK/JTREG failures. With Best Regards, Aleksei

3 10

RFR: 8011697(ScriptEngine "js" randomly means either "rhino" or "nashorn", but should instead select one)
by Srinivas Dama 02 Feb '18

02 Feb '18

Hi, Please review http://cr.openjdk.java.net/~sdama/8011697/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8011697 Fix is to make sure ScriptEngineManager always returns a particular engine on all platforms consistently. Regards, Srinivas

3 7

[11] RFR of 8146656: Wrong Months Array for DateFormatSymbols
by Rachna Goel 23 Jan '18

23 Jan '18

Hi, Kindly review API Doc fix for java.text.DateFormatSymbols. JBS Issue : https://bugs.openjdk.java.net/browse/JDK-8146656 Webrev: http://cr.openjdk.java.net/~rgoel/8146656/webrev/ CSR: https://bugs.openjdk.java.net/browse/JDK-8191414 -- Thanks, Rachna

4 9

Re: RFR 4358774: Add null InputStream and OutputStream
by Brian Burkhalter 12 Jan '18

12 Jan '18

Hi Roger, I agree. It does not seem that whatever performance improvement might accrue from not using the Objects methods is offset by the increased code readability in addition to mitigating the risks mentioned in [1]. I have reinstated this approach in http://cr.openjdk.java.net/~bpb/4358774/webrev.02/. Thanks, Brian On Dec 7, 2017, at 2:04 PM, Roger Riggs <Roger.Riggs(a)Oracle.com> wrote: > For checking indices, I think you should leverage the work done for java.util.Objects.checkFromIndexSize(...) > as optimized for this purpose in 8135248. That was extensively designed and reviewed for optimal performance. > > Regards, Roger > > [1] https://bugs.openjdk.java.net/browse/JDK-8135248

7 22