RFR: 8276084: Linux DEB Bundler: release number in outputted .deb file should be optional
8276084: Linux DEB Bundler: release number in outputted .deb file should be optional - Commit messages: - Bugfix - Bugfix - Bugfix - 8276084: Linux DEB Bundler: release number in outputted .deb file should be optional Changes: https://git.openjdk.java.net/jdk/pull/6345/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6345&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276084 Stats: 65 lines in 7 files changed: 52 ins; 1 del; 12 mod Patch: https://git.openjdk.java.net/jdk/pull/6345.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6345/head:pull/6345 PR: https://git.openjdk.java.net/jdk/pull/6345
Re: RFR: 8276123: ZipFile::getEntry will not return a file entry when there is a directory entry of the same name within a Zip File
On Wed, 10 Nov 2021 20:51:20 GMT, Lance Andersen wrote:
> Hi all,
>
> This patch addresses a regression introduced in JDK 15 via JDK-8242959 where
> you can no longer access a file entry contained within a Zip file when there
> is also a directory entry with the same name via ZipFile:getEntry().
>
> Once fixed, the behavior will be consistent with earlier JDK releases.
>
> Mach5 tiers 1-3 have been run without failure
>
> Best
> Lance
src/java.base/share/classes/java/util/zip/ZipFile.java line 1642:
> 1640: && entry.startsWith(name) &&
> 1641: entry.charAt(entryLen - 1) == '/') {
> 1642: // Now check for a match with a trailing
> slash
Hello Lance,
Is this a typo in that comment? Should it instead say "... without a trailing
slash"?
-
PR: https://git.openjdk.java.net/jdk/pull/6342
Integrated: 8276536: Update TimeZoneNames files to follow the changes made by JDK-8275766
On Wed, 3 Nov 2021 07:05:08 GMT, Yoshiki Sato wrote: > Please review this PR to update the TimeZoneNames files. @naotoj @coffeys > > Some short name changes are not integrated to the JDK. It was detected by the > change made in JDK-8275766. > - Africa/Juba change was done by 2017c > - Africa/Windhoek changes were done by 2018e > - Antarctica/Macquarie change was done by 2017a This pull request has now been integrated. Changeset: ad3be04d Author:Yoshiki Sato Committer: Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/ad3be04d2ac84836e393d696ff03ddfe72779094 Stats: 88 lines in 11 files changed: 22 ins; 0 del; 66 mod 8276536: Update TimeZoneNames files to follow the changes made by JDK-8275766 Reviewed-by: naoto, coffeys - PR: https://git.openjdk.java.net/jdk/pull/6226
Re: RFR: 8274544: Langtools command's usage were garbled on Japanese Windows [v5]
On Wed, 10 Nov 2021 19:46:09 GMT, Alan Bateman wrote: > If there is resistance to fixing the tools then we might have to re-visit > this. It's not just the JDK tools that are an issue. I think that wrapping some PrintStream into a PrintWriter is a common enough idiom that it is not reasonable to fix all occurrences -- i.e. including those outside of JDK. - PR: https://git.openjdk.java.net/jdk/pull/5771
Re: RFR: 8274544: Langtools command's usage were garbled on Japanese Windows [v5]
On Wed, 10 Nov 2021 20:00:39 GMT, Alan Bateman wrote: > > 1. `PrintStream(OutputStream)` and `PrintStream(OutputStream, boolean)` > > should be redefined so that they internally check if the stream arg is a > > PrintStream, in which case they use the encoding from the `PrinStream` > > instead of the default. > > I think you mean the PrintWriter constructors here. Yes, there is merit in > that. PrintStream is a bit unusual in that it's an OutputStream but it has a > charset because it prints text output. Yes, sorry for the confusion. - PR: https://git.openjdk.java.net/jdk/pull/5771
Re: RFR: 8276123: ZipFile::getEntry will not return a file entry when there is a directory entry of the same name within a Zip File
On Wed, 10 Nov 2021 21:32:32 GMT, Claes Redestad wrote: >> Hi all, >> >> This patch addresses a regression introduced in JDK 15 via JDK-8242959 where >> you can no longer access a file entry contained within a Zip file when there >> is also a directory entry with the same name via ZipFile:getEntry(). >> >> Once fixed, the behavior will be consistent with earlier JDK releases. >> >> Mach5 tiers 1-3 have been run without failure >> >> Best >> Lance > > test/jdk/java/util/zip/ZipFile/ZipFileDuplicateEntryTest.java line 515: > >> 513: } >> 514: >> 515: /** > > Left-over? Thank you for the quick review Claes! I was going to remove this method but chose to leave it in to make it a bit more straight forward to port to JDK 8 (which I have run the test on as well). No strong preferences, just figured leaving it there might make it slightly easier for a back port. - PR: https://git.openjdk.java.net/jdk/pull/6342
Re: RFR: 8276123: ZipFile::getEntry will not return a file entry when there is a directory entry of the same name within a Zip File
On Wed, 10 Nov 2021 20:51:20 GMT, Lance Andersen wrote: > Hi all, > > This patch addresses a regression introduced in JDK 15 via JDK-8242959 where > you can no longer access a file entry contained within a Zip file when there > is also a directory entry with the same name via ZipFile:getEntry(). > > Once fixed, the behavior will be consistent with earlier JDK releases. > > Mach5 tiers 1-3 have been run without failure > > Best > Lance LGTM! test/jdk/java/util/zip/ZipFile/ZipFileDuplicateEntryTest.java line 515: > 513: } > 514: > 515: /** Left-over? - Marked as reviewed by redestad (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6342
Re: RFR: 8274544: Langtools command's usage were garbled on Japanese Windows [v5]
On Mon, 1 Nov 2021 16:10:26 GMT, Ichiroh Takiguchi wrote: >> JEP-400 (UTF-8 by Default) was eabled on JDK18-b13. >> After JDK18-b13, javac and some other langtool command's usage were garbled >> on Japanese Windows. >> These commands use PrintWriter instead of standard out/err with PrintStream. > > Ichiroh Takiguchi has updated the pull request with a new target base due to > a merge or a rebase. The pull request now contains five commits: > > - 8274544: Langtools command's usage were garbled on Japanese Windows > - 8274544: Langtools command's usage were garbled on Japanese Windows > - 8274544: Langtools command's usage were garbled on Japanese Windows > - 8274544: Langtools command's usage were garbled on Japanese Windows > - Langtools command's usage were grabled on Japanese Windows Good suggestions. Filed a JBS issue: https://bugs.openjdk.java.net/browse/JDK-8276970 - PR: https://git.openjdk.java.net/jdk/pull/5771
Re: RFR: 8276536: Update TimeZoneNames files to follow the changes made by JDK-8275766
On Wed, 3 Nov 2021 07:05:08 GMT, Yoshiki Sato wrote: > Please review this PR to update the TimeZoneNames files. @naotoj @coffeys > > Some short name changes are not integrated to the JDK. It was detected by the > change made in JDK-8275766. > - Africa/Juba change was done by 2017c > - Africa/Windhoek changes were done by 2018e > - Antarctica/Macquarie change was done by 2017a Marked as reviewed by coffeys (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6226
RFR: 8276123: ZipFile::getEntry will not return a file entry when there is a directory entry of the same name within a Zip File
Hi all, This patch addresses a regression introduced in JDK 15 via JDK-8242959 where you can no longer access a file entry contained within a Zip file when there is also a directory entry with the same name via ZipFile:getEntry(). Once fixed, the behavior will be consistent with earlier JDK releases. Mach5 tiers 1-3 have been run without failure Best Lance - Commit messages: - Address javadoc typo - Fix for JDK-8276123 Changes: https://git.openjdk.java.net/jdk/pull/6342/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6342&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276123 Stats: 598 lines in 2 files changed: 593 ins; 3 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/6342.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6342/head:pull/6342 PR: https://git.openjdk.java.net/jdk/pull/6342
Integrated: 8276775: ZonedDateTime/OffsetDateTime.toString return invalid ISO-8601 for years <= 1893
On Tue, 9 Nov 2021 22:29:12 GMT, Naoto Sato wrote: > Simple doc clarification where the `toString()` output only conforms to ISO > 8601 if the seconds in the offset are zero. This pull request has now been integrated. Changeset: bce35ac1 Author:Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/bce35ac1d6c4115148468a3240ad459074e0b79e Stats: 7 lines in 2 files changed: 2 ins; 0 del; 5 mod 8276775: ZonedDateTime/OffsetDateTime.toString return invalid ISO-8601 for years <= 1893 Reviewed-by: lancea, iris, bpb, scolebourne, rriggs - PR: https://git.openjdk.java.net/jdk/pull/6321
Integrated: 8276186: Require getAvailableLocales() methods to include Locale.ROOT
On Thu, 4 Nov 2021 16:07:01 GMT, Naoto Sato wrote: > This fix is to require to include `Locale.ROOT` in the returned arrays/set > from `getAvailableLocales()` methods in various locale-sensitive classes. The > implementation has been including `Locale.ROOT` since its inception, it is > simply a doc clarification (+ a test case verifying it). Corresponding CSR > has also been drafted: https://bugs.openjdk.java.net/browse/JDK-8276249 This pull request has now been integrated. Changeset: 0c409cac Author:Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/0c409cac789f1b1d21e09a65db36bb6c72c569db Stats: 108 lines in 10 files changed: 86 ins; 0 del; 22 mod 8276186: Require getAvailableLocales() methods to include Locale.ROOT Reviewed-by: prappo, smarks, iris - PR: https://git.openjdk.java.net/jdk/pull/6258
Re: RFR: 8274544: Langtools command's usage were garbled on Japanese Windows [v5]
On Wed, 10 Nov 2021 19:41:29 GMT, Jonathan Gibbons wrote: > 1. `PrintStream(OutputStream)` and `PrintStream(OutputStream, boolean)` > should be redefined so that they internally check if the stream arg is a > PrintStream, in which case they use the encoding from the `PrinStream` > instead of the default. I think you mean the PrintWriter constructors here. Yes, there is merit in that. PrintStream is a bit unusual in that it's an OutputStream but it has a charset because it prints text output. - PR: https://git.openjdk.java.net/jdk/pull/5771
Re: RFR: 8276186: Require getAvailableLocales() methods to include Locale.ROOT [v2]
On Wed, 10 Nov 2021 19:05:17 GMT, Naoto Sato wrote: >> This fix is to require to include `Locale.ROOT` in the returned arrays/set >> from `getAvailableLocales()` methods in various locale-sensitive classes. >> The implementation has been including `Locale.ROOT` since its inception, it >> is simply a doc clarification (+ a test case verifying it). Corresponding >> CSR has also been drafted: https://bugs.openjdk.java.net/browse/JDK-8276249 > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Using @code tag for `Set`. Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6258
Re: RFR: 8274544: Langtools command's usage were garbled on Japanese Windows [v5]
On Wed, 10 Nov 2021 19:16:58 GMT, Jonathan Gibbons wrote: > Generally, the issue is related to the fact that we wrap a PrintStream in a > PrintWriter ... and, if I understand correctly, the writer ends up with the > wrong character encoding. Is it possible to change PrintWriter and/or > PrintStream so that the correct underlying encoding used by the PrintStream > is also used by the PrintWriter. That way, all existing uses where a > PrintWriter wraps a PrintStream would continue to work without any > modification. JEP 400 has moved the JDK to using UTF-8 as the default charset, a long overdue change. So if you create a PrintStream or a PrintWriter without specifying the charset then it will default to UTF-8. The issue that I think we have with javac and a few other tools is that they are creating PrintWriters on PrintStreams where the underlying streams are stdout/stderr and so using the native encoding. There was exploration into special casing this scenario during JEP 400 that was rejected because it complicates the spec and may not be feasible in cases where there are many layers in the onion. If there is resistance to fixing the tools then we might have to re-visit this. Naoto may have more to say on this. - PR: https://git.openjdk.java.net/jdk/pull/5771
Re: RFR: 8274544: Langtools command's usage were garbled on Japanese Windows [v5]
On Mon, 1 Nov 2021 16:10:26 GMT, Ichiroh Takiguchi wrote: >> JEP-400 (UTF-8 by Default) was eabled on JDK18-b13. >> After JDK18-b13, javac and some other langtool command's usage were garbled >> on Japanese Windows. >> These commands use PrintWriter instead of standard out/err with PrintStream. > > Ichiroh Takiguchi has updated the pull request with a new target base due to > a merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains five additional > commits since the last revision: > > - 8274544: Langtools command's usage were garbled on Japanese Windows > - 8274544: Langtools command's usage were garbled on Japanese Windows > - 8274544: Langtools command's usage were garbled on Japanese Windows > - 8274544: Langtools command's usage were garbled on Japanese Windows > - Langtools command's usage were grabled on Japanese Windows Informally, I suggest one of the following: 1. `PrintStream(OutputStream)` and `PrintStream(OutputStream, boolean)` should be redefined so that they internally check if the stream arg is a PrintStream, in which case they use the encoding from the `PrinStream` instead of the default. 2. or, add new overloads for `PrintStream(PrintStream)` and `PrintStream(PrintStream, boolean)` that are defined to use the character encoding from the `PrintStream` arg. I note that `PrintStream` does not expose a "getter" for the encoding. That seems like a bug in itself, but even without fixing that, `PrintWriter` ought to be able to access the encoding "behind the scenes". - PR: https://git.openjdk.java.net/jdk/pull/5771
Re: RFR: 8274544: Langtools command's usage were garbled on Japanese Windows [v5]
On Mon, 1 Nov 2021 16:10:26 GMT, Ichiroh Takiguchi wrote: >> JEP-400 (UTF-8 by Default) was eabled on JDK18-b13. >> After JDK18-b13, javac and some other langtool command's usage were garbled >> on Japanese Windows. >> These commands use PrintWriter instead of standard out/err with PrintStream. > > Ichiroh Takiguchi has updated the pull request with a new target base due to > a merge or a rebase. The pull request now contains five commits: > > - 8274544: Langtools command's usage were garbled on Japanese Windows > - 8274544: Langtools command's usage were garbled on Japanese Windows > - 8274544: Langtools command's usage were garbled on Japanese Windows > - 8274544: Langtools command's usage were garbled on Japanese Windows > - Langtools command's usage were grabled on Japanese Windows I strongly dislike this proposed changeset. In my opinion, the original change that has provoked the changes here is a bad/incompatible change, and should maybe be reconsidered. The fact that a change in the Java runtime has triggered the need for so many changes in application-style code is some sort of "canary in the coalmine". Generally, the issue is related to the fact that we wrap a PrintStream in a PrintWriter ... and, if I understand correctly, the writer ends up with the wrong character encoding. Is it possible to change PrintWriter and/or PrintStream so that the correct underlying encoding used by the PrintStream is also used by the PrintWriter. That way, all existing uses where a PrintWriter wraps a PrintStream would continue to work without any modification. cc: @jddarcy with his CSR hat on, for the compatibility issues relating to the issue that caused the problems being addressed here. - Changes requested by jjg (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5771
Re: OutputStreamWriter (not) flushing stateful Charsetencoder
Here are the old details I can dig up when we ran into this on JavaMail:
https://bugs.openjdk.java.net/browse/JDK-6995537
https://github.com/javaee/javamail/commit/145d18c1738d3cf33b52bc005835980ff78ce4af
I recall digging through the code years ago and I don't recall if adding
w.write("");
Will trigger the encoder flush. Not that is a great workaround either.
Jason
From: core-libs-dev on behalf of Bernd
Eckenfels
Sent: Wednesday, November 10, 2021 8:12 AM
To: core-libs-dev
Subject: OutputStreamWriter (not) flushing stateful Charsetencoder
(I thought this was discussed a while back on a OpenJDK mailing list, but I
can’t find it. So apologies if this is a duplicate, but I might have seen it on
Apache Commons-io instead - which fixed a similar issue on reader side)
The problem: I have code using a OutputStreamWriter with a customer defined
charset name. this writer is flushed, and the code expects all pending bytes to
be written. However when a stateful charset like cp930 is used, this is not the
case. The final unshift byte for example is only written when the writer is
closed. This is probably because it does not call end of data encode on the
encoder in the flush().
The class does not clearly say or not say what is the correct behavior, however
the flush() is formulated in a way that one could expect it should produce a
complete stream.
So, is this a Bug in the implementation, if not should it be added to the
documentation?
Here is a small JShell reproducer, you see the extra unshift byte (dec 15) only
after the close:
var b = new byte[] { 0x31, (byte)0xef, (byte)0xbc, (byte)0x91 };
var s = new String(b, "UTF-8"); // „1２“ (1 is ascii, 2 is fw)
var bos = new ByteArrayOutputStream();
var w = new OutputStreamWriter(bos, "cp930"); // stateful ebcdic with Shift
chars
w.write(s);
w.flush();
bos.toByteArray()
$8 ==> byte[4] { -15, 14, 66, -15 }
w.close();
bos.toByteArray()
$10 ==> byte[5] { -15, 14, 66, -15, 15 }
--
http://bernd.eckenfels.net
Re: RFR: 8276186: Require getAvailableLocales() methods to include Locale.ROOT [v2]
On Wed, 10 Nov 2021 19:05:17 GMT, Naoto Sato wrote: >> This fix is to require to include `Locale.ROOT` in the returned arrays/set >> from `getAvailableLocales()` methods in various locale-sensitive classes. >> The implementation has been including `Locale.ROOT` since its inception, it >> is simply a doc clarification (+ a test case verifying it). Corresponding >> CSR has also been drafted: https://bugs.openjdk.java.net/browse/JDK-8276249 > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Using @code tag for `Set`. Nice use of MethodHandles.lookup() in the test. - Marked as reviewed by smarks (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6258
Re: RFR: 8276186: Require getAvailableLocales() methods to include Locale.ROOT [v2]
On Wed, 10 Nov 2021 18:29:06 GMT, Pavel Rappo wrote:
>> Naoto Sato has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Using @code tag for `Set`.
>
> src/java.base/share/classes/java/time/format/DecimalStyle.java line 118:
>
>> 116: * Lists all the locales that are supported.
>> 117: *
>> 118: * At a minimum, the returned Set must contain a {@code Locale}
>> instance equal to
>
> A nit, really. Consider applying either of these suggestions:
> Suggestion:
>
> * At a minimum, the returned {@code Set} must contain a {@code Locale}
> instance equal to
>
> Suggestion:
>
> * At a minimum, the returned set must contain a {@code Locale} instance
> equal to
Thanks. Adopted the former suggestion.
-
PR: https://git.openjdk.java.net/jdk/pull/6258
Re: RFR: 8276186: Require getAvailableLocales() methods to include Locale.ROOT [v2]
> This fix is to require to include `Locale.ROOT` in the returned arrays/set > from `getAvailableLocales()` methods in various locale-sensitive classes. The > implementation has been including `Locale.ROOT` since its inception, it is > simply a doc clarification (+ a test case verifying it). Corresponding CSR > has also been drafted: https://bugs.openjdk.java.net/browse/JDK-8276249 Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Using @code tag for `Set`. - Changes: - all: https://git.openjdk.java.net/jdk/pull/6258/files - new: https://git.openjdk.java.net/jdk/pull/6258/files/1a611bd5..5bdd4917 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6258&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6258&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/6258.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6258/head:pull/6258 PR: https://git.openjdk.java.net/jdk/pull/6258
Re: RFR: 8276186: Require getAvailableLocales() methods to include Locale.ROOT
On Thu, 4 Nov 2021 16:07:01 GMT, Naoto Sato wrote:
> This fix is to require to include `Locale.ROOT` in the returned arrays/set
> from `getAvailableLocales()` methods in various locale-sensitive classes. The
> implementation has been including `Locale.ROOT` since its inception, it is
> simply a doc clarification (+ a test case verifying it). Corresponding CSR
> has also been drafted: https://bugs.openjdk.java.net/browse/JDK-8276249
The change to the existing source looks good, and the new test looks beautiful.
Thanks for doing this.
src/java.base/share/classes/java/time/format/DecimalStyle.java line 118:
> 116: * Lists all the locales that are supported.
> 117: *
> 118: * At a minimum, the returned Set must contain a {@code Locale}
> instance equal to
A nit, really. Consider applying either of these suggestions:
Suggestion:
* At a minimum, the returned {@code Set} must contain a {@code Locale}
instance equal to
Suggestion:
* At a minimum, the returned set must contain a {@code Locale} instance
equal to
-
Marked as reviewed by prappo (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/6258
Re: RFR: 8275535: Retrying a failed authentication on multiple LDAP servers can lead to users blocked
On Wed, 20 Oct 2021 13:35:22 GMT, Martin Balao wrote: > I'd like to propose a fix for JDK-8275535. This fix reverts the behavior to > the state previous to JDK-8160768, where an authentication failure stops from > trying other LDAP servers with the same credentials [1]. After JDK-8160768 we > have 2 possible loops to stop: the one that iterates over different URLs and > the one that iterates over different endpoints (after a DNS query that > returns multiple values). > > No test regressions observed in jdk/com/sun/jndi/ldap. > > -- > [1] - https://hg.openjdk.java.net/jdk/jdk/rev/a609d549992a#l2.137 Update: * CSR: https://bugs.openjdk.java.net/browse/JDK-8276959 - PR: https://git.openjdk.java.net/jdk/pull/6043
Re: RFR: 8276536: Update TimeZoneNames files to follow the changes made by JDK-8275766
On Wed, 3 Nov 2021 07:05:08 GMT, Yoshiki Sato wrote: > Please review this PR to update the TimeZoneNames files. @naotoj @coffeys > > Some short name changes are not integrated to the JDK. It was detected by the > change made in JDK-8275766. > - Africa/Juba change was done by 2017c > - Africa/Windhoek changes were done by 2018e > - Antarctica/Macquarie change was done by 2017a Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6226
RFR: 8275728: Add simple Producer/Consumer microbenchmark for Thread.onSpinWait
This is a microbenchmarks to demonstrate `Thread.onSpinWait` can be used to avoid heavy locks. The microbenchmark differs from [Gil's original benchmark](https://github.com/giltene/GilExamples/tree/master/SpinWaitTest) and [Dmitry's variations](http://cr.openjdk.java.net/~dchuyko/8186670/yield/spinwait.html). Those benchmarks produce/consume data by incrementing a volatile counter. The latency of such operations is almost zero. They also don't use heavy locks. According to [Gil's SpinWaitTest.java](https://github.com/giltene/GilExamples/blob/master/SpinWaitTest/src/main/java/SpinWaitTest.java): > This test can be used to measure and document the impact of > Runtime.onSpinWait() behavior > on thread-to-thread communication latencies. E.g. when the two threads are > pinned to > the two hardware threads of a shared x86 core (with a shared L1), this test > will > demonstrate an estimate the best case thread-to-thread latencies possible on > the > platform Gil's microbenchmark targets SMT cases (x86 hyperthreading). As not all CPUs support SMT, the microbenchmarks cannot demonstrate benefits of `Thread.onSpinWait`. It is actually opposite. They show `Thread.onSpinWait` has negative impact on performance. The microbenchmark from PR uses `BigInteger` to have 100 - 200 ns latencies for producing/consuming data. These latencies can cause either a producer or a consumer to wait each another. Waiting is implemented with `Object.wait`/`Object.notify` which are heavy. `Thread.onSpinWait` can be used in a spin loop to avoid them. **ARM64 results**: - No spin loop Benchmark (maxNum) (spinNum) Mode Cnt ScoreError Units ThreadOnSpinWaitProducerConsumer.trial 100 0 avgt 75 1520.448 ± 40.507 us/op - No `Thread.onSpinWait` intrinsic Benchmark (maxNum) (spinNum) Mode Cnt ScoreError Units ThreadOnSpinWaitProducerConsumer.trial 100125 avgt 75 1580.756 ± 47.501 us/op - `ISB`-based `Thread.onSpinWait` intrinsic Benchmark (maxNum) (spinNum) Mode CntScore Error Units ThreadOnSpinWaitProducerConsumer.trial 100125 avgt 75 617.454 ± 174.431 us/op **X86_64 results**: - No spin loop Benchmark (maxNum) (spinNum) Mode CntScore Error Units ThreadOnSpinWaitProducerConsumer.trial 100125 avgt 75 1417.944 ± 1.691 us/op - No `Thread.onSpinWait` intrinsic Benchmark (maxNum) (spinNum) Mode CntScore Error Units ThreadOnSpinWaitProducerConsumer.trial 100125 avgt 75 1410.987 ± 2.093 us/op - `PAUSE`-based `Thread.onSpinWait` intrinsic Benchmark (maxNum) (spinNum) Mode CntScore Error Units ThreadOnSpinWaitProducerConsumer.trial 100125 avgt 75 217.054 ± 1.283 us/op - Commit messages: - 8275728: Add simple Producer/Consumer microbenchmark for Thread.onSpinWait Changes: https://git.openjdk.java.net/jdk/pull/6338/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6338&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275728 Stats: 204 lines in 1 file changed: 204 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6338.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6338/head:pull/6338 PR: https://git.openjdk.java.net/jdk/pull/6338
Re: RFR: 8276904: Optional.toString() is unnecessarily expensive
I would suggest that we hold this patch until the string interpolation JEP lands. It will offer both better readability *and* better performance, and probably better to have one round of "replace all the toString implementations" than two? On 11/10/2021 1:04 PM, Eamonn McManus wrote: Use string concatenation instead of `String.format`. - Commit messages: - 8276904: Optional.toString() is unnecessarily expensive Changes:https://git.openjdk.java.net/jdk/pull/6337/files Webrev:https://webrevs.openjdk.java.net/?repo=jdk&pr=6337&range=00 Issue:https://bugs.openjdk.java.net/browse/JDK-8276904 Stats: 4 lines in 4 files changed: 0 ins; 0 del; 4 mod Patch:https://git.openjdk.java.net/jdk/pull/6337.diff Fetch: git fetchhttps://git.openjdk.java.net/jdk pull/6337/head:pull/6337 PR:https://git.openjdk.java.net/jdk/pull/6337
RFR: 8276904: Optional.toString() is unnecessarily expensive
Use string concatenation instead of `String.format`. - Commit messages: - 8276904: Optional.toString() is unnecessarily expensive Changes: https://git.openjdk.java.net/jdk/pull/6337/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6337&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276904 Stats: 4 lines in 4 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/6337.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6337/head:pull/6337 PR: https://git.openjdk.java.net/jdk/pull/6337
Re: RFR: 8276947: Clarify how DateTimeFormatterBuilder.appendFraction handles value ranges
On Wed, 10 Nov 2021 13:57:21 GMT, Claes Redestad wrote: > This changes the specification of `DateTimeFormatterBuilder.appendFraction` > to more clearly specify that the formatter will throw an exception if you > attempt to print a value outside of the value range of the given field. > Changes suggested by @jodastephen in #6188. Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6335
Re: RFR: 8276141: XPathFactory set/getProperty method [v6]
On Wed, 10 Nov 2021 11:26:11 GMT, Alan Bateman wrote: >> Yes. Made a clarification to the javadoc, and while we are here, to >> setFeature method as well. > > One other suggestion for wording is to change: > "A property set on the factory is effective to the XPath object created > thereafter" > to something like: > "The property applies to XPath objects that the XPathFactory creates. It has > no impact on XPath objects that are already created." > > I don't have any other comments on the API. The wording is adjusted accordingly. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/6215
Re: RFR: 8276141: XPathFactory set/getProperty method [v9]
> Add setProperty/getProperty methods to the XPath API so that properties can > be supported in the future. Joe Wang has updated the pull request incrementally with one additional commit since the last revision: adjust wording - Changes: - all: https://git.openjdk.java.net/jdk/pull/6215/files - new: https://git.openjdk.java.net/jdk/pull/6215/files/56ede26e..e1157ea6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6215&range=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6215&range=07-08 Stats: 7 lines in 1 file changed: 1 ins; 0 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/6215.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6215/head:pull/6215 PR: https://git.openjdk.java.net/jdk/pull/6215
Re: RFR: 8275007: Java fails to start with null charset if LC_ALL is set to certain locales
On Wed, 10 Nov 2021 03:31:11 GMT, Ichiroh Takiguchi
wrote:
>>> There may be an argument that the JDK should emit a warning at startup.
>>
>> Updated to emit a warning, if `sun.jnu.encoding` is not supported.
>
> Hello @naotoj .
> I appreciate your code update.
> I'd like to confirm one thing.
> If you could not recreate this issue on your test machine, please ignore this
> message.
>
> I tested your code on my CentOS7 x86_64.
> I saw following exception.
>
> $ git log --oneline | head
> e91e9d85327 8276721: G1: Refine G1EvacFailureObjectsSet::iterate
> 8822d41fdcc 8274736: Concurrent read/close of SSLSockets causes SSLSessions
> to be invalidated unnecessarily
> c1e41fe38bb 8276842: G1: Only calculate size in bytes from words when needed
> c8b0ee6b8a0 8276833: G1: Make G1EvacFailureRegions::par_iterate const
> 0699220830a 8268882: C2: assert(n->outcnt() != 0 || C->top() == n ||
> n->is_Proj()) failed: No dead instructions after post-alloc
> d7012fbd604 8276880: Remove java/lang/RuntimeTests/exec/ExecWithDir as
> unnecessary
> f9024d0606e 8230130: javadoc search result dialog shows cut off headers for
> long results
> 055de6f5662 8223358: Incorrect HTML structure in annotation pages
> a60e91259ba 8198626:
> java/awt/KeyboardFocusmanager/TypeAhead/TestDialogTypeAhead.html fails on mac
> dde959dfcef 8276808: java/nio/channels/Channels/TransferTo.java timed out
> $ git status -s
> M src/java.base/share/classes/java/lang/System.java
> M src/java.base/share/classes/sun/nio/fs/Util.java
> M src/java.base/share/native/libjava/jni_util.c
> ?? 8275007.patch
> ?? Hello.java
> $ cat Hello.java
> public class Hello {
> public static void main(String[] args) throws Exception {
> System.out.println("Hello");
> }
> }
> $ env LC_ALL=kk_KZ.pt154 locale
> LANG=ja_JP.UTF-8
> LC_CTYPE="kk_KZ.pt154"
> LC_NUMERIC="kk_KZ.pt154"
> LC_TIME="kk_KZ.pt154"
> LC_COLLATE="kk_KZ.pt154"
> LC_MONETARY="kk_KZ.pt154"
> LC_MESSAGES="kk_KZ.pt154"
> LC_PAPER="kk_KZ.pt154"
> LC_NAME="kk_KZ.pt154"
> LC_ADDRESS="kk_KZ.pt154"
> LC_TELEPHONE="kk_KZ.pt154"
> LC_MEASUREMENT="kk_KZ.pt154"
> LC_IDENTIFICATION="kk_KZ.pt154"
> LC_ALL=kk_KZ.pt154
> $ env LC_ALL=kk_KZ.pt154
> ./build/linux-x86_64-server-release/images/jdk/bin/java Hello.java
> WARNING: The encoding of the underlying platform's file system is not
> supported by the JVM: PT154
> Error: A JNI error has occurred, please check your installation and try again
> Exception in thread "main" java.util.ServiceConfigurationError:
> java.nio.charset.spi.CharsetProvider: Unable to load
> sun.nio.cs.ext.ExtendedCharsets
> at java.base/java.util.ServiceLoader.fail(ServiceLoader.java:586)
> at
> java.base/java.util.ServiceLoader.loadProvider(ServiceLoader.java:861)
> at
> java.base/java.util.ServiceLoader$ModuleServicesLookupIterator.hasNext(ServiceLoader.java:1084)
> at java.base/java.util.ServiceLoader$2.hasNext(ServiceLoader.java:1309)
> at java.base/java.util.ServiceLoader$3.hasNext(ServiceLoader.java:1393)
> at
> java.base/java.nio.charset.Charset$ExtendedProviderHolder$1.run(Charset.java:428)
> at
> java.base/java.nio.charset.Charset$ExtendedProviderHolder$1.run(Charset.java:422)
> at
> java.base/java.security.AccessController.doPrivileged(AccessController.java:318)
> at
> java.base/java.nio.charset.Charset$ExtendedProviderHolder.extendedProviders(Charset.java:422)
> at
> java.base/java.nio.charset.Charset$ExtendedProviderHolder.(Charset.java:418)
> at
> java.base/java.nio.charset.Charset.lookupExtendedCharset(Charset.java:442)
> at java.base/java.nio.charset.Charset.lookup2(Charset.java:472)
> at java.base/java.nio.charset.Charset.lookup(Charset.java:460)
> at java.base/java.nio.charset.Charset.isSupported(Charset.java:501)
> at
> java.base/sun.launcher.LauncherHelper.makePlatformString(LauncherHelper.java:891)
> Caused by: java.lang.ExceptionInInitializerError
> at java.base/sun.nio.fs.UnixFileSystem.(UnixFileSystem.java:51)
> at java.base/sun.nio.fs.LinuxFileSystem.(LinuxFileSystem.java:39)
> at
> java.base/sun.nio.fs.LinuxFileSystemProvider.newFileSystem(LinuxFileSystemProvider.java:46)
> at
> java.base/sun.nio.fs.LinuxFileSystemProvider.newFileSystem(LinuxFileSystemProvider.java:39)
> at
> java.base/sun.nio.fs.UnixFileSystemProvider.(UnixFileSystemProvider.java:55)
> at
> java.base/sun.nio.fs.LinuxFileSystemProvider.(LinuxFileSystemProvider.java:41)
> at
> java.base/sun.nio.fs.DefaultFileSystemProvider.(DefaultFileSystemProvider.java:35)
> at
> java.base/java.nio.file.FileSystems$DefaultFileSystemHolder.getDefaultProvider(FileSystems.java:114)
> at
> java.base/java.nio.file.FileSystems$DefaultFileSystemHolder$1.run(FileSystems.java:103)
> at
> java.base/java.nio.file.FileSystems$DefaultFileSystemHolder$1.run(FileSystems.java:101)
> at
> java.base/java.security.AccessController.doPrivileged(AccessCon
Re: RFR: 8275535: Retrying a failed authentication on multiple LDAP servers can lead to users blocked
On Wed, 10 Nov 2021 12:58:13 GMT, Aleksei Efimov wrote: >> I'd like to propose a fix for JDK-8275535. This fix reverts the behavior to >> the state previous to JDK-8160768, where an authentication failure stops >> from trying other LDAP servers with the same credentials [1]. After >> JDK-8160768 we have 2 possible loops to stop: the one that iterates over >> different URLs and the one that iterates over different endpoints (after a >> DNS query that returns multiple values). >> >> No test regressions observed in jdk/com/sun/jndi/ldap. >> >> -- >> [1] - https://hg.openjdk.java.net/jdk/jdk/rev/a609d549992a#l2.137 > > Hi Martin, > > The change looks reasonable to me. > I would suggest having a CSR logged for this change due to the following > [behavioral > incompatibility](https://wiki.openjdk.java.net/display/csr/Kinds+of+Compatibility): > Before the change - all available endpoints/URLs are tried to create an LDAP > context. > With the proposed change - incorrect credentials will prevent other endpoints > to be exercised to create an LDAP context. > > Having a CSR will also help to document difference in handling > `AuthenticationException` and `NamingException` during construction of an > LDAP context from the list of endpoints acquired from a LDAP DNS provider. Hi @AlekseiEfimov , Thanks for your feedback. I'll open a CSR as suggested and wait for approval before integrating this fix. With that said, I could not find information in the CSR associated to JDK-8160768 (JDK-8192975) about this behavioral change. My intention here is to restore the previous JDK behavior; and not to introduce a new behavior or revert a previously-approved one. Martin.- - PR: https://git.openjdk.java.net/jdk/pull/6043
Re: RFR: 8276947: Clarify how DateTimeFormatterBuilder.appendFraction handles value ranges
On Wed, 10 Nov 2021 13:57:21 GMT, Claes Redestad wrote: > This changes the specification of `DateTimeFormatterBuilder.appendFraction` > to more clearly specify that the formatter will throw an exception if you > attempt to print a value outside of the value range of the given field. > Changes suggested by @jodastephen in #6188. Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6335
Re: RFR: 8276775: ZonedDateTime/OffsetDateTime.toString return invalid ISO-8601 for years <= 1893
On Tue, 9 Nov 2021 22:29:12 GMT, Naoto Sato wrote: > Simple doc clarification where the `toString()` output only conforms to ISO > 8601 if the seconds in the offset are zero. Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6321
Re: RFR: 8276926: Use String.valueOf() when initializing File.separator and File.pathSeparator
On Wed, 10 Nov 2021 09:22:32 GMT, Сергей Цыпанов wrote: > Looking into `File.pathSeparator` I've found out that currently we initialize > it as > > public static final String separator = "" + separatorChar; > > Which after compilation turns into > > NEW java/lang/StringBuilder > DUP > INVOKESPECIAL java/lang/StringBuilder. ()V > LDC "" > INVOKEVIRTUAL java/lang/StringBuilder.append > (Ljava/lang/String;)Ljava/lang/StringBuilder; > GETSTATIC java/io/File.pathSeparatorChar : C > INVOKEVIRTUAL java/lang/StringBuilder.append (C)Ljava/lang/StringBuilder; > INVOKEVIRTUAL java/lang/StringBuilder.toString ()Ljava/lang/String; > PUTSTATIC java/io/File.pathSeparator : Ljava/lang/String; > > This can be simplified to > > public static final String separator = String.valueOf(separatorChar); > > Which is in turn complied into more compact > > GETSTATIC java/io/File.pathSeparatorChar : C > INVOKESTATIC java/lang/String.valueOf (C)Ljava/lang/String; > PUTSTATIC java/io/File.pathSeparator : Ljava/lang/String; > > The changed code is likely to slightly improve start-up time as it allocates > less and does no bound checks. I've filed https://bugs.openjdk.java.net/browse/JDK-8276951 This pattern _is_ quite common - both in the OpenJDK and elsewhere. - PR: https://git.openjdk.java.net/jdk/pull/6326
OutputStreamWriter (not) flushing stateful Charsetencoder
(I thought this was discussed a while back on a OpenJDK mailing list, but I
can’t find it. So apologies if this is a duplicate, but I might have seen it on
Apache Commons-io instead - which fixed a similar issue on reader side)
The problem: I have code using a OutputStreamWriter with a customer defined
charset name. this writer is flushed, and the code expects all pending bytes to
be written. However when a stateful charset like cp930 is used, this is not the
case. The final unshift byte for example is only written when the writer is
closed. This is probably because it does not call end of data encode on the
encoder in the flush().
The class does not clearly say or not say what is the correct behavior, however
the flush() is formulated in a way that one could expect it should produce a
complete stream.
So, is this a Bug in the implementation, if not should it be added to the
documentation?
Here is a small JShell reproducer, you see the extra unshift byte (dec 15) only
after the close:
var b = new byte[] { 0x31, (byte)0xef, (byte)0xbc, (byte)0x91 };
var s = new String(b, "UTF-8"); // „1２“ (1 is ascii, 2 is fw)
var bos = new ByteArrayOutputStream();
var w = new OutputStreamWriter(bos, "cp930"); // stateful ebcdic with Shift
chars
w.write(s);
w.flush();
bos.toByteArray()
$8 ==> byte[4] { -15, 14, 66, -15 }
w.close();
bos.toByteArray()
$10 ==> byte[5] { -15, 14, 66, -15, 15 }
--
http://bernd.eckenfels.net
RFR: 8276947: Clarify how DateTimeFormatterBuilder.appendFraction handles value ranges
This changes the specification of `DateTimeFormatterBuilder.appendFraction` to more clearly specify that the formatter will throw an exception if you attempt to print a value outside of the value range of the given field. Changes suggested by @jodastephen in #6188. - Commit messages: - Clarify how DateTimeFormatterBuilder.appendFraction handles value ranges Changes: https://git.openjdk.java.net/jdk/pull/6335/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6335&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276947 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/6335.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6335/head:pull/6335 PR: https://git.openjdk.java.net/jdk/pull/6335
Re: RFR: 8193682: Infinite loop in ZipOutputStream.close() [v8]
> Hi all, > > Please review this fix for Infinite loop in ZipOutputStream.close(). > The main issue here is when ever there is an exception during close > operations on GZip we are not setting the deflator to a finished state which > is leading to an infinite loop when we try writing on the same GZip instance( > since we use while(!def.finished()) inside the write operation). > > Thanks, > Ravi Ravi Reddy has updated the pull request incrementally with one additional commit since the last revision: 8193682 : Infinite loop in ZipOutputStream.close() - Changes: - all: https://git.openjdk.java.net/jdk/pull/5522/files - new: https://git.openjdk.java.net/jdk/pull/5522/files/bd0ccd7c..b3d7fb74 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5522&range=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5522&range=06-07 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5522.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5522/head:pull/5522 PR: https://git.openjdk.java.net/jdk/pull/5522
Re: RFR: 8275535: Retrying a failed authentication on multiple LDAP servers can lead to users blocked
On Wed, 20 Oct 2021 13:35:22 GMT, Martin Balao wrote: > I'd like to propose a fix for JDK-8275535. This fix reverts the behavior to > the state previous to JDK-8160768, where an authentication failure stops from > trying other LDAP servers with the same credentials [1]. After JDK-8160768 we > have 2 possible loops to stop: the one that iterates over different URLs and > the one that iterates over different endpoints (after a DNS query that > returns multiple values). > > No test regressions observed in jdk/com/sun/jndi/ldap. > > -- > [1] - https://hg.openjdk.java.net/jdk/jdk/rev/a609d549992a#l2.137 Hi Martin, The change looks reasonable to me. I would suggest having a CSR logged for this change due to the following [behavioral incompatibility](https://wiki.openjdk.java.net/display/csr/Kinds+of+Compatibility): Before the change - all available endpoints/URLs are tried to create an LDAP context. With the proposed change - incorrect credentials will prevent other endpoints to be exercised to create an LDAP context. Having a CSR will also help to document difference in handling `AuthenticationException` and `NamingException` during construction of an LDAP context from the list of endpoints acquired from a LDAP DNS provider. - PR: https://git.openjdk.java.net/jdk/pull/6043
Re: RFR: 8276926: Use String.valueOf() when initializing File.separator and File.pathSeparator
On Wed, 10 Nov 2021 09:22:32 GMT, Сергей Цыпанов wrote: > Looking into `File.pathSeparator` I've found out that currently we initialize > it as > > public static final String separator = "" + separatorChar; > > Which after compilation turns into > > NEW java/lang/StringBuilder > DUP > INVOKESPECIAL java/lang/StringBuilder. ()V > LDC "" > INVOKEVIRTUAL java/lang/StringBuilder.append > (Ljava/lang/String;)Ljava/lang/StringBuilder; > GETSTATIC java/io/File.pathSeparatorChar : C > INVOKEVIRTUAL java/lang/StringBuilder.append (C)Ljava/lang/StringBuilder; > INVOKEVIRTUAL java/lang/StringBuilder.toString ()Ljava/lang/String; > PUTSTATIC java/io/File.pathSeparator : Ljava/lang/String; > > This can be simplified to > > public static final String separator = String.valueOf(separatorChar); > > Which is in turn complied into more compact > > GETSTATIC java/io/File.pathSeparatorChar : C > INVOKESTATIC java/lang/String.valueOf (C)Ljava/lang/String; > PUTSTATIC java/io/File.pathSeparator : Ljava/lang/String; > > The changed code is likely to slightly improve start-up time as it allocates > less and does no bound checks. It's unlikely that javac would do this kind of optimization (javac tends to produce code verbatim), but it might be worth filing a RFE. - PR: https://git.openjdk.java.net/jdk/pull/6326
Integrated: 8276926: Use String.valueOf() when initializing File.separator and File.pathSeparator
On Wed, 10 Nov 2021 09:22:32 GMT, Сергей Цыпанов wrote: > Looking into `File.pathSeparator` I've found out that currently we initialize > it as > > public static final String separator = "" + separatorChar; > > Which after compilation turns into > > NEW java/lang/StringBuilder > DUP > INVOKESPECIAL java/lang/StringBuilder. ()V > LDC "" > INVOKEVIRTUAL java/lang/StringBuilder.append > (Ljava/lang/String;)Ljava/lang/StringBuilder; > GETSTATIC java/io/File.pathSeparatorChar : C > INVOKEVIRTUAL java/lang/StringBuilder.append (C)Ljava/lang/StringBuilder; > INVOKEVIRTUAL java/lang/StringBuilder.toString ()Ljava/lang/String; > PUTSTATIC java/io/File.pathSeparator : Ljava/lang/String; > > This can be simplified to > > public static final String separator = String.valueOf(separatorChar); > > Which is in turn complied into more compact > > GETSTATIC java/io/File.pathSeparatorChar : C > INVOKESTATIC java/lang/String.valueOf (C)Ljava/lang/String; > PUTSTATIC java/io/File.pathSeparator : Ljava/lang/String; > > The changed code is likely to slightly improve start-up time as it allocates > less and does no bound checks. This pull request has now been integrated. Changeset: 0f23c6a9 Author:Sergey Tsypanov Committer: Claes Redestad URL: https://git.openjdk.java.net/jdk/commit/0f23c6a9feb3657eb20ff5988a9e2ffca2108af1 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod 8276926: Use String.valueOf() when initializing File.separator and File.pathSeparator Reviewed-by: redestad, jlaskey - PR: https://git.openjdk.java.net/jdk/pull/6326
Re: RFR: 8276926: Use String.valueOf() when initializing File.separator and File.pathSeparator
On Wed, 10 Nov 2021 09:22:32 GMT, Сергей Цыпанов wrote: > Looking into File.pathSeparator I've found out that currently we initialize > them as > > public static final String separator = "" + separatorChar; > > Which after compilation turns into > > NEW java/lang/StringBuilder > DUP > INVOKESPECIAL java/lang/StringBuilder. ()V > LDC "" > INVOKEVIRTUAL java/lang/StringBuilder.append > (Ljava/lang/String;)Ljava/lang/StringBuilder; > GETSTATIC java/io/File.pathSeparatorChar : C > INVOKEVIRTUAL java/lang/StringBuilder.append (C)Ljava/lang/StringBuilder; > INVOKEVIRTUAL java/lang/StringBuilder.toString ()Ljava/lang/String; > PUTSTATIC java/io/File.pathSeparator : Ljava/lang/String; > > This can be simplified to > > public static final String separator = String.valueOf(separatorChar); > > Which is in turn complied into more compact > > GETSTATIC java/io/File.pathSeparatorChar : C > INVOKESTATIC java/lang/String.valueOf (C)Ljava/lang/String; > PUTSTATIC java/io/File.pathSeparator : Ljava/lang/String; > > The changed code is likely to slightly improve start-up time as it allocates > less and does no bound checks. Marked as reviewed by jlaskey (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6326
Re: Fix AbstractLdapNamingEnumeration next to throw NoSuchElementException instead of NullPointerException
Hi Andrew, I've logged a JBS bug for your change: https://bugs.openjdk.java.net/browse/JDK-8276939 You can use it to update your PR title to address the following jcheck error: The commit message does not reference any issue. To add an issue reference to this PR, edit the title to be of the format issue number: message. With Best Regards, Aleksei From: core-libs-dev on behalf of Andrew Luo Sent: Sunday, October 24, 2021 9:47 PM To: core-libs-dev Subject: Fix AbstractLdapNamingEnumeration next to throw NoSuchElementException instead of NullPointerException I have a small PR to fix a bug in AbstractLdapNamingEnumeration - details are in the PR: https://github.com/openjdk/jdk/pull/6095 (Note: I'm already an OpenJDK contributor and have signed an OCA but don't have an author account to be able to create bugs) Thanks, -Andrew
Re: RFR: 8276926: Use String.valueOf() when initializing File.separator and File.pathSeparator
On Wed, 10 Nov 2021 09:22:32 GMT, Сергей Цыпанов wrote: > Looking into File.pathSeparator I've found out that currently we initialize > them as > > public static final String separator = "" + separatorChar; > > Which after compilation turns into > > NEW java/lang/StringBuilder > DUP > INVOKESPECIAL java/lang/StringBuilder. ()V > LDC "" > INVOKEVIRTUAL java/lang/StringBuilder.append > (Ljava/lang/String;)Ljava/lang/StringBuilder; > GETSTATIC java/io/File.pathSeparatorChar : C > INVOKEVIRTUAL java/lang/StringBuilder.append (C)Ljava/lang/StringBuilder; > INVOKEVIRTUAL java/lang/StringBuilder.toString ()Ljava/lang/String; > PUTSTATIC java/io/File.pathSeparator : Ljava/lang/String; > > This can be simplified to > > public static final String separator = String.valueOf(separatorChar); > > Which is in turn complied into more compact > > GETSTATIC java/io/File.pathSeparatorChar : C > INVOKESTATIC java/lang/String.valueOf (C)Ljava/lang/String; > PUTSTATIC java/io/File.pathSeparator : Ljava/lang/String; > > The changed code is likely to slightly improve start-up time as it allocates > less and does no bound checks. FWIW the specific pattern of `"" + foo` could probably profitably translate into `String.valueOf(foo)` regardless of `StringConcatFactory` availability. (And translating `String` concat to use `SCF` in class initializers is likely a pessimization most of the time.) I'm no expert on javac but I suspect implementing (and worse: specifying) minor improvements to the translation might be more effort than it's worth, though, so point fixes like these seem fine to me. - Marked as reviewed by redestad (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6326
Re: RFR: 8276700: Improve java.lang.ref.Cleaner javadocs [v4]
On Wed, 10 Nov 2021 11:40:28 GMT, Anthony Vanelverdinghe wrote: >> This is getting too complicated... >> >> It's a code *example* with a very clear comment that explains a best >> practice: >> >> // A cleaner, preferably one shared within a library >> private static final Cleaner cleaner = Cleaner.create(); >> >> >> We cannot really show the best practice in this example without making the >> example itself more complicated. IMHO, introducing an extra factory method >> here adds nothing but complexity and makes the example more difficult to >> understand (that aside, it should probably be something like >> `MyLibrary.getCleaner()` and not a `createXyz()` method). >> >> I still much more prefer `cleaner = Cleaner.create();` over `cleaner = >> ` (which really is no better in any way, shape or form and creates >> more questions than it provides answers) or `cleaner = ...`, which again >> does not answer the question of how to get a cleaner instance—something I >> asked myself when trying to use the API. In fact *how to get a cleaner >> instance* is not explained by the current javadocs *at all* and it's >> something one *must* know to use this API. >> >> Here's our chance to show how it *can* be done, even if it's not ideal, >> because it does not demonstrate the one-cleaner per library recommendation >> (only mentions it). >> >> And yes, those ellipsis in the `State` class code—I'd love for them to be >> gone, too. It would make the example less abstract and easier to understand >> (what's this state anyway? in most cases it's probably a reference to a >> state, e.g. a native pointer, i.e. a `long`). But, admittedly, that's >> difficult. >> >> So, to summarize, there is always a tradeoff between making an example easy >> to understand, not too complex, but still conveying the most important >> information and best practices. In the end it's a matter of opinion. In this >> case, I will stick to my original code change suggestion, because it adds >> value (answers the question of how to get/create a cleaner instance). > > In short: the current code > > > // A cleaner, preferably one shared within a library > private static final Cleaner cleaner = ; > > > is unhelpful to you, and the proposed code > > > // A cleaner, preferably one shared within a library > private static final Cleaner cleaner = Cleaner.create(); > > > is confusing to me. > > Trying to find a compromise, I'm merely asking that the comment be clarified: > > > // A cleaner (preferably one shared within a library, but for the sake of > example, a new one is created here) > private static final Cleaner cleaner = Cleaner.create(); > > > What do you think? Sounds great! I'll change the code. Thanks for suggesting this compromise. - PR: https://git.openjdk.java.net/jdk/pull/6076
Re: RFR: 8276700: Improve java.lang.ref.Cleaner javadocs [v4]
> Trivial improvement. > > Explicitly show how to create a `Cleaner` instance using `Cleaner.create()`. > Repeat (again) in the code example that the `State` `Runnable `should be > implemented as static class and not reference the instance to be cleaned, to > make the point even more clear to those people who never read the javadoc > *prose*. > > I have signed the OCA a while back as > [hschreiber](https://openjdk.java.net/census#hschreiber). Hendrik Schreiber has updated the pull request incrementally with one additional commit since the last revision: Added extra info to Cleaner comment. - Changes: - all: https://git.openjdk.java.net/jdk/pull/6076/files - new: https://git.openjdk.java.net/jdk/pull/6076/files/f5d4ee92..04c021c3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6076&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6076&range=02-03 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/6076.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6076/head:pull/6076 PR: https://git.openjdk.java.net/jdk/pull/6076
Re: RFR: 8276700: Improve java.lang.ref.Cleaner javadocs [v3]
On Wed, 10 Nov 2021 09:43:58 GMT, Hendrik Schreiber
wrote:
>>> When I see ``, I'm just wondering what those <> type operators are
>>> good for here...
>>
>> What about just replacing `` with `...` then? The `State`
>> constructor and its invocation also have an ellipsis.
>>
>>> BUT, at least it's a working example and not some pseudo code.
>>
>> The example is still not compilable due to the remaining ellipses.
>>
>>> We do want to move to working example code long term, don't we?
>>
>> I agree that examples should be compilable *in the raw Javadoc*. However, in
>> the rendered Javadoc, using ellipses is a well-understood way to keep
>> examples concise and devoid of irrelevant and/or usecase-dependent code.
>> Moreover, when developers copy-paste the example, they'll immediately be
>> pointed to all the places where they need to fill in the blanks, make a
>> choice for a trade-off, etc. On the other hand, by hard-coding a
>> (suboptimal) choice, developers who merely copy-paste the example are
>> unlikely to reconsider the trade-off.
>>
>>> The new example Cleaner instance _is_ shared, though on a pretty small
>>> scale (just among instances of CleaningExample).
>>
>> True, but my point was that the comment says "shared *within a library*". So
>> to me it's confusing to have a comment saying "it's preferred to do A", and
>> then have the code do B on the next line.
>>
>> So I propose to either:
>> * revert the current change & simply replace `` with `...`
>> * update the comment to say: "A cleaner (preferably one shared within a
>> library, but for the sake of example, a new one is created here)"
>>
>> Actually, to have the line be compilable, and at the same time (attempt to)
>> force developers to consider the trade-off, it could be changed to something
>> like:
>>
>>
>> private static final Cleaner cleaner = createCleaner();
>>
>> private static Cleaner createCleaner() {
>> // A cleaner, preferably one shared within a library
>> throw new UnsupportedOperationException("TBD");
>> }
>
> This is getting too complicated...
>
> It's a code *example* with a very clear comment that explains a best practice:
>
> // A cleaner, preferably one shared within a library
> private static final Cleaner cleaner = Cleaner.create();
>
>
> We cannot really show the best practice in this example without making the
> example itself more complicated. IMHO, introducing an extra factory method
> here adds nothing but complexity and makes the example more difficult to
> understand (that aside, it should probably be something like
> `MyLibrary.getCleaner()` and not a `createXyz()` method).
>
> I still much more prefer `cleaner = Cleaner.create();` over `cleaner =
> ` (which really is no better in any way, shape or form and creates
> more questions than it provides answers) or `cleaner = ...`, which again does
> not answer the question of how to get a cleaner instance—something I asked
> myself when trying to use the API. In fact *how to get a cleaner instance* is
> not explained by the current javadocs *at all* and it's something one *must*
> know to use this API.
>
> Here's our chance to show how it *can* be done, even if it's not ideal,
> because it does not demonstrate the one-cleaner per library recommendation
> (only mentions it).
>
> And yes, those ellipsis in the `State` class code—I'd love for them to be
> gone, too. It would make the example less abstract and easier to understand
> (what's this state anyway? in most cases it's probably a reference to a
> state, e.g. a native pointer, i.e. a `long`). But, admittedly, that's
> difficult.
>
> So, to summarize, there is always a tradeoff between making an example easy
> to understand, not too complex, but still conveying the most important
> information and best practices. In the end it's a matter of opinion. In this
> case, I will stick to my original code change suggestion, because it adds
> value (answers the question of how to get/create a cleaner instance).
In short: the current code
// A cleaner, preferably one shared within a library
private static final Cleaner cleaner = ;
is unhelpful to you, and the proposed code
// A cleaner, preferably one shared within a library
private static final Cleaner cleaner = Cleaner.create();
is confusing to me.
Trying to find a compromise, I'm merely asking that the comment be clarified:
// A cleaner (preferably one shared within a library, but for the sake of
example, a new one is created here)
private static final Cleaner cleaner = Cleaner.create();
What do you think?
-
PR: https://git.openjdk.java.net/jdk/pull/6076
Re: RFR: 8276926: Use String.valueOf() when initializing File.separator and File.pathSeparator
On Wed, 10 Nov 2021 10:36:37 GMT, Michael Bien wrote: > it should compile into invokedynamic makeConcatWithConstants on later JDKs As far as I understand `StringConcatFactory` is not available on bootstrap stage and `StringBuilder` is used instead. See https://github.com/openjdk/jdk/pull/3464#issuecomment-818827786 and https://github.com/openjdk/jdk/pull/4507#issuecomment-873896268 (pay attention to class loading order - `j.i.File` is loaded before `StringConcatFactory` - PR: https://git.openjdk.java.net/jdk/pull/6326
Re: RFR: 8276141: XPathFactory set/getProperty method [v6]
On Tue, 9 Nov 2021 06:27:15 GMT, Joe Wang wrote:
>> src/java.xml/share/classes/javax/xml/xpath/XPathFactory.java line 375:
>>
>>> 373: /**
>>> 374: * Sets a property for this {@code XPathFactory} and {@code XPath}
>>> 375: * created by this factory.
>>
>> Are properties inherited by the XPath object at create time? The javadoc for
>> newXPath suggests so and I'm wondering if the javadoc for setProperty needs
>> to consistent with that. Maybe the intention is really that this method sets
>> a XPathFactory property and it is used when configuring the XPath object
>> returned by the newXPath method.
>
> Yes. Made a clarification to the javadoc, and while we are here, to
> setFeature method as well.
One other suggestion for wording is to change:
"A property set on the factory is effective to the XPath object created
thereafter"
to something like:
"The property applies to XPath objects that the XPathFactory creates. It has no
impact on XPath objects that are already created."
I don't have any other comments on the API.
-
PR: https://git.openjdk.java.net/jdk/pull/6215
Re: RFR: 8276775: ZonedDateTime/OffsetDateTime.toString return invalid ISO-8601 for years <= 1893
On Tue, 9 Nov 2021 22:29:12 GMT, Naoto Sato wrote: > Simple doc clarification where the `toString()` output only conforms to ISO > 8601 if the seconds in the offset are zero. Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6321
Re: RFR: 8276926: Use String.valueOf() when initializing File.separator and File.pathSeparator
On Wed, 10 Nov 2021 09:22:32 GMT, Сергей Цыпанов wrote: > Looking into File.pathSeparator I've found out that currently we initialize > them as > > public static final String separator = "" + separatorChar; > > Which after compilation turns into > > NEW java/lang/StringBuilder > DUP > INVOKESPECIAL java/lang/StringBuilder. ()V > LDC "" > INVOKEVIRTUAL java/lang/StringBuilder.append > (Ljava/lang/String;)Ljava/lang/StringBuilder; > GETSTATIC java/io/File.pathSeparatorChar : C > INVOKEVIRTUAL java/lang/StringBuilder.append (C)Ljava/lang/StringBuilder; > INVOKEVIRTUAL java/lang/StringBuilder.toString ()Ljava/lang/String; > PUTSTATIC java/io/File.pathSeparator : Ljava/lang/String; > > This can be simplified to > > public static final String separator = String.valueOf(separatorChar); > > Which is in turn complied into more compact > > GETSTATIC java/io/File.pathSeparatorChar : C > INVOKESTATIC java/lang/String.valueOf (C)Ljava/lang/String; > PUTSTATIC java/io/File.pathSeparator : Ljava/lang/String; > > The changed code is likely to slightly improve start-up time as it allocates > less and does no bound checks. it should compile into invokedynamic makeConcatWithConstants on later JDKs, but valueOf(char) is most likely going to have smaller first-invocation overhead still. - PR: https://git.openjdk.java.net/jdk/pull/6326
Re: RFR: 8276700: Improve java.lang.ref.Cleaner javadocs [v2]
On Wed, 10 Nov 2021 09:41:44 GMT, Alan Bateman wrote: >> Hendrik Schreiber has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Update src/java.base/share/classes/java/lang/ref/Cleaner.java >> >> Making the comment even clearer. >> >> Co-authored-by: Mandy Chung > > src/java.base/share/classes/java/lang/ref/Cleaner.java line 93: > >> 91: * >> 92: *// Static state class, capturing information necessary for >> 93: *// State class captures information necessary for cleanup. > > I think it would be simpler to just drop L92, meaning start with "State class > captures ..." rather than having "state class" and "State class" in the same > sentence. I have removed that line and also changed `in this CleaningExample` -> `in this example`, because it is obvious. - PR: https://git.openjdk.java.net/jdk/pull/6076
Re: RFR: 8276700: Improve java.lang.ref.Cleaner javadocs [v3]
> Trivial improvement. > > Explicitly show how to create a `Cleaner` instance using `Cleaner.create()`. > Repeat (again) in the code example that the `State` `Runnable `should be > implemented as static class and not reference the instance to be cleaned, to > make the point even more clear to those people who never read the javadoc > *prose*. > > I have signed the OCA a while back as > [hschreiber](https://openjdk.java.net/census#hschreiber). Hendrik Schreiber has updated the pull request incrementally with one additional commit since the last revision: Removed duplicate info from comment. - Changes: - all: https://git.openjdk.java.net/jdk/pull/6076/files - new: https://git.openjdk.java.net/jdk/pull/6076/files/3a552acc..f5d4ee92 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6076&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6076&range=01-02 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/6076.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6076/head:pull/6076 PR: https://git.openjdk.java.net/jdk/pull/6076
Re: RFR: 8276700: Improve java.lang.ref.Cleaner javadocs [v2]
On Wed, 10 Nov 2021 09:30:07 GMT, Hendrik Schreiber wrote: >> Trivial improvement. >> >> Explicitly show how to create a `Cleaner` instance using `Cleaner.create()`. >> Repeat (again) in the code example that the `State` `Runnable `should be >> implemented as static class and not reference the instance to be cleaned, to >> make the point even more clear to those people who never read the javadoc >> *prose*. >> >> I have signed the OCA a while back as >> [hschreiber](https://openjdk.java.net/census#hschreiber). > > Hendrik Schreiber has updated the pull request incrementally with one > additional commit since the last revision: > > Update src/java.base/share/classes/java/lang/ref/Cleaner.java > > Making the comment even clearer. > > Co-authored-by: Mandy Chung src/java.base/share/classes/java/lang/ref/Cleaner.java line 93: > 91: * > 92: *// Static state class, capturing information necessary for > 93: *// State class captures information necessary for cleanup. I think it would be simpler to just drop L92, meaning start with "State class captures ..." rather than having "state class" and "State class" in the same sentence. - PR: https://git.openjdk.java.net/jdk/pull/6076
Re: RFR: 8276700: Improve java.lang.ref.Cleaner javadocs [v2]
On Tue, 9 Nov 2021 11:14:37 GMT, Anthony Vanelverdinghe
wrote:
>> Let me add, why I have raised this issue.
>>
>> I was going to migrate some older code which uses the `finalize()` method to
>> the `Cleaner` mechanism. New it it, there seemed to be two pitfalls:
>>
>> 1. Understanding the whole "don't capture an instance reference in your
>> state object"
>> 2. Copying the example (which was in a non-working state, due to pseudo
>> code) and making it work for me
>>
>> With the improvement suggestions, I was trying help people who *only* read
>> the code sample (many do), to become aware of 1. and help them getting going
>> with 2, simply because it's something they can copy and run.
>
>> When I see ``, I'm just wondering what those <> type operators are
>> good for here...
>
> What about just replacing `` with `...` then? The `State`
> constructor and its invocation also have an ellipsis.
>
>> BUT, at least it's a working example and not some pseudo code.
>
> The example is still not compilable due to the remaining ellipses.
>
>> We do want to move to working example code long term, don't we?
>
> I agree that examples should be compilable *in the raw Javadoc*. However, in
> the rendered Javadoc, using ellipses is a well-understood way to keep
> examples concise and devoid of irrelevant and/or usecase-dependent code.
> Moreover, when developers copy-paste the example, they'll immediately be
> pointed to all the places where they need to fill in the blanks, make a
> choice for a trade-off, etc. On the other hand, by hard-coding a (suboptimal)
> choice, developers who merely copy-paste the example are unlikely to
> reconsider the trade-off.
>
>> The new example Cleaner instance _is_ shared, though on a pretty small scale
>> (just among instances of CleaningExample).
>
> True, but my point was that the comment says "shared *within a library*". So
> to me it's confusing to have a comment saying "it's preferred to do A", and
> then have the code do B on the next line.
>
> So I propose to either:
> * revert the current change & simply replace `` with `...`
> * update the comment to say: "A cleaner (preferably one shared within a
> library, but for the sake of example, a new one is created here)"
>
> Actually, to have the line be compilable, and at the same time (attempt to)
> force developers to consider the trade-off, it could be changed to something
> like:
>
>
> private static final Cleaner cleaner = createCleaner();
>
> private static Cleaner createCleaner() {
> // A cleaner, preferably one shared within a library
> throw new UnsupportedOperationException("TBD");
> }
This is getting too complicated...
It's a code *example* with a very clear comment that explains a best practice:
// A cleaner, preferably one shared within a library
private static final Cleaner cleaner = Cleaner.create();
We cannot really show the best practice in this example without making the
example itself more complicated. IMHO, introducing an extra factory method here
adds nothing but complexity and makes the example more difficult to understand
(that aside, it should probably be something like `MyLibrary.getCleaner()` and
not a `createXyz()` method).
I still much more prefer `cleaner = Cleaner.create();` over `cleaner =
` (which really is no better in any way, shape or form and creates
more questions than it provides answers) or `cleaner = ...`, which again does
not answer the question of how to get a cleaner instance—something I asked
myself when trying to use the API. In fact *how to get a cleaner instance* is
not explained by the current javadocs *at all* and it's something one *must*
know to use this API.
Here's our chance to show how it *can* be done, even if it's not ideal, because
it does not demonstrate the one-cleaner per library recommendation (only
mentions it).
And yes, those ellipsis in the `State` class code—I'd love for them to be gone,
too. It would make the example less abstract and easier to understand (what's
this state anyway? in most cases it's probably a reference to a state, e.g. a
native pointer, i.e. a `long`). But, admittedly, that's difficult.
So, to summarize, there is always a tradeoff between making an example easy to
understand, not too complex, but still conveying the most important information
and best practices. In the end it's a matter of opinion. In this case, I will
stick to my original code change suggestion, because it adds value (answers the
question of how to get/create a cleaner instance).
-
PR: https://git.openjdk.java.net/jdk/pull/6076
Re: RFR: 8276700: Improve java.lang.ref.Cleaner javadocs [v2]
On Mon, 8 Nov 2021 23:21:49 GMT, Brent Christian wrote: >> This is what I suggested and makes it clear that *must hold no reference to >> the instance being cleaned*. Maybe you didn't notice it's still there? >> >> >> *// State class captures information necessary for cleanup. >> *// It must hold no reference to the instance being cleaned >> *// and therefore it is a static inner class in this CleaningExample >> ``` > > I like this suggestion - calling out that the static-ness of the State class > is due to the requirement to not hold any references. > Maybe you didn't notice it's still there? Yeah. Sorry, @mlchung. Good suggestion! - PR: https://git.openjdk.java.net/jdk/pull/6076
RFR: 8276926: Use String.valueOf() when initializing File.separator and File.pathSeparator
Looking into File.pathSeparator I've found out that currently we initialize them as public static final String separator = "" + separatorChar; Which after compilation turns into NEW java/lang/StringBuilder DUP INVOKESPECIAL java/lang/StringBuilder. ()V LDC "" INVOKEVIRTUAL java/lang/StringBuilder.append (Ljava/lang/String;)Ljava/lang/StringBuilder; GETSTATIC java/io/File.pathSeparatorChar : C INVOKEVIRTUAL java/lang/StringBuilder.append (C)Ljava/lang/StringBuilder; INVOKEVIRTUAL java/lang/StringBuilder.toString ()Ljava/lang/String; PUTSTATIC java/io/File.pathSeparator : Ljava/lang/String; This can be simplified to public static final String separator = String.valueOf(separatorChar); Which is in turn complied into more compact GETSTATIC java/io/File.pathSeparatorChar : C INVOKESTATIC java/lang/String.valueOf (C)Ljava/lang/String; PUTSTATIC java/io/File.pathSeparator : Ljava/lang/String; The changed code is likely to slightly improve start-up time as it allocates less and does no bound checks. - Commit messages: - 8276926: Use String.valueOf() when initializing File.separator and File.pathSeparator Changes: https://git.openjdk.java.net/jdk/pull/6326/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6326&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276926 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/6326.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6326/head:pull/6326 PR: https://git.openjdk.java.net/jdk/pull/6326
Re: RFR: 8276700: Improve java.lang.ref.Cleaner javadocs [v2]
> Trivial improvement. > > Explicitly show how to create a `Cleaner` instance using `Cleaner.create()`. > Repeat (again) in the code example that the `State` `Runnable `should be > implemented as static class and not reference the instance to be cleaned, to > make the point even more clear to those people who never read the javadoc > *prose*. > > I have signed the OCA a while back as > [hschreiber](https://openjdk.java.net/census#hschreiber). Hendrik Schreiber has updated the pull request incrementally with one additional commit since the last revision: Update src/java.base/share/classes/java/lang/ref/Cleaner.java Making the comment even clearer. Co-authored-by: Mandy Chung - Changes: - all: https://git.openjdk.java.net/jdk/pull/6076/files - new: https://git.openjdk.java.net/jdk/pull/6076/files/ba469687..3a552acc Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6076&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6076&range=00-01 Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/6076.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6076/head:pull/6076 PR: https://git.openjdk.java.net/jdk/pull/6076
