Re: RFR: 8261744: Implement CharsetDecoder ASCII and latin-1 fast-paths [v2]
On Thu, 18 Feb 2021 23:16:54 GMT, Claes Redestad wrote: > Turns out the native code called by `SystemProps.initProperties();` can > initialize a `CharsetDecoder` if `sun.jnu.encoding` is set to certain values, > and this tripped up the initialization to cause `getJavaLangAccess` to be > called before `setJavaLangAccess`. By moving `setJavaLangAccess` to the very > start of `initPhase1` we avoid this possibility, and limit churn. I assume this is tests running with -Dfile.encoding. I can go along with moving setJavaLangAccess to the start of initPhase1. I'll do another pass over the updated patch today. Good work! - PR: https://git.openjdk.java.net/jdk/pull/2574
Re: RFR: 4926314: Optimize Reader.read(CharBuffer) [v7]
On Thu, 18 Feb 2021 20:35:10 GMT, Brian Burkhalter wrote: >> src/java.base/share/classes/java/io/Reader.java line 221: >> >>> 219: // if the last call to read returned -1 or the >>> number of bytes >>> 220: // requested have been read then break >>> 221: } while (n >= 0 && remaining > 0); >> >> The code for case that the char buffer has a backing array looks okay but >> I'm not sure about the direct buffer/other cases. One concern is that this >> is a read method, not a transferXXX method so we shouldn't be calling the >> underlying read several times. You'll see what I mean if you consider the >> scenario where you read < rem, then read again and the second read blocks or >> throws. I'l also concerned about "workBuffer" adding more per-stream >> footprint for cases where skip or read(CB) is used. Objects such as >> InputStreamReader are already a problem due to the underlying stream decoder. > > I think that's what @AlanBateman intended. The `skip()` changes would revert > also (I think) but the C-style array changes can stay. Thanks. Yes, let's keep bring it back to just eliminating the intermediate array when the buffer has a backing array. The other case that been examined separated if needed but we can't use the approach proposed in the current PR because it changes the semantics of read when there is a short-read followed by a blocking or exception throwing read. - PR: https://git.openjdk.java.net/jdk/pull/1915
Integrated: JDK-8260858: Implementation specific property xsltcIsStandalone for XSLTC Serializer
On Sat, 13 Feb 2021 00:16:50 GMT, Joe Wang wrote: > Adds a property similar to 'isStandalone' in JDK-8249867. > > Please note that the test received an auto-format. The material changes were > the two tests marked with bug id 8260858 and related data and methods. This pull request has now been integrated. Changeset: c99eeb01 Author:Joe Wang URL: https://git.openjdk.java.net/jdk/commit/c99eeb01 Stats: 243 lines in 7 files changed: 167 ins; 4 del; 72 mod 8260858: Implementation specific property xsltcIsStandalone for XSLTC Serializer Reviewed-by: lancea, naoto - PR: https://git.openjdk.java.net/jdk/pull/2559
Re: System.getEnv(String name, String def)
I think System.getenv().getOrDefault(key, def) is already an adequate
solution.
On Thu, 18 Feb 2021 at 09:21, Loïc MATHIEU wrote:
> Hi,
>
> Thanks for your replies.
>
> My point here was that using env variables is more and more common to
> configure apps, and they have less convenient support than system
> properties (the other way to provide easy external configuration at launch
> time with key/value).
>
> I agree that Objects.requireNonNullElse(System.getEnv(String key), "n/a"));
> is an easy way to achieve this, but I still think System.getEnv(key, "n/a")
> should be a good addition.
>
> Regards,
>
> Loïc
>
> Le mar. 16 févr. 2021 à 21:59, Remi Forax a écrit :
>
> > - Mail original -
> > > De: "Michael Kuhlmann"
> > > À: "core-libs-dev"
> > > Envoyé: Mardi 16 Février 2021 13:34:30
> > > Objet: Re: System.getEnv(String name, String def)
> >
> > > Hi Rémi,
> > >
> > > I don't want to be pedantic, but I see this as an anti-pattern. You
> > > would create an Optional just to immediately call orElse() on it.
> That's
> > > not how Optional should be used. (But you know that.)
> > >
> > > It's described in Recipe 12 of this Java Magazine article, for
> instance:
> > >
> >
> https://blogs.oracle.com/javamagazine/12-recipes-for-using-the-optional-class-as-its-meant-to-be-used
> >
> > yep, you are right.
> > Optional.ofNullable(...).orElse(...) is not the best pattern in term of
> > readability.
> >
> > >
> > > Best,
> > > Michael
> >
> > regards,
> > Rémi
> >
> > >
> > > On 2/15/21 3:09 PM, Remi Forax wrote:
> > >> Hi Loic,
> > >> You can use Optional.OfNullable() which is a kind of the general
> bridge
> > between
> > >> the nullable world and the non-nullable one.
> > >>
> > >>var fooOptional = Optional.ofNullable(System.getenv("FOO"));
> > >>var fooValue = fooOptional.orElse(defaultValue);
> > >>
> > >> regards,
> > >> Rémi Forax
> > >>
> > >> - Mail original -
> > >>> De: "Loïc MATHIEU"
> > >>> À: "core-libs-dev"
> > >>> Envoyé: Lundi 15 Février 2021 14:59:42
> > >>> Objet: System.getEnv(String name, String def)
> > >>
> > >>> Hello,
> > >>>
> > >>> I wonder if there has already been some discussion to provide
> > >>> a System.getEnv(String name, String def) method that allows to
> return a
> > >>> default value in case the env variable didn't exist.
> > >>>
> > >>> When using system properties instead of env variable, we do have a
> > >>> System.getProperty(String key, String def) variant.
> > >>>
> > >>> Stating the JavaDoc of System.getEnv():
> > >>> *System properties and environment variables are both conceptually
> > mappings
> > >>> between names and values*
> > >>>
> > >>> So if system properties and environment variables are similar
> concepts,
> > >>> they should provide the same functionalities right ?
> > >>>
> > >>> This would be very convenient as more and more people rely on
> > >>> environment variables these days to configure their applications.
> > >>>
> > >>> Regards,
> > >>>
> > > >> Loïc
> >
>
Re: RFR: 8261938: ASN1Formatter.annotate should not return in the finally block [v2]
On Fri, 19 Feb 2021 01:56:19 GMT, Roger Riggs wrote: > The formattters are a test component used both standalone and in the context > of the HexPrinter test utilities. > In typical use, the stream is a wrapped byte array, so there are no > exceptions other than EOF. > The choice of DataInputStream was chosen for the convenience of the methods > to read different types > and (declared) exceptions are an unwelcome artifact. > > Formatters are designed to be nested, where one formatter can call another > and the valuable output > is the formatted string that has been accumulated from the beginning of the > stream. > If an exception was percolated up and the formatted output discarded it would > defeat the purpose. > > If an exception was thrown, it would still return useful information about > the stream to that point. > The documentation could be improved to be clear on that point. Got it. Thanks for your clarification. Updated. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/2620
Re: RFR: 8261938: ASN1Formatter.annotate should not return in the finally block [v3]
> Hi all, > > ASN1Formatter.annotate should be able to throw an IOException according to > this comment [1]. > But it fails to do so due to the return [2] in the finally block, which would > swallow the IOException. > > Generally, it seems not good to put a return statement in a finally block > because it would overwrite other return-statements or Exceptions [3]. > > Thanks. > Best regards, > Jie > > [1] > https://github.com/openjdk/jdk/blob/master/test/lib/jdk/test/lib/hexdump/ASN1Formatter.java#L113 > [2] > https://github.com/openjdk/jdk/blob/master/test/lib/jdk/test/lib/hexdump/ASN1Formatter.java#L120 > [3] > https://stackoverflow.com/questions/17481251/finally-block-does-not-complete-normally-eclipse-warning Jie Fu has updated the pull request incrementally with one additional commit since the last revision: Catch exception and return the accumulated string - Changes: - all: https://git.openjdk.java.net/jdk/pull/2620/files - new: https://git.openjdk.java.net/jdk/pull/2620/files/b537e060..a1f29f33 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2620=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2620=01-02 Stats: 16 lines in 1 file changed: 13 ins; 1 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2620.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2620/head:pull/2620 PR: https://git.openjdk.java.net/jdk/pull/2620
Re: RFR: 8261938: ASN1Formatter.annotate should not return in the finally block [v2]
On Thu, 18 Feb 2021 23:41:08 GMT, Jie Fu wrote: >> An EOFException can occur during the call to annotate() and must return the >> accumulated contents of the StringBuffer. Otherwise it is discarded. > > Thanks @RogerRiggs for your review. > > Just want to make sure: > > 1. AFAIK, for a method, it seems impossible to return a value and throw an > exception at the same time. > Did you mean we just need to return a string with the IOException to be > catched? > > 2. Does it make sense to return a string when an IOException happens? > > Thanks. The formattters are a test component used both standalone and in the context of the HexPrinter test utilities. In typical use, the stream is a wrapped byte array, so there are no exceptions other than EOF. The choice of DataInputStream was chosen for the convenience of the methods to read different types and (declared) exceptions are an unwelcome artifact. Formatters are designed to be nested, where one formatter can call another and the valuable output is the formatted string that has been accumulated from the beginning of the stream. If an exception was percolated up and the formatted output discarded it would defeat the purpose. If an exception was thrown, it would still return useful information about the stream to that point. The documentation could be improved to be clear on that point. - PR: https://git.openjdk.java.net/jdk/pull/2620
Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v3]
On Thu, 18 Feb 2021 16:18:42 GMT, jmehrens
wrote:
>> Yes -- I think in response to this it makes more sense to pull the
>> `ImmutableCollections` classes out for now and only focus on the wrapping of
>> the classes within `Collections` so we aren't blocked by studying and
>> rectifying these inconsistencies.
>
> Maybe it is not correct for UnmodifiableEntrySet::contains to short circuit?
> What if the implementation was changed to:
>
> `public boolean contains(Object o) {
> if (!(o instanceof Map.Entry))
> return c.contains(o); //false, NPE, or CCE
> return c.contains(
> new UnmodifiableEntry<>((Map.Entry) o));
> }`
This however changes the behavior of unmodifiable maps compared to before; i.e.
before for other entry sets, they could not throw exception if the object
passed was not map entry; now they can.
-
PR: https://git.openjdk.java.net/jdk/pull/2596
Re: RFR: 6323374: (coll) Optimize Collections.unmodifiable* and synchronized* [v3]
On Wed, 17 Feb 2021 19:12:19 GMT, Ian Graves wrote:
>> This raises some interesting issues and makes me wonder if we should allow a
>> single-wrap of the `ImmutableCollections` classes for now to make this less
>> onerous.
>
> Yes -- I think in response to this it makes more sense to pull the
> `ImmutableCollections` classes out for now and only focus on the wrapping of
> the classes within `Collections` so we aren't blocked by studying and
> rectifying these inconsistencies.
Maybe it is not correct for UnmodifiableEntrySet::contains to short circuit?
What if the implementation was changed to:
`public boolean contains(Object o) {
if (!(o instanceof Map.Entry))
return c.contains(o); //false, NPE, or CCE
return c.contains(
new UnmodifiableEntry<>((Map.Entry) o));
}`
-
PR: https://git.openjdk.java.net/jdk/pull/2596
Re: RFR: 8261938: ASN1Formatter.annotate should not return in the finally block [v2]
On Thu, 18 Feb 2021 14:53:17 GMT, Roger Riggs wrote: >> Jie Fu has updated the pull request incrementally with one additional commit >> since the last revision: >> >> Update the copyright year > > An EOFException can occur during the call to annotate() and must return the > accumulated contents of the StringBuffer. Otherwise it is discarded. Thanks @RogerRiggs for your review. Just want to make sure: 1. AFAIK, for a method, it seems impossible to return a value and throw an exception at the same time. Did you mean we just need to return a string with the IOException to be catched? 2. Does it make sense to return a string when an IOException happens? Thanks. - PR: https://git.openjdk.java.net/jdk/pull/2620
Re: RFR: 8261744: Implement CharsetDecoder ASCII and latin-1 fast-paths [v2]
On Wed, 17 Feb 2021 17:19:58 GMT, Alan Bateman wrote: > > Right, I'm not exactly sure why the more limited changes I attempted in > > [5f4e87f](https://github.com/openjdk/jdk/commit/5f4e87f50f49e64b8616063c176ea35632b0347e) > > failed. In that change I simply changed the initialization order, which > > made the failing (closed) tests pass locally - but not in our CI. Since > > this is in initPhase1 there should be no concurrency possible. > > The Reference Handler thread is started by the initializer in > jl.ref.Reference so could be a candidate. The Finalizer thread is another but > this should VM.awaitInitLevel(1) and not touch JLA until initPhase1 is done. Turns out the native code called by `SystemProps.initProperties();` can initialize a `CharsetDecoder` if `sun.jnu.encoding` is set to certain values, and this tripped up the initialization to cause `getJavaLangAccess` to be called before `setJavaLangAccess`. By moving `setJavaLangAccess` to the very start of `initPhase1` we avoid this possibility, and limit churn. - PR: https://git.openjdk.java.net/jdk/pull/2574
Re: RFR: 8261744: Implement CharsetDecoder ASCII and latin-1 fast-paths [v4]
> This patch exposes a couple of intrinsics used by String to speed up ASCII > checking and byte[] -> char[] inflation, which can be used by latin1 and > ASCII-compatible CharsetDecoders to speed up decoding operations. > > - Fast-path implemented for all standard charsets, with up to 10x performance > improvements in microbenchmarks reading Strings from ByteArrayInputStream. > - Cleanup of StreamDecoder/-Encoder with some minor improvements when > interpreting > - Reworked creation of JavaLangAccess to be safely published for > CharsetDecoders/-Encoders used for setting up System.out/in. As JLA and these > encoders are created during System.initPhase1 the current sequence caused the > initialization to became unstable and a few tests were consistently getting > an NPE. > > Testing: tier1-3 Claes Redestad has updated the pull request incrementally with one additional commit since the last revision: Revert JavaLangAccessImpl changes - Changes: - all: https://git.openjdk.java.net/jdk/pull/2574/files - new: https://git.openjdk.java.net/jdk/pull/2574/files/367be2a5..e2316007 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2574=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2574=02-03 Stats: 176 lines in 2 files changed: 6 ins; 10 del; 160 mod Patch: https://git.openjdk.java.net/jdk/pull/2574.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2574/head:pull/2574 PR: https://git.openjdk.java.net/jdk/pull/2574
Integrated: 8261977: Fix comment for getPrefixed() in canonicalize_md.c
On Thu, 18 Feb 2021 19:35:09 GMT, Alexey Semenyuk wrote: > Follow up for JDK-8235397 fix This pull request has now been integrated. Changeset: 0c31d5b9 Author:Alexey Semenyuk URL: https://git.openjdk.java.net/jdk/commit/0c31d5b9 Stats: 5 lines in 1 file changed: 0 ins; 4 del; 1 mod 8261977: Fix comment for getPrefixed() in canonicalize_md.c Reviewed-by: alanb - PR: https://git.openjdk.java.net/jdk/pull/2631
Re: RFR: 4926314: Optimize Reader.read(CharBuffer) [v7]
On Wed, 17 Feb 2021 15:37:11 GMT, Alan Bateman wrote: >> Philippe Marschall has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - Replace c-style array declarations >> - Share work buffer between #skip and #read > > src/java.base/share/classes/java/io/Reader.java line 221: > >> 219: // if the last call to read returned -1 or the >> number of bytes >> 220: // requested have been read then break >> 221: } while (n >= 0 && remaining > 0); > > The code for case that the char buffer has a backing array looks okay but I'm > not sure about the direct buffer/other cases. One concern is that this is a > read method, not a transferXXX method so we shouldn't be calling the > underlying read several times. You'll see what I mean if you consider the > scenario where you read < rem, then read again and the second read blocks or > throws. I'l also concerned about "workBuffer" adding more per-stream > footprint for cases where skip or read(CB) is used. Objects such as > InputStreamReader are already a problem due to the underlying stream decoder. I think that's what @AlanBateman intended. The `skip()` changes would revert also (I think) but the C-style array changes can stay. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/1915
Re: RFR: 4926314: Optimize Reader.read(CharBuffer) [v7]
On Wed, 17 Feb 2021 15:37:11 GMT, Alan Bateman wrote: >> Philippe Marschall has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - Replace c-style array declarations >> - Share work buffer between #skip and #read > > src/java.base/share/classes/java/io/Reader.java line 221: > >> 219: // if the last call to read returned -1 or the >> number of bytes >> 220: // requested have been read then break >> 221: } while (n >= 0 && remaining > 0); > > The code for case that the char buffer has a backing array looks okay but I'm > not sure about the direct buffer/other cases. One concern is that this is a > read method, not a transferXXX method so we shouldn't be calling the > underlying read several times. You'll see what I mean if you consider the > scenario where you read < rem, then read again and the second read blocks or > throws. I'l also concerned about "workBuffer" adding more per-stream > footprint for cases where skip or read(CB) is used. Objects such as > InputStreamReader are already a problem due to the underlying stream decoder. Right. So you propose to revert the off-heap path to the current master? That would be fine with me. The original bug and my motivation was only about the backing array case, the rest crept in. That would certainly keep the risk and impact lower. - PR: https://git.openjdk.java.net/jdk/pull/1915
Re: RFR: 8148937: (str) Adapt StringJoiner for Compact Strings
On Thu, 18 Feb 2021 15:19:43 GMT, Claes Redestad wrote:
>> Hello,
>>
>> as of now `java.util.StringJoiner` still uses `char[]` as a storage for
>> joined Strings.
>>
>> This applies for the cases when all joined Strings as well as delimiter,
>> prefix and suffix contain only ASCII symbols.
>>
>> As a result when `StringJoiner.toString()` is called `byte[]` stored in
>> Strings is inflated in order to fill in `char[]` and after that `char[]` is
>> compressed when constructor of String is called:
>> String delimiter = this.delimiter;
>> char[] chars = new char[this.len + addLen];
>> int k = getChars(this.prefix, chars, 0);
>> if (size > 0) {
>> k += getChars(elts[0], chars, k);// inflate byte[]
>>
>> for(int i = 1; i < size; ++i) {
>> k += getChars(delimiter, chars, k);
>> k += getChars(elts[i], chars, k);
>> }
>> }
>>
>> k += getChars(this.suffix, chars, k);
>> return new String(chars);// compress char[] -> byte[]
>> This can be improved by utilizing new method `String.getBytes(byte[], int,
>> int, byte, int)` [introduced](https://github.com/openjdk/jdk/pull/402) in
>> [JDK-8224986](https://bugs.openjdk.java.net/browse/JDK-8254082)
>> covering both cases when resulting String is Latin1 or UTF-16
>>
>> I've prepared a patch along with benchmark proving that this change is
>> correct and brings improvement.
>>
>> @BenchmarkMode(Mode.AverageTime)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
>> public class StringJoinerBenchmark {
>>
>> @Benchmark
>> public String stringJoiner(Data data) {
>> String[] stringArray = data.stringArray;
>> return Joiner.joinWithStringJoiner(stringArray);
>> }
>>
>> @State(Scope.Thread)
>> public static class Data {
>>
>> @Param({"latin", "cyrillic", "mixed"})
>> private String mode;
>>
>> @Param({"8", "32", "64"})
>> private int length;
>>
>> @Param({"5", "10", "100"})
>> private int count;
>>
>> private String[] stringArray;
>>
>> @Setup
>> public void setup() {
>> RandomStringGenerator generator = new RandomStringGenerator();
>>
>> stringArray = new String[count];
>>
>> for (int i = 0; i < count; i++) {
>> String alphabet = getAlphabet(i, mode);
>> stringArray[i] = generator.randomString(alphabet, length);
>> }
>> }
>>
>> private static String getAlphabet(int index, String mode) {
>> var latin = "abcdefghijklmnopqrstuvwxyz"; //English
>> var cyrillic = "абвгдеёжзиклмнопрстуфхцчшщьыъэюя"; // Russian
>>
>> String alphabet;
>> switch (mode) {
>> case "mixed" -> alphabet = index % 2 == 0 ? cyrillic : latin;
>> case "latin" -> alphabet = latin;
>> case "cyrillic" -> alphabet = cyrillic;
>> default -> throw new RuntimeException("Illegal mode " + mode);
>> }
>> return alphabet;
>> }
>> }
>> }
>>
>>
>> (count) (length)(mode)
>> Java 14 patched Units
>> stringJoiner 5 8 latin 78.836
>> ± 0.20867.546 ± 0.500 ns/op
>> stringJoiner 532 latin 92.877
>> ± 0.42266.760 ± 0.498 ns/op
>> stringJoiner 564 latin 115.423
>> ± 0.88373.224 ± 0.289 ns/op
>> stringJoiner 10 8 latin 152.587
>> ± 0.429 161.427 ± 0.635 ns/op
>> stringJoiner 1032 latin 189.998
>> ± 0.478 164.099 ± 0.963 ns/op
>> stringJoiner 1064 latin 238.679
>> ± 1.419 176.825 ± 0.533 ns/op
>> stringJoiner100 8 latin 1215.612
>> ± 17.413 1541.802 ± 126.166 ns/op
>> stringJoiner10032 latin 1699.998
>> ± 28.407 1563.341 ± 4.439 ns/op
>> stringJoiner10064 latin 2289.388
>> ± 45.319 2215.931 ± 137.583 ns/op
>> stringJoiner 5 8 cyrillic 96.692
>> ± 0.94780.946 ± 0.371 ns/op
>> stringJoiner 532 cyrillic 107.806
>> ± 0.42984.717 ± 0.541 ns/op
>> stringJoiner 564 cyrillic 150.762
>> ± 2.26796.214 ± 1.251 ns/op
>> stringJoiner 10 8 cyrillic 190.570
>> ± 0.381 182.754 ± 0.678 ns/op
>> stringJoiner 1032 cyrillic 240.239
>> ± 1.110 187.991 ± 1.575 ns/op
>> stringJoiner 1064 cyrillic 382.493
>> ± 2.692 219.358
Re: RFR: 8261744: Implement CharsetDecoder ASCII and latin-1 fast-paths [v2]
On Wed, 17 Feb 2021 17:22:21 GMT, Alan Bateman wrote: >>> Right, I'm not exactly sure why the more limited changes I attempted in >>> [5f4e87f](https://github.com/openjdk/jdk/commit/5f4e87f50f49e64b8616063c176ea35632b0347e) >>> failed. In that change I simply changed the initialization order, which >>> made the failing (closed) tests pass locally - but not in our CI. Since >>> this is in initPhase1 there should be no concurrency possible. >> >> The Reference Handler thread is started by the initializer in >> jl.ref.Reference so could be a candidate. The Finalizer thread is another >> but this should VM.awaitInitLevel(1) and not touch JLA until initPhase1 is >> done. > >> but you're probably right and it would be good to make the name more >> explicit when exporting it outside of the package internal use. How about >> `inflateBytesToChars`? > > That should be okay. > > > Is there a reason > > > `sun.nio.cs.ISO_8859_1.Encoder#implEncodeISOArray(char[], int, byte[], > > > int, int)` wasn't moved to `JavaLangAccess` as well? > > > > > > Exposing StringUTF16.compress for Latin-1 and ASCII-compatible encoders > > seem very reasonable, which I was thinking of exploring next as a separate > > RFE. > > Maybe I misunderstood. The intrinsified method you point out here pre-dates > the work in JDK 9 to similarly intrinsify char[]->byte[] compaction in > StringUTF16, see https://bugs.openjdk.java.net/browse/JDK-6896617 > > It might be worthwhile cleaning this up. Not having to route via > SharedSecrets -> JavaLangAccess does speed things up during > startup/interpretation, at the cost of some code duplication. My understanding was ISO_8859_1$Encoder.implEncodeISOArray and StringUTF16.compress are ultimately hooked up to the same intrinsic. I find it inconsistent that ISO_8859_1$Encoder access an encoding intrinsitc directly while ISO_8859_1$Decoder and others access a decoding intrinsic indirectly through JavaLangAccess. I realize this RFE is about decoding so keeping encoding to a different RFE may indeed be better. - PR: https://git.openjdk.java.net/jdk/pull/2574
Re: RFR: 8261977: Fix comment for getPrefixed() in canonicalize_md.c
On Thu, 18 Feb 2021 19:35:09 GMT, Alexey Semenyuk wrote: > Follow up for JDK-8235397 fix Thanks for this, it means we can move this code to the right place in the future. - Marked as reviewed by alanb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2631
Re: RFR: 8261483: jdk/dynalink/TypeConverterFactoryMemoryLeakTest.java failed with "AssertionError: Should have GCd a method handle by now"
On Wed, 17 Feb 2021 19:44:35 GMT, Attila Szegedi wrote: > 8261483: jdk/dynalink/TypeConverterFactoryMemoryLeakTest.java failed with > "AssertionError: Should have GCd a method handle by now" @plevart can I bother you for a follow-up review of my original issue? (Alternatively, @shipilev?) Unfortunately the tests as I wrote them are flaking on somewhat loaded CI servers. Shame on me for using timing-based tests… Thanks for the consideration. I changed the flaky tests to use an upper limit on number of iterations instead of a maximum duration, so they won't be time sensitive anymore. I'm also running them with a separate VM using a very small max heap (4M) so the GC condition triggers quickly. Finally, I measured the number of iterations they need to succeed with this heap size; the numbers are deterministic across executions, but I also added a generous padding (least iterations needed by a test is 30, most needed is 219, I'm allowing the tests to run until 5000.) - PR: https://git.openjdk.java.net/jdk/pull/2617
RFR: 8261977: Fix comment for getPrefixed() in canonicalize_md.c
Follow up for JDK-8235397 fix - Commit messages: - Fix comment for getPrefixed() in canonicalize_md.c Changes: https://git.openjdk.java.net/jdk/pull/2631/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2631=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8261977 Stats: 5 lines in 1 file changed: 0 ins; 4 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2631.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2631/head:pull/2631 PR: https://git.openjdk.java.net/jdk/pull/2631
Re: RFR: 8261483: jdk/dynalink/TypeConverterFactoryMemoryLeakTest.java failed with "AssertionError: Should have GCd a method handle by now" [v2]
> 8261483: jdk/dynalink/TypeConverterFactoryMemoryLeakTest.java failed with > "AssertionError: Should have GCd a method handle by now" Attila Szegedi has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8261483: Try to eliminate flakiness of the test by extending its allowed runtime and reducing the memory - Changes: - all: https://git.openjdk.java.net/jdk/pull/2617/files - new: https://git.openjdk.java.net/jdk/pull/2617/files/989dfb64..3afec32b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2617=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2617=00-01 Stats: 20 lines in 2 files changed: 5 ins; 8 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/2617.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2617/head:pull/2617 PR: https://git.openjdk.java.net/jdk/pull/2617
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v11]
On Mon, 15 Feb 2021 19:47:00 GMT, Andrey Turbanov
wrote:
>> 8080272 Refactor I/O stream copying to use
>> InputStream.transferTo/readAllBytes and Files.copy
>
> Andrey Turbanov has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8080272: Refactor I/O stream copying to use java.io.InputStream.transferTo
> remove unnecessary file.exists() check
src/java.base/share/classes/sun/security/provider/certpath/X509CertPath.java
line 228:
> 226: try {
> 227: if (is.markSupported() == false) {
> 228: // Copy the entire input stream into an InputStream that
> does
I don't think you should remove lines 228-232. These methods are called by
methods of CertificateFactory that take InputStream (which may contain a stream
of security data) and they are designed such that they try to read one
Certificate, CRL, or CertPath from the InputStream and leave the InputStream
ready to parse the next structure instead of consuming all of the bytes. Thus
they check if the InputStream supports mark in order to try to preserve that
behavior. If mark is not supported, then it's ok to use
InputStream.readAllBytes, otherwise, leave the stream as-is.
-
PR: https://git.openjdk.java.net/jdk/pull/1853
Re: RFR: 6245663: (spec) File.renameTo(File) changes the file-system object, not the File instance [v4]
On Thu, 18 Feb 2021 17:34:04 GMT, Brian Burkhalter wrote: >> Please review this minor specification update to highlight that >> `File.renameTo(File)` does not modify the `File` instance on which the >> method is invoked. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 6245663: Simplify new verbiage Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2618
Re: RFR: 6245663: (spec) File.renameTo(File) changes the file-system object, not the File instance [v4]
On Thu, 18 Feb 2021 17:34:04 GMT, Brian Burkhalter wrote: >> Please review this minor specification update to highlight that >> `File.renameTo(File)` does not modify the `File` instance on which the >> method is invoked. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 6245663: Simplify new verbiage Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2618
Re: RFR: 8261940: Fix references to IOException in BigDecimal javadoc [v2]
> Noticed by some of Jon Gibbons's doc linting & checking tooling, this > changeset fixes two javadoc issues for BigDecimal's serialization-related > methods, improving the serial form page. Joe Darcy has updated the pull request incrementally with one additional commit since the last revision: Respond to review feedback. - Changes: - all: https://git.openjdk.java.net/jdk/pull/2623/files - new: https://git.openjdk.java.net/jdk/pull/2623/files/27ff1d78..e3a1659f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2623=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2623=00-01 Stats: 5 lines in 1 file changed: 1 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/2623.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2623/head:pull/2623 PR: https://git.openjdk.java.net/jdk/pull/2623
Integrated: 8261940: Fix references to IOException in BigDecimal javadoc
On Thu, 18 Feb 2021 06:03:41 GMT, Joe Darcy wrote: > Noticed by some of Jon Gibbons's doc linting & checking tooling, this > changeset fixes two javadoc issues for BigDecimal's serialization-related > methods, improving the serial form page. This pull request has now been integrated. Changeset: c4664e64 Author:Joe Darcy URL: https://git.openjdk.java.net/jdk/commit/c4664e64 Stats: 3 lines in 1 file changed: 1 ins; 0 del; 2 mod 8261940: Fix references to IOException in BigDecimal javadoc Reviewed-by: alanb, chegar, iris, bpb - PR: https://git.openjdk.java.net/jdk/pull/2623
Re: RFR: 8261940: Fix references to IOException in BigDecimal javadoc [v2]
On Thu, 18 Feb 2021 16:52:19 GMT, Brian Burkhalter wrote: >> Joe Darcy has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Respond to review feedback. > > I concur with @AlanBateman about importing `IOException`. Will integrate using the suggested import of java.io.IOException; thanks. - PR: https://git.openjdk.java.net/jdk/pull/2623
Re: RFR: 6245663: (spec) File.renameTo(File) changes the file-system object, not the File instance [v2]
On Thu, 18 Feb 2021 16:59:54 GMT, Alan Bateman wrote:
>> Like so?
>> --- a/src/java.base/share/classes/java/io/File.java
>> +++ b/src/java.base/share/classes/java/io/File.java
>> @@ -1376,7 +1376,9 @@ public class File
>> * file from one filesystem to another, it might not be atomic, and it
>> * might not succeed if a file with the destination abstract pathname
>> * already exists. The return value should always be checked to make
>> sure
>> - * that the rename operation was successful.
>> + * that the rename operation was successful. As instances of {@code
>> File}
>> + * are immutable, this File object is not changed to name the
>> destination
>> + * file or directory.
>> *
>> * Note that the {@link java.nio.file.Files} class defines the
>> {@link
>> * java.nio.file.Files#move move} method to move or rename a file in a
>
> yes, I think that works.
[CSR](https://bugs.openjdk.java.net/browse/JDK-8261935) also so updated.
-
PR: https://git.openjdk.java.net/jdk/pull/2618
Re: RFR: 6245663: (spec) File.renameTo(File) changes the file-system object, not the File instance [v4]
> Please review this minor specification update to highlight that > `File.renameTo(File)` does not modify the `File` instance on which the method > is invoked. Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 6245663: Simplify new verbiage - Changes: - all: https://git.openjdk.java.net/jdk/pull/2618/files - new: https://git.openjdk.java.net/jdk/pull/2618/files/e7ff82d2..e6a23ab4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2618=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2618=02-03 Stats: 4 lines in 1 file changed: 0 ins; 2 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2618.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2618/head:pull/2618 PR: https://git.openjdk.java.net/jdk/pull/2618
Re: System.getEnv(String name, String def)
Hi,
Thanks for your replies.
My point here was that using env variables is more and more common to
configure apps, and they have less convenient support than system
properties (the other way to provide easy external configuration at launch
time with key/value).
I agree that Objects.requireNonNullElse(System.getEnv(String key), "n/a"));
is an easy way to achieve this, but I still think System.getEnv(key, "n/a")
should be a good addition.
Regards,
Loïc
Le mar. 16 févr. 2021 à 21:59, Remi Forax a écrit :
> - Mail original -
> > De: "Michael Kuhlmann"
> > À: "core-libs-dev"
> > Envoyé: Mardi 16 Février 2021 13:34:30
> > Objet: Re: System.getEnv(String name, String def)
>
> > Hi Rémi,
> >
> > I don't want to be pedantic, but I see this as an anti-pattern. You
> > would create an Optional just to immediately call orElse() on it. That's
> > not how Optional should be used. (But you know that.)
> >
> > It's described in Recipe 12 of this Java Magazine article, for instance:
> >
> https://blogs.oracle.com/javamagazine/12-recipes-for-using-the-optional-class-as-its-meant-to-be-used
>
> yep, you are right.
> Optional.ofNullable(...).orElse(...) is not the best pattern in term of
> readability.
>
> >
> > Best,
> > Michael
>
> regards,
> Rémi
>
> >
> > On 2/15/21 3:09 PM, Remi Forax wrote:
> >> Hi Loic,
> >> You can use Optional.OfNullable() which is a kind of the general bridge
> between
> >> the nullable world and the non-nullable one.
> >>
> >>var fooOptional = Optional.ofNullable(System.getenv("FOO"));
> >>var fooValue = fooOptional.orElse(defaultValue);
> >>
> >> regards,
> >> Rémi Forax
> >>
> >> - Mail original -
> >>> De: "Loïc MATHIEU"
> >>> À: "core-libs-dev"
> >>> Envoyé: Lundi 15 Février 2021 14:59:42
> >>> Objet: System.getEnv(String name, String def)
> >>
> >>> Hello,
> >>>
> >>> I wonder if there has already been some discussion to provide
> >>> a System.getEnv(String name, String def) method that allows to return a
> >>> default value in case the env variable didn't exist.
> >>>
> >>> When using system properties instead of env variable, we do have a
> >>> System.getProperty(String key, String def) variant.
> >>>
> >>> Stating the JavaDoc of System.getEnv():
> >>> *System properties and environment variables are both conceptually
> mappings
> >>> between names and values*
> >>>
> >>> So if system properties and environment variables are similar concepts,
> >>> they should provide the same functionalities right ?
> >>>
> >>> This would be very convenient as more and more people rely on
> >>> environment variables these days to configure their applications.
> >>>
> >>> Regards,
> >>>
> > >> Loïc
>
Re: RFR: 6245663: (spec) File.renameTo(File) changes the file-system object, not the File instance [v2]
On Thu, 18 Feb 2021 16:43:09 GMT, Brian Burkhalter wrote:
>> It might be clearer if the end of the sentence were changed to something
>> like "... this File object is not changed to name destination file or
>> directory".
>
> Like so?
> --- a/src/java.base/share/classes/java/io/File.java
> +++ b/src/java.base/share/classes/java/io/File.java
> @@ -1376,7 +1376,9 @@ public class File
> * file from one filesystem to another, it might not be atomic, and it
> * might not succeed if a file with the destination abstract pathname
> * already exists. The return value should always be checked to make
> sure
> - * that the rename operation was successful.
> + * that the rename operation was successful. As instances of {@code
> File}
> + * are immutable, this File object is not changed to name the destination
> + * file or directory.
> *
> * Note that the {@link java.nio.file.Files} class defines the {@link
> * java.nio.file.Files#move move} method to move or rename a file in a
yes, I think that works.
-
PR: https://git.openjdk.java.net/jdk/pull/2618
Re: RFR: 8261940: Fix references to IOException in BigDecimal javadoc
On Thu, 18 Feb 2021 06:03:41 GMT, Joe Darcy wrote: > Noticed by some of Jon Gibbons's doc linting & checking tooling, this > changeset fixes two javadoc issues for BigDecimal's serialization-related > methods, improving the serial form page. I concur with @AlanBateman about importing `IOException`. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2623
Re: RFR: 6245663: (spec) File.renameTo(File) changes the file-system object, not the File instance [v2]
On Thu, 18 Feb 2021 09:24:06 GMT, Alan Bateman wrote:
>> No, I intended `filesystem` but in the sense of "object in the filesystem"
>> but it does seem awkward.
>
> It might be clearer if the end of the sentence were changed to something like
> "... this File object is not changed to name destination file or directory".
Like so?
--- a/src/java.base/share/classes/java/io/File.java
+++ b/src/java.base/share/classes/java/io/File.java
@@ -1376,7 +1376,9 @@ public class File
* file from one filesystem to another, it might not be atomic, and it
* might not succeed if a file with the destination abstract pathname
* already exists. The return value should always be checked to make sure
- * that the rename operation was successful.
+ * that the rename operation was successful. As instances of {@code File}
+ * are immutable, this File object is not changed to name the destination
+ * file or directory.
*
* Note that the {@link java.nio.file.Files} class defines the {@link
* java.nio.file.Files#move move} method to move or rename a file in a
-
PR: https://git.openjdk.java.net/jdk/pull/2618
Re: RFR: 8261940: Fix references to IOException in BigDecimal javadoc
On Thu, 18 Feb 2021 06:03:41 GMT, Joe Darcy wrote: > Noticed by some of Jon Gibbons's doc linting & checking tooling, this > changeset fixes two javadoc issues for BigDecimal's serialization-related > methods, improving the serial form page. Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2623
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v20]
> This PR is to introduce a new random number API for the JDK. The primary API > is found in RandomGenerator and RandomGeneratorFactory. Further description > can be found in the JEP https://openjdk.java.net/jeps/356 . > > javadoc can be found at > http://cr.openjdk.java.net/~jlaskey/prng/doc/api/java.base/java/util/random/package-summary.html > > old PR: https://github.com/openjdk/jdk/pull/1273 Jim Laskey has updated the pull request incrementally with one additional commit since the last revision: Correct copyright notice. - Changes: - all: https://git.openjdk.java.net/jdk/pull/1292/files - new: https://git.openjdk.java.net/jdk/pull/1292/files/7d0ebfc9..9861b4e4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1292=19 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1292=18-19 Stats: 28 lines in 1 file changed: 6 ins; 0 del; 22 mod Patch: https://git.openjdk.java.net/jdk/pull/1292.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1292/head:pull/1292 PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v19]
> This PR is to introduce a new random number API for the JDK. The primary API > is found in RandomGenerator and RandomGeneratorFactory. Further description > can be found in the JEP https://openjdk.java.net/jeps/356 . > > javadoc can be found at > http://cr.openjdk.java.net/~jlaskey/prng/doc/api/java.base/java/util/random/package-summary.html > > old PR: https://github.com/openjdk/jdk/pull/1273 Jim Laskey has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 50 commits: - Merge branch 'master' into 8248862 - Update tests for RandomGeneratorFactory.all() - Merge branch 'master' into 8248862 - Flatten out use of all() - Add review edits - Added table of available algorithms. - Merge branch 'master' into 8248862 - Update SplittableRandom to remove unnecessary overrides - Updated API based on CSR review. - Update package info to credit LMX algorithm - ... and 40 more: https://git.openjdk.java.net/jdk/compare/f94a8452...7d0ebfc9 - Changes: https://git.openjdk.java.net/jdk/pull/1292/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1292=18 Stats: 13341 lines in 26 files changed: 11195 ins; 2055 del; 91 mod Patch: https://git.openjdk.java.net/jdk/pull/1292.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1292/head:pull/1292 PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: 8148937: (str) Adapt StringJoiner for Compact Strings
On Thu, 18 Feb 2021 14:30:15 GMT, Сергей Цыпанов
wrote:
> Hello,
>
> as of now `java.util.StringJoiner` still uses `char[]` as a storage for
> joined Strings.
>
> This applies for the cases when all joined Strings as well as delimiter,
> prefix and suffix contain only ASCII symbols.
>
> As a result when `StringJoiner.toString()` is called `byte[]` stored in
> Strings is inflated in order to fill in `char[]` and after that `char[]` is
> compressed when constructor of String is called:
> String delimiter = this.delimiter;
> char[] chars = new char[this.len + addLen];
> int k = getChars(this.prefix, chars, 0);
> if (size > 0) {
> k += getChars(elts[0], chars, k);// inflate byte[]
>
> for(int i = 1; i < size; ++i) {
> k += getChars(delimiter, chars, k);
> k += getChars(elts[i], chars, k);
> }
> }
>
> k += getChars(this.suffix, chars, k);
> return new String(chars);// compress char[] -> byte[]
> This can be improved by utilizing new method `String.getBytes(byte[], int,
> int, byte, int)` [introduced](https://github.com/openjdk/jdk/pull/402) in
> [JDK-8224986](https://bugs.openjdk.java.net/browse/JDK-8254082)
> covering both cases when resulting String is Latin1 or UTF-16
>
> I've prepared a patch along with benchmark proving that this change is
> correct and brings improvement.
>
> @BenchmarkMode(Mode.AverageTime)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
> public class StringJoinerBenchmark {
>
> @Benchmark
> public String stringJoiner(Data data) {
> String[] stringArray = data.stringArray;
> return Joiner.joinWithStringJoiner(stringArray);
> }
>
> @State(Scope.Thread)
> public static class Data {
>
> @Param({"latin", "cyrillic", "mixed"})
> private String mode;
>
> @Param({"8", "32", "64"})
> private int length;
>
> @Param({"5", "10", "100"})
> private int count;
>
> private String[] stringArray;
>
> @Setup
> public void setup() {
> RandomStringGenerator generator = new RandomStringGenerator();
>
> stringArray = new String[count];
>
> for (int i = 0; i < count; i++) {
> String alphabet = getAlphabet(i, mode);
> stringArray[i] = generator.randomString(alphabet, length);
> }
> }
>
> private static String getAlphabet(int index, String mode) {
> var latin = "abcdefghijklmnopqrstuvwxyz"; //English
> var cyrillic = "абвгдеёжзиклмнопрстуфхцчшщьыъэюя"; // Russian
>
> String alphabet;
> switch (mode) {
> case "mixed" -> alphabet = index % 2 == 0 ? cyrillic : latin;
> case "latin" -> alphabet = latin;
> case "cyrillic" -> alphabet = cyrillic;
> default -> throw new RuntimeException("Illegal mode " + mode);
> }
> return alphabet;
> }
> }
> }
>
>
> (count) (length)(mode)
> Java 14 patched Units
> stringJoiner 5 8 latin 78.836
> ± 0.20867.546 ± 0.500 ns/op
> stringJoiner 532 latin 92.877
> ± 0.42266.760 ± 0.498 ns/op
> stringJoiner 564 latin 115.423
> ± 0.88373.224 ± 0.289 ns/op
> stringJoiner 10 8 latin 152.587
> ± 0.429 161.427 ± 0.635 ns/op
> stringJoiner 1032 latin 189.998
> ± 0.478 164.099 ± 0.963 ns/op
> stringJoiner 1064 latin 238.679
> ± 1.419 176.825 ± 0.533 ns/op
> stringJoiner100 8 latin 1215.612
> ± 17.413 1541.802 ± 126.166 ns/op
> stringJoiner10032 latin 1699.998
> ± 28.407 1563.341 ± 4.439 ns/op
> stringJoiner10064 latin 2289.388
> ± 45.319 2215.931 ± 137.583 ns/op
> stringJoiner 5 8 cyrillic 96.692
> ± 0.94780.946 ± 0.371 ns/op
> stringJoiner 532 cyrillic 107.806
> ± 0.42984.717 ± 0.541 ns/op
> stringJoiner 564 cyrillic 150.762
> ± 2.26796.214 ± 1.251 ns/op
> stringJoiner 10 8 cyrillic 190.570
> ± 0.381 182.754 ± 0.678 ns/op
> stringJoiner 1032 cyrillic 240.239
> ± 1.110 187.991 ± 1.575 ns/op
> stringJoiner 1064 cyrillic 382.493
> ± 2.692 219.358 ± 0.967 ns/op
> stringJoiner100 8 cyrillic 1737.435
> ± 16.171
Re: RFR: 8261728: SimpleDateFormat should link to DateTimeFormatter [v2]
On Wed, 17 Feb 2021 20:21:57 GMT, Naoto Sato wrote: >> Please review this simple doc fix. A CSR will be filed accordingly. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Made the additional text an @apiNote Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2616
Re: RFR: 8261938: ASN1Formatter.annotate should not return in the finally block [v2]
On Thu, 18 Feb 2021 02:46:53 GMT, Jie Fu wrote: >> Hi all, >> >> ASN1Formatter.annotate should be able to throw an IOException according to >> this comment [1]. >> But it fails to do so due to the return [2] in the finally block, which >> would swallow the IOException. >> >> Generally, it seems not good to put a return statement in a finally block >> because it would overwrite other return-statements or Exceptions [3]. >> >> Thanks. >> Best regards, >> Jie >> >> [1] >> https://github.com/openjdk/jdk/blob/master/test/lib/jdk/test/lib/hexdump/ASN1Formatter.java#L113 >> [2] >> https://github.com/openjdk/jdk/blob/master/test/lib/jdk/test/lib/hexdump/ASN1Formatter.java#L120 >> [3] >> https://stackoverflow.com/questions/17481251/finally-block-does-not-complete-normally-eclipse-warning > > Jie Fu has updated the pull request incrementally with one additional commit > since the last revision: > > Update the copyright year An EOFException can occur during the call to annotate() and must return the accumulated contents of the StringBuffer. Otherwise it is discarded. - Changes requested by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2620
Re: RFR: 8261621: Delegate Unicode history from JLS to j.l.Character [v2]
On Thu, 18 Feb 2021 03:56:49 GMT, David Holmes wrote: >> The table is informative and should not be construed as specification. >> The wording "has supported" should be sufficient. > >> >> >> The table is informative and should not be construed as specification. >> The wording "has supported" should be sufficient. > > If this is not specification then doesn't that imply that any provider of any > version of OpenJDK would be free to support, or not, whatever version of > Unicode that they chose? Surely a minimum supported version must be part of > the platform specification? The current version of Unicode is specified in a normative statement just before the table. "Character information is based on the Unicode Standard, version 13.0." The table is not a specification of past revisions. - PR: https://git.openjdk.java.net/jdk/pull/2538
RFR: 8148937: (str) Adapt StringJoiner for Compact Strings
Hello,
as of now `java.util.StringJoiner` still uses `char[]` as a storage for joined
Strings.
This applies for the cases when all joined Strings as well as delimiter, prefix
and suffix contain only ASCII symbols.
As a result when `StringJoiner.toString()` is called `byte[]` stored in Strings
is inflated in order to fill in `char[]` and after that `char[]` is compressed
when constructor of String is called:
String delimiter = this.delimiter;
char[] chars = new char[this.len + addLen];
int k = getChars(this.prefix, chars, 0);
if (size > 0) {
k += getChars(elts[0], chars, k);// inflate byte[]
for(int i = 1; i < size; ++i) {
k += getChars(delimiter, chars, k);
k += getChars(elts[i], chars, k);
}
}
k += getChars(this.suffix, chars, k);
return new String(chars);// compress char[] -> byte[]
This can be improved by utilizing new method `String.getBytes(byte[], int, int,
byte, int)` [introduced](https://github.com/openjdk/jdk/pull/402) in
[JDK-8224986](https://bugs.openjdk.java.net/browse/JDK-8254082)
covering both cases when resulting String is Latin1 or UTF-16
I've prepared a patch along with benchmark proving that this change is correct
and brings improvement.
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
public class StringJoinerBenchmark {
@Benchmark
public String stringJoiner(Data data) {
String[] stringArray = data.stringArray;
return Joiner.joinWithStringJoiner(stringArray);
}
@State(Scope.Thread)
public static class Data {
@Param({"latin", "cyrillic", "mixed"})
private String mode;
@Param({"8", "32", "64"})
private int length;
@Param({"5", "10", "100"})
private int count;
private String[] stringArray;
@Setup
public void setup() {
RandomStringGenerator generator = new RandomStringGenerator();
stringArray = new String[count];
for (int i = 0; i < count; i++) {
String alphabet = getAlphabet(i, mode);
stringArray[i] = generator.randomString(alphabet, length);
}
}
private static String getAlphabet(int index, String mode) {
var latin = "abcdefghijklmnopqrstuvwxyz"; //English
var cyrillic = "абвгдеёжзиклмнопрстуфхцчшщьыъэюя"; // Russian
String alphabet;
switch (mode) {
case "mixed" -> alphabet = index % 2 == 0 ? cyrillic : latin;
case "latin" -> alphabet = latin;
case "cyrillic" -> alphabet = cyrillic;
default -> throw new RuntimeException("Illegal mode " + mode);
}
return alphabet;
}
}
}
(count) (length)(mode)
Java 14 patched Units
stringJoiner 5 8 latin 78.836 ±
0.20867.546 ± 0.500 ns/op
stringJoiner 532 latin 92.877 ±
0.42266.760 ± 0.498 ns/op
stringJoiner 564 latin 115.423 ±
0.88373.224 ± 0.289 ns/op
stringJoiner 10 8 latin 152.587 ±
0.429 161.427 ± 0.635 ns/op
stringJoiner 1032 latin 189.998 ±
0.478 164.099 ± 0.963 ns/op
stringJoiner 1064 latin 238.679 ±
1.419 176.825 ± 0.533 ns/op
stringJoiner100 8 latin 1215.612 ±
17.413 1541.802 ± 126.166 ns/op
stringJoiner10032 latin 1699.998 ±
28.407 1563.341 ± 4.439 ns/op
stringJoiner10064 latin 2289.388 ±
45.319 2215.931 ± 137.583 ns/op
stringJoiner 5 8 cyrillic 96.692 ±
0.94780.946 ± 0.371 ns/op
stringJoiner 532 cyrillic 107.806 ±
0.42984.717 ± 0.541 ns/op
stringJoiner 564 cyrillic 150.762 ±
2.26796.214 ± 1.251 ns/op
stringJoiner 10 8 cyrillic 190.570 ±
0.381 182.754 ± 0.678 ns/op
stringJoiner 1032 cyrillic 240.239 ±
1.110 187.991 ± 1.575 ns/op
stringJoiner 1064 cyrillic 382.493 ±
2.692 219.358 ± 0.967 ns/op
stringJoiner100 8 cyrillic 1737.435 ±
16.171 1658.379 ± 56.225 ns/op
stringJoiner10032 cyrillic 2321.106 ±
13.583 1942.028 ± 3.418 ns/op
stringJoiner10064 cyrillic 3189.563 ±
10.135 2484.796 ± 7.572 ns/op
stringJoiner
Re: RFR: 8261880: Change nested classes in java.base to static nested classes where possible [v2]
On Wed, 17 Feb 2021 16:38:03 GMT, Сергей Цыпанов wrote: >> Non-static classes hold a link to their parent classes, which in many cases >> can be avoided. > > Сергей Цыпанов has updated the pull request incrementally with one additional > commit since the last revision: > > 8261880: Remove static from declarations of Holder nested classes The changes in java/net look ok to me. - PR: https://git.openjdk.java.net/jdk/pull/2589
Re: RFR: 8252883: AccessDeniedException caused by delayed file deletion on Windows [v5]
On Wed, 17 Feb 2021 14:47:03 GMT, Evan Whelan wrote: >> Hi, >> >> Please review this fix for JDK-8252883. This handles the case when an >> AccessDeniedException is being thrown on Windows, due to a delay in deleting >> the lock file it is trying to write to. >> >> This fix passes all testing. >> >> Kind regards, >> Evan > > Evan Whelan has updated the pull request incrementally with one additional > commit since the last revision: > > 8252883: Added IllegalArgumentException for incorrect usage LGTM - Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2572
Re: RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy
On Thu, 18 Feb 2021 07:12:34 GMT, Andrey Turbanov wrote: >> Hi @turbanoff, I'm happy to sponsor but I see two comments by @marschall - >> have they been addressed? >> https://github.com/openjdk/jdk/pull/1853#discussion_r572815422 >> https://github.com/openjdk/jdk/pull/1853#discussion_r572380746 > > @FrauBoes fixed in the last commit. Is there any way to de-_integrate_ PR (to > include last commit)? To re-integrate, just run the /integrate command again. Before doing that, could someone from security libs acknowledge the changes in their area? - PR: https://git.openjdk.java.net/jdk/pull/1853
Re: RFR: 8261940: Fix references to IOException in BigDecimal javadoc
On Thu, 18 Feb 2021 06:03:41 GMT, Joe Darcy wrote: > Noticed by some of Jon Gibbons's doc linting & checking tooling, this > changeset fixes two javadoc issues for BigDecimal's serialization-related > methods, improving the serial form page. Marked as reviewed by chegar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2623
Re: RFR: 6245663: (spec) File.renameTo(File) changes the file-system object, not the File instance [v2]
On Wed, 17 Feb 2021 22:28:11 GMT, Brian Burkhalter wrote:
>> src/java.base/share/classes/java/io/File.java line 1381:
>>
>>> 1379: * that the rename operation was successful. As instances of
>>> {@code File}
>>> 1380: * are immutable, the abstract pathname represented by this
>>> {@code File}
>>> 1381: * object does not itself change although the filesystem object
>>> it denoted
>>
>> I guess you meant `file` object here, instead of `filesystem`?
>
> No, I intended `filesystem` but in the sense of "object in the filesystem"
> but it does seem awkward.
It might be clearer if the end of the sentence were changed to something like
"... this File object is not changed to name destination file or directory".
-
PR: https://git.openjdk.java.net/jdk/pull/2618
Re: RFR: 8261940: Fix references to IOException in BigDecimal javadoc
On Thu, 18 Feb 2021 06:03:41 GMT, Joe Darcy wrote: > Noticed by some of Jon Gibbons's doc linting & checking tooling, this > changeset fixes two javadoc issues for BigDecimal's serialization-related > methods, improving the serial form page. Looks okay but it might be simpler to just import java.io.IOException and avoid having two usages of the fully qualified name in the source file. - Marked as reviewed by alanb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2623
