Re: RFR: 8186958: Need method to create pre-sized HashMap [v7]
On Thu, 24 Mar 2022 17:43:31 GMT, XenoAmess wrote: >> 8186958: Need method to create pre-sized HashMap > > XenoAmess has updated the pull request incrementally with two additional > commits since the last revision: > > - update jmh > - refine javadoc; refine implement when expectedSize < 0 I'll sponsor this PR, and I can create a CSR as well. - PR: https://git.openjdk.java.net/jdk/pull/7928
Re: RFR: 8186958: Need method to create pre-sized HashMap [v7]
On Thu, 24 Mar 2022 17:43:31 GMT, XenoAmess wrote: >> 8186958: Need method to create pre-sized HashMap > > XenoAmess has updated the pull request incrementally with two additional > commits since the last revision: > > - update jmh > - refine javadoc; refine implement when expectedSize < 0 > /csr @ChrisHegarty @liach Hi. actually I don't know how to create a CSR request. I have no account on your internal jira. So I write an email to `csr-disc...@openjdk.java.net` , but no response. If there be anything more I should do now, please just tell me. Thanks a lot. - PR: https://git.openjdk.java.net/jdk/pull/7928
Re: RFR: 8186958: Need method to create pre-sized HashMap [v7]
On Sat, 26 Mar 2022 12:51:04 GMT, liach wrote: >>> You probably wanna allow for a non-NEW instance for the corner case where >>> the given size is 0 - no elements. >> >> @ChrisHegarty I guess we shouldn't. >> >> I want to make it 100% equals to `new HashMap()` constructor, thus migrate >> all usecases. >> >> So if we apply this, and when the original usage use this map object as a >> lock, or put some elements after the call(sometimes people cannot decide if >> they would really put elements in this map), bad things would happen. >> >> Besides, there already a function` Map.of()` for such functionality, so >> programmer should use that instead if they really want a shared empty map. > > hash maps are modifiable. empty instances can be changed, and returning such > shared instances are inherently unsafe. Yeah, sorry. Ignore my comment, I was wrong. I completely agree with your reasoning. - PR: https://git.openjdk.java.net/jdk/pull/7928
Re: RFR: 8282429: StringBuilder/StringBuffer.toString() skip compressing for UTF16 strings [v5]
On Wed, 23 Mar 2022 00:35:14 GMT, Xin Liu wrote: >> If AbstractStringBuilder only grow, the inflated value which has been >> encoded in UTF16 can't be compressed. >> toString() can skip compression in this case. This can save an >> ArrayAllocation in StringUTF16::compress(). >> >> java.io.BufferedRead::readLine() is a case that StringBuilder grows only. >> >> In microbench, we expect to see that allocation/op reduces 20%. The initial >> capacity of StringBuilder is S in bytes. When it encounters the 1st >> character that can't be encoded in LATIN1, it inflates and allocate a new >> array of 2*S. `toString()` will try to compress that value so it need to >> allocate S bytes. The last step allocates 2*S bytes because it has to copy >> the string. so it requires to allocate 5 * S bytes in total. By skipping >> the failed compression, it only allocates 4 * S bytes. that's 20%. In real >> execution, we observe 16% allocation reduction, tracked by JMH GC profiler >> `gc.alloc.rate.norm `. I think it's because HotSpot can't track all >> allocations. >> >> Not only allocation drops, the runtime performance(ns/op) also increases >> from 3.34% to 18.91%. >> >> Before: >> >> $$make test >> TEST="micro:org.openjdk.bench.java.lang.StringBuilders.toStringWithMixedChars" >> MICRO="OPTIONS=-prof gc -gc true -o before.log -jvm >> $HOME/Devel/jdk_baseline/bin/java" >> >> Benchmark >> (MIXED_SIZE) Mode Cnt Score Error Units >> StringBuilders.toStringWithMixedChars >> 128 avgt 15 649.846 ± 76.291 ns/op >> StringBuilders.toStringWithMixedChars:·gc.alloc.rate >> 128 avgt 15 872.855 ± 128.259 MB/sec >> StringBuilders.toStringWithMixedChars:·gc.alloc.rate.norm >> 128 avgt 15 880.121 ± 0.050B/op >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space >> 128 avgt 15 707.730 ± 194.421 MB/sec >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space.norm >> 128 avgt 15 706.602 ± 94.504B/op >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space >> 128 avgt 15 0.001 ± 0.002 MB/sec >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space.norm >> 128 avgt 15 0.001 ± 0.001B/op >> StringBuilders.toStringWithMixedChars:·gc.count >> 128 avgt 15 113.000counts >> StringBuilders.toStringWithMixedChars:·gc.time >> 128 avgt 1585.000ms >> StringBuilders.toStringWithMixedChars >> 256 avgt 15 1316.652 ± 112.771 ns/op >> StringBuilders.toStringWithMixedChars:·gc.alloc.rate >> 256 avgt 15 800.864 ± 76.869 MB/sec >> StringBuilders.toStringWithMixedChars:·gc.alloc.rate.norm >> 256 avgt 15 1648.288 ± 0.162B/op >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space >> 256 avgt 15 599.736 ± 174.001 MB/sec >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space.norm >> 256 avgt 15 1229.669 ± 318.518B/op >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space >> 256 avgt 15 0.001 ± 0.001 MB/sec >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space.norm >> 256 avgt 15 0.001 ± 0.002B/op >> StringBuilders.toStringWithMixedChars:·gc.count >> 256 avgt 15 133.000counts >> StringBuilders.toStringWithMixedChars:·gc.time >> 256 avgt 1592.000ms >> StringBuilders.toStringWithMixedChars >>1024 avgt 15 5204.303 ± 418.115 ns/op >> StringBuilders.toStringWithMixedChars:·gc.alloc.rate >>1024 avgt 15 768.730 ± 72.945 MB/sec >> StringBuilders.toStringWithMixedChars:·gc.alloc.rate.norm >>1024 avgt 15 6256.844 ± 0.358B/op >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space >>1024 avgt 15 655.852 ± 121.602 MB/sec >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space.norm >>1024 avgt 15 5315.265 ± 578.878B/op >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space >>1024 avgt 15 0.002 ± 0.002 MB/sec >> StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space.norm >>1024 avgt 15 0.014 ± 0.011B/op >> StringBuilders.toStringWithMixedChars:·gc.count >>1024 avgt 1596.000counts >> StringBuilders.toStringWithMixedChars:·gc.time
Re: RFR: 8282662: Use List/Set.of() factory methods to reduce memory consumption [v3]
On Thu, 10 Mar 2022 08:52:17 GMT, Сергей Цыпанов wrote: >> `List.of()` along with `Set.of()` create unmodifiable `List/Set` but with >> smaller footprint comparing to `Arrays.asList()` / `new HashSet()` when >> called with vararg of size 0, 1, 2. >> >> In general replacement of `Arrays.asList()` with `List.of()` is dubious as >> the latter is null-hostile, however in some cases we are sure that arguments >> are non-null. Into this PR I've included the following cases (in addition to >> those where the argument is proved to be non-null at compile-time): >> - `MethodHandles.longestParameterList()` never returns null >> - parameter types are never null >> - interfaces used for proxy construction and returned from >> `Class.getInterfaces()` are never null >> - exceptions types of method signature are never null > > Сергей Цыпанов has updated the pull request incrementally with one additional > commit since the last revision: > > 8282662: Revert dubious changes in MethodType There were changes with `Set.of()`, but I've reverted them as dubious. I'll rename the ticket and PR's title. As of the second question I didn't look namely for `Collections.addAll(set, elements)`, if I find any feasible for replacement with `Set.of()` then I'll add it. - PR: https://git.openjdk.java.net/jdk/pull/7729
Re: RFR: 8282664: Unroll by hand StringUTF16 and StringLatin1 polynomial hash loops [v2]
On Fri, 4 Mar 2022 17:44:44 GMT, Ludovic Henry wrote: >> Despite the hash value being cached for Strings, computing the hash still >> represents a significant CPU usage for applications handling lots of text. >> >> Even though it would be generally better to do it through an enhancement to >> the autovectorizer, the complexity of doing it by hand is trivial and the >> gain is sizable (2x speedup) even without the Vector API. The algorithm has >> been proposed by Richard Startin and Paul Sandoz [1]. >> >> Speedup are as follows on a `Intel(R) Xeon(R) E-2276G CPU @ 3.80GHz` >> >> >> Benchmark(size) Mode Cnt Score >>Error Units >> StringHashCode.Algorithm.scalarLatin1 0 avgt 25 2.111 >> ± 0.210 ns/op >> StringHashCode.Algorithm.scalarLatin1 1 avgt 25 3.500 >> ± 0.127 ns/op >> StringHashCode.Algorithm.scalarLatin110 avgt 25 7.001 >> ± 0.099 ns/op >> StringHashCode.Algorithm.scalarLatin1 100 avgt 2561.285 >> ± 0.444 ns/op >> StringHashCode.Algorithm.scalarLatin1 1000 avgt 25 628.995 >> ± 0.846 ns/op >> StringHashCode.Algorithm.scalarLatin1 1 avgt 25 6307.990 >> ± 4.071 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled16 0 avgt 25 2.358 >> ± 0.092 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled16 1 avgt 25 3.631 >> ± 0.159 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled16 10 avgt 25 7.049 >> ± 0.019 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled16 100 avgt 2533.626 >> ± 1.218 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled161000 avgt 25 317.811 >> ± 1.225 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled16 1 avgt 25 3212.333 >> ± 14.621 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled80 avgt 25 2.356 >> ± 0.097 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled81 avgt 25 3.630 >> ± 0.158 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled8 10 avgt 25 8.724 >> ± 0.065 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled8 100 avgt 2532.402 >> ± 0.019 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled8 1000 avgt 25 321.949 >> ± 0.251 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled81 avgt 25 3202.083 >> ± 1.667 ns/op >> StringHashCode.Algorithm.scalarUTF16 0 avgt 25 2.135 >> ± 0.191 ns/op >> StringHashCode.Algorithm.scalarUTF16 1 avgt 25 5.202 >> ± 0.362 ns/op >> StringHashCode.Algorithm.scalarUTF16 10 avgt 2511.105 >> ± 0.112 ns/op >> StringHashCode.Algorithm.scalarUTF16100 avgt 2575.974 >> ± 0.702 ns/op >> StringHashCode.Algorithm.scalarUTF16 1000 avgt 25 716.429 >> ± 3.290 ns/op >> StringHashCode.Algorithm.scalarUTF16 1 avgt 25 7095.459 >> ± 43.847 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled160 avgt 25 2.381 >> ± 0.038 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled161 avgt 25 5.268 >> ± 0.422 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled16 10 avgt 2511.248 >> ± 0.178 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled16 100 avgt 2552.966 >> ± 0.089 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled16 1000 avgt 25 450.912 >> ± 1.834 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled161 avgt 25 4403.988 >> ± 2.927 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled8 0 avgt 25 2.401 >> ± 0.032 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled8 1 avgt 25 5.091 >> ± 0.396 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled810 avgt 2512.801 >> ± 0.189 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled8 100 avgt 2552.068 >> ± 0.032 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled8 1000 avgt 25 453.270 >> ± 0.340 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled8 1 avgt 25 4433.112 >> ± 2.699 ns/op >> >> >> At Datadog, we handle a great amount of text (through logs management for >> example), and hashing String represents a large part of our CPU usage. It's >> very unlikely that we are the only one as String.hashCode is such a core >> feature of the JVM-based languages with its use in HashMap for example. >> Having even only a 2x speedup would allow us to save thousands of CPU cores >> per month and improve correspondingly the energy/carbon impact. >> >> [1] >> https://static.rainfocus.com/oracle/oow18/sess/1525822677955001tLqU/PF/codeone18-vector-API-DEV5081_1540354883936001Q3Sv.pdf > > Ludovic Henry has updated the pull request incrementally with one additional > commit since
Re: RFR: 8282662: Use List/Set.of() factory methods to reduce memory consumption [v3]
On Thu, 10 Mar 2022 08:52:17 GMT, Сергей Цыпанов wrote: >> `List.of()` along with `Set.of()` create unmodifiable `List/Set` but with >> smaller footprint comparing to `Arrays.asList()` / `new HashSet()` when >> called with vararg of size 0, 1, 2. >> >> In general replacement of `Arrays.asList()` with `List.of()` is dubious as >> the latter is null-hostile, however in some cases we are sure that arguments >> are non-null. Into this PR I've included the following cases (in addition to >> those where the argument is proved to be non-null at compile-time): >> - `MethodHandles.longestParameterList()` never returns null >> - parameter types are never null >> - interfaces used for proxy construction and returned from >> `Class.getInterfaces()` are never null >> - exceptions types of method signature are never null > > Сергей Цыпанов has updated the pull request incrementally with one additional > commit since the last revision: > > 8282662: Revert dubious changes in MethodType Just curious, this issue asks to replace set constructions with `Set.of`, but there is no such code changes in this pull request. Is there any set creation patterns that aren't detected by your ide cleanups, such as `Collections.addAll(set, elements)` for creating hash set contents from array? - PR: https://git.openjdk.java.net/jdk/pull/7729
Integrated: 8283720: ProblemList java/time/test/java/time/TestZoneOffset.java
On Sat, 26 Mar 2022 13:04:33 GMT, Daniel D. Daugherty wrote: > A trivial fix to ProblemList java/time/test/java/time/TestZoneOffset.java. This pull request has now been integrated. Changeset: c587b29b Author:Daniel D. Daugherty URL: https://git.openjdk.java.net/jdk/commit/c587b29bc9ca7e6d3879fda7df099b7411624f19 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod 8283720: ProblemList java/time/test/java/time/TestZoneOffset.java Reviewed-by: alanb - PR: https://git.openjdk.java.net/jdk/pull/7970
Re: RFR: 8283720: ProblemList java/time/test/java/time/TestZoneOffset.java
On Sat, 26 Mar 2022 13:25:00 GMT, Alan Bateman wrote: >> A trivial fix to ProblemList java/time/test/java/time/TestZoneOffset.java. > > Marked as reviewed by alanb (Reviewer). @AlanBateman - Thanks for the fast review! Especially for a Saturday... - PR: https://git.openjdk.java.net/jdk/pull/7970
Re: RFR: 8283720: ProblemList java/time/test/java/time/TestZoneOffset.java
On Sat, 26 Mar 2022 13:04:33 GMT, Daniel D. Daugherty wrote: > A trivial fix to ProblemList java/time/test/java/time/TestZoneOffset.java. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7970
RFR: 8283720: ProblemList java/time/test/java/time/TestZoneOffset.java
A trivial fix to ProblemList java/time/test/java/time/TestZoneOffset.java. - Commit messages: - 8283720: ProblemList java/time/test/java/time/TestZoneOffset.java Changes: https://git.openjdk.java.net/jdk/pull/7970/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7970=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8283720 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/7970.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7970/head:pull/7970 PR: https://git.openjdk.java.net/jdk/pull/7970
Re: RFR: 8186958: Need method to create pre-sized HashMap [v7]
On Sat, 26 Mar 2022 12:14:25 GMT, XenoAmess wrote: >> src/java.base/share/classes/java/util/HashMap.java line 2584: >> >>> 2582: >>> 2583: /** >>> 2584: * Creates a new, empty HashMap with an initial table size >> >> You probably wanna allow for a non-NEW instance for the corner case where >> the given size is 0 - no elements. > >> You probably wanna allow for a non-NEW instance for the corner case where >> the given size is 0 - no elements. > > @ChrisHegarty I guess we shouldn't. > > I want to make it 100% equals to `new HashMap()` constructor, thus migrate > all usecases. > > So if we apply this, and when the original usage use this map object as a > lock, or put some elements after the call(sometimes people cannot decide if > they would really put elements in this map), bad things would happen. > > Besides, there already a function` Map.of()` for such functionality, so > programmer should use that instead if they really want a shared empty map. hash maps are modifiable. empty instances can be changed, and returning such shared instances are inherently unsafe. - PR: https://git.openjdk.java.net/jdk/pull/7928
Re: RFR: 8186958: Need method to create pre-sized HashMap [v7]
On Sat, 26 Mar 2022 10:53:05 GMT, Chris Hegarty wrote: > You probably wanna allow for a non-NEW instance for the corner case where the > given size is 0 - no elements. @ChrisHegarty I guess we shouldn't. I want to make it 100% equals to `new HashMap()` constructor, thus migrate all usecases. So if we apply this, and when the original usage use this map object as a lock, or put some elements after the call(sometimes people cannot decide if they would really put elements in this map), bad things would happen. Besides, there already a function` Map.of()` for such functionality, so programmer should use that instead if they really want a shared empty map. - PR: https://git.openjdk.java.net/jdk/pull/7928
Re: RFR: 8282819: Deprecate Locale class constructors [v3]
On Fri, 25 Mar 2022 22:51:23 GMT, Naoto Sato wrote: >> Proposing to deprecate the constructors in the `java.util.Locale` class. >> There is already a factory method and a builder to return singletons, so >> there is no need to have constructors anymore unless one purposefully wants >> to create `ill-formed` Locale objects, which is discouraged. We cannot >> terminally deprecate those constructors for the compatibility to serialized >> objects created with older JDKs. Please see the draft CSR for more detail. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Fixed a build failure Hi Naoto, I think the changes look good. One suggestion As part of the PR, we should probably update test/jdk/java/util/Locale/LocaleTest.java. or perhaps add a new test for Locale.of() for completeness. - PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8186958: Need method to create pre-sized HashMap [v7]
On Thu, 24 Mar 2022 17:43:31 GMT, XenoAmess wrote: >> 8186958: Need method to create pre-sized HashMap > > XenoAmess has updated the pull request incrementally with two additional > commits since the last revision: > > - update jmh > - refine javadoc; refine implement when expectedSize < 0 This is a very nice addition. In Elasticsearch we have such API points, which are tedious to get right and test. src/java.base/share/classes/java/util/HashMap.java line 2584: > 2582: > 2583: /** > 2584: * Creates a new, empty HashMap with an initial table size You probably wanna allow for a non-NEW instance for the corner case where the given size is 0 - no elements. - PR: https://git.openjdk.java.net/jdk/pull/7928
Re: RFR: 8283683: Make ThreadLocalRandom a final class
On Fri, 25 Mar 2022 13:32:21 GMT, Jaikiran Pai wrote: > Can I please get a review of this change which marks the `ThreadLocalRandom` > class as `final`? Related JBS issue > https://bugs.openjdk.java.net/browse/JDK-8283683. > > A CSR has been filed too https://bugs.openjdk.java.net/browse/JDK-8283688. > > tier1, tier2 and tier3 tests have been run with this change and no related > failures have been noticed. LGTM. - Marked as reviewed by chegar (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7958
