Re: RFR: 8315585: Optimization for decimal to string
On Mon, 4 Sep 2023 04:58:08 GMT, 温绍锦 wrote: > BigDecimal is a commonly used class in business development, It is often > necessary to perform toString or toPlainString operations on BigDecimal. > > The current version uses StringBuilder resulting in multiple memory > allocations, I made a modification to improve performance. > > Because BigDecimal uses stringCache to cache the result of toString, the > performance of toString needs special treatment before testing, such as > clearing stringCache by unsafe operation before each test, The performance of > toString is similar to that of toEngineering. > > The performance data is as follows: > > ## 1. benchmark script > > sh make/devkit/createJMHBundle.sh > bash configure --with-jmh=build/jmh/jars > make test TEST="micro:java.math.BigDecimals.*ToPlainString" > make test TEST="micro:java.math.BigDecimals.*ToEngineering" > > > ## 2. benchmark environment > * virtual machine : > [aliyun_ecs_c8i.xlarge](https://help.aliyun.com/zh/ecs/user-guide/overview-of-instance-families#c8i) > * cpu intel xeon sapphire rapids (x64) > > ## 3. benchmark result > > -BigDecimals.testHugeToPlainString avgt 15 188.691 ± 0.822 > ns/op (baseline) > -BigDecimals.testLargeToPlainStringavgt 1536.656 ± 0.065 ns/op > -BigDecimals.testSmallToPlainStringavgt 1534.342 ± 0.068 ns/op > -BigDecimals.testToPlainString avgt 15 1719.494 ± 24.886 ns/op > > +Benchmark Mode Cnt ScoreError > Units (optimize) > +BigDecimals.testHugeToPlainString avgt 15 133.972 ? 0.328 > ns/op (+40.84%) > +BigDecimals.testLargeToPlainStringavgt 1514.957 ? 0.047 > ns/op (145.07%) > +BigDecimals.testSmallToPlainStringavgt 1512.045 ? 0.036 > ns/op (+185.11) > +BigDecimals.testToPlainString avgt 15 1643.500 ? 3.217 > ns/op (+4.62%) > > -Benchmark Mode Cnt ScoreError > Units (baseline) > -BigDecimals.testHugeToEngineeringString avgt 15 207.621 ± 5.018 ns/op > -BigDecimals.testLargeToEngineeringString avgt 1535.658 ± 3.144 ns/op > -BigDecimals.testSmallToEngineeringString avgt 1515.142 ± 0.053 ns/op > -BigDecimals.testToEngineeringString avgt 15 1813.959 ± 12.842 ns/op > > +Benchmark Mode Cnt ScoreError > Units (optimize) > +BigDecimals.testHugeToEngineeringString avgt 15 142.110 ? 0.987 > ns/op (+45.09%) > +BigDecimals.testLargeToEngineeringString avgt 1512.509 ? 0.056 > ns/op (+185.05%) > +BigDecimals.testSmallToEngineer... @liach can you help me to review this PR? - PR Comment: https://git.openjdk.org/jdk/pull/1#issuecomment-1705981814
Re: RFR: 8311207: Cleanup for Optimization for UUID.toString [v10]
> [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 two additional commits since the last revision: - remove redundant parentheses - fix java doc, big-endian -> little-endian - Changes: - all: https://git.openjdk.org/jdk/pull/14745/files - new: https://git.openjdk.org/jdk/pull/14745/files/69962433..3f6d35df Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14745=09 - incr: https://webrevs.openjdk.org/?repo=jdk=14745=08-09 Stats: 3 lines in 2 files 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: 8311207: Cleanup for Optimization for UUID.toString [v9]
On Tue, 5 Sep 2023 02:46: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 11 additional commits since the > last revision: > > - 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 > - Merge pull request #1 from liachmodded/optimization_for_uuid_to_string > >Suggested HexDigits change > - Suggested HexDigits change > - use big endian > - ... and 1 more: https://git.openjdk.org/jdk/compare/ac7b7229...69962433 src/java.base/share/classes/java/util/HexDigits.java line 68: > 66: > 67: /** > 68: * Return a big-endian packed integer for the 4 ASCII bytes for an > input unsigned 2-byte integer. Suggestion: * Returns a little-endian packed integer for the 4 ASCII bytes for an input unsigned 2-byte integer. src/java.base/share/classes/java/util/HexDigits.java line 77: > 75: > 76: /** > 77: * Return a big-endian packed long for the 8 ASCII bytes for an input > unsigned 4-byte integer. Suggestion: * Returns a little-endian packed long for the 8 ASCII bytes for an input unsigned 4-byte integer. src/java.base/share/classes/java/util/UUID.java line 494: > 492: buf, > 493: 24, > 494: HexDigits.packDigits(((int) (lsb >> 40)), (int) (lsb >> > 32), ((int) lsb) >> 24, ((int) lsb) >> 16)); Suggestion: HexDigits.packDigits((int) (lsb >> 40), (int) (lsb >> 32), ((int) lsb) >> 24, ((int) lsb) >> 16)); - PR Review Comment: https://git.openjdk.org/jdk/pull/14745#discussion_r1315331223 PR Review Comment: https://git.openjdk.org/jdk/pull/14745#discussion_r1315331130 PR Review Comment: https://git.openjdk.org/jdk/pull/14745#discussion_r1315314460
Re: RFR: 8311207: Cleanup for Optimization for UUID.toString [v9]
> [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 11 additional commits since the last revision: - 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 - Merge pull request #1 from liachmodded/optimization_for_uuid_to_string Suggested HexDigits change - Suggested HexDigits change - use big endian - ... and 1 more: https://git.openjdk.org/jdk/compare/97e85d94...69962433 - Changes: - all: https://git.openjdk.org/jdk/pull/14745/files - new: https://git.openjdk.org/jdk/pull/14745/files/25cf0144..69962433 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14745=08 - incr: https://webrevs.openjdk.org/?repo=jdk=14745=07-08 Stats: 148919 lines in 2866 files changed: 61136 ins; 70985 del; 16798 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: 8311207: Cleanup for Optimization for UUID.toString [v6]
On Sat, 8 Jul 2023 07:14:12 GMT, 温绍锦 wrote:
>> src/java.base/share/classes/java/util/HexDigits.java line 44:
>>
>>> 42: * 0 -> '00' -> ('0' << 8) | '0' -> 0x3030
>>> 43: * 1 -> '01' -> ('0' << 8) | '1' -> 0x3130
>>> 44: * 2 -> '02' -> ('0' << 8) | '2' -> 0x3230
>>
>> The description of the array content does not need to be exhaustive; just
>> sufficient to understand how it is indexed and the meaning of the values.
>
> The comment is already there, I think the value on the right is easier to
> understand in hexadecimal
comment has been removed
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14745#discussion_r1315275178
Re: RFR: 8311207: Cleanup for Optimization for UUID.toString [v8]
> [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: use ByteArrayLittleEndian - Changes: - all: https://git.openjdk.org/jdk/pull/14745/files - new: https://git.openjdk.org/jdk/pull/14745/files/d6bb2725..25cf0144 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14745=07 - incr: https://webrevs.openjdk.org/?repo=jdk=14745=06-07 Stats: 78 lines in 2 files changed: 1 ins; 49 del; 28 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: 8310929: Optimization for Integer.toString [v19]
On Mon, 4 Sep 2023 16:13:53 GMT, Claes Redestad wrote:
>> 温绍锦 has updated the pull request incrementally with one additional commit
>> since the last revision:
>>
>> add comments
>
> src/java.base/share/classes/java/lang/StringUTF16.java line 1534:
>
>> 1532: */
>> 1533: static int getChars(int i, int index, byte[] buf) {
>> 1534: // Used by trusted callers. Assumes all necessary bounds
>> checks have been done by the caller.
>
> Now these are a bit redundant since these methods are surrounded by a comment
> to the same effect. I prefer inline comments like these, but we should avoid
> having both.
Comments are redundant, but I can't find a solution
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14699#discussion_r1315237072
Re: RFR: 8310929: Optimization for Integer.toString [v20]
> 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... 温绍锦 has updated the pull request incrementally with one additional commit since the last revision: update copyright date info - Changes: - all: https://git.openjdk.org/jdk/pull/14699/files - new: https://git.openjdk.org/jdk/pull/14699/files/f37ed8a5..18904a93 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14699=19 - incr: https://webrevs.openjdk.org/?repo=jdk=14699=18-19 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/14699.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14699/head:pull/14699 PR: https://git.openjdk.org/jdk/pull/14699
Re: RFR: 8314949: linux PPC64 Big Endian: Implementation of Foreign Function & Memory API [v7]
> I've found a way to solve the remaining FFI problem on linux PPC64 Big > Endian. Large structs (>8 Bytes) which are passed in registers or on stack > require shifting the Bytes in the last slot if the size is not a multiple of > 8. This PR adds the required functionality to the Java code. > > Please review and provide feedback. There may be better ways to implement it. > I just found one which works and makes the tests pass: > > Test summary > == >TEST TOTAL PASS FAIL ERROR > >jtreg:test/jdk/java/foreign 8888 0 0 > > > > Note: This PR should be considered as preparation work for AIX which also > uses ABIv1. Martin Doerr has updated the pull request incrementally with one additional commit since the last revision: Add comments. Simplify condition. - Changes: - all: https://git.openjdk.org/jdk/pull/15417/files - new: https://git.openjdk.org/jdk/pull/15417/files/d493950d..9c06646b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15417=06 - incr: https://webrevs.openjdk.org/?repo=jdk=15417=05-06 Stats: 4 lines in 1 file changed: 2 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/15417.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15417/head:pull/15417 PR: https://git.openjdk.org/jdk/pull/15417
Re: RFR: 8315578: PPC builds are broken after JDK-8304913
On Mon, 4 Sep 2023 07:26:37 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.
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`.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15556#discussion_r1315180685
Re: RFR: 8310929: Optimization for Integer.toString [v19]
On Mon, 4 Sep 2023 16:00:05 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:
>
> add comments
Looks good to me. Please be patient and wait for @RogerRiggs to approve this,
too (today is a holiday in the US)
src/java.base/share/classes/java/lang/StringUTF16.java line 1534:
> 1532: */
> 1533: static int getChars(int i, int index, byte[] buf) {
> 1534: // Used by trusted callers. Assumes all necessary bounds
> checks have been done by the caller.
Now these are a bit redundant since these methods are surrounded by a comment
to the same effect. I prefer inline comments like these, but we should avoid
having both.
-
Marked as reviewed by redestad (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/14699#pullrequestreview-1609789061
PR Review Comment: https://git.openjdk.org/jdk/pull/14699#discussion_r1315096657
Re: RFR: 8310929: Optimization for Integer.toString [v19]
> 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... 温绍锦 has updated the pull request incrementally with one additional commit since the last revision: add comments - Changes: - all: https://git.openjdk.org/jdk/pull/14699/files - new: https://git.openjdk.org/jdk/pull/14699/files/ed1c58bb..f37ed8a5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14699=18 - incr: https://webrevs.openjdk.org/?repo=jdk=14699=17-18 Stats: 4 lines in 2 files changed: 4 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/14699.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14699/head:pull/14699 PR: https://git.openjdk.org/jdk/pull/14699
Re: RFR: 8312522: Implementation of Foreign Function & Memory API [v9]
On Mon, 4 Sep 2023 11:33: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 one additional > commit since the last revision: > > remove unsupported linker test Adding a few more minor fixes: - Clarifying javadoc comments wrt which exceptions are thrown by handles derived from layouts: https://github.com/openjdk/panama-foreign/pull/869 & https://github.com/openjdk/panama-foreign/pull/872 - Fixing typos: https://github.com/openjdk/panama-foreign/pull/873 - Missing `@implspec` in AddressLayout: https://github.com/openjdk/panama-foreign/pull/877 - Correct the exception that is thrown for an unsupported access using a var handle: https://github.com/openjdk/panama-foreign/pull/876 Only the latter change is really significant (see the original PR) - PR Comment: https://git.openjdk.org/jdk/pull/15103#issuecomment-1705475550
Re: RFR: 8312522: Implementation of Foreign Function & Memory API [v10]
> 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 five additional commits since the last revision: - 8315096: Allowed access modes in memory segment should depend on layout alignment Reviewed-by: psandoz - Add missing @implSpec to AddressLayout Reviewed-by: pminborg - Fix misc typos in FFM API javadoc Reviewed-by: jvernee - Clarify javadoc w.r.t. exceptions thrown by a memory access var handle (part two) Reviewed-by: pminborg - Clarify javadoc w.r.t. exceptions thrown by a memory access var handle Reviewed-by: jvernee - Changes: - all: https://git.openjdk.org/jdk/pull/15103/files - new: https://git.openjdk.org/jdk/pull/15103/files/ea3daaed..de3e7479 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15103=09 - incr: https://webrevs.openjdk.org/?repo=jdk=15103=08-09 Stats: 306 lines in 6 files changed: 218 ins; 12 del; 76 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 [v18]
On Mon, 4 Sep 2023 15:50: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: > > move Integer/Long.getChars to StringLatin1 src/java.base/share/classes/java/lang/StringLatin1.java line 119: > 117: return ret; > 118: } > 119: I think this is an improvement since it better groups these latin1 formatting methods together. Add a copy of the comment from `StringUTF16` about bound-checking here? - PR Review Comment: https://git.openjdk.org/jdk/pull/14699#discussion_r1315079267
Re: RFR: 8310929: Optimization for Integer.toString [v18]
> 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... 温绍锦 has updated the pull request incrementally with one additional commit since the last revision: move Integer/Long.getChars to StringLatin1 - Changes: - all: https://git.openjdk.org/jdk/pull/14699/files - new: https://git.openjdk.org/jdk/pull/14699/files/0d149fa9..ed1c58bb Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14699=17 - incr: https://webrevs.openjdk.org/?repo=jdk=14699=16-17 Stats: 339 lines in 6 files changed: 162 ins; 164 del; 13 mod Patch: https://git.openjdk.org/jdk/pull/14699.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14699/head:pull/14699 PR: https://git.openjdk.org/jdk/pull/14699
Re: RFR: 8314949: linux PPC64 Big Endian: Implementation of Foreign Function & Memory API [v6]
On Mon, 4 Sep 2023 15:38:19 GMT, Martin Doerr wrote: >> I've found a way to solve the remaining FFI problem on linux PPC64 Big >> Endian. Large structs (>8 Bytes) which are passed in registers or on stack >> require shifting the Bytes in the last slot if the size is not a multiple of >> 8. This PR adds the required functionality to the Java code. >> >> Please review and provide feedback. There may be better ways to implement >> it. I just found one which works and makes the tests pass: >> >> Test summary >> == >>TEST TOTAL PASS FAIL ERROR >> >>jtreg:test/jdk/java/foreign 8888 0 0 >> >> >> >> Note: This PR should be considered as preparation work for AIX which also >> uses ABIv1. > > Martin Doerr has updated the pull request incrementally with one additional > commit since the last revision: > > Add comment. Remove obsolete assignment. Thanks! `test/jdk/java/foreign` tests have passed on linux x86_64, ppc64 and ppc64le on my side. - PR Comment: https://git.openjdk.org/jdk/pull/15417#issuecomment-1705465170
Re: RFR: 8314949: linux PPC64 Big Endian: Implementation of Foreign Function & Memory API [v5]
On Mon, 4 Sep 2023 14:53:33 GMT, Jorn Vernee wrote:
>> Martin Doerr has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Separate type cast from Shift bindings.
>
> src/java.base/share/classes/jdk/internal/foreign/abi/Binding.java line 425:
>
>> 423: if (type == int.class || isSubIntType(type)) {
>> 424: bindings.add(Binding.cast(type, long.class));
>> 425: type = long.class;
>
> This line seems redundant now (`type` is not used below).
Good catch! Removed.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15417#discussion_r1315066254
Re: RFR: 8314949: linux PPC64 Big Endian: Implementation of Foreign Function & Memory API [v6]
> I've found a way to solve the remaining FFI problem on linux PPC64 Big > Endian. Large structs (>8 Bytes) which are passed in registers or on stack > require shifting the Bytes in the last slot if the size is not a multiple of > 8. This PR adds the required functionality to the Java code. > > Please review and provide feedback. There may be better ways to implement it. > I just found one which works and makes the tests pass: > > Test summary > == >TEST TOTAL PASS FAIL ERROR > >jtreg:test/jdk/java/foreign 8888 0 0 > > > > Note: This PR should be considered as preparation work for AIX which also > uses ABIv1. Martin Doerr has updated the pull request incrementally with one additional commit since the last revision: Add comment. Remove obsolete assignment. - Changes: - all: https://git.openjdk.org/jdk/pull/15417/files - new: https://git.openjdk.org/jdk/pull/15417/files/27c4cc1b..d493950d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15417=05 - incr: https://webrevs.openjdk.org/?repo=jdk=15417=04-05 Stats: 16 lines in 2 files changed: 15 ins; 1 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/15417.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15417/head:pull/15417 PR: https://git.openjdk.org/jdk/pull/15417
Re: RFR: 8314949: linux PPC64 Big Endian: Implementation of Foreign Function & Memory API [v5]
On Mon, 4 Sep 2023 14:56:18 GMT, Martin Doerr wrote: >> I've found a way to solve the remaining FFI problem on linux PPC64 Big >> Endian. Large structs (>8 Bytes) which are passed in registers or on stack >> require shifting the Bytes in the last slot if the size is not a multiple of >> 8. This PR adds the required functionality to the Java code. >> >> Please review and provide feedback. There may be better ways to implement >> it. I just found one which works and makes the tests pass: >> >> Test summary >> == >>TEST TOTAL PASS FAIL ERROR >> >>jtreg:test/jdk/java/foreign 8888 0 0 >> >> >> >> Note: This PR should be considered as preparation work for AIX which also >> uses ABIv1. > > Martin Doerr has updated the pull request incrementally with one additional > commit since the last revision: > > Separate type cast from Shift bindings. Latest version looks great! I've started a CI job as well. Will approve when that comes back green. - PR Comment: https://git.openjdk.org/jdk/pull/15417#issuecomment-1705454528
Re: RFR: 8314949: linux PPC64 Big Endian: Implementation of Foreign Function & Memory API [v5]
On Mon, 4 Sep 2023 14:56:18 GMT, Martin Doerr wrote: >> I've found a way to solve the remaining FFI problem on linux PPC64 Big >> Endian. Large structs (>8 Bytes) which are passed in registers or on stack >> require shifting the Bytes in the last slot if the size is not a multiple of >> 8. This PR adds the required functionality to the Java code. >> >> Please review and provide feedback. There may be better ways to implement >> it. I just found one which works and makes the tests pass: >> >> Test summary >> == >>TEST TOTAL PASS FAIL ERROR >> >>jtreg:test/jdk/java/foreign 8888 0 0 >> >> >> >> Note: This PR should be considered as preparation work for AIX which also >> uses ABIv1. > > Martin Doerr has updated the pull request incrementally with one additional > commit since the last revision: > > Separate type cast from Shift bindings. Thank you! I have added an example to the CallArranger. Please take a look. - PR Comment: https://git.openjdk.org/jdk/pull/15417#issuecomment-1705453151
Re: RFR: 8314949: linux PPC64 Big Endian: Implementation of Foreign Function & Memory API [v4]
On Mon, 4 Sep 2023 14:54:05 GMT, Martin Doerr wrote: > Do you see a good place for such a comment? PPC CallArranger seems like a good place to me. We have a similar explanation comment in the AArch64 CallArranger. > Maybe it would be better to use a different size for the last chunk. Maybe > three or five Bytes. That's even less straight-forward. Does the size matter that much? It just changes the shift amount right? Could use `short` as type for `z` as well. - PR Comment: https://git.openjdk.org/jdk/pull/15417#issuecomment-1705409739
Re: RFR: 8314949: linux PPC64 Big Endian: Implementation of Foreign Function & Memory API [v5]
On Mon, 4 Sep 2023 14:56:18 GMT, Martin Doerr wrote:
>> I've found a way to solve the remaining FFI problem on linux PPC64 Big
>> Endian. Large structs (>8 Bytes) which are passed in registers or on stack
>> require shifting the Bytes in the last slot if the size is not a multiple of
>> 8. This PR adds the required functionality to the Java code.
>>
>> Please review and provide feedback. There may be better ways to implement
>> it. I just found one which works and makes the tests pass:
>>
>> Test summary
>> ==
>>TEST TOTAL PASS FAIL ERROR
>>
>>jtreg:test/jdk/java/foreign 8888 0 0
>>
>>
>>
>> Note: This PR should be considered as preparation work for AIX which also
>> uses ABIv1.
>
> Martin Doerr has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Separate type cast from Shift bindings.
src/java.base/share/classes/jdk/internal/foreign/abi/Binding.java line 425:
> 423: if (type == int.class || isSubIntType(type)) {
> 424: bindings.add(Binding.cast(type, long.class));
> 425: type = long.class;
This line seems redundant now (`type` is not used below).
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15417#discussion_r1315035355
Re: RFR: 8314949: linux PPC64 Big Endian: Implementation of Foreign Function & Memory API [v3]
On Mon, 4 Sep 2023 14:50:37 GMT, Martin Doerr wrote: >>> as it would no longer need an input type? >> >> Yes. Then both shift ops would always operate on `long`. > > I've changed it. Note that I need many more conversions because buffer > load/store also use subtypes of `int`. Please take a look at my updated > version (after commit number 5). Thanks, your changes look great. - PR Review Comment: https://git.openjdk.org/jdk/pull/15417#discussion_r1315040087
Re: RFR: 8314949: linux PPC64 Big Endian: Implementation of Foreign Function & Memory API [v4]
On Mon, 4 Sep 2023 12:22:00 GMT, Martin Doerr wrote: >> I've found a way to solve the remaining FFI problem on linux PPC64 Big >> Endian. Large structs (>8 Bytes) which are passed in registers or on stack >> require shifting the Bytes in the last slot if the size is not a multiple of >> 8. This PR adds the required functionality to the Java code. >> >> Please review and provide feedback. There may be better ways to implement >> it. I just found one which works and makes the tests pass: >> >> Test summary >> == >>TEST TOTAL PASS FAIL ERROR >> >>jtreg:test/jdk/java/foreign 8888 0 0 >> >> >> >> Note: This PR should be considered as preparation work for AIX which also >> uses ABIv1. > > Martin Doerr has updated the pull request incrementally with one additional > commit since the last revision: > > Split Shift into ShiftLeft and ShiftRight + minor improvements. Hmm. Do you see a good place for such a comment? Maybe it would be better to use a different size for the last chunk. Maybe three or five Bytes. That's even less straight-forward. - PR Comment: https://git.openjdk.org/jdk/pull/15417#issuecomment-1705403519
Re: RFR: 8314949: linux PPC64 Big Endian: Implementation of Foreign Function & Memory API [v5]
> I've found a way to solve the remaining FFI problem on linux PPC64 Big > Endian. Large structs (>8 Bytes) which are passed in registers or on stack > require shifting the Bytes in the last slot if the size is not a multiple of > 8. This PR adds the required functionality to the Java code. > > Please review and provide feedback. There may be better ways to implement it. > I just found one which works and makes the tests pass: > > Test summary > == >TEST TOTAL PASS FAIL ERROR > >jtreg:test/jdk/java/foreign 8888 0 0 > > > > Note: This PR should be considered as preparation work for AIX which also > uses ABIv1. Martin Doerr has updated the pull request incrementally with one additional commit since the last revision: Separate type cast from Shift bindings. - Changes: - all: https://git.openjdk.org/jdk/pull/15417/files - new: https://git.openjdk.org/jdk/pull/15417/files/2f1c6775..27c4cc1b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15417=04 - incr: https://webrevs.openjdk.org/?repo=jdk=15417=03-04 Stats: 77 lines in 2 files changed: 38 ins; 17 del; 22 mod Patch: https://git.openjdk.org/jdk/pull/15417.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15417/head:pull/15417 PR: https://git.openjdk.org/jdk/pull/15417
Re: RFR: 8314949: linux PPC64 Big Endian: Implementation of Foreign Function & Memory API [v4]
On Mon, 4 Sep 2023 12:44:51 GMT, Jorn Vernee wrote: >> Martin Doerr has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Split Shift into ShiftLeft and ShiftRight + minor improvements. > > src/java.base/share/classes/jdk/internal/foreign/abi/BindingSpecializer.java > line 736: > >> 734: cb.i2l(); >> 735: popType(int.class); >> 736: pushType(long.class); > > These should happen along every code path, so, also in the case where the > input type is `long`. This will help catch cases where a previous binding > left something else than a `long` on the type stack. Ok. - PR Review Comment: https://git.openjdk.org/jdk/pull/15417#discussion_r1315030542
Re: RFR: 8314949: linux PPC64 Big Endian: Implementation of Foreign Function & Memory API [v3]
On Mon, 4 Sep 2023 14:33:08 GMT, Jorn Vernee wrote: >>> I think it's worth it in order to have a cleaner contract for the shift >>> ops, should we want to use them for anything else in the future, but also >>> just to make them easier to understand for future readers. >> >> I agree that having a cleaner contract for the shift binding would prove >> useful in the long run. If we do that, we can also simplify the binding >> itself, as it would no longer need an input type? > >> as it would no longer need an input type? > > Yes. Then both shift ops would always operate on `long`. I've changed it. Note that I need many more conversions because buffer load/store also use subtypes of `int`. Please take a look at my updated version (after commit number 5). - PR Review Comment: https://git.openjdk.org/jdk/pull/15417#discussion_r1315032737
Re: RFR: 8314949: linux PPC64 Big Endian: Implementation of Foreign Function & Memory API [v3]
On Mon, 4 Sep 2023 14:27:27 GMT, Maurizio Cimadamore wrote: > as it would no longer need an input type? Yes. Then both shift ops would always operate on `long`. - PR Review Comment: https://git.openjdk.org/jdk/pull/15417#discussion_r1315016599
Re: RFR: 8314949: linux PPC64 Big Endian: Implementation of Foreign Function & Memory API [v3]
On Mon, 4 Sep 2023 14:18:44 GMT, Jorn Vernee wrote: > If this struct is passed as an argument, then the load of the second 'half' > of the struct would look like this: It would perhaps be cleaner if in the MSB/LSB comments we said: LSBs are zzz...z LSBs are 000...0 (e.g. avoid to refer to MSBs in the first, since those bytes are not exactly zero, they are the padding bytes) > I think it's worth it in order to have a cleaner contract for the shift ops, > should we want to use them for anything else in the future, but also just to > make them easier to understand for future readers. I agree that having a cleaner contract for the shift binding would prove useful in the long run. If we do that, we can also simplify the binding itself, as it would no longer need an input type? - PR Comment: https://git.openjdk.org/jdk/pull/15417#issuecomment-1705366497 PR Review Comment: https://git.openjdk.org/jdk/pull/15417#discussion_r1315010943
Re: RFR: 8310929: Optimization for Integer.toString [v17]
On Fri, 1 Sep 2023 18:40:55 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 with a new target base due to a merge or a > rebase. The pull request now contains 16 commits: > > - remove unused import > - UTF16 & Latin1 use same lookup table > - assert bounds check > - Integer/Long toString test against compact strings > >Co-authored-by: liach > - Adapt endian in pack > >Co-authored-by: liach > - use upper case for static final field > - private static final Unsafe > - code format > - simplify code > - format code & comment > - ... and 6 more: https://git.openjdk.org/jdk/compare/2f7c65ec...0d149fa9 A comment on bounds checks. Before `StringUTF16::getChars(int, int, byte[])` it's explicitly spelled out that the important bounds checks must have been done already: // Used by trusted callers. Assumes all necessary bounds checks have // been done by the caller. This is not spelled out for `Integer/Long.getChars(int/long, int, byte[])` but might as well have been since all the usage thereof are from trusted sources that calculate the indexes to fit the string inside the byte buffer. Perhaps `Integer/Long.getChars` and `PACKED_DIGITS` should all be moved to `java.lang.StringLatin1` and be annotated with the same comment as the equivalent methods in `java.lang.StringUTF16`? - PR Comment: https://git.openjdk.org/jdk/pull/14699#issuecomment-1705358539
Re: RFR: 8314949: linux PPC64 Big Endian: Implementation of Foreign Function & Memory API [v3]
On Mon, 4 Sep 2023 12:20:52 GMT, Martin Doerr wrote:
>> Sorry for the delay, I've been on vacation.
>
>> Sorry for the delay, I've been on vacation.
>
> No problem. Hope you had a good time! Thanks for your feedback.
@TheRealMDoerr We've been discussing the shifts in order to wrap our heads
around it, and we've ended up with this diagram in order to try and visualize
what happens:
Let's say we have a struct with 3 ints:
struct Foo {
int x;
int y;
int z;
};
If this struct is passed as an argument, then the load of the second 'half' of
the struct would look like this:
offset : 0 32 . 64 . 96 128
values : ||| (can't touch bits
96..128)
Load int : V++
| |
++|
VV
In register: | (MSBs are 0)
Shift left : | (LSBs are zero)
Write long : V V
Result : |||
So, the 'Result' is padded at the tail with zeros.
Does that seem right? Does it seem useful to add this diagram as a comment
somewhere, for us when we come back to this code a year from now? Thanks
-
PR Comment: https://git.openjdk.org/jdk/pull/15417#issuecomment-1705350592
Re: RFR: 8315454: Add a way to create an immutable snapshot of a BitSet [v5]
> This PR proposes adding a new method to BitSet that provides an immutable > snapshot of the set in the form of an `IntPredicate`. > > The predicate is eligible for constant folding. > > Here are some classes in the JDK that would benefit directly from > constant-folding of BitSets: > > PoolReader (6) > URLEncoder (1) - Updated in this PR > HtmlTree (2) > > More over, the implementation of the predicate is @ValueBased and this would > provide additional benefits in the future. > > Initial benchmarks with the URLEncoder show encouraging results: > > > Name (encodeChars) (maxLength) (unchanged) Cnt > Base Error Test Error Unit Diff% > URLEncodeDecode.testEncodeUTF8 61024 0 15 > 2,371 ± 0,016 2,073 ± 0,184 ms/op 12,6% (p = 0,000*) > URLEncodeDecode.testEncodeUTF8 61024 75 15 > 1,772 ± 0,013 1,387 ± 0,032 ms/op 21,8% (p = 0,000*) > URLEncodeDecode.testEncodeUTF8 61024 100 15 > 1,230 ± 0,009 1,140 ± 0,011 ms/op 7,3% (p = 0,000*) Per Minborg has updated the pull request incrementally with two additional commits since the last revision: - Update test/jdk/java/util/BitSet/ImmutableBitSet.java Co-authored-by: Claes Redestad - Remove factory check and add copyright message - Changes: - all: https://git.openjdk.org/jdk/pull/15530/files - new: https://git.openjdk.org/jdk/pull/15530/files/6f80e0b6..80b5aacf Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15530=04 - incr: https://webrevs.openjdk.org/?repo=jdk=15530=03-04 Stats: 32 lines in 2 files changed: 25 ins; 6 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/15530.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15530/head:pull/15530 PR: https://git.openjdk.org/jdk/pull/15530
Re: RFR: 8314949: linux PPC64 Big Endian: Implementation of Foreign Function & Memory API [v4]
On Mon, 4 Sep 2023 12:22:00 GMT, Martin Doerr wrote: >> I've found a way to solve the remaining FFI problem on linux PPC64 Big >> Endian. Large structs (>8 Bytes) which are passed in registers or on stack >> require shifting the Bytes in the last slot if the size is not a multiple of >> 8. This PR adds the required functionality to the Java code. >> >> Please review and provide feedback. There may be better ways to implement >> it. I just found one which works and makes the tests pass: >> >> Test summary >> == >>TEST TOTAL PASS FAIL ERROR >> >>jtreg:test/jdk/java/foreign 8888 0 0 >> >> >> >> Note: This PR should be considered as preparation work for AIX which also >> uses ABIv1. > > Martin Doerr has updated the pull request incrementally with one additional > commit since the last revision: > > Split Shift into ShiftLeft and ShiftRight + minor improvements. src/java.base/share/classes/jdk/internal/foreign/abi/BindingSpecializer.java line 736: > 734: cb.i2l(); > 735: popType(int.class); > 736: pushType(long.class); These should happen along every code path, so, also in the case where the input type is `long`. This will help catch cases where a previous binding left something else than a `long` on the type stack. - PR Review Comment: https://git.openjdk.org/jdk/pull/15417#discussion_r1314900942
Re: RFR: 8314949: linux PPC64 Big Endian: Implementation of Foreign Function & Memory API [v3]
On Mon, 4 Sep 2023 12:19:42 GMT, Martin Doerr wrote: >> src/java.base/share/classes/jdk/internal/foreign/abi/Binding.java line 398: >> >>> 396: bindings.add(Binding.cast(type, int.class)); >>> 397: type = int.class; >>> 398: } >> >> Part of the casts are handled here with explicit cast bindings, but the >> widening from int -> long, and narrowing from long -> int are handled >> implicitly as part of the ShiftLeft implementation. I'd much prefer if all >> the type conversions are handled with explicit cast bindings. This would >> also semantically simplify the shift operator, since it would just handle >> the actual shifting. > > I guess we would need to add additional bindings for that? Is is worth adding > more just for a big endian corner case? Or can that be done with the existing > ones? A `Cast` case for int -> long and long -> int would need to be added. Given the existing setup, that should only be a few lines of code for each. (See e.g. for int -> long https://github.com/openjdk/jdk/compare/master...JornVernee:jdk:I2L). I don't think the cost is that high. > Is is worth adding more just for a big endian corner case? I think it's worth it in order to have a cleaner contract for the shift ops, should we want to use them for anything else in the future, but also just to make them easier to understand for future readers. - PR Review Comment: https://git.openjdk.org/jdk/pull/15417#discussion_r1314892030
Re: RFR: 8308804: Improve UUID.randomUUID performance with bulk/scalable PRNG access [v7]
On Thu, 1 Jun 2023 09:37:29 GMT, Aleksey Shipilev wrote:
>> UUID is the very important class that is used to track identities of objects
>> in large scale systems. On some of our systems, `UUID.randomUUID` takes >1%
>> of total CPU time, and is frequently a scalability bottleneck due to
>> `SecureRandom` synchronization.
>>
>> The major issue with UUID code itself is that it reads from the single
>> `SecureRandom` instance by 16 bytes. So the heavily contended `SecureRandom`
>> is bashed with very small requests. This also has a chilling effect on other
>> users of `SecureRandom`, when there is a heavy UUID generation traffic.
>>
>> We can improve this by doing the bulk reads from the backing SecureRandom
>> and possibly striping the reads across many instances of it.
>>
>>
>> Benchmark Mode Cnt Score Error Units
>>
>> ### AArch64 (m6g.4xlarge, Graviton, 16 cores)
>>
>> # Before
>> UUIDRandomBench.single thrpt 15 3.545 ± 0.058 ops/us
>> UUIDRandomBench.max thrpt 15 1.832 ± 0.059 ops/us ; negative scaling
>>
>> # After
>> UUIDRandomBench.single thrpt 15 4.823 ± 0.023 ops/us
>> UUIDRandomBench.max thrpt 15 6.561 ± 0.054 ops/us ; positive
>> scaling, ~1.5x
>>
>> ### x86_64 (c6.8xlarge, Xeon, 18 cores)
>>
>> # Before
>> UUIDRandomBench.single thrpt 15 2.710 ± 0.038 ops/us
>> UUIDRandomBench.max thrpt 15 1.880 ± 0.029 ops/us ; negative
>> scaling
>>
>> # After
>> BenchmarkMode Cnt Score Error Units
>> UUIDRandomBench.single thrpt 15 3.109 ± 0.026 ops/us
>> UUIDRandomBench.max thrpt 15 3.561 ± 0.071 ops/us ; positive
>> scaling, ~1.2x
>>
>>
>> Note that there is still a scalability bottleneck in current default random
>> (`NativePRNG`), because it synchronizes over a singleton instance for SHA1
>> mixer, then the engine itself, etc. -- it is quite a whack-a-mole to figure
>> out the synchronization story there. The scalability fix in current default
>> `SecureRandom` would be much more intrusive and risky, since it would change
>> a core crypto class with unknown bug fanout.
>>
>> Using the bulk reads even when the underlying PRNG is heavily synchronized
>> is still a win. A more scalable PRNG would benefit from this as well. This
>> PR adds a system property to select the PRNG implementation, and there we
>> can clearly see the benefit with more scalable PRNG sources:
>>
>>
>> Benchmark Mode Cnt Score Error Units
>>
>> ### x86_64 (c6.8xlarge, Xeon, 18 cores)
>>
>> # Before, hacked `new SecureRandom()` to
>> `SecureRandom.getInstance("SHA1PRNG")`
>> UUIDRandomBench.single thrpt ...
>
> Aleksey Shipilev has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains 13 commits:
>
> - Revert test changes
> - Merge branch 'master' into JDK-8308804-uuid-buffers
> - Simplify clinit
> - Remove the properties
> - Swap lsb/msb
> - Fine-tune exceptions
> - Handle privileged properties
> - Use ByteArray to convert. Do version/variant preparations straight on
> locals. Move init out of optimistic lock section.
> - More touchups
> - Comment updates
> - ... and 3 more: https://git.openjdk.org/jdk/compare/4460429d...fd7eaa1a
I see this PR was closed out as stale and not integrated. I am curious if there
is anything I could do to help push this over the line. As highlighted by
@shipilev in the PR description, UUID generation is often a critical component
of some systems and contention on SecureRandom can become a scalability
bottleneck that I would love to reduce.
Thanks!
-
PR Comment: https://git.openjdk.org/jdk/pull/14135#issuecomment-1705192263
Re: RFR: 8314949: linux PPC64 Big Endian: Implementation of Foreign Function & Memory API [v3]
On Mon, 4 Sep 2023 07:26:36 GMT, Jorn Vernee wrote: > Sorry for the delay, I've been on vacation. No problem. Hope you had a good time! Thanks for your feedback. > src/java.base/share/classes/jdk/internal/foreign/abi/Binding.java line 398: > >> 396: bindings.add(Binding.cast(type, int.class)); >> 397: type = int.class; >> 398: } > > Part of the casts are handled here with explicit cast bindings, but the > widening from int -> long, and narrowing from long -> int are handled > implicitly as part of the ShiftLeft implementation. I'd much prefer if all > the type conversions are handled with explicit cast bindings. This would also > semantically simplify the shift operator, since it would just handle the > actual shifting. I guess we would need to add additional bindings for that? Is is worth adding more just for a big endian corner case? Or can that be done with the existing ones? - PR Comment: https://git.openjdk.org/jdk/pull/15417#issuecomment-1705174117 PR Review Comment: https://git.openjdk.org/jdk/pull/15417#discussion_r1314874972
Re: RFR: 8314949: linux PPC64 Big Endian: Implementation of Foreign Function & Memory API [v3]
On Mon, 4 Sep 2023 07:06:39 GMT, Jorn Vernee wrote:
>> Martin Doerr has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Remove unnecessary imports.
>
> src/java.base/share/classes/jdk/internal/foreign/abi/Binding.java line 695:
>
>> 693: * Negative [shiftAmount] shifts right and converts to int if
>> needed.
>> 694: */
>> 695: record ShiftLeft(int shiftAmount, Class type) implements Binding
>> {
>
> Please split this into 2 binding operators: one for ShiftLeft and another for
> (arithmetic) ShiftRight, rather than mixing the 2 implementations into a
> single operator.
>
> For the right shift you could then also use a positive shift amount, and
> avoid the double negation that's needed right now.
Done.
> src/java.base/share/classes/jdk/internal/foreign/abi/Binding.java line 703:
>
>> 701: if (last != ((type == long.class) ? long.class :
>> int.class))
>> 702: throw new IllegalArgumentException(
>> 703: String.format("Invalid operand type: %s.
>> integral type expected", last));
>
> Why not use `SharedUtils.checkType` here?
>
> Suggestion:
>
> SharedUtils.checkType(last, type);
Done.
> src/java.base/share/classes/jdk/internal/foreign/abi/Binding.java line 711:
>
>> 709: stack.push(type == long.class ? long.class : int.class);
>> 710: } else
>> 711: throw new IllegalArgumentException("shiftAmount 0 not
>> supported");
>
> Please handle this validation in the static `shiftLeft` method. That's also
> where we do validation for other binding ops
Done.
> src/java.base/share/classes/jdk/internal/foreign/abi/BindingSpecializer.java
> line 737:
>
>> 735: cb.i2l();
>> 736: typeStack.pop();
>> 737: typeStack.push(long.class);
>
> Please use `popType` and `pushType` instead of manipulating the type stack
> manually. The former also does some verification. This should happen along
> every control path. Even if the binding is 1-to-1 (like this one) and doesn't
> change the type of the value, this would validate that the input type is
> correct, and make sure that any bugs are caught early.
Done.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15417#discussion_r1314872201
PR Review Comment: https://git.openjdk.org/jdk/pull/15417#discussion_r1314871165
PR Review Comment: https://git.openjdk.org/jdk/pull/15417#discussion_r1314871596
PR Review Comment: https://git.openjdk.org/jdk/pull/15417#discussion_r1314871489
Re: RFR: 8314949: linux PPC64 Big Endian: Implementation of Foreign Function & Memory API [v4]
> I've found a way to solve the remaining FFI problem on linux PPC64 Big > Endian. Large structs (>8 Bytes) which are passed in registers or on stack > require shifting the Bytes in the last slot if the size is not a multiple of > 8. This PR adds the required functionality to the Java code. > > Please review and provide feedback. There may be better ways to implement it. > I just found one which works and makes the tests pass: > > Test summary > == >TEST TOTAL PASS FAIL ERROR > >jtreg:test/jdk/java/foreign 8888 0 0 > > > > Note: This PR should be considered as preparation work for AIX which also > uses ABIv1. Martin Doerr has updated the pull request incrementally with one additional commit since the last revision: Split Shift into ShiftLeft and ShiftRight + minor improvements. - Changes: - all: https://git.openjdk.org/jdk/pull/15417/files - new: https://git.openjdk.org/jdk/pull/15417/files/430fa018..2f1c6775 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15417=03 - incr: https://webrevs.openjdk.org/?repo=jdk=15417=02-03 Stats: 95 lines in 4 files changed: 41 ins; 19 del; 35 mod Patch: https://git.openjdk.org/jdk/pull/15417.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15417/head:pull/15417 PR: https://git.openjdk.org/jdk/pull/15417
Re: RFR: 8298619: java/io/File/GetXSpace.java is failing [v4]
On Wed, 29 Mar 2023 18:05:46 GMT, Brian Burkhalter wrote: >> Modify the `Space` instances used for size comparison to be created with >> total number of bytes derived from the Windows `diskFree` utility instead of >> Cygwin’s `df`. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 8305157: Clean up It seems after refactoring test started to fail on my Win11 VM. STDOUT: --- Testing volumes Total bytes : 299,877,003,264 (279.3 GB) Total quota free bytes : 202,708,971,520 (188.8 GB) Unavailable pool bytes : 0 ( 0.0 KB) Quota unavailable pool bytes: 0 ( 0.0 KB) Used bytes : 92,833,345,536 ( 86.5 GB) Total Reserved bytes: 4,334,686,208 ( 4.0 GB) Volume storage reserved bytes : 4,296,867,840 ( 4.0 GB) Available committed bytes : 0 ( 0.0 KB) Pool available bytes: 0 ( 0.0 KB) SecurityManager = null C:\ (299877003264): getSpace0 total = 299877003264 free = 202708967424 usable = 202708967424 getXSpace total = 299877003264 free = 202708967424 usable = 202708967424 STDERR: WARNING: A terminally deprecated method in java.lang.System has been called WARNING: System::setSecurityManager has been called by GetXSpace (file:/C:/Projects/jdk/build/windows-x86_64-server-slowdebug/test-support/jtreg_test_jdk_java_io_File/classes/2/java/io/File/GetXSpace.d/) WARNING: Please consider reporting this to the maintainers of GetXSpace WARNING: System::setSecurityManager will be removed in a future release java.lang.RuntimeException: The parameter is incorrect at GetXSpace.getSpace0(Native Method) at GetXSpace$Space.(GetXSpace.java:112) at GetXSpace.testVolumes(GetXSpace.java:431) at GetXSpace.main(GetXSpace.java:469) at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) at java.base/java.lang.reflect.Method.invoke(Method.java:580) at com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138) at java.base/java.lang.Thread.run(Thread.java:1570) JavaTest Message: Test threw exception: java.lang.RuntimeException: The parameter is incorrect - PR Comment: https://git.openjdk.org/jdk/pull/12397#issuecomment-1705164515
Re: RFR: JDK-8314272: Improve java.util.prefs.BackingStoreException: Couldn't get file lock. [v2]
On Fri, 1 Sep 2023 13:27:58 GMT, Alan Bateman wrote: > > Hi Alan , Your assumption 'I assume the use of System.getProperty is > > problematic when running with a SM.' is most likely correct. > > You'll need to test with a SM that denies reading the system property to be > sure. There are classes in many tool APIs (you've listed some) where this > doesn't arise. > For java.management/share/classes/sun/management/VMManagementImpl.java I tried with a security manager and had to grant property access so that it worked properly. For java.desktop/share/classes/sun/awt/FontConfiguration.java this can be called from all code working with fonts so it needs to be handled too (e.g. PrivilegedAction). Should I file a JBS issue for those two ? Some of the others listed might indeed fall into the tool APIs you mentioned. - PR Comment: https://git.openjdk.org/jdk/pull/15308#issuecomment-1705142502
Re: RFR: 8315454: Add a way to create an immutable snapshot of a BitSet [v4]
On Mon, 4 Sep 2023 06:54:40 GMT, Per Minborg wrote:
>> This PR proposes adding a new method to BitSet that provides an immutable
>> snapshot of the set in the form of an `IntPredicate`.
>>
>> The predicate is eligible for constant folding.
>>
>> Here are some classes in the JDK that would benefit directly from
>> constant-folding of BitSets:
>>
>> PoolReader (6)
>> URLEncoder (1) - Updated in this PR
>> HtmlTree (2)
>>
>> More over, the implementation of the predicate is @ValueBased and this would
>> provide additional benefits in the future.
>>
>> Initial benchmarks with the URLEncoder show encouraging results:
>>
>>
>> Name (encodeChars) (maxLength) (unchanged) Cnt
>> Base Error Test Error Unit Diff%
>> URLEncodeDecode.testEncodeUTF8 61024 0 15
>> 2,371 ± 0,016 2,073 ± 0,184 ms/op 12,6% (p = 0,000*)
>> URLEncodeDecode.testEncodeUTF8 61024 75 15
>> 1,772 ± 0,013 1,387 ± 0,032 ms/op 21,8% (p = 0,000*)
>> URLEncodeDecode.testEncodeUTF8 61024 100 15
>> 1,230 ± 0,009 1,140 ± 0,011 ms/op 7,3% (p = 0,000*)
>
> Per Minborg has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Do not propagate Integer.MAX_VALUE BitSets
Looks good to me. Since we're not adding any public API I think it's ok to
include the `URLEncoder` changes so that we can use that benchmark as a proxy
for verifying `BitSet::asPredicate` performance.
src/java.base/share/classes/jdk/internal/util/ImmutableBitSetPredicate.java
line 60:
> 58: public static IntPredicate of(BitSet original) {
> 59: // Do not propagate the Integer.MAX_VALUE issue
> 60: if (Integer.toUnsignedLong(original.length()) >
> Integer.MAX_VALUE) {
I'm not so sure about this. While there are issues related to
`BitSet::set(Integer.MAX_VALUE)` (makes `BitSet::length` overflow to negative),
the `get/test` operation is not adversely affected. Since this is an internal
API adding limitations here seem a bit gratuitous and could instead be
discussed if/when taking this public.
test/jdk/java/util/BitSet/ImmutableBitSet.java line 2:
> 1: /*
> 2: * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
Suggestion:
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
-
Marked as reviewed by redestad (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/15530#pullrequestreview-1609364523
PR Review Comment: https://git.openjdk.org/jdk/pull/15530#discussion_r1314830577
PR Review Comment: https://git.openjdk.org/jdk/pull/15530#discussion_r1314830746
Re: RFR: 8312522: Implementation of Foreign Function & Memory API [v9]
> 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: remove unsupported linker test - Changes: - all: https://git.openjdk.org/jdk/pull/15103/files - new: https://git.openjdk.org/jdk/pull/15103/files/470fcb9d..ea3daaed Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15103=08 - incr: https://webrevs.openjdk.org/?repo=jdk=15103=07-08 Stats: 40 lines in 1 file changed: 0 ins; 40 del; 0 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: JDK-8314272: Improve java.util.prefs.BackingStoreException: Couldn't get file lock. [v4]
> 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 - Changes: - all: https://git.openjdk.org/jdk/pull/15308/files - new: https://git.openjdk.org/jdk/pull/15308/files/d385dac1..b63064ec Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15308=03 - incr: https://webrevs.openjdk.org/?repo=jdk=15308=02-03 Stats: 6 lines in 1 file changed: 4 ins; 1 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/15308.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15308/head:pull/15308 PR: https://git.openjdk.org/jdk/pull/15308
Re: RFR: 8315097: Rename createJavaProcessBuilder [v3]
On Wed, 30 Aug 2023 09:23:55 GMT, Leo Korinth wrote: >> Rename createJavaProcessBuilder so that it is not used by mistake instead of >> createTestJvm. >> >> I have used the following sed script: `find -name "*.java" | xargs -n 1 sed >> -i -e >> "s/createJavaProcessBuilder(/createJavaProcessBuilderIgnoreTestJavaOpts(/g"` >> >> Then I have manually modified ProcessTools.java. In that file I have moved >> one version of createJavaProcessBuilder so that it is close to the other >> version. Then I have added a javadoc comment in bold telling: >> >>/** >> * Create ProcessBuilder using the java launcher from the jdk to >> * be tested. >> * >> * Please observe that you likely should use >> * createTestJvm() instead of this method because createTestJvm() >> * will add JVM options from "test.vm.opts" and "test.java.opts" >> * and this method will not do that. >> * >> * @param command Arguments to pass to the java command. >> * @return The ProcessBuilder instance representing the java command. >> */ >> >> >> I have used the name createJavaProcessBuilderIgnoreTestJavaOpts because of >> the name of Utils.prependTestJavaOpts that adds those VM flags. If you have >> a better name I could do a rename of the method. I kind of like that it is >> long and clumsy, that makes it harder to use... >> >> I have run tier 1 testing, and I have started more exhaustive testing. > > Leo Korinth has updated the pull request incrementally with one additional > commit since the last revision: > > fix static import I have created an alternative that uses enums to force the user to make a decision: https://github.com/openjdk/jdk/compare/master...lkorinth:jdk:+process_tools . Another alternative is to do the same but instead using an enum (I think it is not as good). A third alternative is to use the current pull request with a better name. What do you prefer? Do you have a better alternative? Do someone still think the current code is good? I think what we have today is inferior to all these improvements, and I would like to make it harder to develop bad test cases. - PR Comment: https://git.openjdk.org/jdk/pull/15452#issuecomment-1705068700
Re: RFR: 8312522: Implementation of Foreign Function & Memory API [v8]
> 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: fix TestIllegalLink on x86 - Changes: - all: https://git.openjdk.org/jdk/pull/15103/files - new: https://git.openjdk.org/jdk/pull/15103/files/efc5ef4a..470fcb9d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15103=07 - incr: https://webrevs.openjdk.org/?repo=jdk=15103=06-07 Stats: 7 lines in 1 file changed: 0 ins; 7 del; 0 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
Withdrawn: 8180892: Correct handling of annotations on parameters
On Wed, 26 Apr 2023 04:34:34 GMT, Chen Liang wrote: > This patch aims to correct handling of annotations on parameters with the > help of `MethodParameters` attribute, which will be always available once > #9862 is integrated. > > It utilizes and expands upon the existing parameter matching logic present in > `Executable::getAllGenericParameterTypes`, and delegate parameter > parameterized types, parameter annotation, and parameter type annotation > accesses through the matched results, if matching is available. If matching > failed, it falls back to existing heuristics that works without > `MethodParameters` attributes for annotations, in > `Executable::handleParameterNumberMismatch` and > `TypeAnnotationParser::buildAnnotatedTypes` (renamed > `buildAnnotatedTypesWithHeuristics` in this patch) > > `ParameterMappingTest` covers these scenarios with class files that have > `MethodParameters` or `Signature` attributes stripped or preserved to ensure > the new Reflection API implementation works for both class files generated > before #9862 and after its integration. > > Also special thanks to Joe Darcy for reviewing 8304918 (#13183); this brings > much convenience to this patch. This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/13664
Re: RFR: 8288899: java/util/concurrent/ExecutorService/CloseTest.java failed with "InterruptedException: sleep interrupted" [v23]
> 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 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 67 additional commits since the last revision: - Merge branch 'openjdk:master' into JDK-8288899 - Avoid jtreg test group - Ignore more stray interrupts by test harness - Merge branch 'openjdk:master' into JDK-8288899 - Fix tests, undo workarounds - Avoid unwanted jtreg interrupts; undo unnecessary changes - Merge branch 'openjdk:master' into JDK-8288899 - Clear interrupts at top level - Conditionalize new tests - Isolate unexpected interrupt status issues - ... and 57 more: https://git.openjdk.org/jdk/compare/892dc0cf...551a9cf8 - Changes: - all: https://git.openjdk.org/jdk/pull/14301/files - new: https://git.openjdk.org/jdk/pull/14301/files/9d590699..551a9cf8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14301=22 - incr: https://webrevs.openjdk.org/?repo=jdk=14301=21-22 Stats: 1434 lines in 37 files changed: 1186 ins; 157 del; 91 mod Patch: https://git.openjdk.org/jdk/pull/14301.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14301/head:pull/14301 PR: https://git.openjdk.org/jdk/pull/14301
Re: RFR: 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 use on > com.sun.tools.classfile and should be converted to Classfile API. Following tier2 test is affected by the change and fails: test/jdk/java/time/nontestng/java/time/chrono/HijrahConfigTest.java - PR Comment: https://git.openjdk.org/jdk/pull/15529#issuecomment-1704925079
Re: RFR: 8310929: Optimization for Integer.toString [v17]
On Fri, 1 Sep 2023 18:40:55 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 with a new target base due to a merge or a > rebase. The pull request now contains 16 commits: > > - remove unused import > - UTF16 & Latin1 use same lookup table > - assert bounds check > - Integer/Long toString test against compact strings > >Co-authored-by: liach > - Adapt endian in pack > >Co-authored-by: liach > - use upper case for static final field > - private static final Unsafe > - code format > - simplify code > - format code & comment > - ... and 6 more: https://git.openjdk.org/jdk/compare/2f7c65ec...0d149fa9 @RogerRiggs Is it okay to use asserts to check boundaries? - PR Comment: https://git.openjdk.org/jdk/pull/14699#issuecomment-1704914969
Re: RFR: 8311207: Cleanup for Optimization for UUID.toString [v7]
On Sat, 8 Jul 2023 00:20:20 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 incrementally with one additional commit > since the last revision: > > fix typo @RogerRiggs What changes do I need to make to start the integration? - PR Comment: https://git.openjdk.org/jdk/pull/14745#issuecomment-1704910527
Re: RFR: 8233160: Add java.vendor.url.bug to list of recognized standard system properties
On Fri, 1 Sep 2023 14:59:19 GMT, Jaikiran Pai wrote: > I asked for Mark's and Joe's inputs on this in the linked JBS issue > https://bugs.openjdk.org/browse/JDK-8233160. Mark has suggested that we keep > this `java.vendor.url.bug` system property non-optional and also not to add > any expectations of the value for this property to be a parsable URL. Okay but just to mention that in recent years, the system properties table have added "may be interpreted as a " to the description of the value of several properties so that it can be used at run-time. A bug reporting URL is probably only interesting to print out or log but maybe it's not too off-piste to think about a popup window or something might want to call URI.create and represent it as a URI. - PR Comment: https://git.openjdk.org/jdk/pull/15504#issuecomment-1704896205
Re: RFR: 8315373: Change VirtualThread to unmount after freezing, re-mount before thawing [v2]
On Fri, 1 Sep 2023 11:32:45 GMT, Markus Grönlund wrote: >> Thanks. One other thing that I see when more testing with generational ZGC >> is that ZPageAllocator::alloc_page will record an event and this can happen >> during a mount transition. So I think JfrStackTrace::record also needs to >> test if the current thread is a virtual thread without a continuation >> mounted. > > That is not good. Such a check would be attributed to all event sites, making > them all slower. Markus has suggested, and provided the change, to handle the synchronous case in JfrStackTrace. The testing is looking good with the combined changes. - PR Review Comment: https://git.openjdk.org/jdk/pull/15492#discussion_r1314643730
Re: RFR: 8314094: java/lang/ProcessHandle/InfoTest.java fails on Windows when run as user with Administrator privileges [v2]
On Fri, 1 Sep 2023 08:32:20 GMT, Christoph Langer wrote: >> On Windows, the test java/lang/ProcessHandle/InfoTest.java can fail when run >> as user that is member of the Administrators group. In that case new files >> are not owned by the user but instead by BUILTIN\ADMINISTRATORS. This breaks >> the assumptions of the test's whoami check. My suggestion is to cater for >> this case and don't fail the test but write a warning message to stdout that >> a whoami check is not correctly possible. > > Christoph Langer 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: > > - Use System.getProperty(user.name) for the Windows Adminstrators case > - Merge branch 'master' into JDK-8314094 > - JDK-8314094 Looks good to me. - Marked as reviewed by azeller (Author). PR Review: https://git.openjdk.org/jdk/pull/15222#pullrequestreview-1609013076
Re: RFR: 8314491: Linux: jexec launched via PATH fails to find java [v7]
On Mon, 4 Sep 2023 08:01:16 GMT, Julian Waters wrote: > Is this all clear for sponsorship? Yes, I have checked with @dholmes-ora - PR Comment: https://git.openjdk.org/jdk/pull/15343#issuecomment-1704827486
Re: RFR: 8312522: Implementation of Foreign Function & Memory API [v7]
> 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 test for unmodifiable canonical layouts map - Changes: - all: https://git.openjdk.org/jdk/pull/15103/files - new: https://git.openjdk.org/jdk/pull/15103/files/fd0512f8..efc5ef4a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15103=06 - incr: https://webrevs.openjdk.org/?repo=jdk=15103=05-06 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: 8315373: Change VirtualThread to unmount after freezing, re-mount before thawing [v2]
> In the virtual thread implementation, thread identity switches to the carrier > before freezing and switches back to the virtual thread after thawing. This > was a forced move due to issues getting JVMTI to work with virtual threads. > JVMTI can now hide events during transitions so we can invert the sequence > back to mounting before running the continuation, unmounting after freezing, > and re-mounting after thawing. This sequence is important for future changes > that will initiate the freezing from the VM. > > The change requires an update to the JFR thread sampler to skip sampling when > it samples during a transition. > > Testing: tier1-5 Alan Bateman 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: - Check for transition during stack walk for synchronous case - Merge - Initial commit - Changes: - all: https://git.openjdk.org/jdk/pull/15492/files - new: https://git.openjdk.org/jdk/pull/15492/files/04a920d6..18f63cbd Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15492=01 - incr: https://webrevs.openjdk.org/?repo=jdk=15492=00-01 Stats: 3227 lines in 105 files changed: 2522 ins; 385 del; 320 mod Patch: https://git.openjdk.org/jdk/pull/15492.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15492/head:pull/15492 PR: https://git.openjdk.org/jdk/pull/15492
Re: RFR: 8314094: java/lang/ProcessHandle/InfoTest.java fails on Windows when run as user with Administrator privileges [v2]
On Fri, 1 Sep 2023 08:32:20 GMT, Christoph Langer wrote: >> On Windows, the test java/lang/ProcessHandle/InfoTest.java can fail when run >> as user that is member of the Administrators group. In that case new files >> are not owned by the user but instead by BUILTIN\ADMINISTRATORS. This breaks >> the assumptions of the test's whoami check. My suggestion is to cater for >> this case and don't fail the test but write a warning message to stdout that >> a whoami check is not correctly possible. > > Christoph Langer 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: > > - Use System.getProperty(user.name) for the Windows Adminstrators case > - Merge branch 'master' into JDK-8314094 > - JDK-8314094 OK, the latest update seems to work. I'll integrate tomorrow, unless I hear concerns. - PR Comment: https://git.openjdk.org/jdk/pull/15222#issuecomment-1704820273
Re: RFR: 8312522: Implementation of Foreign Function & Memory API [v6]
> 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 with a new target base due to a merge or a rebase. The pull request now contains 22 commits: - Merge branch 'master' into JEP22 - Merge branch 'master' into JEP22 - remove spurious imports - enable fallback linker on linux x86 in GHA - make Arena::allocate abstract - 8313894: Rename isTrivial linker option to critical Reviewed-by: pminborg, mcimadamore - 8313680: Disallow combining caputreCallState with isTrivial Reviewed-by: mcimadamore - Merge branch 'master' into JEP22 - use immutable map for fallback linker canonical layouts - 8313265: Move the FFM API out of preview Reviewed-by: mcimadamore - ... and 12 more: https://git.openjdk.org/jdk/compare/adfc1d6c...fd0512f8 - Changes: https://git.openjdk.org/jdk/pull/15103/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15103=05 Stats: 2839 lines in 233 files changed: 1257 ins; 894 del; 688 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: 8314491: Linux: jexec launched via PATH fails to find java [v7]
On Wed, 30 Aug 2023 20:34:01 GMT, Vladimir Petko wrote: >> 8314491: Linux: jexec launched via PATH fails to find java > > Vladimir Petko 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 ten additional > commits since the last revision: > > - Merge branch 'master' into 8314491-jexec-resolve-symlink > - Merge branch 'master' into 8314491-jexec-resolve-symlink > - declare error in-place > - remove unused imports > - Review comment: use /proc/self/exe as the backup option > - Merge branch 'master' into 8314491-jexec-resolve-symlink > - correct copyright statement > - Use /proc/self/exec to identify path to the executable. > - Create failing test for jexec in PATH issue Is this all clear for sponsorship? - PR Comment: https://git.openjdk.org/jdk/pull/15343#issuecomment-1704796665
RFR: 8315579: SPARC64 builds are broken after JDK-8304913
Similar to other issues in the same area. I have no SPARC 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 SPARC Zero build completes fine. - Commit messages: - Fix Changes: https://git.openjdk.org/jdk/pull/15557/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15557=00 Issue: https://bugs.openjdk.org/browse/JDK-8315579 Stats: 13 lines in 3 files changed: 12 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/15557.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15557/head:pull/15557 PR: https://git.openjdk.org/jdk/pull/15557
RFR: 8315578: PPC builds are broken after JDK-8304913
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. - Commit messages: - Fix Changes: https://git.openjdk.org/jdk/pull/15556/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15556=00 Issue: https://bugs.openjdk.org/browse/JDK-8315578 Stats: 12 lines in 3 files changed: 12 ins; 0 del; 0 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: 8314949: linux PPC64 Big Endian: Implementation of Foreign Function & Memory API [v3]
On Fri, 25 Aug 2023 09:24:15 GMT, Maurizio Cimadamore wrote: >> The ABI says: "An aggregate or union smaller than one doubleword in size is >> padded so that it appears in the least significant bits of the doubleword. >> All others are padded, if necessary, at their tail." >> [https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi.html#PARAM-PASS]. >> I have written examples which pass 9 and 15 Bytes. >> In the first case, we need to get 0x0001020304050607 in the first argument >> and 0x08XX into the second argument (X is "don't care"). Shift >> amount is 7. >> In the second case, we need to get 0x0001020304050607 in the first argument >> and 0x08090a0b0c0d0eXX into the second argument. Shift amount is 1. >> In other words, we need shift amounts between 1 and 7. Stack slots and >> registers are always 64 bit on PPC64. > > Got it - I found these representations: > > https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi-1.7.html#BYTEORDER > > Very helpful. So you have e.g. a `short` value (loaded from somewhere) and > you have to store it on a double-word. Now, if you just stored it at offset > 0, you will write the bits 0-15, which are the "most" significant bits in > big-endian representation. So, it's backwards. I believe FFM will take care > of endianness, so that the bytes 0-7 and 8-15 will be "swapped" when writing > into the double-word (right?) but their base offset (0) is still off, as they > should really start at offset 48. Hence the shift. After looking for a while, I think having the new binding operator is needed. e.g. for stores: we load a 32 bit value from the end of a struct, then the low-order bits of the value needs to be padded with zeros to get a 64 bit register value, leaving the original 32 bit value in the high order bits. This can't be handled by the current cast operator. e.g. if we had an int -> long cast conversion, then in the resulting value the low-order bits would be occupied by the 32 bit value, which is incorrect. - PR Review Comment: https://git.openjdk.org/jdk/pull/15417#discussion_r1314524955
Re: RFR: 8314949: linux PPC64 Big Endian: Implementation of Foreign Function & Memory API [v3]
On Fri, 25 Aug 2023 10:59:45 GMT, Martin Doerr wrote:
>> I've found a way to solve the remaining FFI problem on linux PPC64 Big
>> Endian. Large structs (>8 Bytes) which are passed in registers or on stack
>> require shifting the Bytes in the last slot if the size is not a multiple of
>> 8. This PR adds the required functionality to the Java code.
>>
>> Please review and provide feedback. There may be better ways to implement
>> it. I just found one which works and makes the tests pass:
>>
>> Test summary
>> ==
>>TEST TOTAL PASS FAIL ERROR
>>
>>jtreg:test/jdk/java/foreign 8888 0 0
>>
>>
>>
>> Note: This PR should be considered as preparation work for AIX which also
>> uses ABIv1.
>
> Martin Doerr has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Remove unnecessary imports.
Sorry for the delay, I've been on vacation.
src/java.base/share/classes/jdk/internal/foreign/abi/Binding.java line 398:
> 396: bindings.add(Binding.cast(type, int.class));
> 397: type = int.class;
> 398: }
Part of the casts are handled here with explicit cast bindings, but the
widening from int -> long, and narrowing from long -> int are handled
implicitly as part of the ShiftLeft implementation. I'd much prefer if all the
type conversions are handled with explicit cast bindings. This would also
semantically simplify the shift operator, since it would just handle the actual
shifting.
src/java.base/share/classes/jdk/internal/foreign/abi/Binding.java line 695:
> 693: * Negative [shiftAmount] shifts right and converts to int if
> needed.
> 694: */
> 695: record ShiftLeft(int shiftAmount, Class type) implements Binding {
Please split this into 2 binding operators: one for ShiftLeft and another for
(arithmetic) ShiftRight, rather than mixing the 2 implementations into a single
operator.
For the right shift you could then also use a positive shift amount, and avoid
the double negation that's needed right now.
src/java.base/share/classes/jdk/internal/foreign/abi/Binding.java line 703:
> 701: if (last != ((type == long.class) ? long.class :
> int.class))
> 702: throw new IllegalArgumentException(
> 703: String.format("Invalid operand type: %s.
> integral type expected", last));
Why not use `SharedUtils.checkType` here?
Suggestion:
SharedUtils.checkType(last, type);
src/java.base/share/classes/jdk/internal/foreign/abi/Binding.java line 711:
> 709: stack.push(type == long.class ? long.class : int.class);
> 710: } else
> 711: throw new IllegalArgumentException("shiftAmount 0 not
> supported");
Please handle this validation in the static `shiftLeft` method. That's also
where we do validation for other binding ops
src/java.base/share/classes/jdk/internal/foreign/abi/BindingSpecializer.java
line 737:
> 735: cb.i2l();
> 736: typeStack.pop();
> 737: typeStack.push(long.class);
Please use `popType` and `pushType` instead of manipulating the type stack
manually. The former also does some verification. This should happen along
every control path. Even if the binding is 1-to-1 (like this one) and doesn't
change the type of the value, this would validate that the input type is
correct, and make sure that any bugs are caught early.
-
PR Review: https://git.openjdk.org/jdk/pull/15417#pullrequestreview-1595698959
PR Review Comment: https://git.openjdk.org/jdk/pull/15417#discussion_r1305681864
PR Review Comment: https://git.openjdk.org/jdk/pull/15417#discussion_r1314518216
PR Review Comment: https://git.openjdk.org/jdk/pull/15417#discussion_r1305642804
PR Review Comment: https://git.openjdk.org/jdk/pull/15417#discussion_r1305668315
PR Review Comment: https://git.openjdk.org/jdk/pull/15417#discussion_r1305650457
Re: RFR: 8233160: Add java.vendor.url.bug to list of recognized standard system properties
On Thu, 31 Aug 2023 06:53:45 GMT, Jaikiran Pai wrote: > Can I please get a review of this change which proposes to address > https://bugs.openjdk.org/browse/JDK-8233160? > > It has been noted in https://bugs.openjdk.org/browse/JDK-8232753 that: > >> The java.vendor.url.bug property has been defined by every Sun/Oracle JDK >> going all the way back to JDK 5 (and possibly earlier; JDK 5 is the oldest >> release that I can still run on my development machine). Yet, it's never >> been specified. > > The OpenJDK builds too by default set a value for this system property. > > The commit in this PR updates the javadoc of `System.getProperties()` to > include this system property in the list of specified properties. > Additionally, the `test/jdk/java/lang/System/PropertyTest.java` test has been > updated to verify that this property is indeed available in the Properties > returned by `System.getProperites()`. > > I'll create a CSR shortly for this change. I've now created a CSR for this issue https://bugs.openjdk.org/browse/JDK-8315601 - PR Comment: https://git.openjdk.org/jdk/pull/15504#issuecomment-1704735619
Re: RFR: 8315454: Add a way to create an immutable snapshot of a BitSet [v4]
> This PR proposes adding a new method to BitSet that provides an immutable > snapshot of the set in the form of an `IntPredicate`. > > The predicate is eligible for constant folding. > > Here are some classes in the JDK that would benefit directly from > constant-folding of BitSets: > > PoolReader (6) > URLEncoder (1) - Updated in this PR > HtmlTree (2) > > More over, the implementation of the predicate is @ValueBased and this would > provide additional benefits in the future. > > Initial benchmarks with the URLEncoder show encouraging results: > > > Name (encodeChars) (maxLength) (unchanged) Cnt > Base Error Test Error Unit Diff% > URLEncodeDecode.testEncodeUTF8 61024 0 15 > 2,371 ± 0,016 2,073 ± 0,184 ms/op 12,6% (p = 0,000*) > URLEncodeDecode.testEncodeUTF8 61024 75 15 > 1,772 ± 0,013 1,387 ± 0,032 ms/op 21,8% (p = 0,000*) > URLEncodeDecode.testEncodeUTF8 61024 100 15 > 1,230 ± 0,009 1,140 ± 0,011 ms/op 7,3% (p = 0,000*) Per Minborg has updated the pull request incrementally with one additional commit since the last revision: Do not propagate Integer.MAX_VALUE BitSets - Changes: - all: https://git.openjdk.org/jdk/pull/15530/files - new: https://git.openjdk.org/jdk/pull/15530/files/c3977aa3..6f80e0b6 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15530=03 - incr: https://webrevs.openjdk.org/?repo=jdk=15530=02-03 Stats: 6 lines in 1 file changed: 6 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/15530.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15530/head:pull/15530 PR: https://git.openjdk.org/jdk/pull/15530
Re: RFR: 8311220: Optimization for StringLatin UpperLower [v7]
> # Benchmark Result > > > sh make/devkit/createJMHBundle.sh > bash configure --with-jmh=build/jmh/jars > make test TEST="micro:java.lang.StringUpperLower.*" > > > > ## 1. > [aliyun_ecs_c8i.xlarge](https://help.aliyun.com/document_detail/25378.html#c8i) > * cpu : intel xeon sapphire rapids (x64) > > ``` diff > -Benchmark Mode Cnt Score Error Units (baseline) > -StringUpperLower.lowerToLower avgt 15 27.180 ± 0.017 ns/op > -StringUpperLower.lowerToUpper avgt 15 47.196 ± 0.066 ns/op > -StringUpperLower.mixedToLower avgt 15 32.307 ± 0.072 ns/op > -StringUpperLower.mixedToUpper avgt 15 44.005 ± 0.414 ns/op > -StringUpperLower.upperToLower avgt 15 32.310 ± 0.033 ns/op > -StringUpperLower.upperToUpper avgt 15 42.053 ± 0.341 ns/op > > +Benchmark Mode Cnt Score Error Units (Update 01) > +StringUpperLower.lowerToLower avgt 15 16.964 ± 0.021 ns/op (+60.96%) > +StringUpperLower.lowerToUpper avgt 15 46.491 ± 0.036 ns/op (+1.51%) > +StringUpperLower.mixedToLower avgt 15 35.947 ± 0.254 ns/op (-10.12%) > +StringUpperLower.mixedToUpper avgt 15 41.976 ± 0.326 ns/op (+4.83%) > +StringUpperLower.upperToLower avgt 15 33.466 ± 4.036 ns/op (-3.45%) > +StringUpperLower.upperToUpper avgt 15 17.446 ± 1.036 ns/op (+141.04%) > > +Benchmark Mode Cnt Score Error Units (Update 00) > +StringUpperLower.lowerToLower avgt 15 16.976 ± 0.043 ns/op (+60.160) > +StringUpperLower.lowerToUpper avgt 15 46.373 ± 0.086 ns/op (+1.77%) > +StringUpperLower.mixedToLower avgt 15 32.018 ± 0.061 ns/op (+0.9%) > +StringUpperLower.mixedToUpper avgt 15 42.019 ± 0.473 ns/op (+4.72%) > +StringUpperLower.upperToLower avgt 15 32.052 ± 0.051 ns/op (+0.8%) > +StringUpperLower.upperToUpper avgt 15 16.978 ± 0.190 ns/op (+147.69%) > > > ## 2. > [aliyun_ecs_c8a.xlarge](https://help.aliyun.com/document_detail/25378.html#c8a) > * cpu : amd epc genoa (x64) > > ``` diff > -Benchmark Mode Cnt Score Error Units (baseline) > -StringUpperLower.lowerToLower avgt 15 22.164 ± 0.021 ns/op > -StringUpperLower.lowerToUpper avgt 15 46.113 ± 0.047 ns/op > -StringUpperLower.mixedToLower avgt 15 28.501 ± 0.037 ns/op > -StringUpperLower.mixedToUpper avgt 15 38.782 ± 0.038 ns/op > -StringUpperLower.upperToLower avgt 15 28.625 ± 0.162 ns/op > -StringUpperLower.upperToUpper avgt 15 27.960 ± 0.038 ns/op > > +Benchmark Mode Cnt Score Error Units (Update 01) > +StringUpperLower.lowerToLo... 温绍锦 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 seven additional commits since the last revision: - Merge branch 'master' of github.com:wenshao/jdk into optimization_for_string_latin1_upper_lower - rename hasNotUpperCaseEx to hasUpperCaseMapping - rename isLowerCaseEx to hasNotUpperCaseEx - use hex literal - add method CharacterDataLatin1#isLowerCaseEx - remove unnecessary code - optimization for StringLatin1 UpperLower - Changes: - all: https://git.openjdk.org/jdk/pull/14751/files - new: https://git.openjdk.org/jdk/pull/14751/files/26aa8b19..a38f8870 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14751=06 - incr: https://webrevs.openjdk.org/?repo=jdk=14751=05-06 Stats: 148954 lines in 2871 files changed: 61152 ins; 70989 del; 16813 mod Patch: https://git.openjdk.org/jdk/pull/14751.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14751/head:pull/14751 PR: https://git.openjdk.org/jdk/pull/14751
