Re: RFR: 8309130: x86_64 AVX512 intrinsics for Arrays.sort methods (int, long, float and double arrays) [v33]
On Thu, 31 Aug 2023 21:31:40 GMT, Srinivas Vamsi Parasa wrote: >> The goal is to develop faster sort routines for x86_64 CPUs by taking >> advantage of AVX512 instructions. This enhancement provides an order of >> magnitude speedup for Arrays.sort() using int, long, float and double arrays. >> >> This PR shows upto ~7x improvement for 32-bit datatypes (int, float) and >> upto ~4.5x improvement for 64-bit datatypes (long, double) as shown in the >> performance data below. >> >> >> **Arrays.sort performance data using JMH benchmarks for arrays with random >> data** >> >> |Arrays.sort benchmark | Array Size | Baseline >> (us/op)| AVX512 Sort (us/op) | Speedup | >> |--- | --- | --- | --- | --- >> | >> |ArraysSort.doubleSort | 10 | 0.034 | 0.035 >> | 1.0 | >> |ArraysSort.doubleSort | 25 | 0.116 | 0.089 >> | 1.3 | >> |ArraysSort.doubleSort | 50 | 0.282 | 0.291 >> | 1.0 | >> |ArraysSort.doubleSort | 75 | 0.474 | 0.358 >> | 1.3 | >> |ArraysSort.doubleSort | 100 | 0.654 | 0.623 >> | 1.0 | >> |ArraysSort.doubleSort | 1000| 9.274 | 6.331 >> | 1.5 | >> |ArraysSort.doubleSort | 1 | 323.339 | 71.228 >> | **4.5** | >> |ArraysSort.doubleSort | 10 | 4471.871| >> 1002.748| **4.5** | >> |ArraysSort.doubleSort | 100 | 51660.742 | >> 12921.295 | **4.0** | >> |ArraysSort.floatSort| 10 | 0.045 | 0.046 >> | 1.0 | >> |ArraysSort.floatSort| 25 | 0.103 | 0.084 >> | 1.2 | >> |ArraysSort.floatSort| 50 | 0.285 | 0.33 >> | 0.9 | >> |ArraysSort.floatSort| 75 | 0.492 | 0.346 >> | 1.4 | >> |ArraysSort.floatSort| 100 | 0.597 | 0.326 >> | 1.8 | >> |ArraysSort.floatSort| 1000| 9.811 | 5.294 >> | 1.9 | >> |ArraysSort.floatSort| 1 | 323.955 | 50.547 >> | **6.4** | >> |ArraysSort.floatSort| 10 | 4326.38 | 731.152 >> | **5.9** | >> |ArraysSort.floatSort| 100 | 52413.88| >> 8409.193| **6.2** | >> |ArraysSort.intSort | 10 | 0.033 | 0.033 >> | 1.0 | >> |ArraysSort.intSort | 25 | 0.086 | 0.051 >> | 1.7 | >> |ArraysSort.intSort | 50 | 0.236 | 0.151 >> | 1.6 | >> |ArraysSort.intSort | 75 | 0.416 | 0.332 >> | 1.3 | >> |ArraysSort.intSort | 100 | 0.63| 0.521 >> | 1.2 | >> |ArraysSort.intSort | 1000| 10.518 | 4.698 >> | 2.2 | >> |ArraysSort.intSort | 1 | 309.659 | 42.518 >> | **7.3** | >> |ArraysSort.intSort | 10 | 4130.917| >> 573.956 | **7.2** | >> |ArraysSort.intSort | 100 | 49876.307 | >> 6712.812| **7.4** | >> |ArraysSort.longSort | 10 | 0.036 | 0.037 >> | 1.0 | >> |ArraysSort.longSort | 25 | 0.094 | 0.08 >> | 1.2 | >> |ArraysSort.longSort | 50 | 0.218 | 0.227 >> | 1.0 | >> |ArraysSort.longSort | 75 | 0.466 | 0.402 >> | 1.2 | >> |ArraysSort.longSort | 100 | 0.76| 0.58 >> | 1.3 | >> |ArraysSort.longSort | 1000| 10.449 | 6 > > Srinivas Vamsi Parasa has updated the pull request incrementally with one > additional commit since the last revision: > > Change name of the avxsort library to libx86_64_sort Would it be possible to provide a clear summary on why libx86_64_sort is being added? I'm trying to understand why these weren't linked into libjvm. - PR Comment: https://git.openjdk.org/jdk/pull/14227#issuecomment-1711101611
Integrated: 8310929: Optimization for Integer.toString
On Wed, 28 Jun 2023 17:06:56 GMT, 温绍锦 wrote: > Optimization for: > Integer.toString > Long.toString > StringBuilder#append(int) > > # Benchmark Result > > > sh make/devkit/createJMHBundle.sh > bash configure --with-jmh=build/jmh/jars > make test TEST="micro:java.lang.Integers.toString*" > make test TEST="micro:java.lang.Longs.toString*" > make test TEST="micro:java.lang.StringBuilders.toStringCharWithInt8" > > > ## 1. > [aliyun_ecs_c8i.xlarge](https://help.aliyun.com/document_detail/25378.html#c8i) > * cpu : intel xeon sapphire rapids (x64) > > ``` diff > -Benchmark (size) Mode Cnt Score Error Units (baseline) > -Integers.toStringBig 500 avgt 15 6.825 ± 0.023 us/op > -Integers.toStringSmall 500 avgt 15 4.823 ± 0.023 us/op > -Integers.toStringTiny 500 avgt 15 3.878 ± 0.101 us/op > > +Benchmark (size) Mode Cnt Score Error Units (PR Update > 04 f4aa1989) > +Integers.toStringBig 500 avgt 15 6.002 ± 0.054 us/op (+13.71%) > +Integers.toStringSmall 500 avgt 15 4.025 ± 0.020 us/op (+19.82%) > +Integers.toStringTiny 500 avgt 15 3.874 ± 0.067 us/op (+0.10%) > > -Benchmark(size) Mode Cnt Score Error Units (baseline) > -Longs.toStringBig 500 avgt 15 9.224 ± 0.021 us/op > -Longs.toStringSmall 500 avgt 15 4.621 ± 0.087 us/op > > +Benchmark(size) Mode Cnt Score Error Units (PR Update 04 > f4aa1989) > +Longs.toStringBig 500 avgt 15 7.483 ± 0.018 us/op (+23.26%) > +Longs.toStringSmall 500 avgt 15 4.020 ± 0.016 us/op (+14.95%) > > -Benchmark Mode Cnt ScoreError Units > (baseline) > -StringBuilders.toStringCharWithInt8 avgt 1589.327 ± 0.733 ns/op > > +BenchmarkMode Cnt Score Error Units (PR > Update 04 f4aa1989) > +StringBuilders.toStringCharWithInt8 avgt 15 36.639 ± 0.422 ns/op > (+143.80%) > > > > ## 2. > [aliyun_ecs_c8a.xlarge](https://help.aliyun.com/document_detail/25378.html#c8a) > * cpu : amd epc genoa (x64) > > ``` diff > -Benchmark (size) Mode Cnt Score Error Units (baseline) > -Integers.toStringBig 500 avgt 15 6.753 ± 0.007 us/op > -Integers.toStringSmall 500 avgt 15 4.470 ± 0.005 us/op > -Integers.toStringTiny 500 avgt 15 2.764 ± 0.020 us/op > > +Benchmark (size) Mode Cnt Score Error Units (PR Update > 04 f4aa1989) > +Integers.toStringBig 500 avgt 15 5.036 ± 0.005 us/op (+34.09%) > +Integers.toStringSmall 500 avgt 15 3.491 ± 0.025 us/op (+28.04%) > +Integers.toStringTiny 5... This pull request has now been integrated. Changeset: 4b43c25f Author:shaojin.wensj Committer: Yi Yang URL: https://git.openjdk.org/jdk/commit/4b43c25fe382b5ee805a2d1b173fdd32d8da7fad Stats: 361 lines in 8 files changed: 189 ins; 146 del; 26 mod 8310929: Optimization for Integer.toString Reviewed-by: redestad, rriggs - PR: https://git.openjdk.org/jdk/pull/14699
Re: RFR: 8315789: Minor HexFormat performance improvements
On Wed, 6 Sep 2023 13:36:22 GMT, Claes Redestad wrote: > This PR seeks to improve formatting of hex digits using `java.util.HexFormat` > somewhat. > > This is achieved getting rid of a couple of lookup tables, caching the result > of `HexFormat.of().withUpperCase()`, and removing tiny allocation that > happens in the `formatHex(A, byte)` method. Improvements range from 20-40% on > throughput, and some operations allocate less: > > > Name Cnt Base Error Test Error Unit > Diff% > HexFormatBench.appenderLower15 1,330 ± 0,021 1,065 ± 0,067 us/op > 19,9% (p = 0,000*) > :gc.alloc.rate15 11,481 ± 0,185 0,007 ± 0,000 MB/sec > -99,9% (p = 0,000*) > :gc.alloc.rate.norm 15 16,009 ± 0,000 0,007 ± 0,000 B/op > -100,0% (p = 0,000*) > :gc.count 15 3,000 0,000 counts > :gc.time 3 2,000ms > HexFormatBench.appenderLowerCached 15 1,317 ± 0,013 1,065 ± 0,054 us/op > 19,1% (p = 0,000*) > :gc.alloc.rate15 11,590 ± 0,111 0,007 ± 0,000 MB/sec > -99,9% (p = 0,000*) > :gc.alloc.rate.norm 15 16,009 ± 0,000 0,007 ± 0,000 B/op > -100,0% (p = 0,000*) > :gc.count 15 3,000 0,000 counts > :gc.time 3 2,000ms > HexFormatBench.appenderUpper15 1,330 ± 0,022 1,065 ± 0,036 us/op > 19,9% (p = 0,000*) > :gc.alloc.rate15 34,416 ± 0,559 0,007 ± 0,000 MB/sec > -100,0% (p = 0,000*) > :gc.alloc.rate.norm 15 48,009 ± 0,000 0,007 ± 0,000 B/op > -100,0% (p = 0,000*) > :gc.count 15 0,000 0,000 counts > HexFormatBench.appenderUpperCached 15 1,353 ± 0,009 1,033 ± 0,014 us/op > 23,6% (p = 0,000*) > :gc.alloc.rate15 11,284 ± 0,074 0,007 ± 0,000 MB/sec > -99,9% (p = 0,000*) > :gc.alloc.rate.norm 15 16,009 ± 0,000 0,007 ± 0,000 B/op > -100,0% (p = 0,000*) > :gc.count 15 3,000 0,000 counts > :gc.time 3 2,000ms > HexFormatBench.toHexLower 15 0,198 ± 0,001 0,119 ± 0,008 us/op > 40,1% (p = 0,000*) > :gc.alloc.rate15 0,007 ± 0,000 0,007 ± 0,000 MB/sec > -0,0% (p = 0,816 ) > :gc.alloc.rate.norm 15 0,001 ± 0,000 0,001 ± 0,000 B/op > -40,1% (p = 0,000*) > :gc.count 15 0,000 0,000... Should we test the performance comparison of toHexDigits(long)? I've done some work before, and I didn't find a way to perform better than a lookup table without using a lookup table. - PR Comment: https://git.openjdk.org/jdk/pull/15591#issuecomment-1710944624
Re: RFR: 8287843: File::getCanonicalFile doesn't work for \?\C:\ style paths DOS device paths [v2]
> In the Windows implementation of java.io.File.getCanonicalPath, strip any > long path or UNC prefix before canonicalizing the remainder of the pathname. Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 8287843: Handle "\\?\UNC"; add bad paths to test - Changes: - all: https://git.openjdk.org/jdk/pull/15603/files - new: https://git.openjdk.org/jdk/pull/15603/files/0d4290cc..d9d84b8e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15603=01 - incr: https://webrevs.openjdk.org/?repo=jdk=15603=00-01 Stats: 16 lines in 2 files changed: 11 ins; 0 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/15603.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15603/head:pull/15603 PR: https://git.openjdk.org/jdk/pull/15603
Re: RFR: 8309130: x86_64 AVX512 intrinsics for Arrays.sort methods (int, long, float and double arrays) [v33]
On Thu, 31 Aug 2023 21:31:40 GMT, Srinivas Vamsi Parasa wrote: >> The goal is to develop faster sort routines for x86_64 CPUs by taking >> advantage of AVX512 instructions. This enhancement provides an order of >> magnitude speedup for Arrays.sort() using int, long, float and double arrays. >> >> This PR shows upto ~7x improvement for 32-bit datatypes (int, float) and >> upto ~4.5x improvement for 64-bit datatypes (long, double) as shown in the >> performance data below. >> >> >> **Arrays.sort performance data using JMH benchmarks for arrays with random >> data** >> >> |Arrays.sort benchmark | Array Size | Baseline >> (us/op)| AVX512 Sort (us/op) | Speedup | >> |--- | --- | --- | --- | --- >> | >> |ArraysSort.doubleSort | 10 | 0.034 | 0.035 >> | 1.0 | >> |ArraysSort.doubleSort | 25 | 0.116 | 0.089 >> | 1.3 | >> |ArraysSort.doubleSort | 50 | 0.282 | 0.291 >> | 1.0 | >> |ArraysSort.doubleSort | 75 | 0.474 | 0.358 >> | 1.3 | >> |ArraysSort.doubleSort | 100 | 0.654 | 0.623 >> | 1.0 | >> |ArraysSort.doubleSort | 1000| 9.274 | 6.331 >> | 1.5 | >> |ArraysSort.doubleSort | 1 | 323.339 | 71.228 >> | **4.5** | >> |ArraysSort.doubleSort | 10 | 4471.871| >> 1002.748| **4.5** | >> |ArraysSort.doubleSort | 100 | 51660.742 | >> 12921.295 | **4.0** | >> |ArraysSort.floatSort| 10 | 0.045 | 0.046 >> | 1.0 | >> |ArraysSort.floatSort| 25 | 0.103 | 0.084 >> | 1.2 | >> |ArraysSort.floatSort| 50 | 0.285 | 0.33 >> | 0.9 | >> |ArraysSort.floatSort| 75 | 0.492 | 0.346 >> | 1.4 | >> |ArraysSort.floatSort| 100 | 0.597 | 0.326 >> | 1.8 | >> |ArraysSort.floatSort| 1000| 9.811 | 5.294 >> | 1.9 | >> |ArraysSort.floatSort| 1 | 323.955 | 50.547 >> | **6.4** | >> |ArraysSort.floatSort| 10 | 4326.38 | 731.152 >> | **5.9** | >> |ArraysSort.floatSort| 100 | 52413.88| >> 8409.193| **6.2** | >> |ArraysSort.intSort | 10 | 0.033 | 0.033 >> | 1.0 | >> |ArraysSort.intSort | 25 | 0.086 | 0.051 >> | 1.7 | >> |ArraysSort.intSort | 50 | 0.236 | 0.151 >> | 1.6 | >> |ArraysSort.intSort | 75 | 0.416 | 0.332 >> | 1.3 | >> |ArraysSort.intSort | 100 | 0.63| 0.521 >> | 1.2 | >> |ArraysSort.intSort | 1000| 10.518 | 4.698 >> | 2.2 | >> |ArraysSort.intSort | 1 | 309.659 | 42.518 >> | **7.3** | >> |ArraysSort.intSort | 10 | 4130.917| >> 573.956 | **7.2** | >> |ArraysSort.intSort | 100 | 49876.307 | >> 6712.812| **7.4** | >> |ArraysSort.longSort | 10 | 0.036 | 0.037 >> | 1.0 | >> |ArraysSort.longSort | 25 | 0.094 | 0.08 >> | 1.2 | >> |ArraysSort.longSort | 50 | 0.218 | 0.227 >> | 1.0 | >> |ArraysSort.longSort | 75 | 0.466 | 0.402 >> | 1.2 | >> |ArraysSort.longSort | 100 | 0.76| 0.58 >> | 1.3 | >> |ArraysSort.longSort | 1000| 10.449 | 6 > > Srinivas Vamsi Parasa has updated the pull request incrementally with one > additional commit since the last revision: > > Change name of the avxsort library to libx86_64_sort Thank you for the summary. > Currently there is a regression of up to 20%, when the intrinsic is disabled. > This has been root caused to two reasons: > > * performance loss due to using a generalized intrinsic for all data > types (`int, long, float, double`) with multiple indirections and `switch()` > in the fallback method implementation. > Ok, i cannot guarantee the fallback lambda approach does not regress :-) > * performance loss due to passing a `pivotIndices` array to the partition > methods and modifying it in place. Passing the pivot indices explicitly
Re: RFR: 8311207: Cleanup for Optimization for UUID.toString [v12]
> [PR 14578 ](https://github.com/openjdk/jdk/pull/14578) still has unresolved > discussions, continue to make improvements. > > # Benchmark Result > > > sh make/devkit/createJMHBundle.sh > bash configure --with-jmh=build/jmh/jars > make test TEST="micro:java.util.UUIDBench.toString" > > > ## 1. > [aliyun_ecs_c8i.xlarge](https://help.aliyun.com/document_detail/25378.html#c8i) > * cpu : intel xeon sapphire rapids (x64) > > ``` diff > -Benchmark (size) Mode Cnt Score Error Units (baseline) > -UUIDBench.toString 2 thrpt 15 62.019 ± 0.622 ops/us > > +Benchmark (size) Mode Cnt Score Error Units > +UUIDBench.toString 2 thrpt 15 82.998 ± 0.739 ops/us (+33.82%) > > > ## 2. > [aliyun_ecs_c8a.xlarge](https://help.aliyun.com/document_detail/25378.html#c8a) > * cpu : amd epc genoa (x64) > > ``` diff > -Benchmark (size) Mode Cnt Score Error Units (baseline) > -UUIDBench.toString 2 thrpt 15 88.668 ± 0.672 ops/us > > +Benchmark (size) Mode Cnt Score Error Units > +UUIDBench.toString 2 thrpt 15 89.229 ± 0.271 ops/us (+0.63%) > > > > ## 3. > [aliyun_ecs_c8y.xlarge](https://help.aliyun.com/document_detail/25378.html#c8y) > * cpu : aliyun yitian 710 (aarch64) > ``` diff > -Benchmark (size) Mode Cnt Score Error Units (baseline) > -UUIDBench.toString 2 thrpt 15 49.382 ± 2.160 ops/us > > +Benchmark (size) Mode Cnt Score Error Units > +UUIDBench.toString 2 thrpt 15 49.636 ± 1.974 ops/us (+0.51%) > > > ## 4. MacBookPro M1 Pro > ``` diff > -Benchmark (size) Mode CntScore Error Units (baseline) > -UUIDBench.toString 2 thrpt 15 103.543 ± 0.963 ops/us > > +Benchmark (size) Mode CntScore Error Units > +UUIDBench.toString 2 thrpt 15 110.976 ± 0.685 ops/us (+7.17%) > > > ## 5. Orange Pi 5 Plus > > ``` diff > -Benchmark (size) Mode Cnt Score Error Units (baseline) > -UUIDBench.toString 2 thrpt 15 33.532 ± 0.396 ops/us > > +Benchmark (size) Mode Cnt Score Error Units (PR) > +UUIDBench.toString 2 thrpt 15 33.054 ± 0.190 ops/us (-4.42%) 温绍锦 has updated the pull request incrementally with one additional commit since the last revision: reversed how & hi - Changes: - all: https://git.openjdk.org/jdk/pull/14745/files - new: https://git.openjdk.org/jdk/pull/14745/files/2d36332b..747e8135 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14745=11 - incr: https://webrevs.openjdk.org/?repo=jdk=14745=10-11 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/14745.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14745/head:pull/14745 PR: https://git.openjdk.org/jdk/pull/14745
Re: RFR: 8315789: Minor HexFormat performance improvements
On Wed, 6 Sep 2023 13:36:22 GMT, Claes Redestad wrote: > This PR seeks to improve formatting of hex digits using `java.util.HexFormat` > somewhat. > > This is achieved getting rid of a couple of lookup tables, caching the result > of `HexFormat.of().withUpperCase()`, and removing tiny allocation that > happens in the `formatHex(A, byte)` method. Improvements range from 20-40% on > throughput, and some operations allocate less: > > > Name Cnt Base Error Test Error Unit > Diff% > HexFormatBench.appenderLower15 1,330 ± 0,021 1,065 ± 0,067 us/op > 19,9% (p = 0,000*) > :gc.alloc.rate15 11,481 ± 0,185 0,007 ± 0,000 MB/sec > -99,9% (p = 0,000*) > :gc.alloc.rate.norm 15 16,009 ± 0,000 0,007 ± 0,000 B/op > -100,0% (p = 0,000*) > :gc.count 15 3,000 0,000 counts > :gc.time 3 2,000ms > HexFormatBench.appenderLowerCached 15 1,317 ± 0,013 1,065 ± 0,054 us/op > 19,1% (p = 0,000*) > :gc.alloc.rate15 11,590 ± 0,111 0,007 ± 0,000 MB/sec > -99,9% (p = 0,000*) > :gc.alloc.rate.norm 15 16,009 ± 0,000 0,007 ± 0,000 B/op > -100,0% (p = 0,000*) > :gc.count 15 3,000 0,000 counts > :gc.time 3 2,000ms > HexFormatBench.appenderUpper15 1,330 ± 0,022 1,065 ± 0,036 us/op > 19,9% (p = 0,000*) > :gc.alloc.rate15 34,416 ± 0,559 0,007 ± 0,000 MB/sec > -100,0% (p = 0,000*) > :gc.alloc.rate.norm 15 48,009 ± 0,000 0,007 ± 0,000 B/op > -100,0% (p = 0,000*) > :gc.count 15 0,000 0,000 counts > HexFormatBench.appenderUpperCached 15 1,353 ± 0,009 1,033 ± 0,014 us/op > 23,6% (p = 0,000*) > :gc.alloc.rate15 11,284 ± 0,074 0,007 ± 0,000 MB/sec > -99,9% (p = 0,000*) > :gc.alloc.rate.norm 15 16,009 ± 0,000 0,007 ± 0,000 B/op > -100,0% (p = 0,000*) > :gc.count 15 3,000 0,000 counts > :gc.time 3 2,000ms > HexFormatBench.toHexLower 15 0,198 ± 0,001 0,119 ± 0,008 us/op > 40,1% (p = 0,000*) > :gc.alloc.rate15 0,007 ± 0,000 0,007 ± 0,000 MB/sec > -0,0% (p = 0,816 ) > :gc.alloc.rate.norm 15 0,001 ± 0,000 0,001 ± 0,000 B/op > -40,1% (p = 0,000*) > :gc.count 15 0,000 0,000... I took `ByteArrayLittleEndian` for a spin on a (new) micro for `String toHexDigits(byte)`, and that gives us ~4% (MacBook M1): Name Cnt BaseError TestError Unit Diff% HexFormatBench.toHexDigits 15 1,943 ± 0,014 1,862 ± 0,009 us/op 4,1% (p = 0,000*) * = significant I'm not sure 4% is a large enough win on a micro to motivate the lookup-table based approach. I'd rather see us investigate if we could consolidate these overlapping utility classes (`HexFormat` and `HexDigits`) on a lookup-table free solution. - PR Comment: https://git.openjdk.org/jdk/pull/15591#issuecomment-1710889268
Re: RFR: 8309130: x86_64 AVX512 intrinsics for Arrays.sort methods (int, long, float and double arrays) [v33]
On Thu, 31 Aug 2023 21:31:40 GMT, Srinivas Vamsi Parasa wrote: >> The goal is to develop faster sort routines for x86_64 CPUs by taking >> advantage of AVX512 instructions. This enhancement provides an order of >> magnitude speedup for Arrays.sort() using int, long, float and double arrays. >> >> This PR shows upto ~7x improvement for 32-bit datatypes (int, float) and >> upto ~4.5x improvement for 64-bit datatypes (long, double) as shown in the >> performance data below. >> >> >> **Arrays.sort performance data using JMH benchmarks for arrays with random >> data** >> >> |Arrays.sort benchmark | Array Size | Baseline >> (us/op)| AVX512 Sort (us/op) | Speedup | >> |--- | --- | --- | --- | --- >> | >> |ArraysSort.doubleSort | 10 | 0.034 | 0.035 >> | 1.0 | >> |ArraysSort.doubleSort | 25 | 0.116 | 0.089 >> | 1.3 | >> |ArraysSort.doubleSort | 50 | 0.282 | 0.291 >> | 1.0 | >> |ArraysSort.doubleSort | 75 | 0.474 | 0.358 >> | 1.3 | >> |ArraysSort.doubleSort | 100 | 0.654 | 0.623 >> | 1.0 | >> |ArraysSort.doubleSort | 1000| 9.274 | 6.331 >> | 1.5 | >> |ArraysSort.doubleSort | 1 | 323.339 | 71.228 >> | **4.5** | >> |ArraysSort.doubleSort | 10 | 4471.871| >> 1002.748| **4.5** | >> |ArraysSort.doubleSort | 100 | 51660.742 | >> 12921.295 | **4.0** | >> |ArraysSort.floatSort| 10 | 0.045 | 0.046 >> | 1.0 | >> |ArraysSort.floatSort| 25 | 0.103 | 0.084 >> | 1.2 | >> |ArraysSort.floatSort| 50 | 0.285 | 0.33 >> | 0.9 | >> |ArraysSort.floatSort| 75 | 0.492 | 0.346 >> | 1.4 | >> |ArraysSort.floatSort| 100 | 0.597 | 0.326 >> | 1.8 | >> |ArraysSort.floatSort| 1000| 9.811 | 5.294 >> | 1.9 | >> |ArraysSort.floatSort| 1 | 323.955 | 50.547 >> | **6.4** | >> |ArraysSort.floatSort| 10 | 4326.38 | 731.152 >> | **5.9** | >> |ArraysSort.floatSort| 100 | 52413.88| >> 8409.193| **6.2** | >> |ArraysSort.intSort | 10 | 0.033 | 0.033 >> | 1.0 | >> |ArraysSort.intSort | 25 | 0.086 | 0.051 >> | 1.7 | >> |ArraysSort.intSort | 50 | 0.236 | 0.151 >> | 1.6 | >> |ArraysSort.intSort | 75 | 0.416 | 0.332 >> | 1.3 | >> |ArraysSort.intSort | 100 | 0.63| 0.521 >> | 1.2 | >> |ArraysSort.intSort | 1000| 10.518 | 4.698 >> | 2.2 | >> |ArraysSort.intSort | 1 | 309.659 | 42.518 >> | **7.3** | >> |ArraysSort.intSort | 10 | 4130.917| >> 573.956 | **7.2** | >> |ArraysSort.intSort | 100 | 49876.307 | >> 6712.812| **7.4** | >> |ArraysSort.longSort | 10 | 0.036 | 0.037 >> | 1.0 | >> |ArraysSort.longSort | 25 | 0.094 | 0.08 >> | 1.2 | >> |ArraysSort.longSort | 50 | 0.218 | 0.227 >> | 1.0 | >> |ArraysSort.longSort | 75 | 0.466 | 0.402 >> | 1.2 | >> |ArraysSort.longSort | 100 | 0.76| 0.58 >> | 1.3 | >> |ArraysSort.longSort | 1000| 10.449 | 6 > > Srinivas Vamsi Parasa has updated the pull request incrementally with one > additional commit since the last revision: > > Change name of the avxsort library to libx86_64_sort Hello Paul, Thank you for reviewing the code and providing your suggestions! > Focusing on the Java changes, I am glad you managed to introduce intrinsics > into the main Java sorting loop, with leaf array sorting and array > partitioning thereby taking advantage of existing functionality. > Using the decomposed approach, which you initially suggested, it's now possible to speedup both `Arrays.sort()` and `Arrays.parallelSort()` as only the partition and leaf level sort methods are being intrinsified. > Initially i advised trying to make the intrinsic work across heap
Re: RFR: 8308995: Update Network IO JFR events to be static mirror events [v3]
On Tue, 22 Aug 2023 07:18:21 GMT, Alan Bateman wrote:
>> src/java.base/share/classes/java/net/Socket.java line 1133:
>>
>>> 1131: return parent.getSoTimeout();
>>> 1132: } catch (Throwable t) {
>>> 1133: // ignored - avoiding exceptions in jfr event data
>>> gathering
>>
>> This should be SocketException, not Throwable. That said, I think it would
>> be useful to know why the SocketReadEvent includes the timeout. Is this used
>> to see If read durations are close to the timeout? I assume once this code
>> is fixed to deal with the exceptional case that the need to include the
>> timeout for the success case will mostly go away.
>
> Were you able to find out what the timeout is used for?
No. I think it's a relic from the distant past though. I think the timeout
field should be removed. It's not used on SocketChannel at all, and it doesn't
seem useful on Socket.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14342#discussion_r1319152153
Re: RFR: 8308995: Update Network IO JFR events to be static mirror events [v5]
> The socket read/write JFR events currently use instrumentation of java.base > code using templates in the jdk.jfr modules. This results in some java.base > code residing in the jdk.jfr module which is undesirable. > > JDK19 added static support for event classes. The old instrumentor classes > should be replaced with mirror events using the static support. > > In the java.base module: > Added two new events, jdk.internal.event.SocketReadEvent and > jdk.internal.event.SocketWriteEvent. > java.net.Socket and sun.nio.ch.SocketChannelImpl were changed to make use of > the new events. > > In the jdk.jfr module: > jdk.jfr.events.SocketReadEvent and jdk.jfr.events.SocketWriteEvent were > changed to be mirror events. > In the package jdk.jfr.internal.instrument, the classes > SocketChannelImplInstrumentor, SocketInputStreamInstrumentor, and > SocketOutputStreamInstrumentor were removed. The JDKEvents class was updated > to reflect all of those changes. > > The existing tests in test/jdk/jdk/jfr/event/io continue to pass with the new > implementation: > Passed: jdk/jfr/event/io/TestSocketChannelEvents.java > Passed: jdk/jfr/event/io/TestSocketEvents.java > > I added a micro benchmark which measures the overhead of handling the jfr > socket events. > test/micro/org/openjdk/bench/java/net/SocketEventOverhead.java. > It needs access the jdk.internal.event package, which is done at runtime with > annotations that add the extra arguments. > At compile time the build arguments had to be augmented in > make/test/BuildMicrobenchmark.gmk Tim Prinzing has updated the pull request incrementally with one additional commit since the last revision: More changes from review: I didn't like the name of the helper method 'checkForCommit' because it doesn't indicate that it might commit the event. I also don't like 'commitEvent' because it might not. Since JFR events are sort of like a queue I went with a name from collections and called it 'offer' so using it is something like 'SocketReadEvent.offer(...)' which seems like it gets the idea across better. Also improved the javadoc for it. Removed the comments about being instrumented by JFR in Socket.SocketInputStream and Socket.SocketOutputStream. I went ahead and moved the event commiting out of the finally block so that we don't emit events when the read/write did not actually happen. The bugid JDK-8310979 will be used to determine if more should be done in this area. The implRead and implWrite were moved up with the other support methods for read/write. - Changes: - all: https://git.openjdk.org/jdk/pull/14342/files - new: https://git.openjdk.org/jdk/pull/14342/files/d6f7df72..9fa27459 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14342=04 - incr: https://webrevs.openjdk.org/?repo=jdk=14342=03-04 Stats: 151 lines in 5 files changed: 57 ins; 81 del; 13 mod Patch: https://git.openjdk.org/jdk/pull/14342.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14342/head:pull/14342 PR: https://git.openjdk.org/jdk/pull/14342
Integrated: 8268829: Provide an optimized way to walk the stack with Class object only
On Mon, 21 Aug 2023 20:07:20 GMT, Mandy Chung wrote: > 8268829: Provide an optimized way to walk the stack with Class object only > > `StackWalker::walk` creates one `StackFrame` per frame and the current > implementation > allocates one `StackFrameInfo` and one `MemberName` objects per frame. Some > frameworks > like logging may only interest in the Class object but not the method name > nor the BCI, > for example, filters out its implementation classes to find the caller class. > It's > similar to `StackWalker::getCallerClass` but allows a predicate to filter out > the element. > > This PR proposes to add `Option::DROP_METHOD_INFO` enum that requests to drop > the method information. If no method information is needed, a `StackWalker` > with `DROP_METHOD_INFO` > can be used instead and such stack walker will save the overhead of > extracting the method information > and the memory used for the stack walking. > > New factory methods to take a parameter to specify the kind of stack walker > to be created are defined. > This provides a simple way for existing code, for example logging frameworks, > to take advantage of > this enhancement with the least change as it can keep the existing function > for traversing > `StackFrame`s. > > For example: to find the first caller filtering a known list of > implementation class, > existing code can create a stack walker instance with `DROP_METHOD_INFO` > option: > > > StackWalker walker = StackWalker.getInstance(Option.DROP_METHOD_INFO, > Option.RETAIN_CLASS_REFERENCE); > Optional> callerClass = walker.walk(s -> > s.map(StackFrame::getDeclaringClass) > .filter(Predicate.not(implClasses::contains)) > .findFirst()); > > > If method information is accessed on the `StackFrame`s produced by this stack > walker such as > `StackFrame::getMethodName`, then `UnsupportedOperationException` will be > thrown. > > Javadoc & specdiff > > https://cr.openjdk.org/~mchung/api/java.base/java/lang/StackWalker.html > https://cr.openjdk.org/~mchung/jdk22/specdiff/overview-summary.html > > Alternatives Considered > One alternative is to provide a new API: > ` T walkClass(Function, ? extends T> function)` > > In this case, the caller would need to pass a function that takes a stream > of `Class` object instead of `StackFrame`. Existing code would have to > modify calls to the `walk` method to `walkClass` and the function body. > > ### Implementation Details > > A `StackWalker` configured with `DROP_METHOD_INFO` option creates > `ClassFrameInfo[]` > buffer that is filled by the VM during stack walking. `Sta... This pull request has now been integrated. Changeset: 111ecdba Author:Mandy Chung URL: https://git.openjdk.org/jdk/commit/111ecdbaf58e5c0b3a64e0eca8a291df295e71b0 Stats: 1340 lines in 34 files changed: 718 ins; 358 del; 264 mod 8268829: Provide an optimized way to walk the stack with Class object only 8210375: StackWalker::getCallerClass throws UnsupportedOperationException Reviewed-by: coleenp, dfuchs, bchristi - PR: https://git.openjdk.org/jdk/pull/15370
Re: RFR: 6228794: java.text.ChoiceFormat pattern behavior is not well documented. [v2]
On Thu, 7 Sep 2023 19:35:06 GMT, Naoto Sato wrote: > This should be excluded from the pattern syntax I presume you meant that the syntax should _include_ the required ascending order of limits. Updated to make this apparent, I also adjusted the syntax a little bit to be more accurate. The previous syntax could technically have an empty _Number_ - PR Review Comment: https://git.openjdk.org/jdk/pull/15392#discussion_r1319085087
Re: RFR: 6228794: java.text.ChoiceFormat pattern behavior is not well documented. [v4]
> Please review this PR and associated > [CSR](https://bugs.openjdk.org/browse/JDK-8314546) which expands on the > `java.text.ChoiceFormat` specification regarding its pattern. > > `j.text.ChoiceFormat` provides an example pattern in the class description, > but beyond that it does not specify any well-defined syntax for creating a > pattern. In addition, methods related to the pattern String are > under-specified. > > The wording for `getLimits()` and `getFormats()` was also adjusted, as there > are other ways to set the limits and formats beyond the constructor. > > The pattern syntax may be easier to view -> > https://cr.openjdk.org/~jlu/api/java.base/java/text/ChoiceFormat.html Justin Lu has updated the pull request incrementally with one additional commit since the last revision: Review: Make ascending format clear, improve syntax, remove unicode sequences - Changes: - all: https://git.openjdk.org/jdk/pull/15392/files - new: https://git.openjdk.org/jdk/pull/15392/files/bbb99eb4..e3dafe6e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15392=03 - incr: https://webrevs.openjdk.org/?repo=jdk=15392=02-03 Stats: 5 lines in 1 file changed: 1 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/15392.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15392/head:pull/15392 PR: https://git.openjdk.org/jdk/pull/15392
Re: RFR: 8309130: x86_64 AVX512 intrinsics for Arrays.sort methods (int, long, float and double arrays) [v33]
On Thu, 31 Aug 2023 21:31:40 GMT, Srinivas Vamsi Parasa wrote: >> The goal is to develop faster sort routines for x86_64 CPUs by taking >> advantage of AVX512 instructions. This enhancement provides an order of >> magnitude speedup for Arrays.sort() using int, long, float and double arrays. >> >> This PR shows upto ~7x improvement for 32-bit datatypes (int, float) and >> upto ~4.5x improvement for 64-bit datatypes (long, double) as shown in the >> performance data below. >> >> >> **Arrays.sort performance data using JMH benchmarks for arrays with random >> data** >> >> |Arrays.sort benchmark | Array Size | Baseline >> (us/op)| AVX512 Sort (us/op) | Speedup | >> |--- | --- | --- | --- | --- >> | >> |ArraysSort.doubleSort | 10 | 0.034 | 0.035 >> | 1.0 | >> |ArraysSort.doubleSort | 25 | 0.116 | 0.089 >> | 1.3 | >> |ArraysSort.doubleSort | 50 | 0.282 | 0.291 >> | 1.0 | >> |ArraysSort.doubleSort | 75 | 0.474 | 0.358 >> | 1.3 | >> |ArraysSort.doubleSort | 100 | 0.654 | 0.623 >> | 1.0 | >> |ArraysSort.doubleSort | 1000| 9.274 | 6.331 >> | 1.5 | >> |ArraysSort.doubleSort | 1 | 323.339 | 71.228 >> | **4.5** | >> |ArraysSort.doubleSort | 10 | 4471.871| >> 1002.748| **4.5** | >> |ArraysSort.doubleSort | 100 | 51660.742 | >> 12921.295 | **4.0** | >> |ArraysSort.floatSort| 10 | 0.045 | 0.046 >> | 1.0 | >> |ArraysSort.floatSort| 25 | 0.103 | 0.084 >> | 1.2 | >> |ArraysSort.floatSort| 50 | 0.285 | 0.33 >> | 0.9 | >> |ArraysSort.floatSort| 75 | 0.492 | 0.346 >> | 1.4 | >> |ArraysSort.floatSort| 100 | 0.597 | 0.326 >> | 1.8 | >> |ArraysSort.floatSort| 1000| 9.811 | 5.294 >> | 1.9 | >> |ArraysSort.floatSort| 1 | 323.955 | 50.547 >> | **6.4** | >> |ArraysSort.floatSort| 10 | 4326.38 | 731.152 >> | **5.9** | >> |ArraysSort.floatSort| 100 | 52413.88| >> 8409.193| **6.2** | >> |ArraysSort.intSort | 10 | 0.033 | 0.033 >> | 1.0 | >> |ArraysSort.intSort | 25 | 0.086 | 0.051 >> | 1.7 | >> |ArraysSort.intSort | 50 | 0.236 | 0.151 >> | 1.6 | >> |ArraysSort.intSort | 75 | 0.416 | 0.332 >> | 1.3 | >> |ArraysSort.intSort | 100 | 0.63| 0.521 >> | 1.2 | >> |ArraysSort.intSort | 1000| 10.518 | 4.698 >> | 2.2 | >> |ArraysSort.intSort | 1 | 309.659 | 42.518 >> | **7.3** | >> |ArraysSort.intSort | 10 | 4130.917| >> 573.956 | **7.2** | >> |ArraysSort.intSort | 100 | 49876.307 | >> 6712.812| **7.4** | >> |ArraysSort.longSort | 10 | 0.036 | 0.037 >> | 1.0 | >> |ArraysSort.longSort | 25 | 0.094 | 0.08 >> | 1.2 | >> |ArraysSort.longSort | 50 | 0.218 | 0.227 >> | 1.0 | >> |ArraysSort.longSort | 75 | 0.466 | 0.402 >> | 1.2 | >> |ArraysSort.longSort | 100 | 0.76| 0.58 >> | 1.3 | >> |ArraysSort.longSort | 1000| 10.449 | 6 > > Srinivas Vamsi Parasa has updated the pull request incrementally with one > additional commit since the last revision: > > Change name of the avxsort library to libx86_64_sort Focusing on the Java changes, I am glad you managed to introduce intrinsics into the main Java sorting loop, with leaf array sorting and array partitioning thereby taking advantage of existing functionality. Initially i advised trying to make the intrinsic work across heap and off-heap data, e.g., sorting `MemorySegment`. I still think that is a worthy goal, but we don't need to do that now, but I still think intrinsics should be base+offset shaped in preparation for that. I think we need to summarize future planned steps, some i believe were discussed with regards to
Re: RFR: 8311207: Cleanup for Optimization for UUID.toString [v11]
On Tue, 5 Sep 2023 15:48:21 GMT, 温绍锦 wrote:
>> [PR 14578 ](https://github.com/openjdk/jdk/pull/14578) still has unresolved
>> discussions, continue to make improvements.
>>
>> # Benchmark Result
>>
>>
>> sh make/devkit/createJMHBundle.sh
>> bash configure --with-jmh=build/jmh/jars
>> make test TEST="micro:java.util.UUIDBench.toString"
>>
>>
>> ## 1.
>> [aliyun_ecs_c8i.xlarge](https://help.aliyun.com/document_detail/25378.html#c8i)
>> * cpu : intel xeon sapphire rapids (x64)
>>
>> ``` diff
>> -Benchmark (size) Mode Cnt Score Error Units (baseline)
>> -UUIDBench.toString 2 thrpt 15 62.019 ± 0.622 ops/us
>>
>> +Benchmark (size) Mode Cnt Score Error Units
>> +UUIDBench.toString 2 thrpt 15 82.998 ± 0.739 ops/us (+33.82%)
>>
>>
>> ## 2.
>> [aliyun_ecs_c8a.xlarge](https://help.aliyun.com/document_detail/25378.html#c8a)
>> * cpu : amd epc genoa (x64)
>>
>> ``` diff
>> -Benchmark (size) Mode Cnt Score Error Units (baseline)
>> -UUIDBench.toString 2 thrpt 15 88.668 ± 0.672 ops/us
>>
>> +Benchmark (size) Mode Cnt Score Error Units
>> +UUIDBench.toString 2 thrpt 15 89.229 ± 0.271 ops/us (+0.63%)
>>
>>
>>
>> ## 3.
>> [aliyun_ecs_c8y.xlarge](https://help.aliyun.com/document_detail/25378.html#c8y)
>> * cpu : aliyun yitian 710 (aarch64)
>> ``` diff
>> -Benchmark (size) Mode Cnt Score Error Units (baseline)
>> -UUIDBench.toString 2 thrpt 15 49.382 ± 2.160 ops/us
>>
>> +Benchmark (size) Mode Cnt Score Error Units
>> +UUIDBench.toString 2 thrpt 15 49.636 ± 1.974 ops/us (+0.51%)
>>
>>
>> ## 4. MacBookPro M1 Pro
>> ``` diff
>> -Benchmark (size) Mode CntScore Error Units (baseline)
>> -UUIDBench.toString 2 thrpt 15 103.543 ± 0.963 ops/us
>>
>> +Benchmark (size) Mode CntScore Error Units
>> +UUIDBench.toString 2 thrpt 15 110.976 ± 0.685 ops/us (+7.17%)
>>
>>
>> ## 5. Orange Pi 5 Plus
>>
>> ``` diff
>> -Benchmark (size) Mode Cnt Score Error Units (baseline)
>> -UUIDBench.toString 2 thrpt 15 33.532 ± 0.396 ops/us
>>
>> +Benchmark (size) Mode Cnt Score Error Units (PR)
>> +UUIDBench.toString 2 thrpt 15 33.054 ± 0.190 ops/us (-4.42%)
>
> 温绍锦 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 14 additional commits since the
> last revision:
>
> - Merge branch 'master' into optimization_for_uuid_to_string
> - remove redundant parentheses
> - fix java doc, big-endian -> little-endian
> - Merge branch 'master' into optimization_for_uuid_to_string
> - use ByteArrayLittleEndian
> - fix typo
> - code style
> - Explain the rationale
>
>Co-authored-by: liach
> - private static final Unsafe
> - revert to the previous version
> - ... and 4 more: https://git.openjdk.org/jdk/compare/b801e29e...2d36332b
src/java.base/share/classes/java/util/HexDigits.java line 54:
> 52: for (int j = 0; j < 16; j++) {
> 53: short lo = (short) ((j < 10 ? j + '0' : j - 10 + 'a') <<
> 8);
> 54: digits[(i << 4) + j] = (short) (hi | lo);
The variable naming and this `hi | lo` expression are counter-intuitive; they
should be reversed to be consistent with the little-endian bit layout.
A description of the DIGITS array layout and indexing is needed; it will help
with the comprehension of the masking and shifts in the packDigits methods.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14745#discussion_r1318950015
Re: RFR: 6228794: java.text.ChoiceFormat pattern behavior is not well documented. [v2]
On Thu, 7 Sep 2023 17:18:33 GMT, Justin Lu wrote:
>> src/java.base/share/classes/java/text/ChoiceFormat.java line 136:
>>
>>> 134: * Limit Relation Format
>>> 135: * Limit:
>>> 136: * {@code String} that can be parsed as a {@code double} /
>>> "" ({@code U+221E}) / "-" (-{@code U+221E}).
>>
>> Could the first choice be more specific? Also, do `SubPattern`s need to be
>> sorted with `Limit`s?
>
>> Could the first choice be more specific?
>
> Updated with a more syntactical definition. Please let me know if you think
> this is _too_ specific.
>
>> Also, do SubPatterns need to be sorted with Limits?
>
> If I am interpreting the question right, you're asking if a subPattern
> **needs** a limit to be valid. If that's the question, the answer is yes,
> `ChoiceFormat` is designed to have an equal amount of limits and formats
> (whether from the arrays, or the String pattern). But please let me know if I
> misinterpreted the question.
>
> subpattern "xyz" where both _limit_ and _relation_ are omitted
>
> jshell> var a = new ChoiceFormat("0#abc|xyz")
> a ==> java.text.ChoiceFormat@17863
> jshell> a.getLimits()
> $21 ==> double[1] { 0.0 }
> jshell> a.getFormats()
> $22 ==> String[1] { "abc" }
>
>
> subpattern "#xyz" where _limit_ is omitted
>
> jshell> var b = new ChoiceFormat("0#abc|#xyz")
> | Exception java.lang.IllegalArgumentException: Each interval must contain a
> number before a format
Please let me know if you think this is too specific.
I think the updated one is better.
For the second part, I meant:
jshell> new ChoiceFormat("0#def|-1#abc|xyz")
| Exception java.lang.IllegalArgumentException: Incorrect order of intervals,
must be in ascending order
This should be excluded from the pattern syntax
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15392#discussion_r1319041104
Re: RFR: 8268829: Provide an optimized way to walk the stack with Class object only [v12]
On Thu, 7 Sep 2023 19:27:14 GMT, Mandy Chung wrote: >> 8268829: Provide an optimized way to walk the stack with Class object only >> >> `StackWalker::walk` creates one `StackFrame` per frame and the current >> implementation >> allocates one `StackFrameInfo` and one `MemberName` objects per frame. Some >> frameworks >> like logging may only interest in the Class object but not the method name >> nor the BCI, >> for example, filters out its implementation classes to find the caller >> class. It's >> similar to `StackWalker::getCallerClass` but allows a predicate to filter >> out the element. >> >> This PR proposes to add `Option::DROP_METHOD_INFO` enum that requests to >> drop the method information. If no method information is needed, a >> `StackWalker` with `DROP_METHOD_INFO` >> can be used instead and such stack walker will save the overhead of >> extracting the method information >> and the memory used for the stack walking. >> >> New factory methods to take a parameter to specify the kind of stack walker >> to be created are defined. >> This provides a simple way for existing code, for example logging >> frameworks, to take advantage of >> this enhancement with the least change as it can keep the existing function >> for traversing >> `StackFrame`s. >> >> For example: to find the first caller filtering a known list of >> implementation class, >> existing code can create a stack walker instance with `DROP_METHOD_INFO` >> option: >> >> >> StackWalker walker = StackWalker.getInstance(Option.DROP_METHOD_INFO, >> Option.RETAIN_CLASS_REFERENCE); >> Optional> callerClass = walker.walk(s -> >> s.map(StackFrame::getDeclaringClass) >> .filter(Predicate.not(implClasses::contains)) >> .findFirst()); >> >> >> If method information is accessed on the `StackFrame`s produced by this >> stack walker such as >> `StackFrame::getMethodName`, then `UnsupportedOperationException` will be >> thrown. >> >> Javadoc & specdiff >> >> https://cr.openjdk.org/~mchung/api/java.base/java/lang/StackWalker.html >> https://cr.openjdk.org/~mchung/jdk22/specdiff/overview-summary.html >> >> Alternatives Considered >> One alternative is to provide a new API: >> ` T walkClass(Function, ? extends T> function)` >> >> In this case, the caller would need to pass a function that takes a stream >> of `Class` object instead of `StackFrame`. Existing code would have to >> modify calls to the `walk` method to `walkClass` and the function body. >> >> ### Implementation Details >> >> A `StackWalker` configured with `DROP_METHOD_INFO` ... > > Mandy Chung has updated the pull request incrementally with one additional > commit since the last revision: > > Fix @Param due to the rename from default to class+method Marked as reviewed by dfuchs (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/15370#pullrequestreview-1616112038
Re: RFR: 8268829: Provide an optimized way to walk the stack with Class object only [v12]
> 8268829: Provide an optimized way to walk the stack with Class object only > > `StackWalker::walk` creates one `StackFrame` per frame and the current > implementation > allocates one `StackFrameInfo` and one `MemberName` objects per frame. Some > frameworks > like logging may only interest in the Class object but not the method name > nor the BCI, > for example, filters out its implementation classes to find the caller class. > It's > similar to `StackWalker::getCallerClass` but allows a predicate to filter out > the element. > > This PR proposes to add `Option::DROP_METHOD_INFO` enum that requests to drop > the method information. If no method information is needed, a `StackWalker` > with `DROP_METHOD_INFO` > can be used instead and such stack walker will save the overhead of > extracting the method information > and the memory used for the stack walking. > > New factory methods to take a parameter to specify the kind of stack walker > to be created are defined. > This provides a simple way for existing code, for example logging frameworks, > to take advantage of > this enhancement with the least change as it can keep the existing function > for traversing > `StackFrame`s. > > For example: to find the first caller filtering a known list of > implementation class, > existing code can create a stack walker instance with `DROP_METHOD_INFO` > option: > > > StackWalker walker = StackWalker.getInstance(Option.DROP_METHOD_INFO, > Option.RETAIN_CLASS_REFERENCE); > Optional> callerClass = walker.walk(s -> > s.map(StackFrame::getDeclaringClass) > .filter(Predicate.not(implClasses::contains)) > .findFirst()); > > > If method information is accessed on the `StackFrame`s produced by this stack > walker such as > `StackFrame::getMethodName`, then `UnsupportedOperationException` will be > thrown. > > Javadoc & specdiff > > https://cr.openjdk.org/~mchung/api/java.base/java/lang/StackWalker.html > https://cr.openjdk.org/~mchung/jdk22/specdiff/overview-summary.html > > Alternatives Considered > One alternative is to provide a new API: > ` T walkClass(Function, ? extends T> function)` > > In this case, the caller would need to pass a function that takes a stream > of `Class` object instead of `StackFrame`. Existing code would have to > modify calls to the `walk` method to `walkClass` and the function body. > > ### Implementation Details > > A `StackWalker` configured with `DROP_METHOD_INFO` option creates > `ClassFrameInfo[]` > buffer that is filled by the VM during stack walking. `Sta... Mandy Chung has updated the pull request incrementally with one additional commit since the last revision: Fix @Param due to the rename from default to class+method - Changes: - all: https://git.openjdk.org/jdk/pull/15370/files - new: https://git.openjdk.org/jdk/pull/15370/files/0e6abc42..c3746f5c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15370=11 - incr: https://webrevs.openjdk.org/?repo=jdk=15370=10-11 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/15370.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15370/head:pull/15370 PR: https://git.openjdk.org/jdk/pull/15370
Re: RFR: 8268829: Provide an optimized way to walk the stack with Class object only [v11]
On Thu, 7 Sep 2023 18:51:44 GMT, Daniel Fuchs wrote:
>> Mandy Chung has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> review feedback
>
> test/micro/org/openjdk/bench/java/lang/StackWalkBench.java line 60:
>
>> 58: static StackWalker walker(String name) {
>> 59: return switch (name) {
>> 60: case "class+method" -> WALKER;
>
> don't you need to also change "default" into "class+method" in the
> `@Param({"default", "class_only"})` annotations below?
thanks for catching it. will fix.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15370#discussion_r1319009375
Re: RFR: 8312522: Implementation of Foreign Function & Memory API [v18]
> This patch contains the implementation of the foreign linker & memory API JEP > for Java 22. The initial patch is composed of commits brought over directly > from the [panama-foreign repo](https://github.com/openjdk/panama-foreign). > The main changes found in this patch come from the following PRs: > > 1. https://github.com/openjdk/panama-foreign/pull/836 Where previous > iterations supported converting Java strings to and from native strings in > the UTF-8 encoding, we've extended the supported encoding to all the > encodings found in the `java.nio.charset.StandardCharsets` class. > 2. https://github.com/openjdk/panama-foreign/pull/838 We dropped the > `MemoryLayout::sequenceLayout` factory method which inferred the size of the > sequence to be `Long.MAX_VALUE`, as this led to confusion among clients. A > client is now required to explicitly specify the sequence size. > 3. https://github.com/openjdk/panama-foreign/pull/839 A new API was added: > `Linker::canonicalLayouts`, which exposes a map containing the > platform-specific mappings of common C type names to memory layouts. > 4. https://github.com/openjdk/panama-foreign/pull/840 Memory access > varhandles, as well as byte offset and slice handles derived from memory > layouts, now feature an additional 'base offset' coordinate that is added to > the offset computed by the handle. This allows composing these handles with > other offset computation strategies that may not be based on the same memory > layout. This addresses use-cases where clients are working with 'dynamic' > layouts, whose size might not be known statically, such as variable length > arrays, or variable size matrices. > 5. https://github.com/openjdk/panama-foreign/pull/841 Remove this now > redundant API. Clients can simply use the difference between the base address > of two memory segments. > 6. https://github.com/openjdk/panama-foreign/pull/845 Disambiguate uses of > `SegmentAllocator::allocateArray`, by renaming methods that both allocate + > initialize memory segments to `allocateFrom`. (see the original PR for the > problematic case) > 7. https://github.com/openjdk/panama-foreign/pull/846 Improve the > documentation for variadic functions. > 8. https://github.com/openjdk/panama-foreign/pull/849 Several test fixes to > make sure the `jdk_foreign` tests can pass on 32-bit machines, taking > linux-x86 as a test bed. > 9. https://github.com/openjdk/panama-foreign/pull/850 Make the linker API > required. The `Linker::nativeLinker` method is not longer allowed to throw an > `UnsupportedOperationException` on ... Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: add code snippet - Changes: - all: https://git.openjdk.org/jdk/pull/15103/files - new: https://git.openjdk.org/jdk/pull/15103/files/2f50adbf..86a7e227 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15103=17 - incr: https://webrevs.openjdk.org/?repo=jdk=15103=16-17 Stats: 6 lines in 1 file changed: 5 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/15103.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15103/head:pull/15103 PR: https://git.openjdk.org/jdk/pull/15103
Re: RFR: 8315850: Improve AbstractMap anonymous Iterator classes
On Thu, 7 Sep 2023 18:05:51 GMT, Stuart Marks wrote: >> Hello, >> In Java, sharing code may have a runtime cost (or not) depending on the >> shape of the code, because the VM may have to speculate on the class of some >> value at runtime. >> Here, in hasNext() and remove(), the VM has to find the class of the field >> 'i' at runtime. >> >> Iterators are performance sentive (they are used in for loop) and for that >> they need the escape analysis to work. >> If the iterator code is polymorphic, so part of the code may be unknown so >> the escape analysis will consider that the iterator escape. So usually, it's >> a good practice to not share the iterator code. >> >> Also the field should be package private (or even maybe private) but not >> protected. Protected is very rare in Java because usually classes that share >> implementation details are in the same package (or in the same nestmate >> nest). > > @forax Note that HashMap uses a similar subclassing idiom for the iterators > of the keySet, values, and entrySet collections in order to share the > iterator logic: > > https://github.com/openjdk/jdk/blob/jdk-21%2B35/src/java.base/share/classes/java/util/HashMap.java#L1626 Hi @stuart-marks , for HashMap, all the fields of HashIterator have only one implementation, in this patch, the field of AbstractIterator is typed Iterator (so multiple implementations at runtime). This is not the same code shape. I think the best is to write a simple to test that iterate over both the keySet() and the values() of an implementation of AbstractMap that only defines size() and get() and use -XX:+PrintInlining to see if the VM reports a profile pollution for the code before and after this change. - PR Comment: https://git.openjdk.org/jdk/pull/15615#issuecomment-1710632697
Re: RFR: 8268829: Provide an optimized way to walk the stack with Class object only [v11]
On Thu, 7 Sep 2023 18:22:40 GMT, Mandy Chung wrote:
>> 8268829: Provide an optimized way to walk the stack with Class object only
>>
>> `StackWalker::walk` creates one `StackFrame` per frame and the current
>> implementation
>> allocates one `StackFrameInfo` and one `MemberName` objects per frame. Some
>> frameworks
>> like logging may only interest in the Class object but not the method name
>> nor the BCI,
>> for example, filters out its implementation classes to find the caller
>> class. It's
>> similar to `StackWalker::getCallerClass` but allows a predicate to filter
>> out the element.
>>
>> This PR proposes to add `Option::DROP_METHOD_INFO` enum that requests to
>> drop the method information. If no method information is needed, a
>> `StackWalker` with `DROP_METHOD_INFO`
>> can be used instead and such stack walker will save the overhead of
>> extracting the method information
>> and the memory used for the stack walking.
>>
>> New factory methods to take a parameter to specify the kind of stack walker
>> to be created are defined.
>> This provides a simple way for existing code, for example logging
>> frameworks, to take advantage of
>> this enhancement with the least change as it can keep the existing function
>> for traversing
>> `StackFrame`s.
>>
>> For example: to find the first caller filtering a known list of
>> implementation class,
>> existing code can create a stack walker instance with `DROP_METHOD_INFO`
>> option:
>>
>>
>> StackWalker walker = StackWalker.getInstance(Option.DROP_METHOD_INFO,
>> Option.RETAIN_CLASS_REFERENCE);
>> Optional> callerClass = walker.walk(s ->
>> s.map(StackFrame::getDeclaringClass)
>> .filter(Predicate.not(implClasses::contains))
>> .findFirst());
>>
>>
>> If method information is accessed on the `StackFrame`s produced by this
>> stack walker such as
>> `StackFrame::getMethodName`, then `UnsupportedOperationException` will be
>> thrown.
>>
>> Javadoc & specdiff
>>
>> https://cr.openjdk.org/~mchung/api/java.base/java/lang/StackWalker.html
>> https://cr.openjdk.org/~mchung/jdk22/specdiff/overview-summary.html
>>
>> Alternatives Considered
>> One alternative is to provide a new API:
>> ` T walkClass(Function, ? extends T> function)`
>>
>> In this case, the caller would need to pass a function that takes a stream
>> of `Class` object instead of `StackFrame`. Existing code would have to
>> modify calls to the `walk` method to `walkClass` and the function body.
>>
>> ### Implementation Details
>>
>> A `StackWalker` configured with `DROP_METHOD_INFO` ...
>
> Mandy Chung has updated the pull request incrementally with one additional
> commit since the last revision:
>
> review feedback
Changes requested by dfuchs (Reviewer).
test/micro/org/openjdk/bench/java/lang/StackWalkBench.java line 60:
> 58: static StackWalker walker(String name) {
> 59: return switch (name) {
> 60: case "class+method" -> WALKER;
don't you need to also change "default" into "class+method" in the
`@Param({"default", "class_only"})` annotations below?
test/micro/org/openjdk/bench/java/lang/StackWalkBench.java line 78:
> 76: public int mark = 4;
> 77:
> 78: @Param({"default", "class_only"})
(I mean here)
-
PR Review: https://git.openjdk.org/jdk/pull/15370#pullrequestreview-1616052545
PR Review Comment: https://git.openjdk.org/jdk/pull/15370#discussion_r1318997721
PR Review Comment: https://git.openjdk.org/jdk/pull/15370#discussion_r1318999463
Re: RFR: 8041488: Locale-Dependent List Patterns [v15]
> Introducing a new formatting class for locale-dependent list patterns. The > class is to provide the functionality from the Unicode Consortium's LDML > specification for [list > patterns](https://www.unicode.org/reports/tr35/tr35-general.html#ListPatterns). > For example, given a list of String as "Monday", "Wednesday", "Friday", its > `format` method would produce "Monday, Wednesday, and Friday" in US English. > A CSR has also been drafted, and its draft javadoc can be viewed here: > https://cr.openjdk.org/~naoto/JDK-8041488-ListPatterns-PR/api.00/java.base/java/text/ListFormat.html Naoto Sato 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 24 additional commits since the last revision: - Reflects review comments - Merge branch 'master' into JDK-8041488-ListPatterns-PR - Incorporating suggested changes - Update src/java.base/share/classes/java/text/ListFormat.java Co-authored-by: Roger Riggs - Update src/java.base/share/classes/java/text/ListFormat.java Co-authored-by: Roger Riggs - Update src/java.base/share/classes/sun/util/locale/provider/LocaleResources.java Co-authored-by: Roger Riggs - Removing unnecessary commas - Added tests - Review comments - Merge branch 'master' into JDK-8041488-ListPatterns-PR - ... and 14 more: https://git.openjdk.org/jdk/compare/85b25b37...bce82a4f - Changes: - all: https://git.openjdk.org/jdk/pull/15130/files - new: https://git.openjdk.org/jdk/pull/15130/files/316ed12d..bce82a4f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15130=14 - incr: https://webrevs.openjdk.org/?repo=jdk=15130=13-14 Stats: 18706 lines in 454 files changed: 13614 ins; 2714 del; 2378 mod Patch: https://git.openjdk.org/jdk/pull/15130.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15130/head:pull/15130 PR: https://git.openjdk.org/jdk/pull/15130
Re: RFR: 8268829: Provide an optimized way to walk the stack with Class object only [v11]
On Thu, 7 Sep 2023 18:22:40 GMT, Mandy Chung wrote: >> 8268829: Provide an optimized way to walk the stack with Class object only >> >> `StackWalker::walk` creates one `StackFrame` per frame and the current >> implementation >> allocates one `StackFrameInfo` and one `MemberName` objects per frame. Some >> frameworks >> like logging may only interest in the Class object but not the method name >> nor the BCI, >> for example, filters out its implementation classes to find the caller >> class. It's >> similar to `StackWalker::getCallerClass` but allows a predicate to filter >> out the element. >> >> This PR proposes to add `Option::DROP_METHOD_INFO` enum that requests to >> drop the method information. If no method information is needed, a >> `StackWalker` with `DROP_METHOD_INFO` >> can be used instead and such stack walker will save the overhead of >> extracting the method information >> and the memory used for the stack walking. >> >> New factory methods to take a parameter to specify the kind of stack walker >> to be created are defined. >> This provides a simple way for existing code, for example logging >> frameworks, to take advantage of >> this enhancement with the least change as it can keep the existing function >> for traversing >> `StackFrame`s. >> >> For example: to find the first caller filtering a known list of >> implementation class, >> existing code can create a stack walker instance with `DROP_METHOD_INFO` >> option: >> >> >> StackWalker walker = StackWalker.getInstance(Option.DROP_METHOD_INFO, >> Option.RETAIN_CLASS_REFERENCE); >> Optional> callerClass = walker.walk(s -> >> s.map(StackFrame::getDeclaringClass) >> .filter(Predicate.not(implClasses::contains)) >> .findFirst()); >> >> >> If method information is accessed on the `StackFrame`s produced by this >> stack walker such as >> `StackFrame::getMethodName`, then `UnsupportedOperationException` will be >> thrown. >> >> Javadoc & specdiff >> >> https://cr.openjdk.org/~mchung/api/java.base/java/lang/StackWalker.html >> https://cr.openjdk.org/~mchung/jdk22/specdiff/overview-summary.html >> >> Alternatives Considered >> One alternative is to provide a new API: >> ` T walkClass(Function, ? extends T> function)` >> >> In this case, the caller would need to pass a function that takes a stream >> of `Class` object instead of `StackFrame`. Existing code would have to >> modify calls to the `walk` method to `walkClass` and the function body. >> >> ### Implementation Details >> >> A `StackWalker` configured with `DROP_METHOD_INFO` ... > > Mandy Chung has updated the pull request incrementally with one additional > commit since the last revision: > > review feedback Looks great - thanks! - Marked as reviewed by bchristi (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15370#pullrequestreview-1616029850
Re: RFR: 8268829: Provide an optimized way to walk the stack with Class object only [v11]
> 8268829: Provide an optimized way to walk the stack with Class object only > > `StackWalker::walk` creates one `StackFrame` per frame and the current > implementation > allocates one `StackFrameInfo` and one `MemberName` objects per frame. Some > frameworks > like logging may only interest in the Class object but not the method name > nor the BCI, > for example, filters out its implementation classes to find the caller class. > It's > similar to `StackWalker::getCallerClass` but allows a predicate to filter out > the element. > > This PR proposes to add `Option::DROP_METHOD_INFO` enum that requests to drop > the method information. If no method information is needed, a `StackWalker` > with `DROP_METHOD_INFO` > can be used instead and such stack walker will save the overhead of > extracting the method information > and the memory used for the stack walking. > > New factory methods to take a parameter to specify the kind of stack walker > to be created are defined. > This provides a simple way for existing code, for example logging frameworks, > to take advantage of > this enhancement with the least change as it can keep the existing function > for traversing > `StackFrame`s. > > For example: to find the first caller filtering a known list of > implementation class, > existing code can create a stack walker instance with `DROP_METHOD_INFO` > option: > > > StackWalker walker = StackWalker.getInstance(Option.DROP_METHOD_INFO, > Option.RETAIN_CLASS_REFERENCE); > Optional> callerClass = walker.walk(s -> > s.map(StackFrame::getDeclaringClass) > .filter(Predicate.not(implClasses::contains)) > .findFirst()); > > > If method information is accessed on the `StackFrame`s produced by this stack > walker such as > `StackFrame::getMethodName`, then `UnsupportedOperationException` will be > thrown. > > Javadoc & specdiff > > https://cr.openjdk.org/~mchung/api/java.base/java/lang/StackWalker.html > https://cr.openjdk.org/~mchung/jdk22/specdiff/overview-summary.html > > Alternatives Considered > One alternative is to provide a new API: > ` T walkClass(Function, ? extends T> function)` > > In this case, the caller would need to pass a function that takes a stream > of `Class` object instead of `StackFrame`. Existing code would have to > modify calls to the `walk` method to `walkClass` and the function body. > > ### Implementation Details > > A `StackWalker` configured with `DROP_METHOD_INFO` option creates > `ClassFrameInfo[]` > buffer that is filled by the VM during stack walking. `Sta... Mandy Chung has updated the pull request incrementally with one additional commit since the last revision: review feedback - Changes: - all: https://git.openjdk.org/jdk/pull/15370/files - new: https://git.openjdk.org/jdk/pull/15370/files/a623b9dc..0e6abc42 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15370=10 - incr: https://webrevs.openjdk.org/?repo=jdk=15370=09-10 Stats: 22 lines in 3 files changed: 21 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/15370.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15370/head:pull/15370 PR: https://git.openjdk.org/jdk/pull/15370
Re: RFR: 8315850: Improve AbstractMap anonymous Iterator classes
On Thu, 7 Sep 2023 12:24:58 GMT, Rémi Forax wrote: >> This PR proposes to slightly improve some iterators of `AbstractMap`: >> >> * Code reuse >> * A field declared `final` >> * Add missing `@Override` annotations > > Hello, > In Java, sharing code may have a runtime cost (or not) depending on the shape > of the code, because the VM may have to speculate on the class of some value > at runtime. > Here, in hasNext() and remove(), the VM has to find the class of the field > 'i' at runtime. > > Iterators are performance sentive (they are used in for loop) and for that > they need the escape analysis to work. > If the iterator code is polymorphic, so part of the code may be unknown so > the escape analysis will consider that the iterator escape. So usually, it's > a good practice to not share the iterator code. > > Also the field should be package private (or even maybe private) but not > protected. Protected is very rare in Java because usually classes that share > implementation details are in the same package (or in the same nestmate nest). @forax Note that HashMap uses a similar subclassing idiom for the iterators of the keySet, values, and entrySet collections in order to share the iterator logic: https://github.com/openjdk/jdk/blob/jdk-21%2B35/src/java.base/share/classes/java/util/HashMap.java#L1626 - PR Comment: https://git.openjdk.org/jdk/pull/15615#issuecomment-1710574566
Re: RFR: 8315850: Improve AbstractMap anonymous Iterator classes [v2]
On Thu, 7 Sep 2023 17:23:20 GMT, Per Minborg wrote: >> This PR proposes to slightly improve some iterators of `AbstractMap`: >> >> * Code reuse >> * A field declared `final` >> * Add missing `@Override` annotations > > Per Minborg has updated the pull request incrementally with one additional > commit since the last revision: > > Remove @Override annotations Note, I had asked @minborg to remove the `@Override` annotations. They're generally not used in the collections framework, and where they are, they add a lot of clutter but not much value. - PR Comment: https://git.openjdk.org/jdk/pull/15615#issuecomment-1710557583
Re: RFR: 8041488: Locale-Dependent List Patterns [v14]
On Tue, 5 Sep 2023 22:47:01 GMT, Naoto Sato wrote: >> Introducing a new formatting class for locale-dependent list patterns. The >> class is to provide the functionality from the Unicode Consortium's LDML >> specification for [list >> patterns](https://www.unicode.org/reports/tr35/tr35-general.html#ListPatterns). >> For example, given a list of String as "Monday", "Wednesday", "Friday", its >> `format` method would produce "Monday, Wednesday, and Friday" in US English. >> A CSR has also been drafted, and its draft javadoc can be viewed here: >> https://cr.openjdk.org/~naoto/JDK-8041488-ListPatterns-PR/api.00/java.base/java/text/ListFormat.html > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Incorporating suggested changes Looking very good. src/java.base/share/classes/java/text/ListFormat.java line 95: > 93: * On parsing, if some ambiguity is found in the input string, such as > delimiting > 94: * sequences being found in the input string, may produce the result that > when formatted is not a > 95: * round-trip with the corresponding formatting. For example, a two > element String list Suggestion: * On parsing, if some ambiguity is found in the input string, such as delimiting * sequences in the input string, the result, when formatted with the same formatting, does not * re-produce the input string . For example, a two element String list src/java.base/share/classes/java/text/ListFormat.java line 345: > 343: * of Object. > 344: * @param toAppendTowhere the text is to be appended > 345: * @param posIgnored. Not used in ListFormat. May be null Curious, why not used? I could see a use to identity the string inserted to enable highlighting or other markup around the new string. - Marked as reviewed by rriggs (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15130#pullrequestreview-1615880405 PR Review Comment: https://git.openjdk.org/jdk/pull/15130#discussion_r1318891558 PR Review Comment: https://git.openjdk.org/jdk/pull/15130#discussion_r1318935751
Re: RFR: 6228794: java.text.ChoiceFormat pattern behavior is not well documented. [v2]
On Wed, 6 Sep 2023 23:04:59 GMT, Naoto Sato wrote:
> Could the first choice be more specific?
Updated with a more syntactical definition. Please let me know if you think
this is _too_ specific.
> Also, do SubPatterns need to be sorted with Limits?
If I am interpreting the question right, you're asking if a subPattern
**needs** a limit to be valid. If that's the question, the answer is yes,
`ChoiceFormat` is designed to have an equal amount of limits and formats
(whether from the arrays, or the String pattern). But please let me know if I
misinterpreted the question.
subpattern "xyz" where both _limit_ and _relation_ are omitted
jshell> var a = new ChoiceFormat("0#abc|xyz")
a ==> java.text.ChoiceFormat@17863
jshell> a.getLimits()
$21 ==> double[1] { 0.0 }
jshell> a.getFormats()
$22 ==> String[1] { "abc" }
subpattern "#xyz" where _limit_ is omitted
jshell> var b = new ChoiceFormat("0#abc|#xyz")
| Exception java.lang.IllegalArgumentException: Each interval must contain a
number before a format
> src/java.base/share/classes/java/text/ChoiceFormat.java line 152:
>
>> 150: *
>> 151: * System.out.println("Format with -INF : " +
>> fmt.format(Double.NEGATIVE_INFINITY));
>> 152: * // ...
>
> Commenting out (with ellipsis) would not print the result below
I felt it was redundant to have both the long lines of println and output, so I
commented out the println(s) except for the ones of interest, which are
`Double.NEGATIVE_INFINITY`, `Double.POSITIVE_INFINITY`, and `Double.NaN`.
However, you're right that commenting it out would not print the below result.
I have updated it so that the println and output lines are combined, to provide
a more concise example.
> src/java.base/share/classes/java/text/ChoiceFormat.java line 407:
>
>> 405:
>> 406: /**
>> 407: * Get the formats of this ChoiceFormat.
>
> Same here
Fixed, as well as in the other instance, I have also updated
https://cr.openjdk.org/~jlu/api/java.base/java/text/ChoiceFormat.html to
reflect the changes. Thanks for the review
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15392#discussion_r1318912438
PR Review Comment: https://git.openjdk.org/jdk/pull/15392#discussion_r1318912460
PR Review Comment: https://git.openjdk.org/jdk/pull/15392#discussion_r1318912398
Re: RFR: 6228794: java.text.ChoiceFormat pattern behavior is not well documented. [v3]
> Please review this PR and associated > [CSR](https://bugs.openjdk.org/browse/JDK-8314546) which expands on the > `java.text.ChoiceFormat` specification regarding its pattern. > > `j.text.ChoiceFormat` provides an example pattern in the class description, > but beyond that it does not specify any well-defined syntax for creating a > pattern. In addition, methods related to the pattern String are > under-specified. > > The wording for `getLimits()` and `getFormats()` was also adjusted, as there > are other ways to set the limits and formats beyond the constructor. > > The pattern syntax may be easier to view -> > https://cr.openjdk.org/~jlu/api/java.base/java/text/ChoiceFormat.html Justin Lu has updated the pull request incrementally with one additional commit since the last revision: Reflect Naoto's comments from 9/6 - Changes: - all: https://git.openjdk.org/jdk/pull/15392/files - new: https://git.openjdk.org/jdk/pull/15392/files/fd1cc831..bbb99eb4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15392=02 - incr: https://webrevs.openjdk.org/?repo=jdk=15392=01-02 Stats: 41 lines in 1 file changed: 16 ins; 17 del; 8 mod Patch: https://git.openjdk.org/jdk/pull/15392.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15392/head:pull/15392 PR: https://git.openjdk.org/jdk/pull/15392
Re: RFR: 8315850: Improve AbstractMap anonymous Iterator classes [v2]
> This PR proposes to slightly improve some iterators of `AbstractMap`: > > * Code reuse > * A field declared `final` > * Add missing `@Override` annotations Per Minborg has updated the pull request incrementally with one additional commit since the last revision: Remove @Override annotations - Changes: - all: https://git.openjdk.org/jdk/pull/15615/files - new: https://git.openjdk.org/jdk/pull/15615/files/1b00c84b..aab2472a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15615=01 - incr: https://webrevs.openjdk.org/?repo=jdk=15615=00-01 Stats: 10 lines in 1 file changed: 0 ins; 10 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/15615.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15615/head:pull/15615 PR: https://git.openjdk.org/jdk/pull/15615
Re: RFR: 8311207: Cleanup for Optimization for UUID.toString [v11]
On Tue, 5 Sep 2023 15:48:21 GMT, 温绍锦 wrote: >> [PR 14578 ](https://github.com/openjdk/jdk/pull/14578) still has unresolved >> discussions, continue to make improvements. >> >> # Benchmark Result >> >> >> sh make/devkit/createJMHBundle.sh >> bash configure --with-jmh=build/jmh/jars >> make test TEST="micro:java.util.UUIDBench.toString" >> >> >> ## 1. >> [aliyun_ecs_c8i.xlarge](https://help.aliyun.com/document_detail/25378.html#c8i) >> * cpu : intel xeon sapphire rapids (x64) >> >> ``` diff >> -Benchmark (size) Mode Cnt Score Error Units (baseline) >> -UUIDBench.toString 2 thrpt 15 62.019 ± 0.622 ops/us >> >> +Benchmark (size) Mode Cnt Score Error Units >> +UUIDBench.toString 2 thrpt 15 82.998 ± 0.739 ops/us (+33.82%) >> >> >> ## 2. >> [aliyun_ecs_c8a.xlarge](https://help.aliyun.com/document_detail/25378.html#c8a) >> * cpu : amd epc genoa (x64) >> >> ``` diff >> -Benchmark (size) Mode Cnt Score Error Units (baseline) >> -UUIDBench.toString 2 thrpt 15 88.668 ± 0.672 ops/us >> >> +Benchmark (size) Mode Cnt Score Error Units >> +UUIDBench.toString 2 thrpt 15 89.229 ± 0.271 ops/us (+0.63%) >> >> >> >> ## 3. >> [aliyun_ecs_c8y.xlarge](https://help.aliyun.com/document_detail/25378.html#c8y) >> * cpu : aliyun yitian 710 (aarch64) >> ``` diff >> -Benchmark (size) Mode Cnt Score Error Units (baseline) >> -UUIDBench.toString 2 thrpt 15 49.382 ± 2.160 ops/us >> >> +Benchmark (size) Mode Cnt Score Error Units >> +UUIDBench.toString 2 thrpt 15 49.636 ± 1.974 ops/us (+0.51%) >> >> >> ## 4. MacBookPro M1 Pro >> ``` diff >> -Benchmark (size) Mode CntScore Error Units (baseline) >> -UUIDBench.toString 2 thrpt 15 103.543 ± 0.963 ops/us >> >> +Benchmark (size) Mode CntScore Error Units >> +UUIDBench.toString 2 thrpt 15 110.976 ± 0.685 ops/us (+7.17%) >> >> >> ## 5. Orange Pi 5 Plus >> >> ``` diff >> -Benchmark (size) Mode Cnt Score Error Units (baseline) >> -UUIDBench.toString 2 thrpt 15 33.532 ± 0.396 ops/us >> >> +Benchmark (size) Mode Cnt Score Error Units (PR) >> +UUIDBench.toString 2 thrpt 15 33.054 ± 0.190 ops/us (-4.42%) > > 温绍锦 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 14 additional commits since the > last revision: > > - Merge branch 'master' into optimization_for_uuid_to_string > - remove redundant parentheses > - fix java doc, big-endian -> little-endian > - Merge branch 'master' into optimization_for_uuid_to_string > - use ByteArrayLittleEndian > - fix typo > - code style > - Explain the rationale > >Co-authored-by: liach > - private static final Unsafe > - revert to the previous version > - ... and 4 more: https://git.openjdk.org/jdk/compare/34d842c9...2d36332b @cl4es Can you help me review this PR? - PR Comment: https://git.openjdk.org/jdk/pull/14745#issuecomment-1710520451
Re: RFR: 8294969: Convert jdk.jdeps javap to use the Classfile API [v11]
On Thu, 20 Jul 2023 09:11:20 GMT, Adam Sotona wrote:
>> javap uses proprietary com.sun.tools.classfile library to parse class files.
>>
>> This patch converts javap to use Classfile API.
>>
>> Please review.
>>
>> Thanks,
>> Adam
>
> Adam Sotona has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains 227 commits:
>
> - Merge branch 'master' into JDK-8294969-javap
> - Merge branch 'master' into JDK-8294969-javap
> - fixed code printing and ConstantPoolException reporting indoex
> - added DydnamicConstantPoolEntry::bootstrapMethodIndex
>fix of javap ConstantWriter to print DynamicConstantPoolEntry without
> accessing BSM attribute
> - extended ClassReader about specific entry-reading methods to avoid class
> cast and throw ConstantPoolException instead
> - throwing ConstantPoolException for invalid BSM entry index
> - Merge branch 'master' into JDK-8294969-javap
> - fixed JavapTask
> - Merge branch 'master' into JDK-8294969-javap
> - Merge branch 'master' into JDK-8294969-javap
>
># Conflicts:
>#
> src/java.base/share/classes/jdk/internal/classfile/constantpool/ConstantPoolException.java
> - ... and 217 more: https://git.openjdk.org/jdk/compare/37c756a7...4960751b
src/java.base/share/classes/jdk/internal/classfile/ClassReader.java line 226:
> 224:
> 225: /**
> 226: * {@return the field ref entry whose index is given at the specified
I think that `ref` in these APIs should be `reference`
src/jdk.jdeps/share/classes/com/sun/tools/javap/AttributeWriter.java line 465:
> 463: indent(+1);
> 464: print("target_platform: #" +
> attr.targetPlatform().index());
> 465: //ToDo find the spec - can be null???
what about this comment?
src/jdk.jdeps/share/classes/com/sun/tools/javap/AttributeWriter.java line 549:
> 547: case SourceIDAttribute attr ->
> 548: constantWriter.write(attr.sourceId().index());
> 549: //case StackMapAttribute ???
I guess this comment can be removed
-
PR Review Comment: https://git.openjdk.org/jdk/pull/11411#discussion_r1318036242
PR Review Comment: https://git.openjdk.org/jdk/pull/11411#discussion_r1318069041
PR Review Comment: https://git.openjdk.org/jdk/pull/11411#discussion_r1318070357
Re: RFR: 8315789: Minor HexFormat performance improvements
On Wed, 6 Sep 2023 13:36:22 GMT, Claes Redestad wrote:
> This PR seeks to improve formatting of hex digits using `java.util.HexFormat`
> somewhat.
>
> This is achieved getting rid of a couple of lookup tables, caching the result
> of `HexFormat.of().withUpperCase()`, and removing tiny allocation that
> happens in the `formatHex(A, byte)` method. Improvements range from 20-40% on
> throughput, and some operations allocate less:
>
>
> Name Cnt Base Error Test Error Unit
> Diff%
> HexFormatBench.appenderLower15 1,330 ± 0,021 1,065 ± 0,067 us/op
> 19,9% (p = 0,000*)
> :gc.alloc.rate15 11,481 ± 0,185 0,007 ± 0,000 MB/sec
> -99,9% (p = 0,000*)
> :gc.alloc.rate.norm 15 16,009 ± 0,000 0,007 ± 0,000 B/op
> -100,0% (p = 0,000*)
> :gc.count 15 3,000 0,000 counts
> :gc.time 3 2,000ms
> HexFormatBench.appenderLowerCached 15 1,317 ± 0,013 1,065 ± 0,054 us/op
> 19,1% (p = 0,000*)
> :gc.alloc.rate15 11,590 ± 0,111 0,007 ± 0,000 MB/sec
> -99,9% (p = 0,000*)
> :gc.alloc.rate.norm 15 16,009 ± 0,000 0,007 ± 0,000 B/op
> -100,0% (p = 0,000*)
> :gc.count 15 3,000 0,000 counts
> :gc.time 3 2,000ms
> HexFormatBench.appenderUpper15 1,330 ± 0,022 1,065 ± 0,036 us/op
> 19,9% (p = 0,000*)
> :gc.alloc.rate15 34,416 ± 0,559 0,007 ± 0,000 MB/sec
> -100,0% (p = 0,000*)
> :gc.alloc.rate.norm 15 48,009 ± 0,000 0,007 ± 0,000 B/op
> -100,0% (p = 0,000*)
> :gc.count 15 0,000 0,000 counts
> HexFormatBench.appenderUpperCached 15 1,353 ± 0,009 1,033 ± 0,014 us/op
> 23,6% (p = 0,000*)
> :gc.alloc.rate15 11,284 ± 0,074 0,007 ± 0,000 MB/sec
> -99,9% (p = 0,000*)
> :gc.alloc.rate.norm 15 16,009 ± 0,000 0,007 ± 0,000 B/op
> -100,0% (p = 0,000*)
> :gc.count 15 3,000 0,000 counts
> :gc.time 3 2,000ms
> HexFormatBench.toHexLower 15 0,198 ± 0,001 0,119 ± 0,008 us/op
> 40,1% (p = 0,000*)
> :gc.alloc.rate15 0,007 ± 0,000 0,007 ± 0,000 MB/sec
> -0,0% (p = 0,816 )
> :gc.alloc.rate.norm 15 0,001 ± 0,000 0,001 ± 0,000 B/op
> -40,1% (p = 0,000*)
> :gc.count 15 0,000 0,000...
You can refer to the PR #14745 I submitted. Using a table with a length of 256
in HexDigits, you can get two digits in one lookup table, and use
ByteArrayLittleEndian.setShort to write two digits at a time.
like this:
public String toHexDigits(byte value) {
byte[] rep = new byte[2];
ByteArrayLittleEndian.setShort(rep, 0, HexDigits.DIGITS[value & 0xff]);
// DIGITS shuld changed to little-endian
try {
return jla.newStringNoRepl(rep, StandardCharsets.ISO_8859_1);
} catch (CharacterCodingException cce) {
throw new AssertionError(cce);
}
}
-
PR Comment: https://git.openjdk.org/jdk/pull/15591#issuecomment-1710508831
Re: RFR: 8315578: PPC builds are broken after JDK-8304913 [v2]
On Thu, 7 Sep 2023 14:07:05 GMT, Roger Riggs wrote: >> Aleksey Shipilev 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 three additional >> commits since the last revision: >> >> - Switch to PPC32 >> - Merge branch 'master' into JDK-8315578-ppc-builds >> - Fix > > src/java.base/share/classes/jdk/internal/util/PlatformProps.java.template > line 62: > >> 60: static final boolean TARGET_ARCH_IS_LOONGARCH64 = >> "@@OPENJDK_TARGET_CPU@@" == "loongarch64"; >> 61: static final boolean TARGET_ARCH_IS_S390= >> "@@OPENJDK_TARGET_CPU@@" == "s390"; >> 62: static final boolean TARGET_ARCH_IS_PPC32 = >> "@@OPENJDK_TARGET_CPU@@" == "ppc"; > > The list is getting longer. I'd suggest/request that all the enums be > resorted alphabetically. (now or later). I think later? Don't want to keep retesting the PR for accidental problems during reshuffle :) - PR Review Comment: https://git.openjdk.org/jdk/pull/15556#discussion_r1318857471
Re: RFR: 8312522: Implementation of Foreign Function & Memory API [v17]
> This patch contains the implementation of the foreign linker & memory API JEP > for Java 22. The initial patch is composed of commits brought over directly > from the [panama-foreign repo](https://github.com/openjdk/panama-foreign). > The main changes found in this patch come from the following PRs: > > 1. https://github.com/openjdk/panama-foreign/pull/836 Where previous > iterations supported converting Java strings to and from native strings in > the UTF-8 encoding, we've extended the supported encoding to all the > encodings found in the `java.nio.charset.StandardCharsets` class. > 2. https://github.com/openjdk/panama-foreign/pull/838 We dropped the > `MemoryLayout::sequenceLayout` factory method which inferred the size of the > sequence to be `Long.MAX_VALUE`, as this led to confusion among clients. A > client is now required to explicitly specify the sequence size. > 3. https://github.com/openjdk/panama-foreign/pull/839 A new API was added: > `Linker::canonicalLayouts`, which exposes a map containing the > platform-specific mappings of common C type names to memory layouts. > 4. https://github.com/openjdk/panama-foreign/pull/840 Memory access > varhandles, as well as byte offset and slice handles derived from memory > layouts, now feature an additional 'base offset' coordinate that is added to > the offset computed by the handle. This allows composing these handles with > other offset computation strategies that may not be based on the same memory > layout. This addresses use-cases where clients are working with 'dynamic' > layouts, whose size might not be known statically, such as variable length > arrays, or variable size matrices. > 5. https://github.com/openjdk/panama-foreign/pull/841 Remove this now > redundant API. Clients can simply use the difference between the base address > of two memory segments. > 6. https://github.com/openjdk/panama-foreign/pull/845 Disambiguate uses of > `SegmentAllocator::allocateArray`, by renaming methods that both allocate + > initialize memory segments to `allocateFrom`. (see the original PR for the > problematic case) > 7. https://github.com/openjdk/panama-foreign/pull/846 Improve the > documentation for variadic functions. > 8. https://github.com/openjdk/panama-foreign/pull/849 Several test fixes to > make sure the `jdk_foreign` tests can pass on 32-bit machines, taking > linux-x86 as a test bed. > 9. https://github.com/openjdk/panama-foreign/pull/850 Make the linker API > required. The `Linker::nativeLinker` method is not longer allowed to throw an > `UnsupportedOperationException` on ... Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: Split long throws clauses in `MemorySegment` javadoc Reviewed-by: jvernee - Changes: - all: https://git.openjdk.org/jdk/pull/15103/files - new: https://git.openjdk.org/jdk/pull/15103/files/55296527..2f50adbf Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15103=16 - incr: https://webrevs.openjdk.org/?repo=jdk=15103=15-16 Stats: 41 lines in 1 file changed: 3 ins; 0 del; 38 mod Patch: https://git.openjdk.org/jdk/pull/15103.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15103/head:pull/15103 PR: https://git.openjdk.org/jdk/pull/15103
Re: RFR: 8315683: Parallelize java/util/concurrent/tck/JSR166TestCase.java [v2]
On Thu, 7 Sep 2023 15:58:26 GMT, Martin Buchholz wrote: >> Soumadipta Roy has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Remove extra new line > > test/jdk/java/util/concurrent/tck/JSR166TestCase.java line 45: > >> 43: * @modules java.management >> 44: * @run junit/othervm/timeout=1000 JSR166TestCase >> 45: * @run junit/othervm/timeout=1000 -Djava.security.manager=allow >> JSR166TestCase > > security manager testing is relatively less important. I would move this to > a lower tier, while moving a test with > -Djsr166.testImplementationDetails=true into tier1 Hi Martin, I will raise another commit trying to add summary for the segregations - PR Review Comment: https://git.openjdk.org/jdk/pull/15619#discussion_r1318854380
Re: RFR: 8315578: PPC builds are broken after JDK-8304913 [v2]
On Thu, 7 Sep 2023 13:51:31 GMT, Aleksey Shipilev wrote: >> Similar to other issues in the same area. I have no PPC32 machine to test >> the build on, but this matches other fixes for other architectures >> (https://github.com/openjdk/jdk/commit/83c096d6e20cd6e1164bc666df1be197a10431eb) >> after this fix the PPC32 Zero build completes fine. > > Aleksey Shipilev 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 three additional > commits since the last revision: > > - Switch to PPC32 > - Merge branch 'master' into JDK-8315578-ppc-builds > - Fix Yeah, so as I see from GHA testing: STARTEDArchTest::checkParams '[8] ppc, PPC32, 32, BIG_ENDIAN, false' java.lang.IllegalArgumentException: No enum constant jdk.internal.util.Architecture.PPC at java.base/java.lang.Enum.valueOf(Enum.java:293) at java.base/jdk.internal.util.Architecture.valueOf(Architecture.java:39) at java.base/jdk.internal.util.Architecture.lookupByName(Architecture.java:99) at ArchTest.checkParams(ArchTest.java:120) at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) at java.base/java.lang.reflect.Method.invoke(Method.java:580) The test would verify that enum names are consistent with `os.arch`. Which would be `ppc`, based on build system pick. Therefore, we cannot do PPC32. I reverted the previous commit... - PR Comment: https://git.openjdk.org/jdk/pull/15556#issuecomment-1710435235
Re: RFR: 8315578: PPC builds are broken after JDK-8304913 [v3]
> Similar to other issues in the same area. I have no PPC32 machine to test the > build on, but this matches other fixes for other architectures > (https://github.com/openjdk/jdk/commit/83c096d6e20cd6e1164bc666df1be197a10431eb) > after this fix the PPC32 Zero build completes fine. Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision: Revert "Switch to PPC32" This reverts commit d829d243f94560dc19699d36b67a73280817b8f6. - Changes: - all: https://git.openjdk.org/jdk/pull/15556/files - new: https://git.openjdk.org/jdk/pull/15556/files/d829d243..df419e36 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15556=02 - incr: https://webrevs.openjdk.org/?repo=jdk=15556=01-02 Stats: 7 lines in 3 files changed: 0 ins; 0 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/15556.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15556/head:pull/15556 PR: https://git.openjdk.org/jdk/pull/15556
Re: Question on why sun.management MBeans are not exported?
What we're referring to is to remove sun.management.Hotspot*, the internal MBeans which are never exposed and registered in the platform MBeanServer. The internal metrics in HotSpot VM should be retained as they are exposed through other ways like jstat, GC logs, etc. Mandy On 9/6/23 11:27 PM, Kirk Pepperdine wrote: Hi, It would be a shame to lose these metrics because many of them have been very useful over time and some would be even more useful with some modifications. For example, the CPU breakouts found in GC logs has been incredibly useful as a proxy measure in helping sort out other issues in systems. So much so that I have analytics built specifically around this in my tooling. Kind regards, Kirk Pepperdine On Sep 6, 2023, at 10:50 AM, Alan Bateman wrote: On 06/09/2023 16:17, Volker Simonis wrote: : I'm familiar with JEP 260. But wouldn't you agree that an "encapsulated" monitoring API is an oxymoron? A monitoring API is by design intended for external usage and completely useless to the platform itself. There's no single usage of the "sun.management" MBeans in the JDK itself (except for jconsole where the encapsulation broke it). My assumption is that the corresponding MBeans in "sun.management" are there for historic reasons (added in JDK 1.5) and would have made much more sense in "com.sun.management" package. But I doubt that they can be classified in the "internal implementation details of the JDK and never intended for external use² category of JEP 260. It's left over from experiments on exposing some internal metrics, I think during JDK 5. It's code that should probably have been removed a long time ago. -Alan
Re: RFR: 8287843: File::getCanonicalFile doesn't work for \?\C:\ style paths DOS device paths
On Thu, 7 Sep 2023 08:33:39 GMT, Alan Bateman wrote:
> Would it be possible to summarise how this behaves when File just represents
> a prefix?
Baseline:
jshell> File f = new File("\\\?\")
f ==> \?
jshell> f.getCanonicalPath()
| Exception java.io.IOException: Bad pathname
|at WinNTFileSystem.canonicalize0 (Native Method)
|at WinNTFileSystem.canonicalize (WinNTFileSystem.java:463)
|at File.getCanonicalPath (File.java:626)
|at (#2:1)
jshell> f = new File("\\\?\\UNC\")
f ==> \?\UNC
jshell> f.getCanonicalPath()
| Exception java.io.IOException: Bad pathname
|at WinNTFileSystem.canonicalize0 (Native Method)
|at WinNTFileSystem.canonicalize (WinNTFileSystem.java:463)
|at File.getCanonicalPath (File.java:626)
|at (#4:1)
Proposed patch:
jshell> File f = new File("\\\?\")
f ==> \?
jshell> f.getCanonicalPath()
| Exception java.io.IOException: Bad pathname
|at WinNTFileSystem.canonicalize0 (Native Method)
|at WinNTFileSystem.canonicalize (WinNTFileSystem.java:472)
|at File.getCanonicalPath (File.java:626)
|at (#2:1)
jshell> f = new File("\\\?\\UNC\")
f ==> \?\UNC
jshell> f.getCanonicalPath()
$4 ==> "C:\\Users\\bpb\\dev\\jdk\\jdk\\UNC"
Presumably the constructor removes the trailing `\`, hence I suppose the
special case of `\?\UNC` should be checked in the code right after the proposed
change.
-
PR Comment: https://git.openjdk.org/jdk/pull/15603#issuecomment-1710409880
Re: RFR: 8315683: Parallelize java/util/concurrent/tck/JSR166TestCase.java [v2]
On Thu, 7 Sep 2023 14:10:20 GMT, Soumadipta Roy wrote: >> This test is running in tier1, and takes about 400 seconds to complete. >> Thus, it drags the execution time of tier1 on large machines. The commit >> includes changing the sequential run of test cases in >> java/util/concurrent/tck/JSR166TestCase.java to the best possible >> combination of parallelization to reduce the total runtime of the overall >> test and causing least possible change to user and system times. The below >> comparison shows existing and newer combinations of parallelizations: >> >> * before : **200.64s user 20.94s system 204% cpu >> 1:48.47 total** >> * fully parallel : **308.84s user 23.75s system 479% cpu >> 1:09.42 total** >> * default-details-tck : **244.69s user 22.03s system 314% cpu 1:24.94 >> total** >> * default-fjp_details-tck : **260.12s user 24.47s system 404% cpu 1:10.31 >> total** >> >> Based on the comparison above, the fourth combination has a result very >> close to full parallelization and at the same time having the least >> deviation of system and user times among the newer combinations. Hence the >> commit includes the changes corresponding to the fourth combination. > > Soumadipta Roy has updated the pull request incrementally with one additional > commit since the last revision: > > Remove extra new line Hello Soumadipta - pleased to "meet" you. If the intent is to split the tests into relatively more and less important variants for placing into separate tiers, that should be clear from the @summary's in the tests. - Changes requested by martin (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15619#pullrequestreview-1615792268
Re: RFR: 8315683: Parallelize java/util/concurrent/tck/JSR166TestCase.java [v2]
On Thu, 7 Sep 2023 14:10:20 GMT, Soumadipta Roy wrote: >> This test is running in tier1, and takes about 400 seconds to complete. >> Thus, it drags the execution time of tier1 on large machines. The commit >> includes changing the sequential run of test cases in >> java/util/concurrent/tck/JSR166TestCase.java to the best possible >> combination of parallelization to reduce the total runtime of the overall >> test and causing least possible change to user and system times. The below >> comparison shows existing and newer combinations of parallelizations: >> >> * before : **200.64s user 20.94s system 204% cpu >> 1:48.47 total** >> * fully parallel : **308.84s user 23.75s system 479% cpu >> 1:09.42 total** >> * default-details-tck : **244.69s user 22.03s system 314% cpu 1:24.94 >> total** >> * default-fjp_details-tck : **260.12s user 24.47s system 404% cpu 1:10.31 >> total** >> >> Based on the comparison above, the fourth combination has a result very >> close to full parallelization and at the same time having the least >> deviation of system and user times among the newer combinations. Hence the >> commit includes the changes corresponding to the fourth combination. > > Soumadipta Roy has updated the pull request incrementally with one additional > commit since the last revision: > > Remove extra new line test/jdk/java/util/concurrent/tck/JSR166TestCase.java line 39: > 37: /* > 38: * @test id=default > 39: * @summary JSR-166 tck tests, in a number of variations. The @summary is stale - it describes the state when there was only one @test test/jdk/java/util/concurrent/tck/JSR166TestCase.java line 45: > 43: * @modules java.management > 44: * @run junit/othervm/timeout=1000 JSR166TestCase > 45: * @run junit/othervm/timeout=1000 -Djava.security.manager=allow > JSR166TestCase security manager testing is relatively less important. I would move this to a lower tier, while moving a test with -Djsr166.testImplementationDetails=true into tier1 - PR Review Comment: https://git.openjdk.org/jdk/pull/15619#discussion_r1318817536 PR Review Comment: https://git.openjdk.org/jdk/pull/15619#discussion_r1318820512
Re: RFR: 8315410: Undocumented exceptions in java.text.StringCharacterIterator [v3]
> Please review this PR and [CSR](https://bugs.openjdk.org/browse/JDK-8315558) > which is a conformance change to document some exceptions in the > specification of java.text.StringCharacterIterator. Justin Lu has updated the pull request incrementally with one additional commit since the last revision: CSR review: replace constructor throws tag with a blanket class statement - Changes: - all: https://git.openjdk.org/jdk/pull/15547/files - new: https://git.openjdk.org/jdk/pull/15547/files/58c3f396..9d44aa07 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15547=02 - incr: https://webrevs.openjdk.org/?repo=jdk=15547=01-02 Stats: 5 lines in 1 file changed: 1 ins; 3 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/15547.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15547/head:pull/15547 PR: https://git.openjdk.org/jdk/pull/15547
Integrated: 8315444: Convert test/jdk/tools to Classfile API
On Fri, 1 Sep 2023 08:14:26 GMT, Qing Xiao wrote: > `/test/jdk/tools/lib/tests/JImageValidator.java`, tests in > `/test/jdk/tools/jlink`, and `/test/jdk/tools/jimage`, > `/test/jdk/java/time/nontestng/java/time/chrono/HijrahConfigTest.java` use on > com.sun.tools.classfile and should be converted to Classfile API. This pull request has now been integrated. Changeset: 2fd870a7 Author:Qing Xiao Committer: Adam Sotona URL: https://git.openjdk.org/jdk/commit/2fd870a74fb35cb55b69f0dc6bf041441d658ffa Stats: 153 lines in 24 files changed: 95 ins; 11 del; 47 mod 8315444: Convert test/jdk/tools to Classfile API Reviewed-by: asotona - PR: https://git.openjdk.org/jdk/pull/15529
Re: RFR: JDK-8314272: Improve java.util.prefs.BackingStoreException: Couldn't get file lock. [v4]
On Mon, 4 Sep 2023 11:01:06 GMT, Matthias Baesken wrote: >> We run into some BackingStoreException: Couldn't get file lock. e.g. here : >> >> [JShell] Exception in thread "main" java.lang.IllegalStateException: >> java.util.prefs.BackingStoreException: Couldn't get file lock. >> [JShell] at >> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder$PreferencesStorage.flush(JShellToolBuilder.java:313) >> [JShell] at >> jdk.jshell/jdk.internal.jshell.tool.JShellTool$ReplayableHistory.storeHistory(JShellTool.java:692) >> [JShell] at >> jdk.jshell/jdk.internal.jshell.tool.JShellTool.start(JShellTool.java:1008) >> [JShell] at >> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder.start(JShellToolBuilder.java:261) >> [JShell] at >> jdk.jshell/jdk.internal.jshell.tool.JShellToolProvider.main(JShellToolProvider.java:120) >> [JShell] Caused by: java.util.prefs.BackingStoreException: Couldn't get file >> lock. >> [JShell] at >> java.prefs/java.util.prefs.FileSystemPreferences.sync(FileSystemPreferences.java:769) >> [JShell] at >> java.prefs/java.util.prefs.FileSystemPreferences.flush(FileSystemPreferences.java:864) >> [JShell] at >> jdk.jshell/jdk.internal.jshell.tool.JShellToolBuilder$PreferencesStorage.flush(JShellToolBuilder.java:311) >> [JShell] ... 4 more >> >> The BackingStoreException should be enhanced e.g. by adding the >> errno/errorCode that is already available in the coding > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > Use PrivilegedAction to get OS name src/java.prefs/unix/classes/java/util/prefs/FileSystemPreferences.java line 36: > 34: import sun.util.logging.PlatformLogger; > 35: > 36: Extra blank line. - PR Review Comment: https://git.openjdk.org/jdk/pull/15308#discussion_r1318702833
Java 21 clinit deadlock
Hello, We switched the Jetty builds to Java 21 a while ago, and they randomly fail with a hard deadlock during class initialization. We tried to understand if we were doing something wrong, but the code compiles fine, and at first glance there seems to be nothing wrong with it, but it may well be a popular Java puzzler that I am not aware of :) We did not see these failures with Java 20, and we do not see them with Java 17. I was wondering if something changed in Java 21 to cause this deadlock? I hope this is the right mailing list for this problem. If not, please let me know. I am an OpenJDK author so I can open an issue about this, but I don't have a reproducer, since it seems a race condition happening randomly. Below you can find more details about this deadlock. Thanks! Taking a thread dump we see one thread in this state: java.lang.Thread.State: RUNNABLE at org.eclipse.jetty.http.HttpFields.build(org.eclipse.jetty.http@12.0.2-SNAPSHOT/HttpFields.java:76) - waiting on the Class initialization monitor for org.eclipse.jetty.http.MutableHttpFields at org.eclipse.jetty.http.HttpFields.(org.eclipse.jetty.http@12.0.2-SNAPSHOT/HttpFields.java:67) "ForkJoinPool-1-worker-1" #23 [1457161] daemon prio=5 os_prio=0 cpu=320.77ms elapsed=8870.98s allocated=18414K defined_classes=928 tid=0x7f6ec0c6a6f0 nid=1457161 waiting on condition [0x7f6e91ffb000] And other threads are in this state (or similar -- the bottom frame is different but still triggering the clinit of HttpFields): java.lang.Thread.State: RUNNABLE at org.eclipse.jetty.http.HttpTester.parseResponse(org.eclipse.jetty.http@12.0.2-SNAPSHOT/HttpTester.java:274) - waiting on the Class initialization monitor for org.eclipse.jetty.http.HttpFields at org.eclipse.jetty.http.HttpTester.parseResponse(org.eclipse.jetty.http@12.0.2-SNAPSHOT/HttpTester.java:261) "ForkJoinPool-1-worker-2" #24 [1457162] daemon prio=5 os_prio=0 cpu=136.15ms elapsed=8870.96s allocated=3303K defined_classes=340 tid=0x7f6e4c01ddb0 nid=1457162 waiting on condition [0x7f6e91efc000] Class hierarchy: MutableHttpFields implements HttpFields.Mutable HttpFields.Mutable extends HttpFields Full thread dump: https://gist.github.com/olamy/7e883adcf51b2410337abfa775a79a0b Code: https://github.com/eclipse/jetty.project/blob/jetty-12.0.1/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java -- Simone Bordet --- Finally, no matter how good the architecture and design are, to deliver bug-free software with optimal performance and reliability, the implementation technique must be flawless. Victoria Livschitz
Re: RFR: 8315578: PPC builds are broken after JDK-8304913 [v2]
On Thu, 7 Sep 2023 13:51:31 GMT, Aleksey Shipilev wrote: >> Similar to other issues in the same area. I have no PPC32 machine to test >> the build on, but this matches other fixes for other architectures >> (https://github.com/openjdk/jdk/commit/83c096d6e20cd6e1164bc666df1be197a10431eb) >> after this fix the PPC32 Zero build completes fine. > > Aleksey Shipilev 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 three additional > commits since the last revision: > > - Switch to PPC32 > - Merge branch 'master' into JDK-8315578-ppc-builds > - Fix src/java.base/share/classes/jdk/internal/util/PlatformProps.java.template line 62: > 60: static final boolean TARGET_ARCH_IS_LOONGARCH64 = > "@@OPENJDK_TARGET_CPU@@" == "loongarch64"; > 61: static final boolean TARGET_ARCH_IS_S390= > "@@OPENJDK_TARGET_CPU@@" == "s390"; > 62: static final boolean TARGET_ARCH_IS_PPC32 = > "@@OPENJDK_TARGET_CPU@@" == "ppc"; The list is getting longer. I'd suggest/request that all the enums be resorted alphabetically. (now or later). - PR Review Comment: https://git.openjdk.org/jdk/pull/15556#discussion_r1318671548
Re: RFR: 8310929: Optimization for Integer.toString [v21]
On Wed, 6 Sep 2023 22:22:12 GMT, 温绍锦 wrote: >> Optimization for: >> Integer.toString >> Long.toString >> StringBuilder#append(int) >> >> # Benchmark Result >> >> >> sh make/devkit/createJMHBundle.sh >> bash configure --with-jmh=build/jmh/jars >> make test TEST="micro:java.lang.Integers.toString*" >> make test TEST="micro:java.lang.Longs.toString*" >> make test TEST="micro:java.lang.StringBuilders.toStringCharWithInt8" >> >> >> ## 1. >> [aliyun_ecs_c8i.xlarge](https://help.aliyun.com/document_detail/25378.html#c8i) >> * cpu : intel xeon sapphire rapids (x64) >> >> ``` diff >> -Benchmark (size) Mode Cnt Score Error Units (baseline) >> -Integers.toStringBig 500 avgt 15 6.825 ± 0.023 us/op >> -Integers.toStringSmall 500 avgt 15 4.823 ± 0.023 us/op >> -Integers.toStringTiny 500 avgt 15 3.878 ± 0.101 us/op >> >> +Benchmark (size) Mode Cnt Score Error Units (PR Update >> 04 f4aa1989) >> +Integers.toStringBig 500 avgt 15 6.002 ± 0.054 us/op (+13.71%) >> +Integers.toStringSmall 500 avgt 15 4.025 ± 0.020 us/op (+19.82%) >> +Integers.toStringTiny 500 avgt 15 3.874 ± 0.067 us/op (+0.10%) >> >> -Benchmark(size) Mode Cnt Score Error Units (baseline) >> -Longs.toStringBig 500 avgt 15 9.224 ± 0.021 us/op >> -Longs.toStringSmall 500 avgt 15 4.621 ± 0.087 us/op >> >> +Benchmark(size) Mode Cnt Score Error Units (PR Update 04 >> f4aa1989) >> +Longs.toStringBig 500 avgt 15 7.483 ± 0.018 us/op (+23.26%) >> +Longs.toStringSmall 500 avgt 15 4.020 ± 0.016 us/op (+14.95%) >> >> -Benchmark Mode Cnt ScoreError Units >> (baseline) >> -StringBuilders.toStringCharWithInt8 avgt 1589.327 ± 0.733 ns/op >> >> +BenchmarkMode Cnt Score Error Units (PR >> Update 04 f4aa1989) >> +StringBuilders.toStringCharWithInt8 avgt 15 36.639 ± 0.422 ns/op >> (+143.80%) >> >> >> >> ## 2. >> [aliyun_ecs_c8a.xlarge](https://help.aliyun.com/document_detail/25378.html#c8a) >> * cpu : amd epc genoa (x64) >> >> ``` diff >> -Benchmark (size) Mode Cnt Score Error Units (baseline) >> -Integers.toStringBig 500 avgt 15 6.753 ± 0.007 us/op >> -Integers.toStringSmall 500 avgt 15 4.470 ± 0.005 us/op >> -Integers.toStringTiny 500 avgt 15 2.764 ± 0.020 us/op >> >> +Benchmark (size) Mode Cnt Score Error Units (PR Update >> 04 f4aa1989) >> +Integers.toStringBig 500 avgt 15 5.036 ± 0.005 us/op (+... > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > use ByteArrayLittleEndian Looks good, thank you for your patience. For future performance PRs, please put the detailed performance data in a separate comment, not the description. The description is included in every email and the perf data gets out of date quickly. - Marked as reviewed by rriggs (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14699#pullrequestreview-1615561446
RFR: 8315683: Parallelize java/util/concurrent/tck/JSR166TestCase.java
This test is running in tier1, and takes about 400 seconds to complete. Thus, it drags the execution time of tier1 on large machines. The commit includes changing the sequential run of test cases in java/util/concurrent/tck/JSR166TestCase.java to the best possible combination of parallelization to reduce the total runtime of the overall test and causing least possible change to user and system times. The below comparison shows existing and newer combinations of parallelizations: before : make test CONF=macosx-aarch64-server-fastdebug 200.64s user 20.94s system 204% cpu 1:48.47 total fully parallel : make test CONF=macosx-aarch64-server-fastdebug 308.84s user 23.75s system 479% cpu 1:09.42 total default-details-tck: make test CONF=macosx-aarch64-server-fastdebug 244.69s user 22.03s system 314% cpu 1:24.94 total default-fjp_details-tck : make test CONF=macosx-aarch64-server-fastdebug 260.12s user 24.47s system 404% cpu 1:10.31 total Based on the comparison above, the fourth combination has a result very close to full parallelization and at the same time having the least deviation of system and user times among the newer combinations. Hence the commit includes the changes corresponding to the fourth combination. - Commit messages: - 8315683: Parallelize java/util/concurrent/tck/JSR166TestCase.java Changes: https://git.openjdk.org/jdk/pull/15619/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15619=00 Issue: https://bugs.openjdk.org/browse/JDK-8315683 Stats: 19 lines in 1 file changed: 15 ins; 2 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/15619.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15619/head:pull/15619 PR: https://git.openjdk.org/jdk/pull/15619
Re: RFR: 8315683: Parallelize java/util/concurrent/tck/JSR166TestCase.java [v2]
> This test is running in tier1, and takes about 400 seconds to complete. Thus, > it drags the execution time of tier1 on large machines. The commit includes > changing the sequential run of test cases in > java/util/concurrent/tck/JSR166TestCase.java to the best possible combination > of parallelization to reduce the total runtime of the overall test and > causing least possible change to user and system times. The below comparison > shows existing and newer combinations of parallelizations: > before : make test CONF=macosx-aarch64-server-fastdebug > 200.64s user 20.94s system 204% cpu 1:48.47 total > fully parallel : make test CONF=macosx-aarch64-server-fastdebug > 308.84s user 23.75s system 479% cpu 1:09.42 total > default-details-tck: make test CONF=macosx-aarch64-server-fastdebug > 244.69s user 22.03s system 314% cpu 1:24.94 total > default-fjp_details-tck : make test CONF=macosx-aarch64-server-fastdebug > 260.12s user 24.47s system 404% cpu 1:10.31 total > > Based on the comparison above, the fourth combination has a result very close > to full parallelization and at the same time having the least deviation of > system and user times among the newer combinations. Hence the commit includes > the changes corresponding to the fourth combination. Soumadipta Roy has updated the pull request incrementally with one additional commit since the last revision: Remove extra new line - Changes: - all: https://git.openjdk.org/jdk/pull/15619/files - new: https://git.openjdk.org/jdk/pull/15619/files/591b1134..1482259c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15619=01 - incr: https://webrevs.openjdk.org/?repo=jdk=15619=00-01 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/15619.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15619/head:pull/15619 PR: https://git.openjdk.org/jdk/pull/15619
Re: RFR: 8315578: PPC builds are broken after JDK-8304913 [v2]
On Thu, 7 Sep 2023 13:45:50 GMT, Aleksey Shipilev wrote: >> One might expect `isPPC()` to return true on PPC64 because it technically >> is. This is the only drawback I see. > > I don't have a strong opinion here, so if you want PPC32, you got it in new > commit :) Yeah, at least for the `isPPC32()` method. I think a Java API should minimize risk of confusion and misunderstandings, not reflect the design of the build system. Thanks! - PR Review Comment: https://git.openjdk.org/jdk/pull/15556#discussion_r1318646086
Re: RFR: 8315578: PPC builds are broken after JDK-8304913 [v2]
On Thu, 7 Sep 2023 13:51:31 GMT, Aleksey Shipilev wrote: >> Similar to other issues in the same area. I have no PPC32 machine to test >> the build on, but this matches other fixes for other architectures >> (https://github.com/openjdk/jdk/commit/83c096d6e20cd6e1164bc666df1be197a10431eb) >> after this fix the PPC32 Zero build completes fine. > > Aleksey Shipilev 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 three additional > commits since the last revision: > > - Switch to PPC32 > - Merge branch 'master' into JDK-8315578-ppc-builds > - Fix Thanks! - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15556#pullrequestreview-1615501484
Re: RFR: 8315578: PPC builds are broken after JDK-8304913 [v2]
On Thu, 7 Sep 2023 12:45:14 GMT, Martin Doerr wrote: >> Yeah, we can technically change to `PPC32`. But the current thing follows >> what build system uses as `OPENJDK_TARGET_CPU` (`ppc`) and what would be >> used as `os.arch` because of that. So, choosing which way to deviate: >> towards build-system/java-props or towards hotspot macros, I think what >> current PR does is the lesser evil. > > One might expect `isPPC()` to return true on PPC64 because it technically is. > This is the only drawback I see. I don't have a strong opinion here, so if you want PPC32, you got it in new commit :) - PR Review Comment: https://git.openjdk.org/jdk/pull/15556#discussion_r1318636285
Re: RFR: 8315578: PPC builds are broken after JDK-8304913 [v2]
> Similar to other issues in the same area. I have no PPC32 machine to test the > build on, but this matches other fixes for other architectures > (https://github.com/openjdk/jdk/commit/83c096d6e20cd6e1164bc666df1be197a10431eb) > after this fix the PPC32 Zero build completes fine. Aleksey Shipilev 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 three additional commits since the last revision: - Switch to PPC32 - Merge branch 'master' into JDK-8315578-ppc-builds - Fix - Changes: - all: https://git.openjdk.org/jdk/pull/15556/files - new: https://git.openjdk.org/jdk/pull/15556/files/218991df..d829d243 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15556=01 - incr: https://webrevs.openjdk.org/?repo=jdk=15556=00-01 Stats: 9123 lines in 245 files changed: 7533 ins; 1023 del; 567 mod Patch: https://git.openjdk.org/jdk/pull/15556.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15556/head:pull/15556 PR: https://git.openjdk.org/jdk/pull/15556
Re: RFR: 8312522: Implementation of Foreign Function & Memory API [v16]
> This patch contains the implementation of the foreign linker & memory API JEP > for Java 22. The initial patch is composed of commits brought over directly > from the [panama-foreign repo](https://github.com/openjdk/panama-foreign). > The main changes found in this patch come from the following PRs: > > 1. https://github.com/openjdk/panama-foreign/pull/836 Where previous > iterations supported converting Java strings to and from native strings in > the UTF-8 encoding, we've extended the supported encoding to all the > encodings found in the `java.nio.charset.StandardCharsets` class. > 2. https://github.com/openjdk/panama-foreign/pull/838 We dropped the > `MemoryLayout::sequenceLayout` factory method which inferred the size of the > sequence to be `Long.MAX_VALUE`, as this led to confusion among clients. A > client is now required to explicitly specify the sequence size. > 3. https://github.com/openjdk/panama-foreign/pull/839 A new API was added: > `Linker::canonicalLayouts`, which exposes a map containing the > platform-specific mappings of common C type names to memory layouts. > 4. https://github.com/openjdk/panama-foreign/pull/840 Memory access > varhandles, as well as byte offset and slice handles derived from memory > layouts, now feature an additional 'base offset' coordinate that is added to > the offset computed by the handle. This allows composing these handles with > other offset computation strategies that may not be based on the same memory > layout. This addresses use-cases where clients are working with 'dynamic' > layouts, whose size might not be known statically, such as variable length > arrays, or variable size matrices. > 5. https://github.com/openjdk/panama-foreign/pull/841 Remove this now > redundant API. Clients can simply use the difference between the base address > of two memory segments. > 6. https://github.com/openjdk/panama-foreign/pull/845 Disambiguate uses of > `SegmentAllocator::allocateArray`, by renaming methods that both allocate + > initialize memory segments to `allocateFrom`. (see the original PR for the > problematic case) > 7. https://github.com/openjdk/panama-foreign/pull/846 Improve the > documentation for variadic functions. > 8. https://github.com/openjdk/panama-foreign/pull/849 Several test fixes to > make sure the `jdk_foreign` tests can pass on 32-bit machines, taking > linux-x86 as a test bed. > 9. https://github.com/openjdk/panama-foreign/pull/850 Make the linker API > required. The `Linker::nativeLinker` method is not longer allowed to throw an > `UnsupportedOperationException` on ... Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: Add support for sliced allocation - Changes: - all: https://git.openjdk.org/jdk/pull/15103/files - new: https://git.openjdk.org/jdk/pull/15103/files/a48a77bc..55296527 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15103=15 - incr: https://webrevs.openjdk.org/?repo=jdk=15103=14-15 Stats: 539 lines in 9 files changed: 413 ins; 56 del; 70 mod Patch: https://git.openjdk.org/jdk/pull/15103.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15103/head:pull/15103 PR: https://git.openjdk.org/jdk/pull/15103
Re: RFR: 8312522: Implementation of Foreign Function & Memory API [v15]
On Thu, 7 Sep 2023 11:39:30 GMT, Jorn Vernee wrote: >> This patch contains the implementation of the foreign linker & memory API >> JEP for Java 22. The initial patch is composed of commits brought over >> directly from the [panama-foreign >> repo](https://github.com/openjdk/panama-foreign). The main changes found in >> this patch come from the following PRs: >> >> 1. https://github.com/openjdk/panama-foreign/pull/836 Where previous >> iterations supported converting Java strings to and from native strings in >> the UTF-8 encoding, we've extended the supported encoding to all the >> encodings found in the `java.nio.charset.StandardCharsets` class. >> 2. https://github.com/openjdk/panama-foreign/pull/838 We dropped the >> `MemoryLayout::sequenceLayout` factory method which inferred the size of the >> sequence to be `Long.MAX_VALUE`, as this led to confusion among clients. A >> client is now required to explicitly specify the sequence size. >> 3. https://github.com/openjdk/panama-foreign/pull/839 A new API was added: >> `Linker::canonicalLayouts`, which exposes a map containing the >> platform-specific mappings of common C type names to memory layouts. >> 4. https://github.com/openjdk/panama-foreign/pull/840 Memory access >> varhandles, as well as byte offset and slice handles derived from memory >> layouts, now feature an additional 'base offset' coordinate that is added to >> the offset computed by the handle. This allows composing these handles with >> other offset computation strategies that may not be based on the same memory >> layout. This addresses use-cases where clients are working with 'dynamic' >> layouts, whose size might not be known statically, such as variable length >> arrays, or variable size matrices. >> 5. https://github.com/openjdk/panama-foreign/pull/841 Remove this now >> redundant API. Clients can simply use the difference between the base >> address of two memory segments. >> 6. https://github.com/openjdk/panama-foreign/pull/845 Disambiguate uses of >> `SegmentAllocator::allocateArray`, by renaming methods that both allocate + >> initialize memory segments to `allocateFrom`. (see the original PR for the >> problematic case) >> 7. https://github.com/openjdk/panama-foreign/pull/846 Improve the >> documentation for variadic functions. >> 8. https://github.com/openjdk/panama-foreign/pull/849 Several test fixes to >> make sure the `jdk_foreign` tests can pass on 32-bit machines, taking >> linux-x86 as a test bed. >> 9. https://github.com/openjdk/panama-foreign/pull/850 Make the linker API >> required. The `Linker::nativeLinker` method is not longer allowed to throw >> an `UnsupportedO... > > Jorn Vernee has updated the pull request incrementally with two additional > commits since the last revision: > > - add name of SysV ABI > - Fix javadoc issues in MemorySegment::copy > >Reviewed-by: jvernee After discussing with Maurizio, I've added one more non-trivial change to this patch, brought over from the panama-foreign repo. This adds a new `SegmentAllocator::allocateFrom` overload which accepts a MemorySegment as an initializer. See the original PR: https://github.com/openjdk/panama-foreign/pull/878 The commit I've added also includes the changes from https://github.com/openjdk/panama-foreign/pull/855 which were required by the first patch. I had to massage the code a bit since the javadoc in the mainline slightly deviates from the one in the panama-foreign repo. Please take a look at the changes here: https://github.com/openjdk/jdk/pull/15103/commits/55296527a029b80dd78a7d1aecb429e793d7d32e - PR Comment: https://git.openjdk.org/jdk/pull/15103#issuecomment-1710117041
Re: RFR: 8315578: PPC builds are broken after JDK-8304913
On Thu, 7 Sep 2023 12:02:02 GMT, Aleksey Shipilev wrote:
>> src/java.base/share/classes/jdk/internal/util/Architecture.java line 164:
>>
>>> 162: public static boolean isPPC() {
>>> 163: return PlatformProps.TARGET_ARCH_IS_PPC;
>>> 164: }
>>
>> Maybe `isPPC32()` would be a better name (also `PPC32` above). Note that
>> hotspot uses `PPC` for all PPC platforms and has the more specific macros
>> `PPC32` and `PPC64`.
>
> Yeah, we can technically change to `PPC32`. But the current thing follows
> what build system uses as `OPENJDK_TARGET_CPU` (`ppc`) and what would be used
> as `os.arch` because of that. So, choosing which way to deviate: towards
> build-system/java-props or towards hotspot macros, I think what current PR
> does is the lesser evil.
One might expect `isPPC()` to return true on PPC64 because it technically is.
This is the only drawback I see.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15556#discussion_r1318550667
Re: RFR: 8315850: Improve AbstractMap anonymous Iterator classes
On Thu, 7 Sep 2023 11:48:51 GMT, Per Minborg wrote: > This PR proposes to slightly improve some iterators of `AbstractMap`: > > * Code reuse > * A field declared `final` > * Add missing `@Override` annotations Hello, In Java, sharing code may have a runtime cost (or not) depending on the shape of the code, because the VM may have to speculate on the class of some value at runtime. Here, in hasNext() and remove(), the VM has to find the class of the field 'i' at runtime. Iterators are performance sentive (they are used in for loop) and for that they need the escape analysis to work. If the iterator code is polymorphic, so part of the code may be unknown so the escape analysis will consider that the iterator escape. So usually, it's a good practice to not share the iterator code. Also the field should be package private (or even maybe private) but not protected. Protected is very rare in Java because usually classes that share implementation details are in the same package (or in the same nestmate nest). - PR Comment: https://git.openjdk.org/jdk/pull/15615#issuecomment-1710058719
Re: RFR: 8310929: Optimization for Integer.toString [v21]
On Wed, 6 Sep 2023 22:22:12 GMT, 温绍锦 wrote: >> Optimization for: >> Integer.toString >> Long.toString >> StringBuilder#append(int) >> >> # Benchmark Result >> >> >> sh make/devkit/createJMHBundle.sh >> bash configure --with-jmh=build/jmh/jars >> make test TEST="micro:java.lang.Integers.toString*" >> make test TEST="micro:java.lang.Longs.toString*" >> make test TEST="micro:java.lang.StringBuilders.toStringCharWithInt8" >> >> >> ## 1. >> [aliyun_ecs_c8i.xlarge](https://help.aliyun.com/document_detail/25378.html#c8i) >> * cpu : intel xeon sapphire rapids (x64) >> >> ``` diff >> -Benchmark (size) Mode Cnt Score Error Units (baseline) >> -Integers.toStringBig 500 avgt 15 6.825 ± 0.023 us/op >> -Integers.toStringSmall 500 avgt 15 4.823 ± 0.023 us/op >> -Integers.toStringTiny 500 avgt 15 3.878 ± 0.101 us/op >> >> +Benchmark (size) Mode Cnt Score Error Units (PR Update >> 04 f4aa1989) >> +Integers.toStringBig 500 avgt 15 6.002 ± 0.054 us/op (+13.71%) >> +Integers.toStringSmall 500 avgt 15 4.025 ± 0.020 us/op (+19.82%) >> +Integers.toStringTiny 500 avgt 15 3.874 ± 0.067 us/op (+0.10%) >> >> -Benchmark(size) Mode Cnt Score Error Units (baseline) >> -Longs.toStringBig 500 avgt 15 9.224 ± 0.021 us/op >> -Longs.toStringSmall 500 avgt 15 4.621 ± 0.087 us/op >> >> +Benchmark(size) Mode Cnt Score Error Units (PR Update 04 >> f4aa1989) >> +Longs.toStringBig 500 avgt 15 7.483 ± 0.018 us/op (+23.26%) >> +Longs.toStringSmall 500 avgt 15 4.020 ± 0.016 us/op (+14.95%) >> >> -Benchmark Mode Cnt ScoreError Units >> (baseline) >> -StringBuilders.toStringCharWithInt8 avgt 1589.327 ± 0.733 ns/op >> >> +BenchmarkMode Cnt Score Error Units (PR >> Update 04 f4aa1989) >> +StringBuilders.toStringCharWithInt8 avgt 15 36.639 ± 0.422 ns/op >> (+143.80%) >> >> >> >> ## 2. >> [aliyun_ecs_c8a.xlarge](https://help.aliyun.com/document_detail/25378.html#c8a) >> * cpu : amd epc genoa (x64) >> >> ``` diff >> -Benchmark (size) Mode Cnt Score Error Units (baseline) >> -Integers.toStringBig 500 avgt 15 6.753 ± 0.007 us/op >> -Integers.toStringSmall 500 avgt 15 4.470 ± 0.005 us/op >> -Integers.toStringTiny 500 avgt 15 2.764 ± 0.020 us/op >> >> +Benchmark (size) Mode Cnt Score Error Units (PR Update >> 04 f4aa1989) >> +Integers.toStringBig 500 avgt 15 5.036 ± 0.005 us/op (+... > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > use ByteArrayLittleEndian Can it be merged now? - PR Comment: https://git.openjdk.org/jdk/pull/14699#issuecomment-1710017528
RFR: 8315850: Improve AbstractMap anonymous Iterator classes
This PR proposes to slightly improve some iterators of `AbstractMap`: * Code reuse * A field declared `final` * Add missing `@Override` annotations - Commit messages: - Improve AbstractMap anonymous Iterator classes Changes: https://git.openjdk.org/jdk/pull/15615/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15615=00 Issue: https://bugs.openjdk.org/browse/JDK-8315850 Stats: 76 lines in 1 file changed: 44 ins; 28 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/15615.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15615/head:pull/15615 PR: https://git.openjdk.org/jdk/pull/15615
Re: RFR: 8315444: Convert test/jdk/tools to Classfile API [v3]
On Wed, 6 Sep 2023 16:44:15 GMT, Qing Xiao wrote: >> `/test/jdk/tools/lib/tests/JImageValidator.java`, tests in >> `/test/jdk/tools/jlink`, and `/test/jdk/tools/jimage`, >> `/test/jdk/java/time/nontestng/java/time/chrono/HijrahConfigTest.java` use >> on com.sun.tools.classfile and should be converted to Classfile API. > > Qing Xiao has updated the pull request incrementally with two additional > commits since the last revision: > > - Change the position of a brace > - Remove unnecessary import of `impl` and added stronger verify in > JImageValidator.java Marked as reviewed by asotona (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/15529#pullrequestreview-1615275932
Re: RFR: 8315578: PPC builds are broken after JDK-8304913
On Mon, 4 Sep 2023 19:54:46 GMT, Martin Doerr wrote:
>> Similar to other issues in the same area. I have no PPC32 machine to test
>> the build on, but this matches other fixes for other architectures
>> (https://github.com/openjdk/jdk/commit/83c096d6e20cd6e1164bc666df1be197a10431eb)
>> after this fix the PPC32 Zero build completes fine.
>
> src/java.base/share/classes/jdk/internal/util/Architecture.java line 164:
>
>> 162: public static boolean isPPC() {
>> 163: return PlatformProps.TARGET_ARCH_IS_PPC;
>> 164: }
>
> Maybe `isPPC32()` would be a better name (also `PPC32` above). Note that
> hotspot uses `PPC` for all PPC platforms and has the more specific macros
> `PPC32` and `PPC64`.
Yeah, we can technically change to `PPC32`. But the current thing follows what
build system uses as `OPENJDK_TARGET_CPU` (`ppc`) and what would be used as
`os.arch` because of that. So, choosing which way to deviate: towards
build-system/java-props or towards hotspot macros, I think what current PR does
is the lesser evil.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15556#discussion_r1318502898
Integrated: 8314260: Unable to load system libraries on Windows when using a SecurityManager
On Tue, 5 Sep 2023 08:52:50 GMT, Per Minborg wrote: > This PR proposes to read the system environment variable "SystemRoot" using a > privileged operation so it will work in the event a default SecurityManager > is in place. > > As the `SecurityManager` is deprecated for removal, no support methods were > added for reading environmental variables. This pull request has now been integrated. Changeset: b408a82f Author:Per Minborg URL: https://git.openjdk.org/jdk/commit/b408a82f9b4ce4441f49d745034ef923a880778f Stats: 26 lines in 3 files changed: 24 ins; 0 del; 2 mod 8314260: Unable to load system libraries on Windows when using a SecurityManager Co-authored-by: Jorn Vernee Reviewed-by: jvernee - PR: https://git.openjdk.org/jdk/pull/15564
Re: RFR: 8314260: Unable to load system libraries on Windows when using a SecurityManager [v4]
On Thu, 7 Sep 2023 07:46:03 GMT, Per Minborg wrote: >> This PR proposes to read the system environment variable "SystemRoot" using >> a privileged operation so it will work in the event a default >> SecurityManager is in place. >> >> As the `SecurityManager` is deprecated for removal, no support methods were >> added for reading environmental variables. > > Per Minborg has updated the pull request incrementally with one additional > commit since the last revision: > > Add privilidged action and restrict grants Marked as reviewed by jvernee (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/15564#pullrequestreview-1615246090
Re: RFR: 8312522: Implementation of Foreign Function & Memory API [v10]
On Wed, 6 Sep 2023 16:03:40 GMT, Paul Sandoz wrote: >> [This SO question](https://stackoverflow.com/a/40348010) points to a gitlab >> repo that seems to have the latest version: >> https://gitlab.com/x86-psABIs/x86-64-ABI But, I'm not sure how stable that >> is, or if that's an authoritative source. >> >> Alternatively, we could refer to the name only: "System V Application Binary >> Interface - AMD64 Architecture Processor Supplement" (or "x86-64 psABI") >> Then people can google for themselves and find it. > > Yeah, its hard to find the official and latest version. Referring to the full > title will help. I've added the name now: https://github.com/openjdk/jdk/pull/15103/commits/a48a77bcdadda65a81ad30abf00e6da46a56b933 - PR Review Comment: https://git.openjdk.org/jdk/pull/15103#discussion_r1318472039
Re: RFR: 8312522: Implementation of Foreign Function & Memory API [v15]
> This patch contains the implementation of the foreign linker & memory API JEP > for Java 22. The initial patch is composed of commits brought over directly > from the [panama-foreign repo](https://github.com/openjdk/panama-foreign). > The main changes found in this patch come from the following PRs: > > 1. https://github.com/openjdk/panama-foreign/pull/836 Where previous > iterations supported converting Java strings to and from native strings in > the UTF-8 encoding, we've extended the supported encoding to all the > encodings found in the `java.nio.charset.StandardCharsets` class. > 2. https://github.com/openjdk/panama-foreign/pull/838 We dropped the > `MemoryLayout::sequenceLayout` factory method which inferred the size of the > sequence to be `Long.MAX_VALUE`, as this led to confusion among clients. A > client is now required to explicitly specify the sequence size. > 3. https://github.com/openjdk/panama-foreign/pull/839 A new API was added: > `Linker::canonicalLayouts`, which exposes a map containing the > platform-specific mappings of common C type names to memory layouts. > 4. https://github.com/openjdk/panama-foreign/pull/840 Memory access > varhandles, as well as byte offset and slice handles derived from memory > layouts, now feature an additional 'base offset' coordinate that is added to > the offset computed by the handle. This allows composing these handles with > other offset computation strategies that may not be based on the same memory > layout. This addresses use-cases where clients are working with 'dynamic' > layouts, whose size might not be known statically, such as variable length > arrays, or variable size matrices. > 5. https://github.com/openjdk/panama-foreign/pull/841 Remove this now > redundant API. Clients can simply use the difference between the base address > of two memory segments. > 6. https://github.com/openjdk/panama-foreign/pull/845 Disambiguate uses of > `SegmentAllocator::allocateArray`, by renaming methods that both allocate + > initialize memory segments to `allocateFrom`. (see the original PR for the > problematic case) > 7. https://github.com/openjdk/panama-foreign/pull/846 Improve the > documentation for variadic functions. > 8. https://github.com/openjdk/panama-foreign/pull/849 Several test fixes to > make sure the `jdk_foreign` tests can pass on 32-bit machines, taking > linux-x86 as a test bed. > 9. https://github.com/openjdk/panama-foreign/pull/850 Make the linker API > required. The `Linker::nativeLinker` method is not longer allowed to throw an > `UnsupportedOperationException` on ... Jorn Vernee has updated the pull request incrementally with two additional commits since the last revision: - add name of SysV ABI - Fix javadoc issues in MemorySegment::copy Reviewed-by: jvernee - Changes: - all: https://git.openjdk.org/jdk/pull/15103/files - new: https://git.openjdk.org/jdk/pull/15103/files/52df58f5..a48a77bc Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15103=14 - incr: https://webrevs.openjdk.org/?repo=jdk=15103=13-14 Stats: 10 lines in 2 files changed: 3 ins; 0 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/15103.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15103/head:pull/15103 PR: https://git.openjdk.org/jdk/pull/15103
Re: RFR: 8310929: Optimization for Integer.toString [v21]
On Wed, 6 Sep 2023 22:22:12 GMT, 温绍锦 wrote: >> Optimization for: >> Integer.toString >> Long.toString >> StringBuilder#append(int) >> >> # Benchmark Result >> >> >> sh make/devkit/createJMHBundle.sh >> bash configure --with-jmh=build/jmh/jars >> make test TEST="micro:java.lang.Integers.toString*" >> make test TEST="micro:java.lang.Longs.toString*" >> make test TEST="micro:java.lang.StringBuilders.toStringCharWithInt8" >> >> >> ## 1. >> [aliyun_ecs_c8i.xlarge](https://help.aliyun.com/document_detail/25378.html#c8i) >> * cpu : intel xeon sapphire rapids (x64) >> >> ``` diff >> -Benchmark (size) Mode Cnt Score Error Units (baseline) >> -Integers.toStringBig 500 avgt 15 6.825 ± 0.023 us/op >> -Integers.toStringSmall 500 avgt 15 4.823 ± 0.023 us/op >> -Integers.toStringTiny 500 avgt 15 3.878 ± 0.101 us/op >> >> +Benchmark (size) Mode Cnt Score Error Units (PR Update >> 04 f4aa1989) >> +Integers.toStringBig 500 avgt 15 6.002 ± 0.054 us/op (+13.71%) >> +Integers.toStringSmall 500 avgt 15 4.025 ± 0.020 us/op (+19.82%) >> +Integers.toStringTiny 500 avgt 15 3.874 ± 0.067 us/op (+0.10%) >> >> -Benchmark(size) Mode Cnt Score Error Units (baseline) >> -Longs.toStringBig 500 avgt 15 9.224 ± 0.021 us/op >> -Longs.toStringSmall 500 avgt 15 4.621 ± 0.087 us/op >> >> +Benchmark(size) Mode Cnt Score Error Units (PR Update 04 >> f4aa1989) >> +Longs.toStringBig 500 avgt 15 7.483 ± 0.018 us/op (+23.26%) >> +Longs.toStringSmall 500 avgt 15 4.020 ± 0.016 us/op (+14.95%) >> >> -Benchmark Mode Cnt ScoreError Units >> (baseline) >> -StringBuilders.toStringCharWithInt8 avgt 1589.327 ± 0.733 ns/op >> >> +BenchmarkMode Cnt Score Error Units (PR >> Update 04 f4aa1989) >> +StringBuilders.toStringCharWithInt8 avgt 15 36.639 ± 0.422 ns/op >> (+143.80%) >> >> >> >> ## 2. >> [aliyun_ecs_c8a.xlarge](https://help.aliyun.com/document_detail/25378.html#c8a) >> * cpu : amd epc genoa (x64) >> >> ``` diff >> -Benchmark (size) Mode Cnt Score Error Units (baseline) >> -Integers.toStringBig 500 avgt 15 6.753 ± 0.007 us/op >> -Integers.toStringSmall 500 avgt 15 4.470 ± 0.005 us/op >> -Integers.toStringTiny 500 avgt 15 2.764 ± 0.020 us/op >> >> +Benchmark (size) Mode Cnt Score Error Units (PR Update >> 04 f4aa1989) >> +Integers.toStringBig 500 avgt 15 5.036 ± 0.005 us/op (+... > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > use ByteArrayLittleEndian Using `ByteArrayLittleEndian` cleaned up the code nicely. The overhead compared to `Unsafe` doesn't seem too concerning (maybe even some wins?) - Marked as reviewed by redestad (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14699#pullrequestreview-1615137576
Re: RFR: 8315004: Runtime.halt() debug logging
On Fri, 25 Aug 2023 09:37:47 GMT, Masanori Yano wrote: > I want to add a log output similar to JDK-8301627 to Runtime.halt(). > To avoid double logging of Runtime.exit(), add a flag to indicate whether > logging was done, and fix it so that logging is done only once. > Could someone please review this fix? I agree that Runtime.halt() should be terminated immediately. However, I don't think this logging feature is for normal use, but for troubleshooting when we can't find where Runtime.halt() is called. So I don't think it will prevent Runtime.halt() from exiting immediately in normal operation. - PR Comment: https://git.openjdk.org/jdk/pull/15426#issuecomment-1709793801
Re: RFR: 8294969: Convert jdk.jdeps javap to use the Classfile API [v12]
> javap uses proprietary com.sun.tools.classfile library to parse class files. > > This patch converts javap to use Classfile API. > > Please review. > > Thanks, > Adam Adam Sotona has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 229 commits: - minor patches - Merge branch 'master' into JDK-8294969 - Merge branch 'master' into JDK-8294969-javap - Merge branch 'master' into JDK-8294969-javap - fixed code printing and ConstantPoolException reporting indoex - added DydnamicConstantPoolEntry::bootstrapMethodIndex fix of javap ConstantWriter to print DynamicConstantPoolEntry without accessing BSM attribute - extended ClassReader about specific entry-reading methods to avoid class cast and throw ConstantPoolException instead - throwing ConstantPoolException for invalid BSM entry index - Merge branch 'master' into JDK-8294969-javap - fixed JavapTask - ... and 219 more: https://git.openjdk.org/jdk/compare/9887cd8a...697079f7 - Changes: https://git.openjdk.org/jdk/pull/11411/files Webrev: https://webrevs.openjdk.org/?repo=jdk=11411=11 Stats: 3831 lines in 29 files changed: 1001 ins; 1701 del; 1129 mod Patch: https://git.openjdk.org/jdk/pull/11411.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/11411/head:pull/11411 PR: https://git.openjdk.org/jdk/pull/11411
Re: RFR: JDK-8315751: RandomTestBsi1999 fails often with timeouts on Linux ppc64le
On Wed, 6 Sep 2023 14:47:20 GMT, Matthias Baesken wrote: > The test RandomTestBsi1999 fails often with timeouts on Linux ppc64le; even > when it succeeds the test takes a lot of time and is close to timing out. > Probably we should increase the timeout value for this test. Looks good. - PR Review: https://git.openjdk.org/jdk/pull/15594#pullrequestreview-1614961811
Re: RFR: 8288899: java/util/concurrent/ExecutorService/CloseTest.java failed with "InterruptedException: sleep interrupted" [v25]
On Wed, 6 Sep 2023 13:07:10 GMT, Doug Lea wrote:
>> Addresses Jdk 8288899 : java/util/concurrent/ExecutorService/CloseTest.java
>> failed with "InterruptedException: sleep interrupted" and related issues.
>>
>> This is a major ForkJoin update (and hard to review -- sorry) that finally
>> addresses incompatibilities between ExecutorService and ForkJoinPool (which
>> claims to implement it), with the goal of avoiding continuing bug reports
>> and incompatibilities. Doing this required reworking internal control to use
>> phaser/seqlock-style versioning schemes (affecting nearly every method) that
>> ensure consistent data structures and actions without requiring global
>> synchronization or locking on every task execution that would massively
>> degrade performance. The previous lack of a solution to this was the main
>> reason for these incompatibilities.
>
> Doug Lea has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Allow ThreadGroup access in tck tests
test/jdk/java/util/concurrent/tck/JSR166TestCase.java line 1687:
> 1685: thread.join(timeoutMillis);
> 1686: break;
> 1687: } catch (InterruptedException ignore) {
@DougLea Shouldn't this deduct the wait-time for the next join if it gets
interrupted? 樂
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14301#discussion_r1318289951
Re: RFR: 8315810: Reimplement sun.reflect.ReflectionFactory::newConstructorForSerialization with method handles
On Wed, 6 Sep 2023 18:34:29 GMT, Mandy Chung wrote:
> This reimplements
> `sun.reflect.ReflectionFactory::newConstructorForSerialization` with method
> handles.
>
> This API currently generates the bytecode which fails the verification
> because `new C; invokespecial A()` where the given class `C` and invoke a
> no-arg constructor of `C`'s first non-`Serializable` superclass `A` is not a
> valid operation per the VM specification. VM special cases the classes
> generated for reflection to skip verification for the constructors generated
> for serialization and externalization. This change will allow such VM hack
> to be removed.
>
> A `jdk.reflect.useOldSerializableConstructor` system property can be set to
> use the old implementation in case if customers run into any compatibility
> issue. I expect this change has very low compatibility risk. This system
> property is undocumented and will be removed in a future release.
src/java.base/share/classes/jdk/internal/reflect/DirectMethodHandleAccessor.java
line 198:
> 196: if (!declaringClass.isAssignableFrom(o.getClass())) {
> 197: throw new IllegalArgumentException("object of type " +
> o.getClass().getName()
> 198: + " is not an instance of " +
> declaringClass.getName());
Suggestion:
+ " is not an instance of " + declaringClass.getName());
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15600#discussion_r1318295076
Re: RFR: 8294969: Convert jdk.jdeps javap to use the Classfile API [v11]
On Wed, 6 Sep 2023 19:53:12 GMT, Vicente Romero wrote: >> Adam Sotona has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 227 commits: >> >> - Merge branch 'master' into JDK-8294969-javap >> - Merge branch 'master' into JDK-8294969-javap >> - fixed code printing and ConstantPoolException reporting indoex >> - added DydnamicConstantPoolEntry::bootstrapMethodIndex >>fix of javap ConstantWriter to print DynamicConstantPoolEntry without >> accessing BSM attribute >> - extended ClassReader about specific entry-reading methods to avoid class >> cast and throw ConstantPoolException instead >> - throwing ConstantPoolException for invalid BSM entry index >> - Merge branch 'master' into JDK-8294969-javap >> - fixed JavapTask >> - Merge branch 'master' into JDK-8294969-javap >> - Merge branch 'master' into JDK-8294969-javap >> >># Conflicts: >># >> src/java.base/share/classes/jdk/internal/classfile/constantpool/ConstantPoolException.java >> - ... and 217 more: https://git.openjdk.org/jdk/compare/37c756a7...4960751b > > src/jdk.jdeps/share/classes/com/sun/tools/javap/TypeAnnotationWriter.java > line 134: > >> 132: private ClassWriter classWriter; >> 133: private Map> pcMap; >> 134: private CodeAttribute lr; > > nit: name `lr` doesn't relate much to the type of the field Right, `lr` historically came from label resolver, I'll fix it. - PR Review Comment: https://git.openjdk.org/jdk/pull/11411#discussion_r1318277017
Re: RFR: 8287843: File::getCanonicalFile doesn't work for \?\C:\ style paths DOS device paths
On Wed, 6 Sep 2023 21:38:39 GMT, Brian Burkhalter wrote: > In the Windows implementation of java.io.File.getCanonicalPath, strip any > long path or UNC prefix before canonicalizing the remainder of the pathname. Would it be possible to summarise how this behaves when File just presents this prefix? It would be a weird thing to do but it may come about from usages of getParent or other methods. I think I'm mostly asking if is possible for the method to return the canonical path of the current directory when it should fail. - PR Comment: https://git.openjdk.org/jdk/pull/15603#issuecomment-1709709146
Re: RFR: 8294969: Convert jdk.jdeps javap to use the Classfile API [v11]
On Wed, 6 Sep 2023 19:57:19 GMT, Vicente Romero wrote: >> Adam Sotona has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 227 commits: >> >> - Merge branch 'master' into JDK-8294969-javap >> - Merge branch 'master' into JDK-8294969-javap >> - fixed code printing and ConstantPoolException reporting indoex >> - added DydnamicConstantPoolEntry::bootstrapMethodIndex >>fix of javap ConstantWriter to print DynamicConstantPoolEntry without >> accessing BSM attribute >> - extended ClassReader about specific entry-reading methods to avoid class >> cast and throw ConstantPoolException instead >> - throwing ConstantPoolException for invalid BSM entry index >> - Merge branch 'master' into JDK-8294969-javap >> - fixed JavapTask >> - Merge branch 'master' into JDK-8294969-javap >> - Merge branch 'master' into JDK-8294969-javap >> >># Conflicts: >># >> src/java.base/share/classes/jdk/internal/classfile/constantpool/ConstantPoolException.java >> - ... and 217 more: https://git.openjdk.org/jdk/compare/37c756a7...4960751b > > test/langtools/tools/javap/malformed/MalformedTest.java line 2: > >> 1: /* >> 2: * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. > > isn't this test testing the same as: > `test/langtools/tools/javap/8260403/T8260403.java`? The principle of the test is the same, but it tests intentionally different class malformations (on a different level). - PR Review Comment: https://git.openjdk.org/jdk/pull/11411#discussion_r1318272202
Re: RFR: 8294969: Convert jdk.jdeps javap to use the Classfile API [v11]
On Wed, 6 Sep 2023 19:55:49 GMT, Vicente Romero wrote:
>> Adam Sotona has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains 227 commits:
>>
>> - Merge branch 'master' into JDK-8294969-javap
>> - Merge branch 'master' into JDK-8294969-javap
>> - fixed code printing and ConstantPoolException reporting indoex
>> - added DydnamicConstantPoolEntry::bootstrapMethodIndex
>>fix of javap ConstantWriter to print DynamicConstantPoolEntry without
>> accessing BSM attribute
>> - extended ClassReader about specific entry-reading methods to avoid class
>> cast and throw ConstantPoolException instead
>> - throwing ConstantPoolException for invalid BSM entry index
>> - Merge branch 'master' into JDK-8294969-javap
>> - fixed JavapTask
>> - Merge branch 'master' into JDK-8294969-javap
>> - Merge branch 'master' into JDK-8294969-javap
>>
>># Conflicts:
>>#
>> src/java.base/share/classes/jdk/internal/classfile/constantpool/ConstantPoolException.java
>> - ... and 217 more: https://git.openjdk.org/jdk/compare/37c756a7...4960751b
>
> test/langtools/tools/javap/8260403/T8260403.java line 42:
>
>> 40: new String[]{"-c", System.getProperty("test.classes") +
>> "/InvalidSignature.class"},
>> 41: new PrintWriter(sw));
>> 42: System.out.println(sw);
>
> is this for debug purpose only?
It is the test fix.
8260403 says "javap should be more robust", however to expect zero exit code
for invalid class is wrong.
It suppose to expect non-zero exit code and fail only on fatal errors or
unhandled CP errors.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/11411#discussion_r1318264327
Re: RFR: 8314260: Unable to load system libraries on Windows when using a SecurityManager [v4]
> This PR proposes to read the system environment variable "SystemRoot" using a > privileged operation so it will work in the event a default SecurityManager > is in place. > > As the `SecurityManager` is deprecated for removal, no support methods were > added for reading environmental variables. Per Minborg has updated the pull request incrementally with one additional commit since the last revision: Add privilidged action and restrict grants - Changes: - all: https://git.openjdk.org/jdk/pull/15564/files - new: https://git.openjdk.org/jdk/pull/15564/files/0cc7c381..16e74316 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15564=03 - incr: https://webrevs.openjdk.org/?repo=jdk=15564=02-03 Stats: 8 lines in 2 files changed: 6 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/15564.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15564/head:pull/15564 PR: https://git.openjdk.org/jdk/pull/15564
Re: RFR: JDK-8315751: RandomTestBsi1999 fails often with timeouts on Linux ppc64le
On Wed, 6 Sep 2023 14:47:20 GMT, Matthias Baesken wrote: > The test RandomTestBsi1999 fails often with timeouts on Linux ppc64le; even > when it succeeds the test takes a lot of time and is close to timing out. > Probably we should increase the timeout value for this test. Hi Martin, thanks for the review ! - PR Comment: https://git.openjdk.org/jdk/pull/15594#issuecomment-1709628033
Integrated: JDK-8315751: RandomTestBsi1999 fails often with timeouts on Linux ppc64le
On Wed, 6 Sep 2023 14:47:20 GMT, Matthias Baesken wrote: > The test RandomTestBsi1999 fails often with timeouts on Linux ppc64le; even > when it succeeds the test takes a lot of time and is close to timing out. > Probably we should increase the timeout value for this test. This pull request has now been integrated. Changeset: 9887cd8a Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/9887cd8adc408a71b045b1a4891cc0d5dede7e0e Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod 8315751: RandomTestBsi1999 fails often with timeouts on Linux ppc64le Reviewed-by: mdoerr - PR: https://git.openjdk.org/jdk/pull/15594
Integrated: JDK-8314121: test tools/jpackage/share/RuntimePackageTest.java#id0 fails on RHEL8
On Fri, 1 Sep 2023 07:22:12 GMT, Matthias Baesken wrote: > on some RHEL Linux 8.X machines , we run into errors in test > tools/jpackage/share/RuntimePackageTest.java#id0 , error can be seen below. > It looks like these errors occur when running the jtreg tests with higher > concurrency, I did not see them when running just the single test. > > When adding some test output , we see the 2 top install dirs below (1 is > expected, not 2) : > ./test/unpacked-rpm/opt > ./test/unpacked-rpm/usr > > error output : > > java.lang.AssertionError: Expected [1]. Actual [2]: Check the package has 1 > top installation directories >at jdk.jpackage.test.TKit.error(TKit.java:273) >at jdk.jpackage.test.TKit.assertEquals(TKit.java:576) >at > jdk.jpackage.test.PackageTest$Handler.verifyRootCountInUnpackedPackage(PackageTest.java:705) >at > jdk.jpackage.test.PackageTest$Handler.lambda$verifyPackageInstalled$3(PackageTest.java:635) >at java.base/java.util.Optional.ifPresent(Optional.java:178) >at > jdk.jpackage.test.PackageTest$Handler.verifyPackageInstalled(PackageTest.java:633) >at jdk.jpackage.test.PackageTest$Handler.accept(PackageTest.java:592) >at jdk.jpackage.test.PackageTest$2.accept(PackageTest.java:504) >at jdk.jpackage.test.PackageTest$2.accept(PackageTest.java:411) >at > jdk.jpackage.test.Functional$ThrowingConsumer.lambda$toConsumer$0(Functional.java:41) >at java.base/java.lang.Iterable.forEach(Iterable.java:75) >at > jdk.jpackage.test.PackageTest.lambda$runActions$24(PackageTest.java:381) >at java.base/java.util.ArrayList.forEach(ArrayList.java:1596) >at > jdk.jpackage.test.PackageTest.lambda$runActions$25(PackageTest.java:380) >at java.base/java.util.ArrayList.forEach(ArrayList.java:1596) >at jdk.jpackage.test.PackageTest.runActions(PackageTest.java:379) >at > jdk.jpackage.test.RunnablePackageTest.run(RunnablePackageTest.java:58) >at RuntimePackageTest.test(RuntimePackageTest.java:83) >at > java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) >at java.base/java.lang.reflect.Method.invoke(Method.java:580) >at jdk.jpackage.test.MethodCall.accept(MethodCall.java:145) >at jdk.jpackage.test.TestInstance.run(TestInstance.java:230) >at jdk.jpackage.test.TKit.lambda$ignoreExceptions$5(TKit.java:141) >at jdk.jpackage.test.TKit.lambda$runTests$3(TKit.java:126) >at > java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1708) >at java.base/jav... This pull request has now been integrated. Changeset: 8107eab3 Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/8107eab3c09b3f9fcf1348c3bf1deb7c4ac2fdf3 Stats: 3 lines in 1 file changed: 3 ins; 0 del; 0 mod 8314121: test tools/jpackage/share/RuntimePackageTest.java#id0 fails on RHEL8 Reviewed-by: lucy, asemenyuk - PR: https://git.openjdk.org/jdk/pull/15528
Re: RFR: JDK-8314121: test tools/jpackage/share/RuntimePackageTest.java#id0 fails on RHEL8 [v2]
On Wed, 6 Sep 2023 07:29:20 GMT, Matthias Baesken wrote: >> on some RHEL Linux 8.X machines , we run into errors in test >> tools/jpackage/share/RuntimePackageTest.java#id0 , error can be seen below. >> It looks like these errors occur when running the jtreg tests with higher >> concurrency, I did not see them when running just the single test. >> >> When adding some test output , we see the 2 top install dirs below (1 is >> expected, not 2) : >> ./test/unpacked-rpm/opt >> ./test/unpacked-rpm/usr >> >> error output : >> >> java.lang.AssertionError: Expected [1]. Actual [2]: Check the package has 1 >> top installation directories >>at jdk.jpackage.test.TKit.error(TKit.java:273) >>at jdk.jpackage.test.TKit.assertEquals(TKit.java:576) >>at >> jdk.jpackage.test.PackageTest$Handler.verifyRootCountInUnpackedPackage(PackageTest.java:705) >>at >> jdk.jpackage.test.PackageTest$Handler.lambda$verifyPackageInstalled$3(PackageTest.java:635) >>at java.base/java.util.Optional.ifPresent(Optional.java:178) >>at >> jdk.jpackage.test.PackageTest$Handler.verifyPackageInstalled(PackageTest.java:633) >>at jdk.jpackage.test.PackageTest$Handler.accept(PackageTest.java:592) >>at jdk.jpackage.test.PackageTest$2.accept(PackageTest.java:504) >>at jdk.jpackage.test.PackageTest$2.accept(PackageTest.java:411) >>at >> jdk.jpackage.test.Functional$ThrowingConsumer.lambda$toConsumer$0(Functional.java:41) >>at java.base/java.lang.Iterable.forEach(Iterable.java:75) >>at >> jdk.jpackage.test.PackageTest.lambda$runActions$24(PackageTest.java:381) >>at java.base/java.util.ArrayList.forEach(ArrayList.java:1596) >>at >> jdk.jpackage.test.PackageTest.lambda$runActions$25(PackageTest.java:380) >>at java.base/java.util.ArrayList.forEach(ArrayList.java:1596) >>at jdk.jpackage.test.PackageTest.runActions(PackageTest.java:379) >>at >> jdk.jpackage.test.RunnablePackageTest.run(RunnablePackageTest.java:58) >>at RuntimePackageTest.test(RuntimePackageTest.java:83) >>at >> java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) >>at java.base/java.lang.reflect.Method.invoke(Method.java:580) >>at jdk.jpackage.test.MethodCall.accept(MethodCall.java:145) >>at jdk.jpackage.test.TestInstance.run(TestInstance.java:230) >>at jdk.jpackage.test.TKit.lambda$ignoreExceptions$5(TKit.java:141) >>at jdk.jpackage.test.TKit.lambda$runTests$3(TKit.java:126) >>at java.base/java.util.ArrayList$ArrayListSpl... > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > remove output from PackageTest.java Hi Alexey, thanks for the review ! - PR Comment: https://git.openjdk.org/jdk/pull/15528#issuecomment-1709620631
Re: RFR: 8313612: Use JUnit in lib-test/jdk tests [v3]
> Modified all tests under lib-test/jdk to use JUnit Qing Xiao has updated the pull request incrementally with one additional commit since the last revision: Change test static method to instance method - Changes: - all: https://git.openjdk.org/jdk/pull/15131/files - new: https://git.openjdk.org/jdk/pull/15131/files/0e15e752..e520735d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15131=02 - incr: https://webrevs.openjdk.org/?repo=jdk=15131=01-02 Stats: 13 lines in 4 files changed: 0 ins; 0 del; 13 mod Patch: https://git.openjdk.org/jdk/pull/15131.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15131/head:pull/15131 PR: https://git.openjdk.org/jdk/pull/15131
Re: RFR: 8308995: Update Network IO JFR events to be static mirror events [v4]
On Wed, 6 Sep 2023 15:55:21 GMT, Tim Prinzing wrote: > I think it's useful if trying to trace the calls (i.e. set to 0ms). > Apparently the security manager was being used for that by some. The SM isn't called once connected so I don't think anyone could have every done that. Yes, you could set the threshold to 0ms to emit event for every read/write but I wonder how useful that would be, maybe I/O stats would be more interesting. - PR Comment: https://git.openjdk.org/jdk/pull/14342#issuecomment-1709568576
Re: Question on why sun.management MBeans are not exported?
Hi, It would be a shame to lose these metrics because many of them have been very useful over time and some would be even more useful with some modifications. For example, the CPU breakouts found in GC logs has been incredibly useful as a proxy measure in helping sort out other issues in systems. So much so that I have analytics built specifically around this in my tooling. Kind regards, Kirk Pepperdine > On Sep 6, 2023, at 10:50 AM, Alan Bateman wrote: > > On 06/09/2023 16:17, Volker Simonis wrote: >> : >> I'm familiar with JEP 260. But wouldn't you agree that an >> "encapsulated" monitoring API is an oxymoron? A monitoring API is by >> design intended for external usage and completely useless to the >> platform itself. There's no single usage of the "sun.management" >> MBeans in the JDK itself (except for jconsole where the encapsulation >> broke it). My assumption is that the corresponding MBeans in >> "sun.management" are there for historic reasons (added in JDK 1.5) and >> would have made much more sense in "com.sun.management" package. But I >> doubt that they can be classified in the "internal implementation >> details of the JDK and never intended for external use² category of >> JEP 260. > It's left over from experiments on exposing some internal metrics, I think > during JDK 5. It's code that should probably have been removed a long time > ago. > > -Alan
