Re: RFR: 8259814: test/jdk/tools/jlink/plugins/CompressorPluginTest.java has compilation issues
On Fri, 15 Jan 2021 05:19:44 GMT, Athijegannathan Sundararajan wrote: > Fixed compilation issues with the test. Test compiles and runs fine locally. Are you going to remove it from the exclude list (test/jdk/ProblemList.txt)? I skimmed through the changes and it looks fine but would like confirmation that it has been run many times on all platforms (some of the jlink tests have taken time to get stable). - PR: https://git.openjdk.java.net/jdk/pull/2091
Re: RFR: 6594730: UUID.getVersion() is only meaningful for Leach-Salz variant
On Thu, 17 Dec 2020 22:16:12 GMT, Roger Riggs wrote: >> I cannot create CSRs, only members can. Deprecating and switching to another >> function results in much more work for users overall because it affects all >> those who use it correctly as well. In other words: 100% >> >> I'm in favor of rejection if that's the way forward. >> >> What's wrong with creating PRs for existing issues? I thought that more >> contributions is actually the reason OpenJDK came to GitHub. > > Often an enhancement is created when there's an idea of an improvement but no > resources to do the research and develop a justification. That work has to be > done when someone has time. Usually ideas are discussed on one of the many > OpenJDK email aliases to validate the idea and the approach before developing > the code. See the list of OpenJDK Mail lists for details. > https://mail.openjdk.java.net/mailman/listinfo > As for moving to GitHub, it was in part to make it easier to collaborate on > the development but also to move from Mercurial to Git. > But GitHub does not replace the need for discussion on the Email Lists. Active - PR: https://git.openjdk.java.net/jdk/pull/1467
RFR: 8259814: test/jdk/tools/jlink/plugins/CompressorPluginTest.java has compilation issues
Fixed compilation issues with the test. Test compiles and runs fine locally. - Commit messages: - 8259814: test/jdk/tools/jlink/plugins/CompressorPluginTest.java has compilation issues Changes: https://git.openjdk.java.net/jdk/pull/2091/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2091=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8259814 Stats: 16 lines in 1 file changed: 5 ins; 0 del; 11 mod Patch: https://git.openjdk.java.net/jdk/pull/2091.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2091/head:pull/2091 PR: https://git.openjdk.java.net/jdk/pull/2091
Integrated: 8259622: TreeMap.computeIfAbsent deviates from spec
On Wed, 13 Jan 2021 09:51:01 GMT, Tagir F. Valeev wrote: > Handle TreeMap::computeIfAbsent when previous mapping with null value existed > (in this case spec requires to overwrite the existing mapping) This pull request has now been integrated. Changeset: 2c8e337d Author:Tagir F. Valeev URL: https://git.openjdk.java.net/jdk/commit/2c8e337d Stats: 26 lines in 2 files changed: 22 ins; 0 del; 4 mod 8259622: TreeMap.computeIfAbsent deviates from spec Reviewed-by: smarks - PR: https://git.openjdk.java.net/jdk/pull/2058
Re: RFR: 8259622: TreeMap.computeIfAbsent deviates from spec [v2]
> Handle TreeMap::computeIfAbsent when previous mapping with null value existed > (in this case spec requires to overwrite the existing mapping) Tagir F. Valeev has updated the pull request incrementally with one additional commit since the last revision: Copyright year updated - Changes: - all: https://git.openjdk.java.net/jdk/pull/2058/files - new: https://git.openjdk.java.net/jdk/pull/2058/files/8f63db72..906619e1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2058=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2058=00-01 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2058.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2058/head:pull/2058 PR: https://git.openjdk.java.net/jdk/pull/2058
Re: [jdk16] RFR: JDK-8259732: JDK 16 L10n resource file update - msg drop 10 [v2]
On Thu, 14 Jan 2021 17:19:11 GMT, Naoto Sato wrote:
>> Leo Jiang 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 two additional commits
>> since the last revision:
>>
>> - Merge branch 'master' into msgdrop
>> - JDK-8259732: JDK 16 L10n resource file update - msg drop 10
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties
> line 518:
>
>> 516: doclet.footer_specified=\
>> 517: The -footer option is no longer supported and will be ignored.\n\
>> 518: It may be removed in a future release.
>
> I believe this is to fix no newline at the end (unrelated to l10n changes).
> Do we need to change the copyright year for this?
Actually I was correcting the L217, changed {) -> {}. But 2 days ago, Jonathan
Gibbons fixed it in another commit 6bb6093. I found his fix after running the
merge. Looks both of us forgot to update the copyright year. Any suggestion?
doclet.help.systemProperties.body=\
The {0} page lists references to system properties.
-
PR: https://git.openjdk.java.net/jdk16/pull/123
Re: RFR: 8259622: TreeMap.computeIfAbsent deviates from spec
On Wed, 13 Jan 2021 09:51:01 GMT, Tagir F. Valeev wrote: > Handle TreeMap::computeIfAbsent when previous mapping with null value existed > (in this case spec requires to overwrite the existing mapping) Marked as reviewed by smarks (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2058
Re: RFR: 8259622: TreeMap.computeIfAbsent deviates from spec
On Thu, 14 Jan 2021 23:25:22 GMT, Stuart Marks wrote: >> Handle TreeMap::computeIfAbsent when previous mapping with null value >> existed (in this case spec requires to overwrite the existing mapping) > > Marked as reviewed by smarks (Reviewer). Thanks for picking this up. Changes look good. Please update copyright years if you get a chance. - PR: https://git.openjdk.java.net/jdk/pull/2058
Re: RFR: 8193031: Collections.addAll is likely to perform worse than Collection.addAll [v7]
On Thu, 14 Jan 2021 15:25:25 GMT, Сергей Цыпанов
wrote:
>> Hello, I feel like this was previously discussed in
>> https://mail.openjdk.java.net/pipermail/core-libs-dev/ but since I cannot
>> find original mail I post this here.
>>
>> Currently `Collections.addAll()` is implemented and documented as:
>> /**
>> * ...
>> * The behavior of this convenience method is identical to that of
>> * {@code c.addAll(Arrays.asList(elements))}, but this method is likely
>> * to run significantly faster under most implementations.
>> */
>> @SafeVarargs
>> public static boolean addAll(Collection c, T... elements)
>> {
>> boolean result = false;
>> for (T element : elements)
>> result |= c.add(element);
>> return result;
>> }
>>
>> But it practice the notation `this method is likely to run significantly
>> faster under most implementations` is completely wrong. When I take this
>> [benchmark](https://github.com/stsypanov/benchmarks/blob/master/benchmark-runners/src/main/java/com/luxoft/logeek/benchmark/collection/CollectionsAddAllVsAddAllBenchmark.java)
>> and run it on JDK 14 I get the following results:
>>(collection) (size)
>> Score Error Units
>> addAllArrayList 10
>> 37.9 ± 1.9 ns/op
>> addAllArrayList 100
>> 83.8 ± 3.4 ns/op
>> addAllArrayList1000
>> 678.2 ±23.0 ns/op
>> collectionsAddAll ArrayList 10
>> 50.9 ± 1.1 ns/op
>> collectionsAddAll ArrayList 100
>> 751.4 ±47.4 ns/op
>> collectionsAddAll ArrayList1000
>> 8839.8 ± 710.7 ns/op
>>
>> addAll HashSet 10
>> 128.4 ± 5.9 ns/op
>> addAll HashSet 100
>> 1864.2 ± 102.4 ns/op
>> addAll HashSet1000
>> 16615.5 ± 1202.6 ns/op
>> collectionsAddAll HashSet 10
>> 172.8 ± 6.0 ns/op
>> collectionsAddAll HashSet 100
>> 2355.8 ± 195.4 ns/op
>> collectionsAddAll HashSet1000
>> 20364.7 ± 1164.0 ns/op
>>
>> addAll ArrayDeque 10
>> 54.0 ± 0.4 ns/op
>> addAll ArrayDeque 100
>> 319.7 ± 2.5 ns/op
>> addAll ArrayDeque1000
>> 3176.9 ±22.2 ns/op
>> collectionsAddAllArrayDeque 10
>> 66.5 ± 1.4 ns/op
>> collectionsAddAllArrayDeque 100
>> 808.1 ±55.9 ns/op
>> collectionsAddAllArrayDeque1000
>> 5639.6 ± 240.9 ns/op
>>
>> addAll CopyOnWriteArrayList 10
>> 18.0 ± 0.7 ns/op
>> addAll CopyOnWriteArrayList 100
>> 39.4 ± 1.7 ns/op
>> addAll CopyOnWriteArrayList1000
>> 371.1 ±17.0 ns/op
>> collectionsAddAll CopyOnWriteArrayList 10
>> 251.9 ±18.4 ns/op
>> collectionsAddAll CopyOnWriteArrayList 100
>> 3405.9 ± 304.8 ns/op
>> collectionsAddAll CopyOnWriteArrayList1000
>> 247496.8 ± 23502.3 ns/op
>>
>> addAllConcurrentLinkedDeque 10
>> 81.4 ± 2.8 ns/op
>> addAllConcurrentLinkedDeque 100
>> 609.1 ±26.4 ns/op
>> addAllConcurrentLinkedDeque1000
>> 4494.5 ± 219.3 ns/op
>> collectionsAddAll ConcurrentLinkedDeque 10
>> 189.8 ± 2.5 ns/op
>> collectionsAddAll ConcurrentLinkedDeque 100
>> 1660.0 ±62.0 ns/op
>> collectionsAddAll ConcurrentLinkedDeque1000
>> 17649.2 ± 300.9 ns/op
>>
>> addAll:·gc.alloc.rate.normArrayList 10
>> 160.0 ± 0.0B/op
>> addAll:·gc.alloc.rate.normArrayList 100
>> 880.0 ± 0.0B/op
>> addAll:·gc.alloc.rate.normArrayList1000
>> 8080.3 ± 0.0B/op
>> collectionsAddAll:·gc.alloc.rate.norm ArrayList 10
>> 80.0 ± 0.0B/op
>>
RFR: 8259707: LDAP channel binding does not work with StartTLS extension
Please review a small patch to enable LDAP TLS Channel Binding with StartTLS Extension. Test from the bug report and jtreg javax/naming tests are passed. - Commit messages: - 8259707: LDAP channel binding does not work with StartTLS extension Changes: https://git.openjdk.java.net/jdk/pull/2085/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2085=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8259707 Stats: 2 lines in 2 files changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2085.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2085/head:pull/2085 PR: https://git.openjdk.java.net/jdk/pull/2085
RE: [PING] RE: 8250678: ModuleDescriptor.Version parsing treats empty segments inconsistently
Hello,
Please reply if anyone can be a sponsor.
Regards,
Masanori Yano
> -Original Message-
> From: Yano, Masanori
> Sent: Wednesday, December 23, 2020 5:01 PM
> To: 'core-libs-dev@openjdk.java.net'
> Subject: [PING] RE: 8250678: ModuleDescriptor.Version parsing treats empty
> segments inconsistently
>
> Hello,
>
> Please reply if anyone can be a sponsor.
>
> Regards,
> Masanori Yano
>
> > -Original Message-
> > From: Yano, Masanori
> > Sent: Wednesday, November 4, 2020 6:03 PM
> > To: 'core-libs-dev@openjdk.java.net'
> > Subject: 8250678: ModuleDescriptor.Version parsing treats empty
> > segments inconsistently
> >
> > Hello.
> >
> > I would like to contribute for JDK-8250678.
> >
> > The 'parse' method of ModuleDescriptor.Version class behaves
> > incorrectly when the input string contains consecutive delimiters.
> >
> > The 'parse' method treats strings as three sections, but the parsing
> > method differs between the method for the version sections and the ones for
> > others.
> > In version sections, the 'parse' method takes a single character in a
> > loop and determines whether it is a delimiter. In pre and build
> > sections, the parse method takes two characters in a loop and determines
> > whether
> the second character is the delimiter.
> > Therefore, if the string contains a sequence of delimiters in pre or
> > build section, the 'parse' method treats character at the odd-numbered
> > position as a token and the one at even-numbered as a delimiter
> >
> > A string containing consecutive delimiters is an incorrect version
> > string, but this behavior does not follow the API specification.
> > https://download.java.net/java/early_access/jdk16/docs/api/java.base/j
> > ava/lang/
> > module/ModuleDescriptor.Version.html
> >
> > Therefore, I propose to fix the parsing method of the pre section and
> > the build section in the same way as the version.
> >
> > Please sponsor the following change.
> >
> > diff -r bdc20ee1a68d
> > src/java.base/share/classes/java/lang/module/ModuleDescriptor.java
> > --- a/src/java.base/share/classes/java/lang/module/ModuleDescriptor.java
> > Fri Sep 04 23:51:26 2020 -0400
> > +++ b/src/java.base/share/classes/java/lang/module/ModuleDescriptor.ja
> > +++ va
> > Wed Oct 28 17:06:47 2020 +0900
> > @@ -1053,13 +1053,6 @@
> >
> > while (i < n) {
> > c = v.charAt(i);
> > -if (c >= '0' && c <= '9')
> > -i = takeNumber(v, i, pre);
> > -else
> > -i = takeString(v, i, pre);
> > -if (i >= n)
> > -break;
> > -c = v.charAt(i);
> > if (c == '.' || c == '-') {
> > i++;
> > continue;
> > @@ -1068,6 +1061,10 @@
> > i++;
> > break;
> > }
> > +if (c >= '0' && c <= '9')
> > +i = takeNumber(v, i, pre);
> > +else
> > +i = takeString(v, i, pre);
> > }
> >
> > if (c == '+' && i >= n)
> > @@ -1075,17 +1072,14 @@
> >
> > while (i < n) {
> > c = v.charAt(i);
> > +if (c == '.' || c == '-' || c == '+') {
> > +i++;
> > +continue;
> > +}
> > if (c >= '0' && c <= '9')
> > i = takeNumber(v, i, build);
> > else
> > i = takeString(v, i, build);
> > -if (i >= n)
> > -break;
> > -c = v.charAt(i);
> > -if (c == '.' || c == '-' || c == '+') {
> > -i++;
> > -continue;
> > -}
> > }
> >
> > this.version = v;
> > diff -r bdc20ee1a68d test/jdk/java/lang/module/VersionTest.java
> > --- a/test/jdk/java/lang/module/VersionTest.javaFri Sep 04 23:51:26 2020
> > -0400
> > +++ b/test/jdk/java/lang/module/VersionTest.javaWed Oct 28 17:06:47
> > 2020 +0900
> > @@ -148,6 +148,8 @@
> > { "1", "1.0.0" },
> > { "1.0", "1.0.0" },
> > { "1.0-beta", "1.0.0-beta" },
> > +{ "1.0-1.1", "1.0-1..1" },
> > +{ "1.0-1+1", "1.0-1.+1" },
> >
> > };
> > }
> >
> > Regards,
> > Masanori Yano
Re: Difference in encoding semantics of URI returned by File.toURI and Path.toUri representing the same file
> On 13. Jan 2021, at 17:50, Daniel Fuchs wrote: > > [...] > It's not clear to me whether NFC should be applied - or what would be > the consequences of applying NFC there (or not). While I can't answer this question, it may be worth mentioning RFC 3987 which might not be directly applicable but also deals with the representation of non-ASCII data in URIs. In section 3 it explicitly asks for NFC-normalization _before_ applying percent-encoding. I can't tell what's the rationale behind this decision but possibly the behaviour of URI.toASCIIString() is in the spirit of these rules.
Re: [jdk16] RFR: 8254350: CompletableFuture.get may swallow InterruptedException
On Thu, 14 Jan 2021 13:59:50 GMT, Kezhu Wang wrote: >> 8254350: CompletableFuture.get may swallow InterruptedException > > @Martin-Buchholz @DougLea @AlanBateman Sorry for rough in. After change > `future.get()` to `future.get(1, TimeUnit.DAYS)`, > `SwallowedInterruptedException` failed. Thanks, Kezhu. I filed: https://bugs.openjdk.java.net/browse/JDK-8259796 - PR: https://git.openjdk.java.net/jdk16/pull/17
Re: [jdk16] RFR: JDK-8259732: JDK 16 L10n resource file update - msg drop 10 [v2]
On Thu, 14 Jan 2021 14:27:25 GMT, Leo Jiang wrote: >> This is the changes for JDK 16 msg drop 10. > > Leo Jiang 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 two additional commits since > the last revision: > > - Merge branch 'master' into msgdrop > - JDK-8259732: JDK 16 L10n resource file update - msg drop 10 src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties line 518: > 516: doclet.footer_specified=\ > 517: The -footer option is no longer supported and will be ignored.\n\ > 518: It may be removed in a future release. I believe this is to fix no newline at the end (unrelated to l10n changes). Do we need to change the copyright year for this? - PR: https://git.openjdk.java.net/jdk16/pull/123
Integrated: 8258956: Memory Leak in StringCoding on ThreadLocal resultCached StringCoding.Result
On Wed, 13 Jan 2021 18:41:05 GMT, Naoto Sato wrote: > Please review the fix to this issue. Unused thread local StringCoding.Result > is now wrapped with SoftReference, which can be GC'ed as needed. I confirmed > it worked as expected with a test case > (https://github.com/openjdk/jdk/blob/8b4e1e1cf8f5d753ed901406f73d67b21557fddb/test/jdk/java/lang/StringCoding/ResultCachedGCTest.java), > but decided not to include it in this PR. This is because I purposefully > chose the size of the byte array and max heap size which is fragile, and > could become noise in a regression test run. This pull request has now been integrated. Changeset: aba3431c Author:Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/aba3431c Stats: 18 lines in 1 file changed: 9 ins; 0 del; 9 mod 8258956: Memory Leak in StringCoding on ThreadLocal resultCached StringCoding.Result Reviewed-by: rriggs, alanb - PR: https://git.openjdk.java.net/jdk/pull/2064
Re: RFR: 8253866: Security Libs Terminology Refresh [v2]
> This is the security libs portion of the effort to replace > archaic/non-inclusive words with more neutral terms (see JDK-8253315 for > details). > > Here are the changes covering core libraries code and tests. Terms were > changed as follows: > > - blacklisted.certs -> blocked.certs (along with supporting make > infrastructure and tests) > - master/slave in KRB5 -> primary/replica > - blacklist in other files -> deny list > - whitelist -> allow list > > Addressing similar issues in upstream 3rd party code is out of scope of this > PR. Such changes will be picked up from their upstream sources. Jamil Nimeh has updated the pull request incrementally with one additional commit since the last revision: Minor grammatical fixes - Changes: - all: https://git.openjdk.java.net/jdk/pull/2074/files - new: https://git.openjdk.java.net/jdk/pull/2074/files/9d44a9e1..45492f55 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2074=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2074=00-01 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2074.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2074/head:pull/2074 PR: https://git.openjdk.java.net/jdk/pull/2074
Re: RFR: 8253866: Security Libs Terminology Refresh [v2]
On Thu, 14 Jan 2021 15:45:43 GMT, Sean Mullan wrote: >> Jamil Nimeh has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Minor grammatical fixes > > src/java.base/share/conf/security/java.security line 455: > >> 453: #max_retries and timeout are optional numerical parameters (default >> 1 and >> 454: #5000, which means once and 5 seconds). Please notes that if any of >> the >> 455: #values defined here is more than what is defined in krb5.conf, it >> will be > > Typo: s/is more/are more/ Done. Good catch. > src/java.base/share/conf/security/java.security line 454: > >> 452: #configuration, but with smaller max_retries and timeout values. >> 453: #max_retries and timeout are optional numerical parameters (default >> 1 and >> 454: #5000, which means once and 5 seconds). Please notes that if any of >> the > > Typo: s/notes/note/ fixed - PR: https://git.openjdk.java.net/jdk/pull/2074
Integrated: 8253866: Security Libs Terminology Refresh
On Thu, 14 Jan 2021 06:32:37 GMT, Jamil Nimeh wrote: > This is the security libs portion of the effort to replace > archaic/non-inclusive words with more neutral terms (see JDK-8253315 for > details). > > Here are the changes covering core libraries code and tests. Terms were > changed as follows: > > - blacklisted.certs -> blocked.certs (along with supporting make > infrastructure and tests) > - master/slave in KRB5 -> primary/replica > - blacklist in other files -> deny list > - whitelist -> allow list > > Addressing similar issues in upstream 3rd party code is out of scope of this > PR. Such changes will be picked up from their upstream sources. This pull request has now been integrated. Changeset: 8554fe6e Author:Jamil Nimeh URL: https://git.openjdk.java.net/jdk/commit/8554fe6e Stats: 351 lines in 17 files changed: 152 ins; 150 del; 49 mod 8253866: Security Libs Terminology Refresh Reviewed-by: erikj, weijun, mullan - PR: https://git.openjdk.java.net/jdk/pull/2074
Re: RFR: 8253866: Security Libs Terminology Refresh
On Thu, 14 Jan 2021 06:32:37 GMT, Jamil Nimeh wrote: > This is the security libs portion of the effort to replace > archaic/non-inclusive words with more neutral terms (see JDK-8253315 for > details). > > Here are the changes covering core libraries code and tests. Terms were > changed as follows: > > - blacklisted.certs -> blocked.certs (along with supporting make > infrastructure and tests) > - master/slave in KRB5 -> primary/replica > - blacklist in other files -> deny list > - whitelist -> allow list > > Addressing similar issues in upstream 3rd party code is out of scope of this > PR. Such changes will be picked up from their upstream sources. Marked as reviewed by mullan (Reviewer). src/java.base/share/conf/security/java.security line 454: > 452: #configuration, but with smaller max_retries and timeout values. > 453: #max_retries and timeout are optional numerical parameters (default > 1 and > 454: #5000, which means once and 5 seconds). Please notes that if any of > the Typo: s/notes/note/ src/java.base/share/conf/security/java.security line 455: > 453: #max_retries and timeout are optional numerical parameters (default > 1 and > 454: #5000, which means once and 5 seconds). Please notes that if any of > the > 455: #values defined here is more than what is defined in krb5.conf, it > will be Typo: s/is more/are more/ - PR: https://git.openjdk.java.net/jdk/pull/2074
Re: [jdk16] RFR: JDK-8259732: JDK 16 L10n resource file update - msg drop 10
On Thu, 14 Jan 2021 14:10:12 GMT, Leo Jiang wrote: >> This is the changes for JDK 16 msg drop 10. > > [webrev.tar.gz](https://github.com/openjdk/jdk16/files/5814846/webrev.tar.gz) > > Since they're Unicode escape sequences in the l10n resource files, so I > attached a human readable webrev, created by `git webrev` and converted. Pls > find this to help your review. @naotoj Would you have time to take a look at this change? Very appreciated! - PR: https://git.openjdk.java.net/jdk16/pull/123
Re: RFR: 8253866: Security Libs Terminology Refresh
On Thu, 14 Jan 2021 06:32:37 GMT, Jamil Nimeh wrote: > This is the security libs portion of the effort to replace > archaic/non-inclusive words with more neutral terms (see JDK-8253315 for > details). > > Here are the changes covering core libraries code and tests. Terms were > changed as follows: > > - blacklisted.certs -> blocked.certs (along with supporting make > infrastructure and tests) > - master/slave in KRB5 -> primary/replica > - blacklist in other files -> deny list > - whitelist -> allow list > > Addressing similar issues in upstream 3rd party code is out of scope of this > PR. Such changes will be picked up from their upstream sources. Code change looks fine to me. - Marked as reviewed by weijun (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2074
Re: RFR: 8193031: Collections.addAll is likely to perform worse than Collection.addAll [v7]
> Hello, I feel like this was previously discussed in
> https://mail.openjdk.java.net/pipermail/core-libs-dev/ but since I cannot
> find original mail I post this here.
>
> Currently `Collections.addAll()` is implemented and documented as:
> /**
> * ...
> * The behavior of this convenience method is identical to that of
> * {@code c.addAll(Arrays.asList(elements))}, but this method is likely
> * to run significantly faster under most implementations.
> */
> @SafeVarargs
> public static boolean addAll(Collection c, T... elements) {
> boolean result = false;
> for (T element : elements)
> result |= c.add(element);
> return result;
> }
>
> But it practice the notation `this method is likely to run significantly
> faster under most implementations` is completely wrong. When I take this
> [benchmark](https://github.com/stsypanov/benchmarks/blob/master/benchmark-runners/src/main/java/com/luxoft/logeek/benchmark/collection/CollectionsAddAllVsAddAllBenchmark.java)
> and run it on JDK 14 I get the following results:
>(collection) (size)
> Score Error Units
> addAllArrayList 10
> 37.9 ± 1.9 ns/op
> addAllArrayList 100
> 83.8 ± 3.4 ns/op
> addAllArrayList1000
> 678.2 ±23.0 ns/op
> collectionsAddAll ArrayList 10
> 50.9 ± 1.1 ns/op
> collectionsAddAll ArrayList 100
> 751.4 ±47.4 ns/op
> collectionsAddAll ArrayList1000
> 8839.8 ± 710.7 ns/op
>
> addAll HashSet 10
> 128.4 ± 5.9 ns/op
> addAll HashSet 100
> 1864.2 ± 102.4 ns/op
> addAll HashSet1000
> 16615.5 ± 1202.6 ns/op
> collectionsAddAll HashSet 10
> 172.8 ± 6.0 ns/op
> collectionsAddAll HashSet 100
> 2355.8 ± 195.4 ns/op
> collectionsAddAll HashSet1000
> 20364.7 ± 1164.0 ns/op
>
> addAll ArrayDeque 10
> 54.0 ± 0.4 ns/op
> addAll ArrayDeque 100
> 319.7 ± 2.5 ns/op
> addAll ArrayDeque1000
> 3176.9 ±22.2 ns/op
> collectionsAddAllArrayDeque 10
> 66.5 ± 1.4 ns/op
> collectionsAddAllArrayDeque 100
> 808.1 ±55.9 ns/op
> collectionsAddAllArrayDeque1000
> 5639.6 ± 240.9 ns/op
>
> addAll CopyOnWriteArrayList 10
> 18.0 ± 0.7 ns/op
> addAll CopyOnWriteArrayList 100
> 39.4 ± 1.7 ns/op
> addAll CopyOnWriteArrayList1000
> 371.1 ±17.0 ns/op
> collectionsAddAll CopyOnWriteArrayList 10
> 251.9 ±18.4 ns/op
> collectionsAddAll CopyOnWriteArrayList 100
> 3405.9 ± 304.8 ns/op
> collectionsAddAll CopyOnWriteArrayList1000
> 247496.8 ± 23502.3 ns/op
>
> addAllConcurrentLinkedDeque 10
> 81.4 ± 2.8 ns/op
> addAllConcurrentLinkedDeque 100
> 609.1 ±26.4 ns/op
> addAllConcurrentLinkedDeque1000
> 4494.5 ± 219.3 ns/op
> collectionsAddAll ConcurrentLinkedDeque 10
> 189.8 ± 2.5 ns/op
> collectionsAddAll ConcurrentLinkedDeque 100
> 1660.0 ±62.0 ns/op
> collectionsAddAll ConcurrentLinkedDeque1000
> 17649.2 ± 300.9 ns/op
>
> addAll:·gc.alloc.rate.normArrayList 10
> 160.0 ± 0.0B/op
> addAll:·gc.alloc.rate.normArrayList 100
> 880.0 ± 0.0B/op
> addAll:·gc.alloc.rate.normArrayList1000
> 8080.3 ± 0.0B/op
> collectionsAddAll:·gc.alloc.rate.norm ArrayList 10
> 80.0 ± 0.0B/op
> collectionsAddAll:·gc.alloc.rate.norm ArrayList 100
> 1400.2 ± 0.0B/op
> collectionsAddAll:·gc.alloc.rate.norm
Re: RFR: 8193031: Collections.addAll is likely to perform worse than Collection.addAll
On Thu, 14 Jan 2021 10:18:23 GMT, Peter Levart wrote: >> ...but we could employ this method to guarantee more than >> `c.addAll(Arrays.asList(elements))` does. So what about: >> >>> The behaviour of this convenience method is similar to that of >>> `c.addAll(Collections.unmodifiableList(Arrays.asList(elements)))` >> >> This means that the method guarantees that the passed in `elements` array >> will not be modified even if given collection `c` is not trusted. Speaking >> of that, perhaps you could try benchmarking such >> `c.addAll(Collections.unmodifiableList(Arrays.asList(elements)))` and see >> how it compares. The speed-up you observed from >> `c.addAll(Arrays.asList(elements))` with some collections was probably a >> result of them calling .toArray() on the argument collection and >> incorporating the resulting array into their own data structure in a >> bulk-copying-way. So >> `c.addAll(Collections.unmodifiableList(Arrays.asList(elements)))` might have >> same performance characteristics while guaranteeing same things about >> argument array. It might be slower when Iterator is employed though because >> unmodifiableList wrapper wraps Iterator(s) too > >> @plevart done. Should I also rename this PR to e.g. 'Fix performance-related >> notation in Collections.addAll'? > > I think the title of JIRA ticket has to be the same as that of PR (with JIRA > bug number prepended) in order to pass all checks needed for integrating. So > If you modify both, it would be fine. I cannot modify JIRA ticket without account there, so let's keep it as is - PR: https://git.openjdk.java.net/jdk/pull/1764
Re: [jdk16] RFR: JDK-8259732: JDK 16 L10n resource file update - msg drop 10 [v2]
> This is the changes for JDK 16 msg drop 10. Leo Jiang 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 two additional commits since the last revision: - Merge branch 'master' into msgdrop - JDK-8259732: JDK 16 L10n resource file update - msg drop 10 - Changes: - all: https://git.openjdk.java.net/jdk16/pull/123/files - new: https://git.openjdk.java.net/jdk16/pull/123/files/230117b4..d72f444a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk16=123=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk16=123=00-01 Stats: 718 lines in 32 files changed: 616 ins; 38 del; 64 mod Patch: https://git.openjdk.java.net/jdk16/pull/123.diff Fetch: git fetch https://git.openjdk.java.net/jdk16 pull/123/head:pull/123 PR: https://git.openjdk.java.net/jdk16/pull/123
Re: [jdk16] RFR: JDK-8259732: JDK 16 L10n resource file update - msg drop 10
On Thu, 14 Jan 2021 14:00:00 GMT, Leo Jiang wrote: > This is the changes for JDK 16 msg drop 10. [webrev.tar.gz](https://github.com/openjdk/jdk16/files/5814846/webrev.tar.gz) Since they're Unicode escape sequences in the l10n resource files, so I attached a human readable webrev, created by `git webrev` and converted. Pls find this to help your review. - PR: https://git.openjdk.java.net/jdk16/pull/123
[jdk16] RFR: JDK-8259732: JDK 16 L10n resource file update - msg drop 10
This is the changes for JDK 16 msg drop 10. - Commit messages: - JDK-8259732: JDK 16 L10n resource file update - msg drop 10 Changes: https://git.openjdk.java.net/jdk16/pull/123/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk16=123=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8259732 Stats: 215 lines in 30 files changed: 118 ins; 16 del; 81 mod Patch: https://git.openjdk.java.net/jdk16/pull/123.diff Fetch: git fetch https://git.openjdk.java.net/jdk16 pull/123/head:pull/123 PR: https://git.openjdk.java.net/jdk16/pull/123
Re: [jdk16] RFR: 8254350: CompletableFuture.get may swallow InterruptedException
On Sun, 13 Dec 2020 03:31:44 GMT, Martin Buchholz wrote: > 8254350: CompletableFuture.get may swallow InterruptedException @Martin-Buchholz @DougLea @AlanBateman Sorry for rough in. After change `future.get()` to `future.get(1, TimeUnit.DAYS)`, `SwallowedInterruptedException` failed. - PR: https://git.openjdk.java.net/jdk16/pull/17
Re: RFR: 8253866: Security Libs Terminology Refresh
On Thu, 14 Jan 2021 06:32:37 GMT, Jamil Nimeh wrote: > This is the security libs portion of the effort to replace > archaic/non-inclusive words with more neutral terms (see JDK-8253315 for > details). > > Here are the changes covering core libraries code and tests. Terms were > changed as follows: > > - blacklisted.certs -> blocked.certs (along with supporting make > infrastructure and tests) > - master/slave in KRB5 -> primary/replica > - blacklist in other files -> deny list > - whitelist -> allow list > > Addressing similar issues in upstream 3rd party code is out of scope of this > PR. Such changes will be picked up from their upstream sources. Build changes look good. - Marked as reviewed by erikj (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2074
Re: RFR: 8193031: Collections.addAll is likely to perform worse than Collection.addAll
On Fri, 8 Jan 2021 11:39:17 GMT, Peter Levart wrote: >> Hi @stsypanov, >> >>> The **behavior** of this convenience method is **identical** to that of >>> c.addAll(Arrays.asList(elements)) >> >> What about: >> >>> The **behaviour** of this convenience method is **similar** to that of >>> c.addAll(Arrays.asList(elements)) >> >> ...since it is not entirely identical. The outcome is usually identical >> because collections usually adhere to the specification, but we can not >> claim the behaviour is identical if the target collection does not adhere. > > ...but we could employ this method to guarantee more than > `c.addAll(Arrays.asList(elements))` does. So what about: > >> The behaviour of this convenience method is similar to that of >> `c.addAll(Collections.unmodifiableList(Arrays.asList(elements)))` > > This means that the method guarantees that the passed in `elements` array > will not be modified even if given collection `c` is not trusted. Speaking of > that, perhaps you could try benchmarking such > `c.addAll(Collections.unmodifiableList(Arrays.asList(elements)))` and see how > it compares. The speed-up you observed from > `c.addAll(Arrays.asList(elements))` with some collections was probably a > result of them calling .toArray() on the argument collection and > incorporating the resulting array into their own data structure in a > bulk-copying-way. So > `c.addAll(Collections.unmodifiableList(Arrays.asList(elements)))` might have > same performance characteristics while guaranteeing same things about > argument array. It might be slower when Iterator is employed though because > unmodifiableList wrapper wraps Iterator(s) too > @plevart done. Should I also rename this PR to e.g. 'Fix performance-related > notation in Collections.addAll'? I think the title of JIRA ticket has to be the same as that of PR (with JIRA bug number prepended) in order to pass all checks needed for integrating. So If you modify both, it would be fine. - PR: https://git.openjdk.java.net/jdk/pull/1764
Re: RFR: 8193031: Collections.addAll is likely to perform worse than Collection.addAll
On Fri, 8 Jan 2021 11:39:17 GMT, Peter Levart wrote: >> Hi @stsypanov, >> >>> The **behavior** of this convenience method is **identical** to that of >>> c.addAll(Arrays.asList(elements)) >> >> What about: >> >>> The **behaviour** of this convenience method is **similar** to that of >>> c.addAll(Arrays.asList(elements)) >> >> ...since it is not entirely identical. The outcome is usually identical >> because collections usually adhere to the specification, but we can not >> claim the behaviour is identical if the target collection does not adhere. > > ...but we could employ this method to guarantee more than > `c.addAll(Arrays.asList(elements))` does. So what about: > >> The behaviour of this convenience method is similar to that of >> `c.addAll(Collections.unmodifiableList(Arrays.asList(elements)))` > > This means that the method guarantees that the passed in `elements` array > will not be modified even if given collection `c` is not trusted. Speaking of > that, perhaps you could try benchmarking such > `c.addAll(Collections.unmodifiableList(Arrays.asList(elements)))` and see how > it compares. The speed-up you observed from > `c.addAll(Arrays.asList(elements))` with some collections was probably a > result of them calling .toArray() on the argument collection and > incorporating the resulting array into their own data structure in a > bulk-copying-way. So > `c.addAll(Collections.unmodifiableList(Arrays.asList(elements)))` might have > same performance characteristics while guaranteeing same things about > argument array. It might be slower when Iterator is employed though because > unmodifiableList wrapper wraps Iterator(s) too @plevart done. Should I also rename this PR to e.g. 'Fix performance-related notation in Collections.addAll'? - PR: https://git.openjdk.java.net/jdk/pull/1764
Re: RFR: 8193031: Collections.addAll is likely to perform worse than Collection.addAll [v6]
> Hello, I feel like this was previously discussed in
> https://mail.openjdk.java.net/pipermail/core-libs-dev/ but since I cannot
> find original mail I post this here.
>
> Currently `Collections.addAll()` is implemented and documented as:
> /**
> * ...
> * The behavior of this convenience method is identical to that of
> * {@code c.addAll(Arrays.asList(elements))}, but this method is likely
> * to run significantly faster under most implementations.
> */
> @SafeVarargs
> public static boolean addAll(Collection c, T... elements) {
> boolean result = false;
> for (T element : elements)
> result |= c.add(element);
> return result;
> }
>
> But it practice the notation `this method is likely to run significantly
> faster under most implementations` is completely wrong. When I take this
> [benchmark](https://github.com/stsypanov/benchmarks/blob/master/benchmark-runners/src/main/java/com/luxoft/logeek/benchmark/collection/CollectionsAddAllVsAddAllBenchmark.java)
> and run it on JDK 14 I get the following results:
>(collection) (size)
> Score Error Units
> addAllArrayList 10
> 37.9 ± 1.9 ns/op
> addAllArrayList 100
> 83.8 ± 3.4 ns/op
> addAllArrayList1000
> 678.2 ±23.0 ns/op
> collectionsAddAll ArrayList 10
> 50.9 ± 1.1 ns/op
> collectionsAddAll ArrayList 100
> 751.4 ±47.4 ns/op
> collectionsAddAll ArrayList1000
> 8839.8 ± 710.7 ns/op
>
> addAll HashSet 10
> 128.4 ± 5.9 ns/op
> addAll HashSet 100
> 1864.2 ± 102.4 ns/op
> addAll HashSet1000
> 16615.5 ± 1202.6 ns/op
> collectionsAddAll HashSet 10
> 172.8 ± 6.0 ns/op
> collectionsAddAll HashSet 100
> 2355.8 ± 195.4 ns/op
> collectionsAddAll HashSet1000
> 20364.7 ± 1164.0 ns/op
>
> addAll ArrayDeque 10
> 54.0 ± 0.4 ns/op
> addAll ArrayDeque 100
> 319.7 ± 2.5 ns/op
> addAll ArrayDeque1000
> 3176.9 ±22.2 ns/op
> collectionsAddAllArrayDeque 10
> 66.5 ± 1.4 ns/op
> collectionsAddAllArrayDeque 100
> 808.1 ±55.9 ns/op
> collectionsAddAllArrayDeque1000
> 5639.6 ± 240.9 ns/op
>
> addAll CopyOnWriteArrayList 10
> 18.0 ± 0.7 ns/op
> addAll CopyOnWriteArrayList 100
> 39.4 ± 1.7 ns/op
> addAll CopyOnWriteArrayList1000
> 371.1 ±17.0 ns/op
> collectionsAddAll CopyOnWriteArrayList 10
> 251.9 ±18.4 ns/op
> collectionsAddAll CopyOnWriteArrayList 100
> 3405.9 ± 304.8 ns/op
> collectionsAddAll CopyOnWriteArrayList1000
> 247496.8 ± 23502.3 ns/op
>
> addAllConcurrentLinkedDeque 10
> 81.4 ± 2.8 ns/op
> addAllConcurrentLinkedDeque 100
> 609.1 ±26.4 ns/op
> addAllConcurrentLinkedDeque1000
> 4494.5 ± 219.3 ns/op
> collectionsAddAll ConcurrentLinkedDeque 10
> 189.8 ± 2.5 ns/op
> collectionsAddAll ConcurrentLinkedDeque 100
> 1660.0 ±62.0 ns/op
> collectionsAddAll ConcurrentLinkedDeque1000
> 17649.2 ± 300.9 ns/op
>
> addAll:·gc.alloc.rate.normArrayList 10
> 160.0 ± 0.0B/op
> addAll:·gc.alloc.rate.normArrayList 100
> 880.0 ± 0.0B/op
> addAll:·gc.alloc.rate.normArrayList1000
> 8080.3 ± 0.0B/op
> collectionsAddAll:·gc.alloc.rate.norm ArrayList 10
> 80.0 ± 0.0B/op
> collectionsAddAll:·gc.alloc.rate.norm ArrayList 100
> 1400.2 ± 0.0B/op
> collectionsAddAll:·gc.alloc.rate.norm
Re: RFR: 8258956: Memory Leak in StringCoding on ThreadLocal resultCached StringCoding.Result
On Wed, 13 Jan 2021 18:41:05 GMT, Naoto Sato wrote: > Please review the fix to this issue. Unused thread local StringCoding.Result > is now wrapped with SoftReference, which can be GC'ed as needed. I confirmed > it worked as expected with a test case > (https://github.com/openjdk/jdk/blob/8b4e1e1cf8f5d753ed901406f73d67b21557fddb/test/jdk/java/lang/StringCoding/ResultCachedGCTest.java), > but decided not to include it in this PR. This is because I purposefully > chose the size of the byte array and max heap size which is fragile, and > could become noise in a regression test run. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2064
