Re: RFR: 8267184: JEP 411: Add -Djava.security.manager=allow to tests calling System.setSecurityManager
On Mon, 17 May 2021 17:51:36 GMT, Weijun Wang wrote:
> Please review the test changes for [JEP
> 411](https://openjdk.java.net/jeps/411).
>
> With JEP 411 and the default value of `-Djava.security.manager` becoming
> `disallow`, tests calling `System.setSecurityManager()` need
> `-Djava.security.manager=allow` when launched. This PR covers such changes
> for tier1 to tier3 (except for the JCK tests).
>
> To make it easier to focus your review on the tests in your area, this PR is
> divided into multiple commits for different areas. Mostly the rule is the
> same as how Skara adds labels, but there are several small tweaks:
>
> 1. When a file is covered by multiple labels, only one is chosen. I make my
> best to avoid putting too many tests into `core-libs`. If a file is covered
> by `core-libs` and another label, I categorized it into the other label.
> 2. If a file is in `core-libs` but contains `/xml/` in its name, it's in the
> `xml` commit.
> 3. If a file is in `core-libs` but contains `/rmi/` in its name, it's in the
> `rmi` commit.
> 4. One file not covered by any label --
> `test/jdk/com/sun/java/accessibility/util/8051626/Bug8051626.java` -- is in
> the `swing` commit.
>
> Due to the size of this PR, no attempt is made to update copyright years for
> all files to minimize unnecessary merge conflict.
>
> Please note that this PR can be integrated before the source changes for JEP
> 411, as the possible values of this system property was already defined long
> time ago in JDK 9.
>
> Most of the change in this PR is a simple adding of
> `-Djava.security.manager=allow` to the `@run main/othervm` line. Sometimes it
> was not `othervm` and we add one. Sometimes there's no `@run` at all and we
> add the line.
>
> There are several tests that launch another Java process that needs to call
> the `System.setSecurityManager()` method, and the system property is added to
> `ProcessBuilder`, `ProcessTools`, or the java command line (if the test is a
> shell test).
>
> 3 langtools tests are added into problem list due to
> [JDK-8265611](https://bugs.openjdk.java.net/browse/JDK-8265611).
>
> 2 SQL tests are moved because they need different options on the `@run` line
> but they are inside a directory that has a `TEST.properties`:
>
> rename test/jdk/java/sql/{testng/test/sql/othervm =>
> permissionTests}/DriverManagerPermissionsTests.java (93%)
> rename test/jdk/javax/sql/{testng/test/rowset/spi =>
> permissionTests}/SyncFactoryPermissionsTests.java (95%)
> ```
>
> The source change for JEP 411 is at https://github.com/openjdk/jdk/pull/4073.
Marked as reviewed by alanb (Reviewer).
The changes look okay but a bit inconsistent on where -Djava...=allow is
inserted for tests that already set other system properties or other
parameters. Not a correctness issue, just looks odd in several places, e.g.
test/jdk/java/lang/System/LoggerFinder/BaseLoggerFinderTest/BaseLoggerFinderTest.java
- the tests sets the system properties after -Xbootclasspath/a: but the change
means it sets properties before and after.
test/jdk/java/lang/Class/getResource/ResourcesTest.java - you've added it in
the middle of the module and class path parameters.
For uses using ProcessTools then it seems to be very random.
-
PR: https://git.openjdk.java.net/jdk/pull/4071
Re: RFR: 8267184: JEP 411: Add -Djava.security.manager=allow to tests calling System.setSecurityManager
On Mon, 17 May 2021 17:51:36 GMT, Weijun Wang wrote:
> Please review the test changes for [JEP
> 411](https://openjdk.java.net/jeps/411).
>
> With JEP 411 and the default value of `-Djava.security.manager` becoming
> `disallow`, tests calling `System.setSecurityManager()` need
> `-Djava.security.manager=allow` when launched. This PR covers such changes
> for tier1 to tier3 (except for the JCK tests).
>
> To make it easier to focus your review on the tests in your area, this PR is
> divided into multiple commits for different areas. Mostly the rule is the
> same as how Skara adds labels, but there are several small tweaks:
>
> 1. When a file is covered by multiple labels, only one is chosen. I make my
> best to avoid putting too many tests into `core-libs`. If a file is covered
> by `core-libs` and another label, I categorized it into the other label.
> 2. If a file is in `core-libs` but contains `/xml/` in its name, it's in the
> `xml` commit.
> 3. If a file is in `core-libs` but contains `/rmi/` in its name, it's in the
> `rmi` commit.
> 4. One file not covered by any label --
> `test/jdk/com/sun/java/accessibility/util/8051626/Bug8051626.java` -- is in
> the `swing` commit.
>
> Due to the size of this PR, no attempt is made to update copyright years for
> all files to minimize unnecessary merge conflict.
>
> Please note that this PR can be integrated before the source changes for JEP
> 411, as the possible values of this system property was already defined long
> time ago in JDK 9.
>
> Most of the change in this PR is a simple adding of
> `-Djava.security.manager=allow` to the `@run main/othervm` line. Sometimes it
> was not `othervm` and we add one. Sometimes there's no `@run` at all and we
> add the line.
>
> There are several tests that launch another Java process that needs to call
> the `System.setSecurityManager()` method, and the system property is added to
> `ProcessBuilder`, `ProcessTools`, or the java command line (if the test is a
> shell test).
>
> 3 langtools tests are added into problem list due to
> [JDK-8265611](https://bugs.openjdk.java.net/browse/JDK-8265611).
>
> 2 SQL tests are moved because they need different options on the `@run` line
> but they are inside a directory that has a `TEST.properties`:
>
> rename test/jdk/java/sql/{testng/test/sql/othervm =>
> permissionTests}/DriverManagerPermissionsTests.java (93%)
> rename test/jdk/javax/sql/{testng/test/rowset/spi =>
> permissionTests}/SyncFactoryPermissionsTests.java (95%)
> ```
>
> The source change for JEP 411 is at https://github.com/openjdk/jdk/pull/4073.
Changes to hotspot-* and serviceability look good.
Thanks,
David
-
Marked as reviewed by dholmes (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/4071
Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal
On Mon, 17 May 2021 18:23:41 GMT, Weijun Wang wrote: > Please review this implementation of [JEP > 411](https://openjdk.java.net/jeps/411). > > The code change is divided into 3 commits. Please review them one by one. > > 1. > https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1 > The essential change for this JEP, including the `@Deprecate` annotations > and spec change. It also update the default value of the > `java.security.manager` system property to "disallow", and necessary test > change following this update. > 2. > https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66 > Manual changes to several files so that the next commit can be generated > programatically. > 3. > https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950 > Automatic changes to other source files to avoid javac warnings on > deprecation for removal > > The 1st and 2nd commits should be reviewed carefully. The 3rd one is > generated programmatically, see the comment below for more details. If you > are only interested in a portion of the 3rd commit and would like to review > it as a separate file, please comment here and I'll generate an individual > webrev. > > Due to the size of this PR, no attempt is made to update copyright years for > any file to minimize unnecessary merge conflict. > > Furthermore, since the default value of `java.security.manager` system > property is now "disallow", most of the tests calling > `System.setSecurityManager()` need to launched with > `-Djava.security.manager=allow`. This is covered in a different PR at > https://github.com/openjdk/jdk/pull/4071. Makefile change looks ok. - Marked as reviewed by erikj (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4073
Re: RFR: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v4]
On Sat, 15 May 2021 02:06:29 GMT, Sandhya Viswanathan wrote: >> This PR contains Short Vector Math Library support related changes for >> [JEP-414 Vector API (Second Incubator)](https://openjdk.java.net/jeps/414), >> in preparation for when targeted. >> >> Intel Short Vector Math Library (SVML) based intrinsics in native x86 >> assembly provide optimized implementation for Vector API transcendental and >> trigonometric methods. >> These methods are built into a separate library instead of being part of >> libjvm.so or jvm.dll. >> >> The following changes are made: >>The source for these methods is placed in the jdk.incubator.vector module >> under src/jdk.incubator.vector/linux/native/libsvml and >> src/jdk.incubator.vector/windows/native/libsvml. >>The assembly source files are named as “*.S” and include files are named >> as “*.S.inc”. >>The corresponding build script is placed at >> make/modules/jdk.incubator.vector/Lib.gmk. >>Changes are made to build system to support dependency tracking for >> assembly files with includes. >>The built native libraries (libsvml.so/svml.dll) are placed in bin >> directory of JDK on Windows and lib directory of JDK on Linux. >>The C2 JIT uses the dll_load and dll_lookup to get the addresses of >> optimized methods from this library. >> >> Build system changes and module library build scripts are contributed by >> Magnus (magnus.ihse.bur...@oracle.com). >> >> Looking forward to your review and feedback. >> >> Performance: >> Micro benchmark Base Optimized Unit Gain(Optimized/Base) >> Double128Vector.ACOS 45.91 87.34 ops/ms 1.90 >> Double128Vector.ASIN 45.06 92.36 ops/ms 2.05 >> Double128Vector.ATAN 19.92 118.36 ops/ms 5.94 >> Double128Vector.ATAN2 15.24 88.17 ops/ms 5.79 >> Double128Vector.CBRT 45.77 208.36 ops/ms 4.55 >> Double128Vector.COS 49.94 245.89 ops/ms 4.92 >> Double128Vector.COSH 26.91 126.00 ops/ms 4.68 >> Double128Vector.EXP 71.64 379.65 ops/ms 5.30 >> Double128Vector.EXPM1 35.95 150.37 ops/ms 4.18 >> Double128Vector.HYPOT 50.67 174.10 ops/ms 3.44 >> Double128Vector.LOG 61.95 279.84 ops/ms 4.52 >> Double128Vector.LOG10 59.34 239.05 ops/ms 4.03 >> Double128Vector.LOG1P 18.56 200.32 ops/ms 10.79 >> Double128Vector.SIN 49.36 240.79 ops/ms 4.88 >> Double128Vector.SINH 26.59 103.75 ops/ms 3.90 >> Double128Vector.TAN 41.05 152.39 ops/ms 3.71 >> Double128Vector.TANH 45.29 169.53 ops/ms 3.74 >> Double256Vector.ACOS 54.21 106.39 ops/ms 1.96 >> Double256Vector.ASIN 53.60 107.99 ops/ms 2.01 >> Double256Vector.ATAN 21.53 189.11 ops/ms 8.78 >> Double256Vector.ATAN2 16.67 140.76 ops/ms 8.44 >> Double256Vector.CBRT 56.45 397.13 ops/ms 7.04 >> Double256Vector.COS 58.26 389.77 ops/ms 6.69 >> Double256Vector.COSH 29.44 151.11 ops/ms 5.13 >> Double256Vector.EXP 86.67 564.68 ops/ms 6.52 >> Double256Vector.EXPM1 41.96 201.28 ops/ms 4.80 >> Double256Vector.HYPOT 66.18 305.74 ops/ms 4.62 >> Double256Vector.LOG 71.52 394.90 ops/ms 5.52 >> Double256Vector.LOG10 65.43 362.32 ops/ms 5.54 >> Double256Vector.LOG1P 19.99 300.88 ops/ms 15.05 >> Double256Vector.SIN 57.06 380.98 ops/ms 6.68 >> Double256Vector.SINH 29.40 117.37 ops/ms 3.99 >> Double256Vector.TAN 44.90 279.90 ops/ms 6.23 >> Double256Vector.TANH 54.08 274.71 ops/ms 5.08 >> Double512Vector.ACOS 55.65 687.54 ops/ms 12.35 >> Double512Vector.ASIN 57.31 777.72 ops/ms 13.57 >> Double512Vector.ATAN 21.42 729.21 ops/ms 34.04 >> Double512Vector.ATAN2 16.37 414.33 ops/ms 25.32 >> Double512Vector.CBRT 56.78 834.38 ops/ms 14.69 >> Double512Vector.COS 59.88 837.04 ops/ms 13.98 >> Double512Vector.COSH 30.34 172.76 ops/ms 5.70 >> Double512Vector.EXP 99.66 1608.12 ops/ms 16.14 >> Double512Vector.EXPM1 43.39 318.61 ops/ms 7.34 >> Double512Vector.HYPOT 73.87 1502.72 ops/ms 20.34 >> Double512Vector.LOG 74.84 996.00 ops/ms 13.31 >> Double512Vector.LOG10 71.12 1046.52 ops/ms 14.72 >> Double512Vector.LOG1P 19.75 776.87 ops/ms 39.34 >> Double512Vector.POW 37.42 384.13 ops/ms 10.26 >> Double512Vector.SIN 59.74 728.45 ops/ms 12.19 >> Double512Vector.SINH 29.47 143.38 ops/ms 4.87 >> Double512Vector.TAN 46.20 587.21 ops/ms 12.71 >> Double512Vector.TANH 57.36 495.42 ops/ms 8.64 >> Double64Vector.ACOS 24.04 73.67 ops/ms 3.06 >> Double64Vector.ASIN 23.78 75.11 ops/ms 3.16 >> Double64Vector.ATAN 14.14 62.81 ops/ms 4.44 >> Double64Vector.ATAN2 10.38 44.43 ops/ms 4.28 >> Double64Vector.CBRT 16.47 107.50 ops/ms 6.53 >> Double64Vector.COS 23.42 152.01 ops/ms 6.49 >> Double64Vector.COSH 17.34 113.34 ops/ms 6.54 >> Double64Vector.EXP 27.08 203.53 ops/ms 7.52 >> Double64Vector.EXPM1 18.77 96.73 ops/ms 5.15 >> Double64Vector.HYPOT 18.54 103.62 ops/ms 5.59 >> Double64Vector.LOG 26.75 142.63 ops/ms 5.33 >> Double64Vector.LOG10 25.85 139.71 ops/ms 5.40 >> Double64Vector.LOG1P 13.26 97.94 ops/ms 7.38 >> Double64Vector.SIN 23.28 146.91 ops/ms 6.31 >> Double64Vector.SINH 17.62 88.59 ops/ms 5.03 >> Double64Vector.TAN 21.00 86.43 ops/ms 4.12 >> Double64Vector.TANH 23.75 111.35 ops/ms 4.69 >> Float128Vector.ACOS 57.52 110.65
Re: RFR: 8265029: Preserve SIZED characteristics on slice operations (skip, limit) [v3]
On Sat, 17 Apr 2021 02:16:44 GMT, Tagir F. Valeev wrote: >> Tagir F. Valeev has updated the pull request incrementally with four >> additional commits since the last revision: >> >> - More benchmarks >> - adjustSize -> exactOutputSize >> >>Co-authored-by: Paul Sandoz >> - adjustedLimit -> normalizedLimit >> >>Co-authored-by: Paul Sandoz >> - adjustSize -> exactOutputSize >> >>Co-authored-by: Paul Sandoz > > I see your concern. I made some additional benchmarks and added them here. > First, CountSized, which just gets the stream size without actual traversal. > We can see how the performance changes depending on number of stream > operations. I also added an optional type profile pollution that makes > `exactOutputSize` virtual method polymorphic. Here's the results: > > Baseline (The `count10Skip` test added just to ensure that patch works) > > Benchmark (pollute) (size) Mode Cnt Score Error Units > count0 false 1 avgt 100 15,648 ±0,182 ns/op > count2 false 1 avgt 100 31,252 ±0,113 ns/op > count4 false 1 avgt 100 47,683 ±0,165 ns/op > count6 false 1 avgt 100 64,417 ±0,203 ns/op > count8 false 1 avgt 100 80,813 ±0,265 ns/op > count10false 1 avgt 100 101,057 ±0,295 ns/op > count10Skipfalse 1 avgt 100 497967,375 ± 5946,108 ns/op > count0 true 1 avgt 100 18,843 ±0,103 ns/op > count2 true 1 avgt 100 33,716 ±0,152 ns/op > count4 true 1 avgt 100 49,062 ±0,208 ns/op > count6 true 1 avgt 100 66,773 ±0,237 ns/op > count8 true 1 avgt 100 82,727 ±0,354 ns/op > count10 true 1 avgt 100 104,499 ±0,299 ns/op > count10Skip true 1 avgt 100 501723,220 ± 6361,932 ns/op > > Type pollution adds some near-constant ~2ns overhead to the non-patched > version as well. > > Patched: > > Benchmark (pollute) (size) Mode CntScore Error Units > count0 false 1 avgt 100 15,363 ± 0,086 ns/op > count2 false 1 avgt 100 33,736 ± 0,138 ns/op > count4 false 1 avgt 100 51,470 ± 0,205 ns/op > count6 false 1 avgt 100 70,407 ± 0,262 ns/op > count8 false 1 avgt 100 89,865 ± 0,262 ns/op > count10false 1 avgt 100 114,423 ± 0,363 ns/op > count10Skipfalse 1 avgt 100 139,963 ± 0,550 ns/op > count0 true 1 avgt 100 26,538 ± 0,084 ns/op > count2 true 1 avgt 100 46,089 ± 0,191 ns/op > count4 true 1 avgt 100 66,560 ± 0,315 ns/op > count6 true 1 avgt 100 87,852 ± 0,288 ns/op > count8 true 1 avgt 100 109,037 ± 0,391 ns/op > count10 true 1 avgt 100 139,759 ± 0,382 ns/op > count10Skip true 1 avgt 100 156,963 ± 1,862 ns/op > > So indeed we have some performance drawback in patched version. Here's a > chart: > >  > I've calculated linear regression on (patched-baseline) times, depending on > the number of ops. It's `y = 1.288x - 0.7078` for clean type profile and `y = > 2.6174x + 6.9489` for polluted type profile. So, in the worst case, we have > circa 2.6ns per operation plus 7ns constant overhead. > > However, using Stream API without actually iterating the stream is very rare > case. And if we iterate, the performance will be dominated by the number of > iterations. I tried to model this case with SumSized benchmark (replacing > count with sum, for 5 and 10 stream elements), but got very confusing results. > > Baseline: > > Benchmark (pollute) (size) Mode CntScoreError Units > sum0true 5 avgt 100 126,425 ± 0,793 ns/op > sum2true 5 avgt 100 195,113 ± 1,359 ns/op > sum4true 5 avgt 100 304,111 ± 8,302 ns/op > sum6true 5 avgt 100 414,841 ± 3,215 ns/op > sum8true 5 avgt 100 507,421 ± 4,781 ns/op > sum10 true 5 avgt 100 633,635 ± 7,105 ns/op > sum0 false 5 avgt 100 45,781 ± 0,258 ns/op > sum2 false 5 avgt 100 86,720 ± 0,573 ns/op > sum4 false 5 avgt 100 195,777 ± 1,145 ns/op > sum6 false 5 avgt 100 291,261 ± 2,091 ns/op > sum8 false 5 avgt 100 376,094 ± 3,283 ns/op > sum10 false 5 avgt 100 492,082 ± 7,914 ns/op > sum0true 10 avgt 100 127,989 ± 0,758 ns/op > sum2true 10 avgt 100 219,991 ± 3,081 ns/op > sum4true 10 avgt 100 374,148 ± 7,426 ns/op > sum6
RFR: 8267184: JEP 411: Add -Djava.security.manager=allow to tests calling System.setSecurityManager
Please review the test changes for [JEP 411](https://openjdk.java.net/jeps/411).
With JEP 411 and the default value of `-Djava.security.manager` becoming
`disallow`, tests calling `System.setSecurityManager()` need
`-Djava.security.manager=allow` when launched. This PR covers such changes for
tier1 to tier3 (except for the JCK tests).
To make it easier to focus your review on the tests in your area, this PR is
divided into multiple commits for different areas. Mostly the rule is the same
as how Skara adds labels, but there are several small tweaks:
1. When a file is covered by multiple labels, only one is chosen. I make my
best to avoid putting too many tests into `core-libs`. If a file is covered by
`core-libs` and another label, I categorized it into the other label.
2. If a file is in `core-libs` but contains `/xml/` in its name, it's in the
`xml` commit.
3. If a file is in `core-libs` but contains `/rmi/` in its name, it's in the
`rmi` commit.
4. One file not covered by any label --
`test/jdk/com/sun/java/accessibility/util/8051626/Bug8051626.java` -- is in the
`swing` commit.
Due to the size of this PR, no attempt is made to update copyright years for
all files to minimize unnecessary merge conflict.
Please note that this PR can be integrated before the source changes for JEP
411, as the possible values of this system property was already defined long
time ago in JDK 9.
Most of the change in this PR is a simple adding of
`-Djava.security.manager=allow` to the `@run main/othervm` line. Sometimes it
was not `othervm` and we add one. Sometimes there's no `@run` at all and we add
the line.
There are several tests that launch another Java process that needs to call the
`System.setSecurityManager()` method, and the system property is added to
`ProcessBuilder`, `ProcessTools`, or the java command line (if the test is a
shell test).
3 langtools tests are added into problem list due to
[JDK-8265611](https://bugs.openjdk.java.net/browse/JDK-8265611).
2 SQL tests are moved because they need different options on the `@run` line
but they are inside a directory that has a `TEST.properties`:
rename test/jdk/java/sql/{testng/test/sql/othervm =>
permissionTests}/DriverManagerPermissionsTests.java (93%)
rename test/jdk/javax/sql/{testng/test/rowset/spi =>
permissionTests}/SyncFactoryPermissionsTests.java (95%)
```
The source change for JEP 411 is at https://github.com/openjdk/jdk/pull/4073.
-
Commit messages:
- test for awt
- test for hotspot-gc
- test for compiler
- test for net
- test for core-libs
- test for beans
- test for xml
- test for nio
- test for hotspot-runtime
- test for security
- ... and 7 more: https://git.openjdk.java.net/jdk/compare/79b39445...900c28c0
Changes: https://git.openjdk.java.net/jdk/pull/4071/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4071=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8267184
Stats: 1024 lines in 852 files changed: 249 ins; 8 del; 767 mod
Patch: https://git.openjdk.java.net/jdk/pull/4071.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/4071/head:pull/4071
PR: https://git.openjdk.java.net/jdk/pull/4071
Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal
On Mon, 17 May 2021 18:23:41 GMT, Weijun Wang wrote:
> Please review this implementation of [JEP
> 411](https://openjdk.java.net/jeps/411).
>
> The code change is divided into 3 commits. Please review them one by one.
>
> 1.
> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1
> The essential change for this JEP, including the `@Deprecate` annotations
> and spec change. It also update the default value of the
> `java.security.manager` system property to "disallow", and necessary test
> change following this update.
> 2.
> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66
> Manual changes to several files so that the next commit can be generated
> programatically.
> 3.
> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950
> Automatic changes to other source files to avoid javac warnings on
> deprecation for removal
>
> The 1st and 2nd commits should be reviewed carefully. The 3rd one is
> generated programmatically, see the comment below for more details. If you
> are only interested in a portion of the 3rd commit and would like to review
> it as a separate file, please comment here and I'll generate an individual
> webrev.
>
> Due to the size of this PR, no attempt is made to update copyright years for
> any file to minimize unnecessary merge conflict.
>
> Furthermore, since the default value of `java.security.manager` system
> property is now "disallow", most of the tests calling
> `System.setSecurityManager()` need to launched with
> `-Djava.security.manager=allow`. This is covered in a different PR at
> https://github.com/openjdk/jdk/pull/4071.
The 3rd commit is the biggest one but it's all generated programmatically. All
changes are simply adding `@SupressWarnings("removal")` to a declaration.
Precisely, of all the diff hunks (leading whitespaces ignored), there are 1607
+ @SuppressWarnings("removal")
There are also 7 cases where an existing annotation is updated
= 2
- @SuppressWarnings("deprecation")
+ @SuppressWarnings({"removal","deprecation"})
= 1
- @SuppressWarnings("Convert2Lambda")
+ @SuppressWarnings({"removal","Convert2Lambda"})
= 1
- @SuppressWarnings({"unchecked", "CloneDeclaresCloneNotSupported"})
+ @SuppressWarnings({"removal","unchecked", "CloneDeclaresCloneNotSupported"})
= 1
- @SuppressWarnings("deprecation") // Use of RMISecurityManager
+ @SuppressWarnings({"removal","deprecation"}) // Use of RMISecurityManager
= 1
- @SuppressWarnings("unchecked") /*To suppress warning from line 1374*/
+ @SuppressWarnings({"removal","unchecked"}) /*To suppress warning from
line 1374*/
= 1
- @SuppressWarnings("fallthrough")
+ @SuppressWarnings({"removal","fallthrough"})
All other cases are new annotation embedded inline:
= 7
- AccessControlContext acc) {
+ @SuppressWarnings("removal") AccessControlContext acc) {
= 4
- AccessControlContext acc,
+ @SuppressWarnings("removal") AccessControlContext acc,
= 3
- AccessControlContext context,
+ @SuppressWarnings("removal") AccessControlContext context,
= 3
- AccessControlContext acc)
+ @SuppressWarnings("removal") AccessControlContext acc)
= 2
- try (InputStream stream = AccessController.doPrivileged(pa)) {
+ try (@SuppressWarnings("removal") InputStream stream =
AccessController.doPrivileged(pa)) {
= 2
- AccessControlContext context, Permission... perms) {
+ @SuppressWarnings("removal") AccessControlContext context, Permission...
perms) {
= 2
- } catch (java.security.AccessControlException e) {
+ } catch (@SuppressWarnings("removal") java.security.AccessControlException e)
{
= 2
- } catch (AccessControlException ace) {
+ } catch (@SuppressWarnings("removal") AccessControlException ace) {
= 2
- AccessControlContext context)
+ @SuppressWarnings("removal") AccessControlContext context)
= 2
- AccessControlContext acc) throws LoginException {
+ @SuppressWarnings("removal") AccessControlContext acc) throws LoginException {
= 2
- } catch (AccessControlException e) {
+ } catch (@SuppressWarnings("removal") AccessControlException e) {
= 1
- public static void addHook(AccessControlContext acc, PlatformEventType type,
Runnable hook) {
+ public static void addHook(@SuppressWarnings("removal") AccessControlContext
acc, PlatformEventType type, Runnable hook) {
= 1
RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal
Please review this implementation of [JEP 411](https://openjdk.java.net/jeps/411). The code change is divided into 3 commits. Please review them one by one. 1. https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1 The essential change for this JEP, including the `@Deprecate` annotations and spec change. It also update the default value of the `java.security.manager` system property to "disallow", and necessary test change following this update. 2. https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66 Manual changes to several files so that the next commit can be generated programatically. 3. https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950 Automatic changes to other source files to avoid javac warnings on deprecation for removal The 1st and 2nd commits should be reviewed carefully. The 3rd one is generated programmatically, see the comment below for more details. If you are only interested in a portion of the 3rd commit and would like to review it as a separate file, please comment here and I'll generate an individual webrev. Due to the size of this PR, no attempt is made to update copyright years for any file to minimize unnecessary merge conflict. Furthermore, since the default value of `java.security.manager` system property is now "disallow", most of the tests calling `System.setSecurityManager()` need to launched with `-Djava.security.manager=allow`. This is covered in a different PR at https://github.com/openjdk/jdk/pull/4071. - Commit messages: - add supresswarnings annotations automatically - manual change before automatic annotating - 8266459: Implement JEP 411: Deprecate the Security Manager for Removal Changes: https://git.openjdk.java.net/jdk/pull/4073/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4073=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8266459 Stats: 2018 lines in 826 files changed: 1884 ins; 9 del; 125 mod Patch: https://git.openjdk.java.net/jdk/pull/4073.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4073/head:pull/4073 PR: https://git.openjdk.java.net/jdk/pull/4073
Integrated: 8264777: Overload optimized FileInputStream::readAllBytes
On Mon, 3 May 2021 20:33:23 GMT, Brian Burkhalter wrote: > Please consider this request to override the `java.io.InputStream` methods > `readAllBytes()` and `readNBytes(int)` in `FileInputStream` with more > performant implementations. The method overrides attempt to read all > requested bytes into a single array of the required size rather than > composing the result from a sequence of smaller arrays. An example of the > performance improvements is as follows. > > **readAllBytes** > Before > > Benchmark(length) Mode Cnt Score >Error Units > ReadAllBytes.readAllBytesFileInputStream 100 thrpt 20 2412.031 > ± 7.309 ops/s > ReadAllBytes.readAllBytesFileInputStream 1000 thrpt 20212.181 > ± 0.369 ops/s > ReadAllBytes.readAllBytesFileInputStream1 thrpt 20 19.860 > ± 0.048 ops/s > ReadAllBytes.readAllBytesFileInputStream 10 thrpt 20 1.298 > ± 0.183 ops/s > > After > > Benchmark(length) Mode Cnt Score >Error Units > ReadAllBytes.readAllBytesFileInputStream 100 thrpt 20 8218.473 > ± 43.425 ops/s > ReadAllBytes.readAllBytesFileInputStream 1000 thrpt 20302.781 > ± 1.273 ops/s > ReadAllBytes.readAllBytesFileInputStream1 thrpt 20 22.461 > ± 0.084 ops/s > ReadAllBytes.readAllBytesFileInputStream 10 thrpt 20 1.502 > ± 0.003 ops/s > > > **readNBytes** > > `N = file_size / 2` > > Before > > Benchmark(length) Mode Cnt Score >Error Units > ReadAllBytes.readHalfBytesFileInputStream 100 thrpt 20 5585.500 > ± 153.353 ops/s > ReadAllBytes.readHalfBytesFileInputStream1000 thrpt 20436.164 > ± 1.104 ops/s > ReadAllBytes.readHalfBytesFileInputStream 1 thrpt 20 40.167 > ± 0.141 ops/s > ReadAllBytes.readHalfBytesFileInputStream 10 thrpt 20 3.291 > ± 0.211 ops/s > > > After > > Benchmark(length) Mode Cnt Score >Error Units > ReadAllBytes.readHalfBytesFileInputStream 100 thrpt 20 15463.210 > ± 66.045 ops/s > ReadAllBytes.readHalfBytesFileInputStream1000 thrpt 20561.752 > ± 0.951 ops/s > ReadAllBytes.readHalfBytesFileInputStream 1 thrpt 20 45.043 > ± 0.102 ops/s > ReadAllBytes.readHalfBytesFileInputStream 10 thrpt 20 4.629 > ± 0.035 ops/s This pull request has now been integrated. Changeset: da4dfde7 Author:Brian Burkhalter URL: https://git.openjdk.java.net/jdk/commit/da4dfde71a176d2b8401782178e854d4c924eba1 Stats: 244 lines in 4 files changed: 238 ins; 1 del; 5 mod 8264777: Overload optimized FileInputStream::readAllBytes Reviewed-by: dfuchs, alanb - PR: https://git.openjdk.java.net/jdk/pull/3845
Re: [External] : Re: JEP draft: Scope Locals
Let's try again: why is that important? What decisions would you make differently if you had this information? What benefit would come from those decisions? Threading is hard. Gee, thanks MIke for pointing that out to me, obviously I'm new to the topic :) Scoped variables are also hard to get right. Combining the two without an explicit api ignores the complexity that will arise as scoped variables are passed to other threads. OK, now we're getting to the actual question; you obviously have some assumptions about the degree of sharing expected, that might be skipping a few steps (and then skipping some more steps by jumping to a solution). So to ask again: what patterns of access / sharing are you assuming, what risks are you anticipating, etc?
Re: RFR: 8262891: Compiler implementation for Pattern Matching for switch (Preview) [v2]
On Mon, 17 May 2021 19:04:11 GMT, Jan Lahoda wrote: >> This is a preview of a patch implementing JEP 406: Pattern Matching for >> switch (Preview): >> https://bugs.openjdk.java.net/browse/JDK-8213076 >> >> The current draft of the specification is here: >> http://cr.openjdk.java.net/~gbierman/jep406/jep406-20210430/specs/patterns-switch-jls.html >> >> A summary of notable parts of the patch: >> -to support cases expressions and patterns in cases, there is a new common >> superinterface for expressions and patterns, `CaseLabelTree`, which >> expressions and patterns implement, and a list of case labels is returned >> from `CaseTree.getLabels()`. >> -to support `case default`, there is an implementation of `CaseLabelTree` >> that represents it (`DefaultCaseLabelTree`). It is used also to represent >> the conventional `default` internally and in the newly added methods. >> -in the parser, parenthesized patterns and expressions need to be >> disambiguated when parsing case labels. >> -Lower has been enhanced to handle `case null` for ordinary >> (boxed-primitive, String, enum) switches. This is a bit tricky for boxed >> primitives, as there is no value that is not part of the input domain so >> that could be used to represent `case null`. Requires a bit shuffling with >> values. >> -TransPatterns has been enhanced to handle the pattern matching switch. It >> produces code that delegates to a new bootstrap method, that will classify >> the input value to the switch and return the case number, to which the >> switch then jumps. To support guards, the switches (and the bootstrap >> method) are restartable. The bootstrap method as such is written very simply >> so far, but could be much more optimized later. >> -nullable type patterns are `case String s, null`/`case null, String >> s`/`case null: case String s:`/`case String s: case null:`, handling of >> these required a few tricks in `Attr`, `Flow` and `TransPatterns`. >> >> The specdiff for the change is here (to be updated): >> http://cr.openjdk.java.net/~jlahoda/8265981/specdiff.preview.01/overview-summary.html > > Jan Lahoda has updated the pull request incrementally with three additional > commits since the last revision: > > - Reflecting recent spec changes. > - Reflecting review comments. > - Reflecting review comments on SwitchBootstraps. It's not clear to me if it's the first change and several will follow or not. The code looks good but the metaprotocol is not the right one IMO, i would prefer the one we discuss in April https://mail.openjdk.java.net/pipermail/amber-spec-experts/2021-April/002992.html But this can be tackle in another PR - Marked as reviewed by fo...@github.com (no known OpenJDK username). PR: https://git.openjdk.java.net/jdk/pull/3863
Re: RFR: 8262891: Compiler implementation for Pattern Matching for switch (Preview) [v2]
> This is a preview of a patch implementing JEP 406: Pattern Matching for > switch (Preview): > https://bugs.openjdk.java.net/browse/JDK-8213076 > > The current draft of the specification is here: > http://cr.openjdk.java.net/~gbierman/jep406/jep406-20210430/specs/patterns-switch-jls.html > > A summary of notable parts of the patch: > -to support cases expressions and patterns in cases, there is a new common > superinterface for expressions and patterns, `CaseLabelTree`, which > expressions and patterns implement, and a list of case labels is returned > from `CaseTree.getLabels()`. > -to support `case default`, there is an implementation of `CaseLabelTree` > that represents it (`DefaultCaseLabelTree`). It is used also to represent the > conventional `default` internally and in the newly added methods. > -in the parser, parenthesized patterns and expressions need to be > disambiguated when parsing case labels. > -Lower has been enhanced to handle `case null` for ordinary (boxed-primitive, > String, enum) switches. This is a bit tricky for boxed primitives, as there > is no value that is not part of the input domain so that could be used to > represent `case null`. Requires a bit shuffling with values. > -TransPatterns has been enhanced to handle the pattern matching switch. It > produces code that delegates to a new bootstrap method, that will classify > the input value to the switch and return the case number, to which the switch > then jumps. To support guards, the switches (and the bootstrap method) are > restartable. The bootstrap method as such is written very simply so far, but > could be much more optimized later. > -nullable type patterns are `case String s, null`/`case null, String s`/`case > null: case String s:`/`case String s: case null:`, handling of these required > a few tricks in `Attr`, `Flow` and `TransPatterns`. > > The specdiff for the change is here (to be updated): > http://cr.openjdk.java.net/~jlahoda/8265981/specdiff.preview.01/overview-summary.html Jan Lahoda has updated the pull request incrementally with three additional commits since the last revision: - Reflecting recent spec changes. - Reflecting review comments. - Reflecting review comments on SwitchBootstraps. - Changes: - all: https://git.openjdk.java.net/jdk/pull/3863/files - new: https://git.openjdk.java.net/jdk/pull/3863/files/5e663d70..54ba974e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3863=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3863=00-01 Stats: 544 lines in 18 files changed: 431 ins; 68 del; 45 mod Patch: https://git.openjdk.java.net/jdk/pull/3863.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3863/head:pull/3863 PR: https://git.openjdk.java.net/jdk/pull/3863
Re: RFR: 8262891: Compiler implementation for Pattern Matching for switch (Preview) [v2]
On Tue, 4 May 2021 20:48:34 GMT, Maurizio Cimadamore
wrote:
>> Jan Lahoda has updated the pull request incrementally with three additional
>> commits since the last revision:
>>
>> - Reflecting recent spec changes.
>> - Reflecting review comments.
>> - Reflecting review comments on SwitchBootstraps.
>
> Good work. There's a lot to take in here. I think overall, it holds up well -
> I like how case labels have been extended to accommodate for patterns. In the
> front-end, I think there are some questions over the split between Attr and
> Flow - maybe it is unavoidable, but I'm not sure why some control-flow
> analysis happens in Attr instead of Flow and I left some questions to make
> sure I understand what's happening.
>
> In the backend it's mostly good work - but overall the structure of the code,
> both in Lower and in TransPattern is getting very difficult to follow, given
> there are many different kind of switches each with its own little twist
> attached to it. It would be nice, I think (maybe in a separate cleanup?) if
> the code paths serving the different switch kinds could be made more
> separate, so that, when reading the code we can worry only about one possible
> code shape. That would make the code easier to document as well.
@mcimadamore, @forax, thanks a lot for comments! I tried to apply the requested
changes in recent commits
(https://github.com/openjdk/jdk/pull/3863/commits/3fc2502a33cec20f39fe571eb25538ee3b459a9b
https://github.com/openjdk/jdk/pull/3863/commits/aeddb85e65bf77ed62dc7fa1ad00c29646d02c99
).
I've also tried to update the implementation for the latest spec changes here:
https://github.com/openjdk/jdk/pull/3863/commits/54ba974e1aac00bbde1c3bd2627f01caaaeda09b
The spec changes are: total patterns are nullable; pattern matching ("new")
statement switches must be complete (as switch expressions).
Any further feedback is welcome!
-
PR: https://git.openjdk.java.net/jdk/pull/3863
Re: RFR: 8264777: Overload optimized FileInputStream::readAllBytes [v9]
On Mon, 17 May 2021 17:53:05 GMT, Brian Burkhalter wrote: >> Please consider this request to override the `java.io.InputStream` methods >> `readAllBytes()` and `readNBytes(int)` in `FileInputStream` with more >> performant implementations. The method overrides attempt to read all >> requested bytes into a single array of the required size rather than >> composing the result from a sequence of smaller arrays. An example of the >> performance improvements is as follows. >> >> **readAllBytes** >> Before >> >> Benchmark(length) Mode Cnt Score >> Error Units >> ReadAllBytes.readAllBytesFileInputStream 100 thrpt 20 2412.031 >> ± 7.309 ops/s >> ReadAllBytes.readAllBytesFileInputStream 1000 thrpt 20212.181 >> ± 0.369 ops/s >> ReadAllBytes.readAllBytesFileInputStream1 thrpt 20 19.860 >> ± 0.048 ops/s >> ReadAllBytes.readAllBytesFileInputStream 10 thrpt 20 1.298 >> ± 0.183 ops/s >> >> After >> >> Benchmark(length) Mode Cnt Score >> Error Units >> ReadAllBytes.readAllBytesFileInputStream 100 thrpt 20 8218.473 >> ± 43.425 ops/s >> ReadAllBytes.readAllBytesFileInputStream 1000 thrpt 20302.781 >> ± 1.273 ops/s >> ReadAllBytes.readAllBytesFileInputStream1 thrpt 20 22.461 >> ± 0.084 ops/s >> ReadAllBytes.readAllBytesFileInputStream 10 thrpt 20 1.502 >> ± 0.003 ops/s >> >> >> **readNBytes** >> >> `N = file_size / 2` >> >> Before >> >> Benchmark(length) Mode Cnt Score >> Error Units >> ReadAllBytes.readHalfBytesFileInputStream 100 thrpt 20 5585.500 >> ± 153.353 ops/s >> ReadAllBytes.readHalfBytesFileInputStream1000 thrpt 20436.164 >> ± 1.104 ops/s >> ReadAllBytes.readHalfBytesFileInputStream 1 thrpt 20 40.167 >> ± 0.141 ops/s >> ReadAllBytes.readHalfBytesFileInputStream 10 thrpt 20 3.291 >> ± 0.211 ops/s >> >> >> After >> >> Benchmark(length) Mode Cnt Score >> Error Units >> ReadAllBytes.readHalfBytesFileInputStream 100 thrpt 20 15463.210 >> ± 66.045 ops/s >> ReadAllBytes.readHalfBytesFileInputStream1000 thrpt 20561.752 >> ± 0.951 ops/s >> ReadAllBytes.readHalfBytesFileInputStream 1 thrpt 20 45.043 >> ± 0.102 ops/s >> ReadAllBytes.readHalfBytesFileInputStream 10 thrpt 20 4.629 >> ± 0.035 ops/s > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 8264777: Merge two try-with-resources blocks in test Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3845
Re: [External] : Re: JEP draft: Scope Locals
Let's back up a lot of steps; you're deep in proposing a solution when you've not even explained what problem you think you're solving. So control question, which I hope will start to expose the assumptions: Why do you think its important to know that a snapshot of a variable has occurred? From the JEP: "In addition, a |Snapshot()| operation that captures the current set of inheritable scope locals is provided. This allows context information to be shared with asynchronous computations." As the provider of the scoped variable, I'd certainly like to know that my scoped variable is being passed into a new scope. Let's try again: why is that important? What decisions would you make differently if you had this information? What benefit would come from those decisions?
Re: RFR: 8264777: Overload optimized FileInputStream::readAllBytes [v9]
> Please consider this request to override the `java.io.InputStream` methods > `readAllBytes()` and `readNBytes(int)` in `FileInputStream` with more > performant implementations. The method overrides attempt to read all > requested bytes into a single array of the required size rather than > composing the result from a sequence of smaller arrays. An example of the > performance improvements is as follows. > > **readAllBytes** > Before > > Benchmark(length) Mode Cnt Score >Error Units > ReadAllBytes.readAllBytesFileInputStream 100 thrpt 20 2412.031 > ± 7.309 ops/s > ReadAllBytes.readAllBytesFileInputStream 1000 thrpt 20212.181 > ± 0.369 ops/s > ReadAllBytes.readAllBytesFileInputStream1 thrpt 20 19.860 > ± 0.048 ops/s > ReadAllBytes.readAllBytesFileInputStream 10 thrpt 20 1.298 > ± 0.183 ops/s > > After > > Benchmark(length) Mode Cnt Score >Error Units > ReadAllBytes.readAllBytesFileInputStream 100 thrpt 20 8218.473 > ± 43.425 ops/s > ReadAllBytes.readAllBytesFileInputStream 1000 thrpt 20302.781 > ± 1.273 ops/s > ReadAllBytes.readAllBytesFileInputStream1 thrpt 20 22.461 > ± 0.084 ops/s > ReadAllBytes.readAllBytesFileInputStream 10 thrpt 20 1.502 > ± 0.003 ops/s > > > **readNBytes** > > `N = file_size / 2` > > Before > > Benchmark(length) Mode Cnt Score >Error Units > ReadAllBytes.readHalfBytesFileInputStream 100 thrpt 20 5585.500 > ± 153.353 ops/s > ReadAllBytes.readHalfBytesFileInputStream1000 thrpt 20436.164 > ± 1.104 ops/s > ReadAllBytes.readHalfBytesFileInputStream 1 thrpt 20 40.167 > ± 0.141 ops/s > ReadAllBytes.readHalfBytesFileInputStream 10 thrpt 20 3.291 > ± 0.211 ops/s > > > After > > Benchmark(length) Mode Cnt Score >Error Units > ReadAllBytes.readHalfBytesFileInputStream 100 thrpt 20 15463.210 > ± 66.045 ops/s > ReadAllBytes.readHalfBytesFileInputStream1000 thrpt 20561.752 > ± 0.951 ops/s > ReadAllBytes.readHalfBytesFileInputStream 1 thrpt 20 45.043 > ± 0.102 ops/s > ReadAllBytes.readHalfBytesFileInputStream 10 thrpt 20 4.629 > ± 0.035 ops/s Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 8264777: Merge two try-with-resources blocks in test - Changes: - all: https://git.openjdk.java.net/jdk/pull/3845/files - new: https://git.openjdk.java.net/jdk/pull/3845/files/17e21c9a..76895a0c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3845=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3845=07-08 Stats: 7 lines in 1 file changed: 0 ins; 4 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/3845.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3845/head:pull/3845 PR: https://git.openjdk.java.net/jdk/pull/3845
Re: RFR: 8264777: Overload optimized FileInputStream::readAllBytes [v8]
On Mon, 17 May 2021 17:00:15 GMT, Brian Burkhalter wrote:
>> Please consider this request to override the `java.io.InputStream` methods
>> `readAllBytes()` and `readNBytes(int)` in `FileInputStream` with more
>> performant implementations. The method overrides attempt to read all
>> requested bytes into a single array of the required size rather than
>> composing the result from a sequence of smaller arrays. An example of the
>> performance improvements is as follows.
>>
>> **readAllBytes**
>> Before
>>
>> Benchmark(length) Mode Cnt Score
>> Error Units
>> ReadAllBytes.readAllBytesFileInputStream 100 thrpt 20 2412.031
>> ± 7.309 ops/s
>> ReadAllBytes.readAllBytesFileInputStream 1000 thrpt 20212.181
>> ± 0.369 ops/s
>> ReadAllBytes.readAllBytesFileInputStream1 thrpt 20 19.860
>> ± 0.048 ops/s
>> ReadAllBytes.readAllBytesFileInputStream 10 thrpt 20 1.298
>> ± 0.183 ops/s
>>
>> After
>>
>> Benchmark(length) Mode Cnt Score
>> Error Units
>> ReadAllBytes.readAllBytesFileInputStream 100 thrpt 20 8218.473
>> ± 43.425 ops/s
>> ReadAllBytes.readAllBytesFileInputStream 1000 thrpt 20302.781
>> ± 1.273 ops/s
>> ReadAllBytes.readAllBytesFileInputStream1 thrpt 20 22.461
>> ± 0.084 ops/s
>> ReadAllBytes.readAllBytesFileInputStream 10 thrpt 20 1.502
>> ± 0.003 ops/s
>>
>>
>> **readNBytes**
>>
>> `N = file_size / 2`
>>
>> Before
>>
>> Benchmark(length) Mode Cnt Score
>> Error Units
>> ReadAllBytes.readHalfBytesFileInputStream 100 thrpt 20 5585.500
>> ± 153.353 ops/s
>> ReadAllBytes.readHalfBytesFileInputStream1000 thrpt 20436.164
>> ± 1.104 ops/s
>> ReadAllBytes.readHalfBytesFileInputStream 1 thrpt 20 40.167
>> ± 0.141 ops/s
>> ReadAllBytes.readHalfBytesFileInputStream 10 thrpt 20 3.291
>> ± 0.211 ops/s
>>
>>
>> After
>>
>> Benchmark(length) Mode Cnt Score
>> Error Units
>> ReadAllBytes.readHalfBytesFileInputStream 100 thrpt 20 15463.210
>> ± 66.045 ops/s
>> ReadAllBytes.readHalfBytesFileInputStream1000 thrpt 20561.752
>> ± 0.951 ops/s
>> ReadAllBytes.readHalfBytesFileInputStream 1 thrpt 20 45.043
>> ± 0.102 ops/s
>> ReadAllBytes.readHalfBytesFileInputStream 10 thrpt 20 4.629
>> ± 0.035 ops/s
>
> Brian Burkhalter has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8264777: Clean up test
Marked as reviewed by alanb (Reviewer).
test/jdk/java/io/FileInputStream/ReadXBytes.java line 69:
> 67:
> 68: try (FileInputStream fis = new FileInputStream(empty)) {
> 69: byte[] b = fis.readAllBytes();
You could move this readAllBytes test up to the previous block if you want.
-
PR: https://git.openjdk.java.net/jdk/pull/3845
Re: RFR: 8266846: Add java.time.InstantSource [v2]
On Mon, 17 May 2021 17:13:58 GMT, Stephen Colebourne
wrote:
>> src/java.base/share/classes/java/time/Clock.java line 128:
>>
>>> 126: * Implementations should implement {@code Serializable} wherever
>>> possible and must
>>> 127: * document whether or not they do support serialization.
>>> 128: *
>>
>> The ImplSpec needs to say how it is implemented.
>> The 'implements InstantSource' can not mandate any particular
>> implementation. Its just an interface the real behavior comes from its
>> implementations. In this case Clock. Referring to the static methods of
>> InstantSource behavior may be sufficient because that behavior is concrete.
>
> There are plenty of examples of interfaces in `java.time` and elsewhere that
> apply restrictions to implementations. Nevertheless, for simplicity and
> expediency I have reinstated the `implSpec` on `Clock`
ok, thanks.
Assertions in interfaces are toothless. Tests can only be written for
implementations.
-
PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8263087: Add a MethodHandle combinator that switches over a set of MethodHandles [v2]
On Mon, 17 May 2021 17:19:16 GMT, Jorn Vernee wrote:
>> This patch adds a `tableSwitch` combinator that can be used to switch over a
>> set of method handles given an index, with a fallback in case the index is
>> out of bounds, much like the `tableswitch` bytecode. Here is a description
>> of how it works (copied from the javadoc):
>>
>> Creates a table switch method handle, which can be used to switch over
>> a set of target
>> method handles, based on a given target index, called selector.
>>
>> For a selector value of {@code n}, where {@code n} falls in the range
>> {@code [0, N)},
>> and where {@code N} is the number of target method handles, the table
>> switch method
>> handle will invoke the n-th target method handle from the list of
>> target method handles.
>>
>> For a selector value that does not fall in the range {@code [0, N)},
>> the table switch
>> method handle will invoke the given fallback method handle.
>>
>> All method handles passed to this method must have the same type, with
>> the additional
>> requirement that the leading parameter be of type {@code int}. The
>> leading parameter
>> represents the selector.
>>
>> Any trailing parameters present in the type will appear on the returned
>> table switch
>> method handle as well. Any arguments assigned to these parameters will
>> be forwarded,
>> together with the selector value, to the selected method handle when
>> invoking it.
>>
>> The combinator does not support specifying the starting index, so the switch
>> cases always run from 0 to however many target handles are specified. A
>> starting index can be added manually with another combination step that
>> filters the input index by adding or subtracting a constant from it, which
>> does not affect performance. One of the reasons for not supporting a
>> starting index is that it allows for more lambda form sharing, but also
>> simplifies the implementation somewhat. I guess an open question is if a
>> convenience overload should be added for that case?
>>
>> Lookup switch can also be simulated by filtering the input through an
>> injection function that translates it into a case index, which has also
>> proven to have the ability to have comparable performance to, or even better
>> performance than, a bytecode-native `lookupswitch` instruction. I plan to
>> add such an injection function to the runtime libraries in the future as
>> well. Maybe at that point it could be evaluated if it's worth it to add a
>> lookup switch combinator as well, but I don't see an immediate need to
>> include it in this patch.
>>
>> The current bytecode intrinsification generates a call for each switch case,
>> which guarantees full inlining of the target method handles. Alternatively
>> we could only have 1 callsite at the end of the switch, where each case just
>> loads the target method handle, but currently this does not allow for
>> inlining of the handles, since they are not constant.
>>
>> Maybe a future C2 optimization could look at the receiver input for
>> invokeBasic call sites, and if the input is a phi node, clone the call for
>> each constant input of the phi. I believe that would allow simplifying the
>> bytecode without giving up on inlining.
>>
>> Some numbers from the added benchmarks:
>>
>> Benchmark(numCases) (offset)
>> (sorted) Mode Cnt Score Error Units
>> MethodHandlesTableSwitchConstant.testSwitch 5 0
>> N/A avgt 30 4.186 � 0.054 ms/op
>> MethodHandlesTableSwitchConstant.testSwitch 5 150
>> N/A avgt 30 4.164 � 0.057 ms/op
>> MethodHandlesTableSwitchConstant.testSwitch 10 0
>> N/A avgt 30 4.124 � 0.023 ms/op
>> MethodHandlesTableSwitchConstant.testSwitch 10 150
>> N/A avgt 30 4.126 � 0.025 ms/op
>> MethodHandlesTableSwitchConstant.testSwitch 25 0
>> N/A avgt 30 4.137 � 0.042 ms/op
>> MethodHandlesTableSwitchConstant.testSwitch 25 150
>> N/A avgt 30 4.113 � 0.016 ms/op
>> MethodHandlesTableSwitchConstant.testSwitch 50 0
>> N/A avgt 30 4.118 � 0.028 ms/op
>> MethodHandlesTableSwitchConstant.testSwitch 50 150
>> N/A avgt 30 4.127 � 0.019 ms/op
>> MethodHandlesTableSwitchConstant.testSwitch 100 0
>> N/A avgt 30 4.116 � 0.013 ms/op
>> MethodHandlesTableSwitchConstant.testSwitch 100 150
>> N/A avgt 30 4.121 � 0.020 ms/op
>> MethodHandlesTableSwitchOpaqueSingle.testSwitch 5 0
>> N/A avgt 30 4.113 � 0.009 ms/op
>> MethodHandlesTableSwitchOpaqueSingle.testSwitch 5 150
>> N/A avgt 30 4.149 � 0.041 ms/op
>>
Re: RFR: 8263202: Update Hebrew/Indonesian/Yiddish ISO 639 language codes to current
On Mon, 17 May 2021 17:14:53 GMT, Erik Joelsson wrote: > I see no relevant build changes to comment on as the build label was only > added because the buildtool java source was touched, so this looks ok from a > build perspective. Removed `build` label. - PR: https://git.openjdk.java.net/jdk/pull/4069
Re: RFR: 8263087: Add a MethodHandle combinator that switches over a set of MethodHandles [v2]
On Fri, 14 May 2021 07:27:08 GMT, Claes Redestad wrote: >> Jorn Vernee has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains two additional >> commits since the last revision: >> >> - - Formatting >>- Reduce benchmark cases >>- Remove NamedFunction::intrinsicName >> - All changes prior to review > > Overall this looks great! > > I have a number of nits and a request to try and remodel so that > `NamedFunction` isn't responsible for holding the arbitrary intrinsic data > that you want to add here. Thanks for the review @cl4es I've addressed your review comments. I've reduced the number of cases in the benchmark to 5, 10, and 25, which is the most interesting area to look at, and also removed the offset cases for the non-constant input benchmarks (proving the offset is constant folded only needs to be done once I think). WRT `NamedFunction::intrinsicName`, I think you're right that we can also just delegate to `IntrinsicMethodHandle::intrinsicName`, and have it indicate the intrinsic. I've implemented that change. As a result, some `NamedFunction` constructor call sites no longer attach the intrinsic name to the `NamedFunction` itself, but re-wrap the `resolvedHandle` they use in an `IntrinsicMethodHandle` with the right intrinsic name. This leads to a nice code simplification from being able to remove all the `NamedFunction` constructor overloads. java/lang/invoke tests are still green, but I'll re-run Tier 1-3 as well to make sure that the difference in `resolvedHandle` being used for some `NamedFunctions` doesn't cause any other problems. - PR: https://git.openjdk.java.net/jdk/pull/3401
Re: RFR: 8266846: Add java.time.InstantSource
On Thu, 13 May 2021 20:52:33 GMT, Stephen Colebourne wrote: > 8266846: Add java.time.InstantSource FWIW, if there is a need to access `VM.getNanoTimeAdjustment(localOffset)` then that should be a separate issue. I don't personally think it would be approved given Valhalla is coming. - PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8266846: Add java.time.InstantSource [v2]
On Mon, 17 May 2021 14:30:20 GMT, Roger Riggs wrote:
>> Stephen Colebourne has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8266846: Add java.time.InstantSource
>
> src/java.base/share/classes/java/time/Clock.java line 128:
>
>> 126: * Implementations should implement {@code Serializable} wherever
>> possible and must
>> 127: * document whether or not they do support serialization.
>> 128: *
>
> The ImplSpec needs to say how it is implemented.
> The 'implements InstantSource' can not mandate any particular implementation.
> Its just an interface the real behavior comes from its implementations. In
> this case Clock. Referring to the static methods of InstantSource behavior
> may be sufficient because that behavior is concrete.
There are plenty of examples of interfaces in `java.time` and elsewhere that
apply restrictions to implementations. Nevertheless, for simplicity and
expediency I have reinstated the `implSpec` on `Clock`
-
PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8263087: Add a MethodHandle combinator that switches over a set of MethodHandles [v2]
> This patch adds a `tableSwitch` combinator that can be used to switch over a
> set of method handles given an index, with a fallback in case the index is
> out of bounds, much like the `tableswitch` bytecode. Here is a description of
> how it works (copied from the javadoc):
>
> Creates a table switch method handle, which can be used to switch over a
> set of target
> method handles, based on a given target index, called selector.
>
> For a selector value of {@code n}, where {@code n} falls in the range
> {@code [0, N)},
> and where {@code N} is the number of target method handles, the table
> switch method
> handle will invoke the n-th target method handle from the list of target
> method handles.
>
> For a selector value that does not fall in the range {@code [0, N)}, the
> table switch
> method handle will invoke the given fallback method handle.
>
> All method handles passed to this method must have the same type, with
> the additional
> requirement that the leading parameter be of type {@code int}. The
> leading parameter
> represents the selector.
>
> Any trailing parameters present in the type will appear on the returned
> table switch
> method handle as well. Any arguments assigned to these parameters will
> be forwarded,
> together with the selector value, to the selected method handle when
> invoking it.
>
> The combinator does not support specifying the starting index, so the switch
> cases always run from 0 to however many target handles are specified. A
> starting index can be added manually with another combination step that
> filters the input index by adding or subtracting a constant from it, which
> does not affect performance. One of the reasons for not supporting a starting
> index is that it allows for more lambda form sharing, but also simplifies the
> implementation somewhat. I guess an open question is if a convenience
> overload should be added for that case?
>
> Lookup switch can also be simulated by filtering the input through an
> injection function that translates it into a case index, which has also
> proven to have the ability to have comparable performance to, or even better
> performance than, a bytecode-native `lookupswitch` instruction. I plan to add
> such an injection function to the runtime libraries in the future as well.
> Maybe at that point it could be evaluated if it's worth it to add a lookup
> switch combinator as well, but I don't see an immediate need to include it in
> this patch.
>
> The current bytecode intrinsification generates a call for each switch case,
> which guarantees full inlining of the target method handles. Alternatively we
> could only have 1 callsite at the end of the switch, where each case just
> loads the target method handle, but currently this does not allow for
> inlining of the handles, since they are not constant.
>
> Maybe a future C2 optimization could look at the receiver input for
> invokeBasic call sites, and if the input is a phi node, clone the call for
> each constant input of the phi. I believe that would allow simplifying the
> bytecode without giving up on inlining.
>
> Some numbers from the added benchmarks:
>
> Benchmark(numCases) (offset)
> (sorted) Mode Cnt Score Error Units
> MethodHandlesTableSwitchConstant.testSwitch 5 0
> N/A avgt 30 4.186 � 0.054 ms/op
> MethodHandlesTableSwitchConstant.testSwitch 5 150
> N/A avgt 30 4.164 � 0.057 ms/op
> MethodHandlesTableSwitchConstant.testSwitch 10 0
> N/A avgt 30 4.124 � 0.023 ms/op
> MethodHandlesTableSwitchConstant.testSwitch 10 150
> N/A avgt 30 4.126 � 0.025 ms/op
> MethodHandlesTableSwitchConstant.testSwitch 25 0
> N/A avgt 30 4.137 � 0.042 ms/op
> MethodHandlesTableSwitchConstant.testSwitch 25 150
> N/A avgt 30 4.113 � 0.016 ms/op
> MethodHandlesTableSwitchConstant.testSwitch 50 0
> N/A avgt 30 4.118 � 0.028 ms/op
> MethodHandlesTableSwitchConstant.testSwitch 50 150
> N/A avgt 30 4.127 � 0.019 ms/op
> MethodHandlesTableSwitchConstant.testSwitch 100 0
> N/A avgt 30 4.116 � 0.013 ms/op
> MethodHandlesTableSwitchConstant.testSwitch 100 150
> N/A avgt 30 4.121 � 0.020 ms/op
> MethodHandlesTableSwitchOpaqueSingle.testSwitch 5 0
> N/A avgt 30 4.113 � 0.009 ms/op
> MethodHandlesTableSwitchOpaqueSingle.testSwitch 5 150
> N/A avgt 30 4.149 � 0.041 ms/op
> MethodHandlesTableSwitchOpaqueSingle.testSwitch 10 0
> N/A avgt 30 4.121 � 0.026 ms/op
> MethodHandlesTableSwitchOpaqueSingle.testSwitch 10
Re: RFR: 8266846: Add java.time.InstantSource [v2]
> 8266846: Add java.time.InstantSource Stephen Colebourne has updated the pull request incrementally with one additional commit since the last revision: 8266846: Add java.time.InstantSource - Changes: - all: https://git.openjdk.java.net/jdk/pull/4016/files - new: https://git.openjdk.java.net/jdk/pull/4016/files/befc7621..7fa9d0ec Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4016=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4016=00-01 Stats: 38 lines in 2 files changed: 31 ins; 0 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/4016.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4016/head:pull/4016 PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8263202: Update Hebrew/Indonesian/Yiddish ISO 639 language codes to current
On Mon, 17 May 2021 16:55:35 GMT, Naoto Sato wrote: > Please review the changes to the subject issue. java.util.Locale class has a > long-standing issue for those obsolete ISO 639 languages where its > normalization ends up in the obsolete codes. This change intends to flip the > normalization towards the current codes, providing a system property for > compatibility behavior. ResourceBundle class is also modified to load either > obsolete/current bundles. For more detail, take a look at the CSR. I see no relevant build changes to comment on as the build label was only added because the buildtool java source was touched, so this looks ok from a build perspective. - PR: https://git.openjdk.java.net/jdk/pull/4069
RFR: 8263202: Update Hebrew/Indonesian/Yiddish ISO 639 language codes to current
Please review the changes to the subject issue. java.util.Locale class has a long-standing issue for those obsolete ISO 639 languages where its normalization ends up in the obsolete codes. This change intends to flip the normalization towards the current codes, providing a system property for compatibility behavior. ResourceBundle class is also modified to load either obsolete/current bundles. For more detail, take a look at the CSR. - Commit messages: - Added more ResourceBundleProvider tests - ResourceBundleProvider and test cases modifications - Eliminated some duplicated code - Changed ResourceBundle javadoc - Locale class description - renamed old resource files - inital commit Changes: https://git.openjdk.java.net/jdk/pull/4069/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4069=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8263202 Stats: 382 lines in 35 files changed: 239 ins; 41 del; 102 mod Patch: https://git.openjdk.java.net/jdk/pull/4069.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4069/head:pull/4069 PR: https://git.openjdk.java.net/jdk/pull/4069
RFR: 8224158: assertion related to NPE at DynamicCallSiteDesc::withArgs should be reworded
This is a very small patch that is just rewording the spec for DynamicCallSiteDesc::withArgs. Basically adding that NPE can also be thrown if the content of the argument is `null` TIA for the review - Commit messages: - 8224158: assertion related to NPE at DynamicCallSiteDesc::withArgs should be reworded Changes: https://git.openjdk.java.net/jdk/pull/4067/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4067=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8224158 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4067.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4067/head:pull/4067 PR: https://git.openjdk.java.net/jdk/pull/4067
Re: RFR: 8264777: Overload optimized FileInputStream::readAllBytes [v8]
> Please consider this request to override the `java.io.InputStream` methods > `readAllBytes()` and `readNBytes(int)` in `FileInputStream` with more > performant implementations. The method overrides attempt to read all > requested bytes into a single array of the required size rather than > composing the result from a sequence of smaller arrays. An example of the > performance improvements is as follows. > > **readAllBytes** > Before > > Benchmark(length) Mode Cnt Score >Error Units > ReadAllBytes.readAllBytesFileInputStream 100 thrpt 20 2412.031 > ± 7.309 ops/s > ReadAllBytes.readAllBytesFileInputStream 1000 thrpt 20212.181 > ± 0.369 ops/s > ReadAllBytes.readAllBytesFileInputStream1 thrpt 20 19.860 > ± 0.048 ops/s > ReadAllBytes.readAllBytesFileInputStream 10 thrpt 20 1.298 > ± 0.183 ops/s > > After > > Benchmark(length) Mode Cnt Score >Error Units > ReadAllBytes.readAllBytesFileInputStream 100 thrpt 20 8218.473 > ± 43.425 ops/s > ReadAllBytes.readAllBytesFileInputStream 1000 thrpt 20302.781 > ± 1.273 ops/s > ReadAllBytes.readAllBytesFileInputStream1 thrpt 20 22.461 > ± 0.084 ops/s > ReadAllBytes.readAllBytesFileInputStream 10 thrpt 20 1.502 > ± 0.003 ops/s > > > **readNBytes** > > `N = file_size / 2` > > Before > > Benchmark(length) Mode Cnt Score >Error Units > ReadAllBytes.readHalfBytesFileInputStream 100 thrpt 20 5585.500 > ± 153.353 ops/s > ReadAllBytes.readHalfBytesFileInputStream1000 thrpt 20436.164 > ± 1.104 ops/s > ReadAllBytes.readHalfBytesFileInputStream 1 thrpt 20 40.167 > ± 0.141 ops/s > ReadAllBytes.readHalfBytesFileInputStream 10 thrpt 20 3.291 > ± 0.211 ops/s > > > After > > Benchmark(length) Mode Cnt Score >Error Units > ReadAllBytes.readHalfBytesFileInputStream 100 thrpt 20 15463.210 > ± 66.045 ops/s > ReadAllBytes.readHalfBytesFileInputStream1000 thrpt 20561.752 > ± 0.951 ops/s > ReadAllBytes.readHalfBytesFileInputStream 1 thrpt 20 45.043 > ± 0.102 ops/s > ReadAllBytes.readHalfBytesFileInputStream 10 thrpt 20 4.629 > ± 0.035 ops/s Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 8264777: Clean up test - Changes: - all: https://git.openjdk.java.net/jdk/pull/3845/files - new: https://git.openjdk.java.net/jdk/pull/3845/files/4fae0209..17e21c9a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3845=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3845=06-07 Stats: 49 lines in 1 file changed: 6 ins; 3 del; 40 mod Patch: https://git.openjdk.java.net/jdk/pull/3845.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3845/head:pull/3845 PR: https://git.openjdk.java.net/jdk/pull/3845
Re: RFR: 8266846: Add java.time.InstantSource
On Mon, 17 May 2021 14:13:06 GMT, Roger Riggs wrote:
>> 8266846: Add java.time.InstantSource
>
> src/java.base/share/classes/java/time/InstantSource.java line 165:
>
>> 163: */
>> 164: static InstantSource fixed(Instant fixedInstant) {
>> 165: return Clock.fixed(fixedInstant, ZoneOffset.UTC);
>
> Instant might also implement InstantSource, returning itself.
> This fixed method would be unnecessary because an Instant is itself a
> InstantSource.
While I can see the appeal, I don't think this would be a good choice. Doing so
would add three methods to `Instant` that are of no value. `millis()` would
duplicate `toEpochMilli()`, `withZone(zone)` would duplicate `atZone(zone)` and
`instant()` would return `this`. Moreover it doesn't respect the rationale of
the interface, which is explictly to return the current instant, not any random
instant.
Effectively, this proposal is a variation of Scala's implicit conversions which
were rejected during the development of JSR-310 as being more likely to cause
bugs than help developers,
-
PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8266846: Add java.time.InstantSource
On Mon, 17 May 2021 14:04:24 GMT, Roger Riggs wrote:
>> 8266846: Add java.time.InstantSource
>
> src/java.base/share/classes/java/time/InstantSource.java line 36:
>
>> 34: * Instances of this interface are used to find the current instant.
>> 35: * As such, an {@code InstantSource} can be used instead of {@link
>> System#currentTimeMillis()}.
>> 36: *
>
> The word 'current' is likely to misleading here. The specification of an
> interface does not have any context in which to describe what the instant
> represents or what it is relative to.
> Given the intended use cases, it is definitely not always related to
> System.currentTimeMillis()
> which is bound to the running system.
> i think the best you could say is that it returns an instant provided by the
> source as does the 'instance()' method.
This is the definition used by `Clock` since Java 8. It is also the purpose of
the interface. ie. this isn't an interface for providing instants in general,
but for providing the _current instant_. I can clarify wrt the meaning of
"current", but I don't see how that word can be avoided.
-
PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v21]
> This PR contains the API and implementation changes for JEP-412 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/412 Maurizio Cimadamore 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 30 additional commits since the last revision: - Merge branch 'master' into JEP-412 - Fix issue with bounded arena allocator - Rename is_entry_blob to is_optimized_entry_blob Rename as_entry_blob to as_optimized_entry_blob Fix misc typos and indentation issues - Take care of remaining references to "Panama" - * Replace is_panama_entry_frame() with is_optimized_entry_frame() * Replace EntryBlob with OptimizedEntryBlob - Address CSR comments - * Remove unused imports * Fix broken javadoc after removal of @throws clauses * Remove other `@CallerSensitive` annotations from `AbstractCLinker` - Merge branch 'master' into JEP-412 - Remove redundant checks for --enable-native-access - Fix issue in snippet in package-info - ... and 20 more: https://git.openjdk.java.net/jdk/compare/893e2ec1...9eb61a46 - Changes: - all: https://git.openjdk.java.net/jdk/pull/3699/files - new: https://git.openjdk.java.net/jdk/pull/3699/files/0c464221..9eb61a46 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3699=20 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3699=19-20 Stats: 23427 lines in 753 files changed: 8549 ins; 9558 del; 5320 mod Patch: https://git.openjdk.java.net/jdk/pull/3699.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3699/head:pull/3699 PR: https://git.openjdk.java.net/jdk/pull/3699
Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v20]
> This PR contains the API and implementation changes for JEP-412 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/412 Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Fix issue with bounded arena allocator - Changes: - all: https://git.openjdk.java.net/jdk/pull/3699/files - new: https://git.openjdk.java.net/jdk/pull/3699/files/352c287f..0c464221 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3699=19 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3699=18-19 Stats: 112 lines in 3 files changed: 61 ins; 37 del; 14 mod Patch: https://git.openjdk.java.net/jdk/pull/3699.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3699/head:pull/3699 PR: https://git.openjdk.java.net/jdk/pull/3699
Integrated: 8264561: javap get NegativeArraySizeException on bad instruction
On Mon, 17 May 2021 13:11:56 GMT, Adam Sotona wrote: > Actual javap implementation reacts on corrupted TABLESWITCH or LOOKUPSWITCH > bytecode instructions resulting to negative array allocation with > NegativeArraySizeException, which is reported to user with stack trace and as > serious internal error. > > The fix in c.s.t.classfile.Instruction is checking for negative array size of > corrupted TABLESWITCH or LOOKUPSWITCH bytecode and throwing > j.l.IllegalStateException instead of the NegativeArraySizeException. > > Another part of the fix in c.s.t.javap.CodeWriter is catching > j.l.IllegalStateException and reporting it as error in the analyzed bytecode, > instead of passing it up and causing serious internal javap error. This pull request has now been integrated. Changeset: cf97252f Author:Adam Sotona URL: https://git.openjdk.java.net/jdk/commit/cf97252f3fd4e7bdb57271b92dd2866101d4a94b Stats: 5 lines in 2 files changed: 4 ins; 0 del; 1 mod 8264561: javap get NegativeArraySizeException on bad instruction Reviewed-by: vromero - PR: https://git.openjdk.java.net/jdk/pull/4061
Re: RFR: 8266846: Add java.time.InstantSource
On Mon, 17 May 2021 07:50:22 GMT, Stephen Colebourne wrote: >> 8266846: Add java.time.InstantSource > > It would good to get confirmation in the review from @dholmes-ora or Mark > Kralj-Taylor that my changes to the precise system clock are correct. > Specifically, I moved the code from instance to static. I don't believe this > changes the thread-safety, but it does need double checking (there was no > discussion in the original thread concerning instance vs static). > > Original thread: > http://mail.openjdk.java.net/pipermail/core-libs-dev/2020-May/066670.html @jodastephen While an implementation can certainly implement the now() method there is nothing useful it can do in the JVM to get anything more than millisecond precision. It needs to perform the logic in currentInstant() which requires a call to VM.getNanoTimeAdjustment(localOffset) which it doesn't have access to. This is precisely why it needs to call Clock to pass in a structure that can be populated with the millis and nanos. - PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8264561: javap get NegativeArraySizeException on bad instruction
On Mon, 17 May 2021 13:11:56 GMT, Adam Sotona wrote: > Actual javap implementation reacts on corrupted TABLESWITCH or LOOKUPSWITCH > bytecode instructions resulting to negative array allocation with > NegativeArraySizeException, which is reported to user with stack trace and as > serious internal error. > > The fix in c.s.t.classfile.Instruction is checking for negative array size of > corrupted TABLESWITCH or LOOKUPSWITCH bytecode and throwing > j.l.IllegalStateException instead of the NegativeArraySizeException. > > Another part of the fix in c.s.t.javap.CodeWriter is catching > j.l.IllegalStateException and reporting it as error in the analyzed bytecode, > instead of passing it up and causing serious internal javap error. lgtm - Marked as reviewed by vromero (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4061
Re: RFR: 8266846: Add java.time.InstantSource
On Thu, 13 May 2021 20:52:33 GMT, Stephen Colebourne
wrote:
> 8266846: Add java.time.InstantSource
Changes requested by rriggs (Reviewer).
src/java.base/share/classes/java/time/Clock.java line 128:
> 126: * Implementations should implement {@code Serializable} wherever
> possible and must
> 127: * document whether or not they do support serialization.
> 128: *
The ImplSpec needs to say how it is implemented.
The 'implements InstantSource' can not mandate any particular implementation.
Its just an interface the real behavior comes from its implementations. In
this case Clock. Referring to the static methods of InstantSource behavior may
be sufficient because that behavior is concrete.
src/java.base/share/classes/java/time/InstantSource.java line 36:
> 34: * Instances of this interface are used to find the current instant.
> 35: * As such, an {@code InstantSource} can be used instead of {@link
> System#currentTimeMillis()}.
> 36: *
The word 'current' is likely to misleading here. The specification of an
interface does not have any context in which to describe what the instant
represents or what it is relative to.
Given the intended use cases, it is definitely not always related to
System.currentTimeMillis()
which is bound to the running system.
i think the best you could say is that it returns an instant provided by the
source as does the 'instance()' method.
src/java.base/share/classes/java/time/InstantSource.java line 64:
> 62: * @implSpec
> 63: * This interface must be implemented with care to ensure other classes
> operate correctly.
> 64: * All implementations must be thread-safe.
It would be useful to expand on the invariants that must be maintained, or
examples of incorrect implementations.
src/java.base/share/classes/java/time/InstantSource.java line 165:
> 163: */
> 164: static InstantSource fixed(Instant fixedInstant) {
> 165: return Clock.fixed(fixedInstant, ZoneOffset.UTC);
Instant might also implement InstantSource, returning itself.
This fixed method would be unnecessary because an Instant is itself a
InstantSource.
-
PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8153490: Cannot setBytes() if incoming buffer's length is bigger than number of elements we want to insert. [v3]
> Fix `SerialBlob.setBytes(long pos, byte[] bytes, int offset, int length)` in > the following cases: > > 1. `pos - 1 + bytes.length - offset > this.length() && pos - 1 + length <= > this.length()` >The original implementation throws `ArrayIndexOutOfBoundsException` but > this case should end successfully. >(test31) > > 2. `pos - 1 + length > this.length()` >The original implementation throws `ArrayIndexOutOfBoundsException` but > this case should end successfully. *1 >(test32) > > 3. `pos == this.length() + 1` >The original implementation throws `SerialException` but this case should > end successfully. *2 >(test33) > > 4. `length < 0` >The original implementation throws `ArrayIndexOutOfBoundsException` but > this case should throw `SerialException`. >(test34) > > Additionally, fix `SerialClob.setString(long pos, String str, int offset, int > length)` in the following cases: > > 1. `offset > str.length()` >The original implementaion throws `StringIndexOutOfBoundsException` but > this case should throw `SerialException`. >(test39) > > 2. `pos - 1 + str.length() - offset > this.length() && pos - 1 + length <= > this.length()` >The original implementation throws `ArrayIndexOutOfBoundsException` but > this case should end successfully. >(test32) > > 3. `pos - 1 + length > this.length()` >The original implementaion throws `SerialException` but this case should > end successfully. *3 >(test40) > > 4. `pos == this.length() + 1` >The original implementaion throws `SerialException` but this case should > end successfully. *4 >(test41) > > 5. `length < 0` >The original implementation throws `StringIndexOutOfBoundsException` but > this case should throw `SerialException`. >(test42) > > > The javadoc has also been modified according to the above. > > *1 The documentation of `Blob.setBytes()` says, "If the end of the Blob value > is reached while writing the array of bytes, then the length of the Blob > value will be increased to accommodate the extra bytes." > > *2 The documentation of `Blob.setBytes()` says, "If the value specified for > pos is greater than the length+1 of the BLOB value then the behavior is > undefined." >So, it should work correctly when pos == length+1 of the BLOB value. > > *3 The documentation of `Clob.setString()` says, "If the end of the Clob > value is eached while writing the given string, then the length of the Clob > value will be increased to accommodate the extra characters." > > *4 The documentation of `Clob.setString()` says, "If the value specified for > pos is greater than the length+1 of the CLOB value then the behavior is > undefined." >So, it should work correctly when pos == length+1 of the CLOB value. Mitsuru Kariya has updated the pull request incrementally with one additional commit since the last revision: Fix for length + offset > Integer.MAX_VALUE case - Changes: - all: https://git.openjdk.java.net/jdk/pull/4001/files - new: https://git.openjdk.java.net/jdk/pull/4001/files/6a0cc1ad..e4346738 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4001=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4001=01-02 Stats: 26 lines in 4 files changed: 24 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/4001.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4001/head:pull/4001 PR: https://git.openjdk.java.net/jdk/pull/4001
Re: [External] : Re: JEP draft: Scope Locals
How so? How do I know if a snapshot of my variable has occurred? Let's back up a lot of steps; you're deep in proposing a solution when you've not even explained what problem you think you're solving. So control question, which I hope will start to expose the assumptions: Why do you think its important to know that a snapshot of a variable has occurred?
Re: RFR: 8266054: VectorAPI rotate operation optimization [v6]
On Mon, 17 May 2021 12:06:33 GMT, Jatin Bhateja wrote: >> Current VectorAPI Java side implementation expresses rotateLeft and >> rotateRight operation using following operations:- >> >> vec1 = lanewise(VectorOperators.LSHL, n) >> vec2 = lanewise(VectorOperators.LSHR, n) >> res = lanewise(VectorOperations.OR, vec1 , vec2) >> >> This patch moves above handling from Java side to C2 compiler which >> facilitates dismantling the rotate operation if target ISA does not support >> a direct rotate instruction. >> >> AVX512 added vector rotate instructions vpro[rl][v][dq] which operate over >> long and integer type vectors. For other cases (i.e. sub-word type vectors >> or for targets which do not support direct rotate operations ) instruction >> sequence comprising of vector SHIFT (LEFT/RIGHT) and vector OR is emitted. >> >> Please find below the performance data for included JMH benchmark. >> Machine: Cascade Lake Server (Intel(R) Xeon(R) Platinum 8280 CPU @ 2.70GHz) >> >> >> Benchmark | (TESTSIZE) | Shift | Baseline AVX3 (ops/ms) | Withopt AVX3 >> (ops/ms) | Gain % | Baseline AVX2 (ops/ms) | Withopt AVX2 (ops/ms) | Gain % >> -- | -- | -- | -- | -- | -- | -- | -- | -- >> | | | | | | | | >> RotateBenchmark.testRotateLeftB | 128.00 | 7.00 | 17223.35 | 17094.69 | >> -0.75 | 17008.32 | 17488.06 | 2.82 >> RotateBenchmark.testRotateLeftB | 128.00 | 7.00 | 8944.98 | 8811.34 | -1.49 >> | 8878.17 | 9218.68 | 3.84 >> RotateBenchmark.testRotateLeftB | 128.00 | 15.00 | 17195.75 | 17137.32 | >> -0.34 | 16789.01 | 17780.34 | 5.90 >> RotateBenchmark.testRotateLeftB | 128.00 | 15.00 | 9052.67 | 8838.60 | -2.36 >> | 8814.62 | 9206.01 | 4.44 >> RotateBenchmark.testRotateLeftB | 128.00 | 31.00 | 17100.19 | 16950.64 | >> -0.87 | 16827.73 | 17720.37 | 5.30 >> RotateBenchmark.testRotateLeftB | 128.00 | 31.00 | 9079.95 | 8471.26 | -6.70 >> | .44 | 9167.68 | 3.14 >> RotateBenchmark.testRotateLeftB | 256.00 | 7.00 | 21231.33 | 21513.08 | 1.33 >> | 21824.51 | 21479.48 | -1.58 >> RotateBenchmark.testRotateLeftB | 256.00 | 7.00 | 11103.62 | 11180.16 | 0.69 >> | 11173.67 | 11529.22 | 3.18 >> RotateBenchmark.testRotateLeftB | 256.00 | 15.00 | 21119.14 | 21552.04 | >> 2.05 | 21693.05 | 21915.37 | 1.02 >> RotateBenchmark.testRotateLeftB | 256.00 | 15.00 | 11048.68 | 11094.20 | >> 0.41 | 11049.90 | 11439.07 | 3.52 >> RotateBenchmark.testRotateLeftB | 256.00 | 31.00 | 21506.31 | 21391.41 | >> -0.53 | 21263.18 | 21986.29 | 3.40 >> RotateBenchmark.testRotateLeftB | 256.00 | 31.00 | 11056.12 | 11232.78 | >> 1.60 | 10941.59 | 11397.09 | 4.16 >> RotateBenchmark.testRotateLeftB | 512.00 | 7.00 | 17976.56 | 18180.85 | 1.14 >> | 1212.26 | 2533.34 | 108.98 >> RotateBenchmark.testRotateLeftB | 512.00 | 15.00 | 17553.70 | 18219.07 | >> 3.79 | 1256.73 | 2537.41 | 101.91 >> RotateBenchmark.testRotateLeftB | 512.00 | 31.00 | 17618.03 | 17738.15 | >> 0.68 | 1214.69 | 2533.83 | 108.60 >> RotateBenchmark.testRotateLeftI | 128.00 | 7.00 | 7258.87 | 7468.88 | 2.89 | >> 7115.12 | 7117.26 | 0.03 >> RotateBenchmark.testRotateLeftI | 128.00 | 7.00 | 3586.65 | 3950.85 | 10.15 >> | 3532.17 | 3595.80 | 1.80 >> RotateBenchmark.testRotateLeftI | 128.00 | 7.00 | 1835.07 | 1999.68 | 8.97 | >> 1789.90 | 1819.93 | 1.68 >> RotateBenchmark.testRotateLeftI | 128.00 | 15.00 | 7273.36 | 7410.91 | 1.89 >> | 7198.60 | 6994.79 | -2.83 >> RotateBenchmark.testRotateLeftI | 128.00 | 15.00 | 3674.98 | 3926.27 | 6.84 >> | 3549.90 | 3755.09 | 5.78 >> RotateBenchmark.testRotateLeftI | 128.00 | 15.00 | 1840.94 | 1882.25 | 2.24 >> | 1801.56 | 1872.89 | 3.96 >> RotateBenchmark.testRotateLeftI | 128.00 | 31.00 | 7457.11 | 7361.48 | -1.28 >> | 6975.33 | 7385.94 | 5.89 >> RotateBenchmark.testRotateLeftI | 128.00 | 31.00 | 3570.74 | 3929.30 | 10.04 >> | 3635.37 | 3736.67 | 2.79 >> RotateBenchmark.testRotateLeftI | 128.00 | 31.00 | 1902.32 | 1960.46 | 3.06 >> | 1812.32 | 1813.88 | 0.09 >> RotateBenchmark.testRotateLeftI | 256.00 | 7.00 | 11174.24 | 12044.52 | 7.79 >> | 11509.87 | 11273.44 | -2.05 >> RotateBenchmark.testRotateLeftI | 256.00 | 7.00 | 5981.47 | 6073.70 | 1.54 | >> 5593.66 | 5661.93 | 1.22 >> RotateBenchmark.testRotateLeftI | 256.00 | 7.00 | 2932.49 | 3069.54 | 4.67 | >> 2950.86 | 2892.42 | -1.98 >> RotateBenchmark.testRotateLeftI | 256.00 | 15.00 | 11764.11 | 12098.63 | >> 2.84 | 11069.52 | 11476.93 | 3.68 >> RotateBenchmark.testRotateLeftI | 256.00 | 15.00 | 5855.20 | 6080.40 | 3.85 >> | 5919.11 | 5607.04 | -5.27 >> RotateBenchmark.testRotateLeftI | 256.00 | 15.00 | 2989.05 | 3048.56 | 1.99 >> | 2902.63 | 2821.83 | -2.78 >> RotateBenchmark.testRotateLeftI | 256.00 | 31.00 | 11652.84 | 11965.40 | >> 2.68 | 11525.62 | 11459.83 | -0.57 >> RotateBenchmark.testRotateLeftI | 256.00 | 31.00 | 5851.82 | 6164.94 | 5.35 >> | 5882.60 | 5842.30 | -0.69 >> RotateBenchmark.testRotateLeftI | 256.00 | 31.00 | 3015.99 | 3043.79 | 0.92 >> | 2963.71 | 2947.97 | -0.53 >> RotateBenchmark.testRotateLeftI | 512.00 | 7.00 |
RFR: 8264561: javap get NegativeArraySizeException on bad instruction
Actual javap implementation reacts on corrupted TABLESWITCH or LOOKUPSWITCH bytecode instructions resulting to negative array allocation with NegativeArraySizeException, which is reported to user with stack trace and as serious internal error. The fix in c.s.t.classfile.Instruction is checking for negative array size of corrupted TABLESWITCH or LOOKUPSWITCH bytecode and throwing j.l.IllegalStateException instead of the NegativeArraySizeException. Another part of the fix in c.s.t.javap.CodeWriter is catching j.l.IllegalStateException and reporting it as error in the analyzed bytecode, instead of passing it up and causing serious internal javap error. - Commit messages: - 8264561: javap get NegativeArraySizeException on bad instruction Changes: https://git.openjdk.java.net/jdk/pull/4061/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4061=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8264561 Stats: 5 lines in 2 files changed: 4 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4061.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4061/head:pull/4061 PR: https://git.openjdk.java.net/jdk/pull/4061
Re: RFR: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v4]
On Sat, 15 May 2021 02:06:29 GMT, Sandhya Viswanathan wrote: >> This PR contains Short Vector Math Library support related changes for >> [JEP-414 Vector API (Second Incubator)](https://openjdk.java.net/jeps/414), >> in preparation for when targeted. >> >> Intel Short Vector Math Library (SVML) based intrinsics in native x86 >> assembly provide optimized implementation for Vector API transcendental and >> trigonometric methods. >> These methods are built into a separate library instead of being part of >> libjvm.so or jvm.dll. >> >> The following changes are made: >>The source for these methods is placed in the jdk.incubator.vector module >> under src/jdk.incubator.vector/linux/native/libsvml and >> src/jdk.incubator.vector/windows/native/libsvml. >>The assembly source files are named as “*.S” and include files are named >> as “*.S.inc”. >>The corresponding build script is placed at >> make/modules/jdk.incubator.vector/Lib.gmk. >>Changes are made to build system to support dependency tracking for >> assembly files with includes. >>The built native libraries (libsvml.so/svml.dll) are placed in bin >> directory of JDK on Windows and lib directory of JDK on Linux. >>The C2 JIT uses the dll_load and dll_lookup to get the addresses of >> optimized methods from this library. >> >> Build system changes and module library build scripts are contributed by >> Magnus (magnus.ihse.bur...@oracle.com). >> >> Looking forward to your review and feedback. >> >> Performance: >> Micro benchmark Base Optimized Unit Gain(Optimized/Base) >> Double128Vector.ACOS 45.91 87.34 ops/ms 1.90 >> Double128Vector.ASIN 45.06 92.36 ops/ms 2.05 >> Double128Vector.ATAN 19.92 118.36 ops/ms 5.94 >> Double128Vector.ATAN2 15.24 88.17 ops/ms 5.79 >> Double128Vector.CBRT 45.77 208.36 ops/ms 4.55 >> Double128Vector.COS 49.94 245.89 ops/ms 4.92 >> Double128Vector.COSH 26.91 126.00 ops/ms 4.68 >> Double128Vector.EXP 71.64 379.65 ops/ms 5.30 >> Double128Vector.EXPM1 35.95 150.37 ops/ms 4.18 >> Double128Vector.HYPOT 50.67 174.10 ops/ms 3.44 >> Double128Vector.LOG 61.95 279.84 ops/ms 4.52 >> Double128Vector.LOG10 59.34 239.05 ops/ms 4.03 >> Double128Vector.LOG1P 18.56 200.32 ops/ms 10.79 >> Double128Vector.SIN 49.36 240.79 ops/ms 4.88 >> Double128Vector.SINH 26.59 103.75 ops/ms 3.90 >> Double128Vector.TAN 41.05 152.39 ops/ms 3.71 >> Double128Vector.TANH 45.29 169.53 ops/ms 3.74 >> Double256Vector.ACOS 54.21 106.39 ops/ms 1.96 >> Double256Vector.ASIN 53.60 107.99 ops/ms 2.01 >> Double256Vector.ATAN 21.53 189.11 ops/ms 8.78 >> Double256Vector.ATAN2 16.67 140.76 ops/ms 8.44 >> Double256Vector.CBRT 56.45 397.13 ops/ms 7.04 >> Double256Vector.COS 58.26 389.77 ops/ms 6.69 >> Double256Vector.COSH 29.44 151.11 ops/ms 5.13 >> Double256Vector.EXP 86.67 564.68 ops/ms 6.52 >> Double256Vector.EXPM1 41.96 201.28 ops/ms 4.80 >> Double256Vector.HYPOT 66.18 305.74 ops/ms 4.62 >> Double256Vector.LOG 71.52 394.90 ops/ms 5.52 >> Double256Vector.LOG10 65.43 362.32 ops/ms 5.54 >> Double256Vector.LOG1P 19.99 300.88 ops/ms 15.05 >> Double256Vector.SIN 57.06 380.98 ops/ms 6.68 >> Double256Vector.SINH 29.40 117.37 ops/ms 3.99 >> Double256Vector.TAN 44.90 279.90 ops/ms 6.23 >> Double256Vector.TANH 54.08 274.71 ops/ms 5.08 >> Double512Vector.ACOS 55.65 687.54 ops/ms 12.35 >> Double512Vector.ASIN 57.31 777.72 ops/ms 13.57 >> Double512Vector.ATAN 21.42 729.21 ops/ms 34.04 >> Double512Vector.ATAN2 16.37 414.33 ops/ms 25.32 >> Double512Vector.CBRT 56.78 834.38 ops/ms 14.69 >> Double512Vector.COS 59.88 837.04 ops/ms 13.98 >> Double512Vector.COSH 30.34 172.76 ops/ms 5.70 >> Double512Vector.EXP 99.66 1608.12 ops/ms 16.14 >> Double512Vector.EXPM1 43.39 318.61 ops/ms 7.34 >> Double512Vector.HYPOT 73.87 1502.72 ops/ms 20.34 >> Double512Vector.LOG 74.84 996.00 ops/ms 13.31 >> Double512Vector.LOG10 71.12 1046.52 ops/ms 14.72 >> Double512Vector.LOG1P 19.75 776.87 ops/ms 39.34 >> Double512Vector.POW 37.42 384.13 ops/ms 10.26 >> Double512Vector.SIN 59.74 728.45 ops/ms 12.19 >> Double512Vector.SINH 29.47 143.38 ops/ms 4.87 >> Double512Vector.TAN 46.20 587.21 ops/ms 12.71 >> Double512Vector.TANH 57.36 495.42 ops/ms 8.64 >> Double64Vector.ACOS 24.04 73.67 ops/ms 3.06 >> Double64Vector.ASIN 23.78 75.11 ops/ms 3.16 >> Double64Vector.ATAN 14.14 62.81 ops/ms 4.44 >> Double64Vector.ATAN2 10.38 44.43 ops/ms 4.28 >> Double64Vector.CBRT 16.47 107.50 ops/ms 6.53 >> Double64Vector.COS 23.42 152.01 ops/ms 6.49 >> Double64Vector.COSH 17.34 113.34 ops/ms 6.54 >> Double64Vector.EXP 27.08 203.53 ops/ms 7.52 >> Double64Vector.EXPM1 18.77 96.73 ops/ms 5.15 >> Double64Vector.HYPOT 18.54 103.62 ops/ms 5.59 >> Double64Vector.LOG 26.75 142.63 ops/ms 5.33 >> Double64Vector.LOG10 25.85 139.71 ops/ms 5.40 >> Double64Vector.LOG1P 13.26 97.94 ops/ms 7.38 >> Double64Vector.SIN 23.28 146.91 ops/ms 6.31 >> Double64Vector.SINH 17.62 88.59 ops/ms 5.03 >> Double64Vector.TAN 21.00 86.43 ops/ms 4.12 >> Double64Vector.TANH 23.75 111.35 ops/ms 4.69 >> Float128Vector.ACOS 57.52 110.65
New java.util.Strings class
Hi, members of the core-libs developers of OpenJDK. I think Java needs a Strings class similar to the java.util.Objects class of Java 16 to be used in method arguments, return types and Stream filters. I have coded it myself here based on the Objects implementation of Java 16 (please have a look): https://github.com/Aliuken/Java-Strings/blob/main/Strings.java In fact, I think it would be useful also adding annotations for method arguments and return types such as: - @NonNull - @NonNullElse(defaultValue) - @NonNullElseGet(class::supplierMethod) - @NonNullNorEmpty - @NonNullNorEmptyElse(defaultValue) - @NonNullNorEmptyElseGet(class::supplierMethod) With that kind of annotations, you could assume thinks like: - an argument or return type cannot have value null, but an Exception - an argument or return type cannot have value null, but a default value What do you think? Waiting for your response. PS: I am sending this email repeated. I have sended it yesterday with my other email account (alber8...@gmail.com), but I wasn't a member of this mailing list. Now I have become a member with this other email account. Regards, Alberto Otero Rodríguez.
Re: RFR: 8266054: VectorAPI rotate operation optimization [v6]
> Current VectorAPI Java side implementation expresses rotateLeft and > rotateRight operation using following operations:- > > vec1 = lanewise(VectorOperators.LSHL, n) > vec2 = lanewise(VectorOperators.LSHR, n) > res = lanewise(VectorOperations.OR, vec1 , vec2) > > This patch moves above handling from Java side to C2 compiler which > facilitates dismantling the rotate operation if target ISA does not support a > direct rotate instruction. > > AVX512 added vector rotate instructions vpro[rl][v][dq] which operate over > long and integer type vectors. For other cases (i.e. sub-word type vectors or > for targets which do not support direct rotate operations ) instruction > sequence comprising of vector SHIFT (LEFT/RIGHT) and vector OR is emitted. > > Please find below the performance data for included JMH benchmark. > Machine: Cascade Lake Server (Intel(R) Xeon(R) Platinum 8280 CPU @ 2.70GHz) > > > Benchmark | (TESTSIZE) | Shift | Baseline AVX3 (ops/ms) | Withopt AVX3 > (ops/ms) | Gain % | Baseline AVX2 (ops/ms) | Withopt AVX2 (ops/ms) | Gain % > -- | -- | -- | -- | -- | -- | -- | -- | -- > | | | | | | | | > RotateBenchmark.testRotateLeftB | 128.00 | 7.00 | 17223.35 | 17094.69 | -0.75 > | 17008.32 | 17488.06 | 2.82 > RotateBenchmark.testRotateLeftB | 128.00 | 7.00 | 8944.98 | 8811.34 | -1.49 | > 8878.17 | 9218.68 | 3.84 > RotateBenchmark.testRotateLeftB | 128.00 | 15.00 | 17195.75 | 17137.32 | > -0.34 | 16789.01 | 17780.34 | 5.90 > RotateBenchmark.testRotateLeftB | 128.00 | 15.00 | 9052.67 | 8838.60 | -2.36 > | 8814.62 | 9206.01 | 4.44 > RotateBenchmark.testRotateLeftB | 128.00 | 31.00 | 17100.19 | 16950.64 | > -0.87 | 16827.73 | 17720.37 | 5.30 > RotateBenchmark.testRotateLeftB | 128.00 | 31.00 | 9079.95 | 8471.26 | -6.70 > | .44 | 9167.68 | 3.14 > RotateBenchmark.testRotateLeftB | 256.00 | 7.00 | 21231.33 | 21513.08 | 1.33 > | 21824.51 | 21479.48 | -1.58 > RotateBenchmark.testRotateLeftB | 256.00 | 7.00 | 11103.62 | 11180.16 | 0.69 > | 11173.67 | 11529.22 | 3.18 > RotateBenchmark.testRotateLeftB | 256.00 | 15.00 | 21119.14 | 21552.04 | 2.05 > | 21693.05 | 21915.37 | 1.02 > RotateBenchmark.testRotateLeftB | 256.00 | 15.00 | 11048.68 | 11094.20 | 0.41 > | 11049.90 | 11439.07 | 3.52 > RotateBenchmark.testRotateLeftB | 256.00 | 31.00 | 21506.31 | 21391.41 | > -0.53 | 21263.18 | 21986.29 | 3.40 > RotateBenchmark.testRotateLeftB | 256.00 | 31.00 | 11056.12 | 11232.78 | 1.60 > | 10941.59 | 11397.09 | 4.16 > RotateBenchmark.testRotateLeftB | 512.00 | 7.00 | 17976.56 | 18180.85 | 1.14 > | 1212.26 | 2533.34 | 108.98 > RotateBenchmark.testRotateLeftB | 512.00 | 15.00 | 17553.70 | 18219.07 | 3.79 > | 1256.73 | 2537.41 | 101.91 > RotateBenchmark.testRotateLeftB | 512.00 | 31.00 | 17618.03 | 17738.15 | 0.68 > | 1214.69 | 2533.83 | 108.60 > RotateBenchmark.testRotateLeftI | 128.00 | 7.00 | 7258.87 | 7468.88 | 2.89 | > 7115.12 | 7117.26 | 0.03 > RotateBenchmark.testRotateLeftI | 128.00 | 7.00 | 3586.65 | 3950.85 | 10.15 | > 3532.17 | 3595.80 | 1.80 > RotateBenchmark.testRotateLeftI | 128.00 | 7.00 | 1835.07 | 1999.68 | 8.97 | > 1789.90 | 1819.93 | 1.68 > RotateBenchmark.testRotateLeftI | 128.00 | 15.00 | 7273.36 | 7410.91 | 1.89 | > 7198.60 | 6994.79 | -2.83 > RotateBenchmark.testRotateLeftI | 128.00 | 15.00 | 3674.98 | 3926.27 | 6.84 | > 3549.90 | 3755.09 | 5.78 > RotateBenchmark.testRotateLeftI | 128.00 | 15.00 | 1840.94 | 1882.25 | 2.24 | > 1801.56 | 1872.89 | 3.96 > RotateBenchmark.testRotateLeftI | 128.00 | 31.00 | 7457.11 | 7361.48 | -1.28 > | 6975.33 | 7385.94 | 5.89 > RotateBenchmark.testRotateLeftI | 128.00 | 31.00 | 3570.74 | 3929.30 | 10.04 > | 3635.37 | 3736.67 | 2.79 > RotateBenchmark.testRotateLeftI | 128.00 | 31.00 | 1902.32 | 1960.46 | 3.06 | > 1812.32 | 1813.88 | 0.09 > RotateBenchmark.testRotateLeftI | 256.00 | 7.00 | 11174.24 | 12044.52 | 7.79 > | 11509.87 | 11273.44 | -2.05 > RotateBenchmark.testRotateLeftI | 256.00 | 7.00 | 5981.47 | 6073.70 | 1.54 | > 5593.66 | 5661.93 | 1.22 > RotateBenchmark.testRotateLeftI | 256.00 | 7.00 | 2932.49 | 3069.54 | 4.67 | > 2950.86 | 2892.42 | -1.98 > RotateBenchmark.testRotateLeftI | 256.00 | 15.00 | 11764.11 | 12098.63 | 2.84 > | 11069.52 | 11476.93 | 3.68 > RotateBenchmark.testRotateLeftI | 256.00 | 15.00 | 5855.20 | 6080.40 | 3.85 | > 5919.11 | 5607.04 | -5.27 > RotateBenchmark.testRotateLeftI | 256.00 | 15.00 | 2989.05 | 3048.56 | 1.99 | > 2902.63 | 2821.83 | -2.78 > RotateBenchmark.testRotateLeftI | 256.00 | 31.00 | 11652.84 | 11965.40 | 2.68 > | 11525.62 | 11459.83 | -0.57 > RotateBenchmark.testRotateLeftI | 256.00 | 31.00 | 5851.82 | 6164.94 | 5.35 | > 5882.60 | 5842.30 | -0.69 > RotateBenchmark.testRotateLeftI | 256.00 | 31.00 | 3015.99 | 3043.79 | 0.92 | > 2963.71 | 2947.97 | -0.53 > RotateBenchmark.testRotateLeftI | 512.00 | 7.00 | 16029.15 | 16189.79 | 1.00 > | 860.43 | 2339.32 | 171.88 > RotateBenchmark.testRotateLeftI | 512.00 | 7.00 | 8078.25 | 8081.84 | 0.04 | > 427.39
Re: RFR: 8266846: Add java.time.InstantSource
On Thu, 13 May 2021 20:52:33 GMT, Stephen Colebourne wrote: > 8266846: Add java.time.InstantSource It would good to get confirmation in the review from @dholmes-ora or Mark Kralj-Taylor that my changes to the precise system clock are correct. Specifically, I moved the code from instance to static. I don't believe this changes the thread-safety, but it does need double checking (there was no discussion in the original thread concerning instance vs static). Original thread: http://mail.openjdk.java.net/pipermail/core-libs-dev/2020-May/066670.html - PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8266846: Add java.time.InstantSource
On Mon, 17 May 2021 07:31:45 GMT, Stephen Colebourne
wrote:
>> src/java.base/share/classes/java/time/InstantSource.java line 93:
>>
>>> 91: * @since 17
>>> 92: */
>>> 93: public interface InstantSource {
>>
>> Should not we add `@FunctionalInterface`? I can easily imagine this
>> interface being used in tests where we can define the `InstantSource` with
>> lambdas.
>
> `@FunctionalInterface` isn't required for use by lambdas. I wasn't initially
> convinced using it here was the right choice.
Right, it's about signaling that it's safe to be used with a
lambda/method-reference, so it's not required but a nice to have, very like
@Override.
-
PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8266846: Add java.time.InstantSource
On Sat, 15 May 2021 21:32:54 GMT, Ralph Goers wrote: >> 8266846: Add java.time.InstantSource > > Can you possibly find a way that this can be used in a garbage free manner? > Providing a MutableInstant interface that allows the Instant object to be > provided and have its values set by a currentInstant(MutableInstant instant) > method would solve the problem nicely for Log4j - or any other app that is > trying to avoid allocating new objects. I believe this also aligns with what > [Michael Hixson](https://github.com/michaelhixson) is asking for. > > An alternative would be for java.time to maintain a pool of Instants that > apps like Log4j can somehow control, but that would seem far more complicated. @rgoers Michael's request has been handled, which was a simple change to the wording. What you need cannot be accomplished because `Instant` has a restricted design due to the Valhalla project. A `MutableInstant` class can have its own `now()` method that does whatever is necessary to initialize it, so there is no need to refer to it here. - PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8266846: Add java.time.InstantSource
On Sat, 15 May 2021 20:53:28 GMT, Istvan Neuwirth
wrote:
>> 8266846: Add java.time.InstantSource
>
> src/java.base/share/classes/java/time/InstantSource.java line 93:
>
>> 91: * @since 17
>> 92: */
>> 93: public interface InstantSource {
>
> Should not we add `@FunctionalInterface`? I can easily imagine this interface
> being used in tests where we can define the `InstantSource` with lambdas.
`@FunctionalInterface` isn't required for use by lambdas. I wasn't initially
convinced using it here was the right choice.
-
PR: https://git.openjdk.java.net/jdk/pull/4016
