Re: RFR: 8265248: Implementation Specific Properties: change prefix, plus add existing properties [v6]

[Joe Wang]

> Update module summary, add a few existing properties and features into the 
> tables.

Joe Wang 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 six additional commits since the 
last revision:

 - Merge branch 'master' into JDK-8265248 before a major commit that will
   involve many (57) classes.
 - Thanks Roger. Changed to fully qualified names. Also made them align left 
instead of center
 - Add legacy property names table
 - replace isAssignableFrom with instanceof
 - Update the CSR. See Update 03 in the CSR
 - 8265248: Implementation Specific Properties: change prefix, plus add 
existing properties

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3644/files
  - new: https://git.openjdk.java.net/jdk/pull/3644/files/70f634de..d4356925

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3644=05
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3644=04-05

  Stats: 566351 lines in 5111 files changed: 50537 ins; 501716 del; 14098 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3644.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3644/head:pull/3644

PR: https://git.openjdk.java.net/jdk/pull/3644

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

[Phil Race]

On Thu, 20 May 2021 04:05:23 GMT, Phil Race  wrote:

>> By converting JDK-8267432 to a bug, there is an extra benefit that we can 
>> fix it after RDP. So I'll convert it now.
>
> That is unfortunate, but nonetheless I think required to be done.
> We have acknowledeged this can't reasonably be called an RFE, so the JEP is 
> introducing bugs/technical debt/call it what you will. This should generally 
> be part of a sandbox for the JEP and fixed before integration of the JEP.
> From my point of view it is a blocker.

The JEP isn't PTT for 17 so there's plenty of time isn't there ?

-

PR: https://git.openjdk.java.net/jdk/pull/4073

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

[Phil Race]

On Thu, 20 May 2021 02:09:57 GMT, Weijun Wang  wrote:

>> I can make it a bug.
>> 
>> I don't want to do it here is because it involves indefinite amount of 
>> manual work and we will see everyone having their preferences. The more time 
>> we spend on this PR the more likely there will be merge conflict. There are 
>> just too many files here.
>> 
>> And no matter if we include that change in this PR or after it, it will be 
>> after the automatic conversion. In fact, the data about which cases are more 
>> worth fixing come from the automatic conversion itself. Also, as the manual 
>> work will be done part by part, if the automatic conversion is after it, 
>> there will be rounds and rounds of history rewriting and force push. This is 
>> quite unfriendly to the reviewers.
>> 
>> Altogether, there are 117 class-level annotations. Unfortunately, 
>> `java.awt.Component` is the one with the biggest class -- estimated number 
>> of bytes that the annotation covers is over 380K. In the client area, the 
>> 2nd place is `java.awt.Container`, and then we have 
>> `sun.font.SunFontManager`, `java.awt.Window`, and `java.awt.Toolkit` which 
>> are over 100KB, and other 25 classes over 10KB, and other 11 classes smaller 
>> than 10KB.
>
> By converting JDK-8267432 to a bug, there is an extra benefit that we can fix 
> it after RDP. So I'll convert it now.

That is unfortunate, but nonetheless I think required to be done.
We have acknowledeged this can't reasonably be called an RFE, so the JEP is 
introducing bugs/technical debt/call it what you will. This should generally be 
part of a sandbox for the JEP and fixed before integration of the JEP.
>From my point of view it is a blocker.

-

PR: https://git.openjdk.java.net/jdk/pull/4073

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

[Weijun Wang]

On Thu, 20 May 2021 02:06:46 GMT, Weijun Wang  wrote:

>> Well .. as an enhancement (P3 or otherwise) I can see it being dropped and 
>> definitely if it misses the fork,
>> and I don't get why you can't do it here. And if it isn't done the JEP isn't 
>> really ready.
>> I already pasted the patch for Component.java and it took about 60 seconds 
>> to do that.
>> Then yes, you would have to deal with the fact that now you need to reapply 
>> your automated tool to the file but this is just work you'd have to have 
>> done anyway if it was already refactored.
>> 
>> I only *noticed* Component and Container. And stopped there to raise the 
>> question. How many more cases are there ?
>
> I can make it a bug.
> 
> I don't want to do it here is because it involves indefinite amount of manual 
> work and we will see everyone having their preferences. The more time we 
> spend on this PR the more likely there will be merge conflict. There are just 
> too many files here.
> 
> And no matter if we include that change in this PR or after it, it will be 
> after the automatic conversion. In fact, the data about which cases are more 
> worth fixing come from the automatic conversion itself. Also, as the manual 
> work will be done part by part, if the automatic conversion is after it, 
> there will be rounds and rounds of history rewriting and force push. This is 
> quite unfriendly to the reviewers.
> 
> Altogether, there are 117 class-level annotations. Unfortunately, 
> `java.awt.Component` is the one with the biggest class -- estimated number of 
> bytes that the annotation covers is over 380K. In the client area, the 2nd 
> place is `java.awt.Container`, and then we have `sun.font.SunFontManager`, 
> `java.awt.Window`, and `java.awt.Toolkit` which are over 100KB, and other 25 
> classes over 10KB, and other 11 classes smaller than 10KB.

By converting JDK-8267432 to a bug, there is an extra benefit that we can fix 
it after RDP. So I'll convert it now.

-

PR: https://git.openjdk.java.net/jdk/pull/4073

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

[Weijun Wang]

On Wed, 19 May 2021 23:50:04 GMT, Phil Race  wrote:

>> I just made it P3 (P4 was the default value), and I will target it to 17 
>> once JEP 411 is targeted 17. But I think it's probably not a good idea to 
>> include it inside *this* PR. There are some middle ground where it's 
>> debatable if a change is worth doing (Ex: which is uglier between an 
>> a-liitle-faraway-annotation and a temporary variable?) so it's not obvious 
>> what the scope of the refactoring can be. Thus it will be divided into 
>> smaller sub-tasks. It's not totally impossible that part of the work will be 
>> delayed to next release.
>
> Well .. as an enhancement (P3 or otherwise) I can see it being dropped and 
> definitely if it misses the fork,
> and I don't get why you can't do it here. And if it isn't done the JEP isn't 
> really ready.
> I already pasted the patch for Component.java and it took about 60 seconds to 
> do that.
> Then yes, you would have to deal with the fact that now you need to reapply 
> your automated tool to the file but this is just work you'd have to have done 
> anyway if it was already refactored.
> 
> I only *noticed* Component and Container. And stopped there to raise the 
> question. How many more cases are there ?

I can make it a bug.

I don't want to do it here is because it involves indefinite amount of manual 
work and we will see everyone having their preferences. The more time we spend 
on this PR the more likely there will be merge conflict. There are just too 
many files here.

And no matter if we include that change in this PR or after it, it will be 
after the automatic conversion. In fact, the data about which cases are more 
worth fixing come from the automatic conversion itself. Also, as the manual 
work will be done part by part, if the automatic conversion is after it, there 
will be rounds and rounds of history rewriting and force push. This is quite 
unfriendly to the reviewers.

Altogether, there are 117 class-level annotations. Unfortunately, 
`java.awt.Component` is the one with the biggest class -- estimated number of 
bytes that the annotation covers is over 380K. In the client area, the 2nd 
place is `java.awt.Container`, and then we have `sun.font.SunFontManager`, 
`java.awt.Window`, and `java.awt.Toolkit` which are over 100KB, and other 25 
classes over 10KB, and other 11 classes smaller than 10KB.

-

PR: https://git.openjdk.java.net/jdk/pull/4073

Re: RFR: 8267190: Optimize Vector API test operations [v2]

[Sandhya Viswanathan]

> Vector API test operations (IS_DEFAULT, IS_FINITE, IS_INFINITE, IS_NAN and 
> IS_NEGATIVE) are computed in three steps:
> 1) reinterpreting the floating point vectors as integral vectors (int/long)
> 2) perform the test in integer domain to get a int/long mask
> 3) reinterpret the int/long mask as float/double mask
> Step 3) currently is very slow. It can be optimized by modifying the Java 
> code to utilize the existing reinterpret intrinsic.
> 
> For the VectorTestPerf attached to the JBS for JDK-8267190, the performance 
> improves as follows:
> 
> Base:
> Benchmark (size) Mode Cnt Score Error Units
> VectorTestPerf.IS_DEFAULT 1024 thrpt 5 223.156 ± 90.452 ops/ms
> VectorTestPerf.IS_FINITE 1024 thrpt 5 223.841 ± 91.685 ops/ms
> VectorTestPerf.IS_INFINITE 1024 thrpt 5 224.561 ± 83.890 ops/ms
> VectorTestPerf.IS_NAN 1024 thrpt 5 223.777 ± 70.629 ops/ms
> VectorTestPerf.IS_NEGATIVE 1024 thrpt 5 218.392 ± 79.806 ops/ms
> 
> With patch:
> Benchmark (size) Mode Cnt Score Error Units
> VectorTestPerf.IS_DEFAULT 1024 thrpt 5 8812.357 ± 40.477 ops/ms
> VectorTestPerf.IS_FINITE 1024 thrpt 5 7425.739 ± 296.622 ops/ms
> VectorTestPerf.IS_INFINITE 1024 thrpt 5 8932.730 ± 269.988 ops/ms
> VectorTestPerf.IS_NAN 1024 thrpt 5 8574.872 ± 498.649 ops/ms
> VectorTestPerf.IS_NEGATIVE 1024 thrpt 5 8838.400 ± 11.849 ops/ms
> 
> Best Regards,
> Sandhya

Sandhya Viswanathan has updated the pull request incrementally with one 
additional commit since the last revision:

  Implement Paul's review comments

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4039/files
  - new: https://git.openjdk.java.net/jdk/pull/4039/files/bb0d4000..b506fc45

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4039=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4039=00-01

  Stats: 806 lines in 31 files changed: 0 ins; 310 del; 496 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4039.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4039/head:pull/4039

PR: https://git.openjdk.java.net/jdk/pull/4039

Re: RFR: 8267190: Optimize Vector API test operations

[Sandhya Viswanathan]

On Wed, 19 May 2021 16:51:33 GMT, Paul Sandoz  wrote:

>> Vector API test operations (IS_DEFAULT, IS_FINITE, IS_INFINITE, IS_NAN and 
>> IS_NEGATIVE) are computed in three steps:
>> 1) reinterpreting the floating point vectors as integral vectors (int/long)
>> 2) perform the test in integer domain to get a int/long mask
>> 3) reinterpret the int/long mask as float/double mask
>> Step 3) currently is very slow. It can be optimized by modifying the Java 
>> code to utilize the existing reinterpret intrinsic.
>> 
>> For the VectorTestPerf attached to the JBS for JDK-8267190, the performance 
>> improves as follows:
>> 
>> Base:
>> Benchmark (size) Mode Cnt Score Error Units
>> VectorTestPerf.IS_DEFAULT 1024 thrpt 5 223.156 ± 90.452 ops/ms
>> VectorTestPerf.IS_FINITE 1024 thrpt 5 223.841 ± 91.685 ops/ms
>> VectorTestPerf.IS_INFINITE 1024 thrpt 5 224.561 ± 83.890 ops/ms
>> VectorTestPerf.IS_NAN 1024 thrpt 5 223.777 ± 70.629 ops/ms
>> VectorTestPerf.IS_NEGATIVE 1024 thrpt 5 218.392 ± 79.806 ops/ms
>> 
>> With patch:
>> Benchmark (size) Mode Cnt Score Error Units
>> VectorTestPerf.IS_DEFAULT 1024 thrpt 5 8812.357 ± 40.477 ops/ms
>> VectorTestPerf.IS_FINITE 1024 thrpt 5 7425.739 ± 296.622 ops/ms
>> VectorTestPerf.IS_INFINITE 1024 thrpt 5 8932.730 ± 269.988 ops/ms
>> VectorTestPerf.IS_NAN 1024 thrpt 5 8574.872 ± 498.649 ops/ms
>> VectorTestPerf.IS_NEGATIVE 1024 thrpt 5 8838.400 ± 11.849 ops/ms
>> 
>> Best Regards,
>> Sandhya
>
> Tier 1 to 3 tests pass on supported platforms

@PaulSandoz @vnkozlov Thanks a lot for the review. 
Paul, I have implemented your review comments. I also changed the switch to 
switch expression. Please take a look.

-

PR: https://git.openjdk.java.net/jdk/pull/4039

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

[Phil Race]

On Wed, 19 May 2021 22:14:20 GMT, Weijun Wang  wrote:

>> I don't think it is a separate P4 enhancement (?) that someone will (maybe) 
>> do next release.
>> I think it should all be taken care of as part of this proposed change.
>
> I just made it P3 (P4 was the default value), and I will target it to 17 once 
> JEP 411 is targeted 17. But I think it's probably not a good idea to include 
> it inside *this* PR. There are some middle ground where it's debatable if a 
> change is worth doing (Ex: which is uglier between an 
> a-liitle-faraway-annotation and a temporary variable?) so it's not obvious 
> what the scope of the refactoring can be. Thus it will be divided into 
> smaller sub-tasks. It's not totally impossible that part of the work will be 
> delayed to next release.

Well .. as an enhancement (P3 or otherwise) I can see it being dropped and 
definitely if it misses the fork,
and I don't get why you can't do it here. And if it isn't done the JEP isn't 
really ready.
I already pasted the patch for Component.java and it took about 60 seconds to 
do that.
Then yes, you would have to deal with the fact that now you need to reapply 
your automated tool to the file but this is just work you'd have to have done 
anyway if it was already refactored.

I only *noticed* Component and Container. And stopped there to raise the 
question. How many more cases are there ?

-

PR: https://git.openjdk.java.net/jdk/pull/4073

Re: RFR: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v13]

[Sandhya Viswanathan]

> 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 ops/ms 1.92
> Float128Vector.ASIN 57.15 117.95 ops/ms 2.06
> Float128Vector.ATAN 22.52 318.74 ops/ms 14.15
> Float128Vector.ATAN2 17.06 246.07 ops/ms 14.42
> 

Re: RFR: 8266846: Add java.time.InstantSource [v4]

[Naoto Sato]

On Wed, 19 May 2021 21:58:08 GMT, Stephen Colebourne  
wrote:

>> 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

I believe that the default execution mode is `agentvm` so it will leave 
unnecessary side effects. To make it run in `othervm` mode, possibly you will 
need to tweak TEST.properties (not tried though).

-

PR: https://git.openjdk.java.net/jdk/pull/4016

Re: RFR: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v12]

[Sandhya Viswanathan]

> 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 ops/ms 1.92
> Float128Vector.ASIN 57.15 117.95 ops/ms 2.06
> Float128Vector.ATAN 22.52 318.74 ops/ms 14.15
> Float128Vector.ATAN2 17.06 246.07 ops/ms 14.42
> 

Re: RFR: 8266310: deadlock while loading the JNI code [v2]

[David Holmes]

On 20/05/2021 2:29 am, Aleksei Voitylov wrote:

On Wed, 19 May 2021 16:21:41 GMT, Aleksei Voitylov  
wrote:


Please review this PR which fixes the deadlock in ClassLoader between the two 
lock objects - a lock object associated with the class being loaded, and the 
ClassLoader.loadedLibraryNames hash map, locked during the native library load 
operation.

Problem being fixed:

The initial reproducer demonstrated a deadlock between the JarFile/ZipFile and 
the hash map. That deadlock exists even when the ZipFile/JarFile lock is 
removed because there's another lock object in the class loader, associated 
with the name of the class being loaded. Such objects are stored in 
ClassLoader.parallelLockMap. The deadlock occurs when JNI_OnLoad() loads 
exactly the same class, whose signature is being verified in another thread.

Proposed fix:

The proposed patch suggests to get rid of locking loadedLibraryNames hash map 
and synchronize on each entry name, as it's done with class names in see 
ClassLoader.getClassLoadingLock(name) method.

The patch introduces nativeLibraryLockMap which holds the lock objects for each 
library name, and the getNativeLibraryLock() private method is used to lazily 
initialize the corresponding lock object. nativeLibraryContext was changed to 
ThreadLocal, so that in any concurrent thread it would have a NativeLibrary 
object on top of the stack, that's being currently loaded/unloaded in that 
thread. nativeLibraryLockMap accumulates the names of all native libraries 
loaded - in line with class loading code, it is not explicitly cleared.

Testing:  jtreg and jck testing with no regressions. A new regression test was 
developed.


Aleksei Voitylov has updated the pull request incrementally with one additional 
commit since the last revision:

   address review comments, add tests


Dear colleagues,

The updated PR addresses review comment regarding ThreadLocal as well as David' 
concern around the lock being held during JNI_OnLoad/JNI_OnUnload calls, and 
ensures all lock objects are deallocated. Multiple threads are allowed to enter 
NativeLibrary.load() to prevent any thread from locking while another thread 
loads a library. Before the update, there could be a class loading lock held by 
a parallel capable class loader, which can deadlock with the library loading 
lock. As proposed by David Holmes, the library loading lock was removed because 
dlopen/LoadLibrary are thread safe and they maintain internal reference 
counters on libraries. There's still a lock being held while a pair of 
containers are read/updated. It's not going to deadlock as there's no lock/wait 
operation performed while that lock is held. Multiple threads may create their 
own copies of NativeLibrary object and register it for auto unloading.

Tests for auto unloading were added along with the PR update. There are now 3 
jtreg tests:
- one checks for deadlock, similar to the one proposed by Chris Hegarty
- two other tests are for library unload.

The major side effect of that multiple threads are allowed to enter is that 
JNI_OnLoad/JNI_OnUnload may be called multiple (but same) number of times from 
concurrent threads. In particular, the number of calls to JNI_OnLoad must be 
equal to the number of calls to JNI_OnUnload after the relevant class loader is 
garbage collected. This may affect the behaviour that relies on specific order 
or the number of JNI_OnLoad/JNI_OnUnload calls. The current JNI specification 
does not mandate how many times JNI_OnLoad/JNI_OnUnload are called. Also, we 
could not locate tests in jck/jtreg/vmTestbase that would rely on the specific 
order or number of calls to JNI_OnLoad/JNI_OnUnload.


But you can't make such a change! That was my point. To fix the deadlock 
we must not hold a lock. But we must ensure only a single call to 
JNI_OnLoad is possible. It is an unsolvable problem with those 
constraints. You can't just change the behaviour of JNI_OnLoad like that.


David
-


Thank you Alan Bateman, David Holmes and Chris Hegarty for your valuable input.

-

PR: https://git.openjdk.java.net/jdk/pull/3976

Re: RFR: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v11]

[Paul Sandoz]

On Wed, 19 May 2021 22:16:18 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: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

[Weijun Wang]

On Wed, 19 May 2021 22:04:57 GMT, Phil Race  wrote:

>> Correct, there are ways to modify the code to make it more 
>> annotation-friendly. We thought about whether it's good to do it before 
>> adding the annotations or after it. Our decision now is to do it after 
>> because it will be more easy to see why it's necessary and we can take time 
>> to do them little by little. A new enhancement at 
>> https://bugs.openjdk.java.net/browse/JDK-8267432 is filed.
>
> I don't think it is a separate P4 enhancement (?) that someone will (maybe) 
> do next release.
> I think it should all be taken care of as part of this proposed change.

I just made it P3 (P4 was the default value), and I will target it to 17 once 
JEP 411 is targeted 17. But I think it's probably not a good idea to include it 
inside *this* PR. There are some middle ground where it's debatable if a change 
is worth doing (Ex: which is uglier between an a-liitle-faraway-annotation and 
a temporary variable?) so it's not obvious what the scope of the refactoring 
can be. Thus it will be divided into smaller sub-tasks. It's not totally 
impossible that part of the work will be delayed to next release.

-

PR: https://git.openjdk.java.net/jdk/pull/4073

Re: RFR: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v2]

[Sandhya Viswanathan]

On Wed, 19 May 2021 22:02:14 GMT, Paul Sandoz  wrote:

>> Tier 1 to 3 tests pass for the default set of build profiles.
>
>> Thanks a lot for the review @PaulSandoz @iwanowww @erikj79.
>> Paul and Vladimir, I have implemented your review comments. Please take a 
>> look.
> 
> `case VECTOR_OP_OR` is still present.

@PaulSandoz Thanks for pointing that out. I had missed git add for some of the 
files.

-

PR: https://git.openjdk.java.net/jdk/pull/3638

Re: RFR: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v11]

[Sandhya Viswanathan]

> 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 ops/ms 1.92
> Float128Vector.ASIN 57.15 117.95 ops/ms 2.06
> Float128Vector.ATAN 22.52 318.74 ops/ms 14.15
> Float128Vector.ATAN2 17.06 246.07 ops/ms 14.42
> 

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

[Phil Race]

On Wed, 19 May 2021 21:53:35 GMT, Weijun Wang  wrote:

>> That's a sad limitation of the annotation stuff then, but I don't think that 
>> it is insurmountable.
>> You can define a static private method to contain this and call it from the 
>> static initializer block.
>> Much better than applying the annotation to an entire class.
>> 
>> --- a/src/java.desktop/share/classes/java/awt/Component.java
>> +++ b/src/java.desktop/share/classes/java/awt/Component.java
>> @@ -618,6 +618,17 @@ public abstract class Component implements 
>> ImageObserver, MenuContainer,
>>   */
>>  static boolean isInc;
>>  static int incRate;
>> +
>> +private static void initIncRate() {
>> +String s = java.security.AccessController.doPrivileged(
>> + new 
>> GetPropertyAction("awt.image.incrementaldraw"));
>> +isInc = (s == null || s.equals("true"));
>> +
>> +s = java.security.AccessController.doPrivileged(
>> +  new GetPropertyAction("awt.image.redrawrate"));
>> +incRate = (s != null) ? Integer.parseInt(s) : 100;
>> +}
>> +
>>  static {
>>  /* ensure that the necessary native libraries are loaded */
>>  Toolkit.loadLibraries();
>> @@ -625,14 +636,7 @@ public abstract class Component implements 
>> ImageObserver, MenuContainer,
>>  if (!GraphicsEnvironment.isHeadless()) {
>>  initIDs();
>>  }
>> -
>> -String s = java.security.AccessController.doPrivileged(
>> -   new 
>> GetPropertyAction("awt.image.incrementaldraw"));
>> -isInc = (s == null || s.equals("true"));
>> -
>> -s = java.security.AccessController.doPrivileged(
>> -new 
>> GetPropertyAction("awt.image.redrawrate"));
>> -incRate = (s != null) ? Integer.parseInt(s) : 100;
>> +initIncRate();
>>  }
>
> Correct, there are ways to modify the code to make it more 
> annotation-friendly. We thought about whether it's good to do it before 
> adding the annotations or after it. Our decision now is to do it after 
> because it will be more easy to see why it's necessary and we can take time 
> to do them little by little. A new enhancement at 
> https://bugs.openjdk.java.net/browse/JDK-8267432 is filed.

I don't think it is a separate P4 enhancement (?) that someone will (maybe) do 
next release.
I think it should all be taken care of as part of this proposed change.

-

PR: https://git.openjdk.java.net/jdk/pull/4073

Re: RFR: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v2]

[Paul Sandoz]

On Mon, 3 May 2021 21:41:26 GMT, Paul Sandoz  wrote:

>> Sandhya Viswanathan has updated the pull request with a new target base due 
>> to a merge or a rebase. The pull request now contains six commits:
>> 
>>  - Merge master
>>  - remove whitespace
>>  - Merge master
>>  - Small fix
>>  - cleanup
>>  - x86 short vector math optimization for Vector API
>
> Tier 1 to 3 tests pass for the default set of build profiles.

> Thanks a lot for the review @PaulSandoz @iwanowww @erikj79.
> Paul and Vladimir, I have implemented your review comments. Please take a 
> look.

`case VECTOR_OP_OR` is still present.

-

PR: https://git.openjdk.java.net/jdk/pull/3638

Re: RFR: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v2]

[Sandhya Viswanathan]

On Mon, 3 May 2021 21:41:26 GMT, Paul Sandoz  wrote:

>> Sandhya Viswanathan has updated the pull request with a new target base due 
>> to a merge or a rebase. The pull request now contains six commits:
>> 
>>  - Merge master
>>  - remove whitespace
>>  - Merge master
>>  - Small fix
>>  - cleanup
>>  - x86 short vector math optimization for Vector API
>
> Tier 1 to 3 tests pass for the default set of build profiles.

Thanks a lot for the review @PaulSandoz @iwanowww @erikj79.
Paul and Vladimir, I have implemented your review comments. Please take a look.

-

PR: https://git.openjdk.java.net/jdk/pull/3638

Re: RFR: 8266846: Add java.time.InstantSource [v3]

[Stephen Colebourne]

On Tue, 18 May 2021 23:18:42 GMT, Stephen Colebourne  
wrote:

>> 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

I've made the obvious changes to fix the offset reflection. However it does now 
mean that the offset is being altered for a singleton where previously it would 
have only affected Clock.systemUtc(). Is the test class spun up in its own JVM 
process? Just want to make sure that manipulating the singleton clock won't 
impact other tests.

-

PR: https://git.openjdk.java.net/jdk/pull/4016

Re: RFR: 8266846: Add java.time.InstantSource [v4]

[Stephen Colebourne]

> 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/425e01a8..c7d9076b

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4016=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4016=02-03

  Stats: 6 lines in 2 files changed: 2 ins; 1 del; 3 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: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

[Weijun Wang]

On Wed, 19 May 2021 19:31:24 GMT, Phil Race  wrote:

>> This happens when a deprecated method is called inside a static block. The 
>> annotation can only be added to a declaration and here it must be the whole 
>> class. The call in this file is
>> 
>> s = java.security.AccessController.doPrivileged(
>> new 
>> GetPropertyAction("awt.image.redrawrate"));
>
> That's a sad limitation of the annotation stuff then, but I don't think that 
> it is insurmountable.
> You can define a static private method to contain this and call it from the 
> static initializer block.
> Much better than applying the annotation to an entire class.
> 
> --- a/src/java.desktop/share/classes/java/awt/Component.java
> +++ b/src/java.desktop/share/classes/java/awt/Component.java
> @@ -618,6 +618,17 @@ public abstract class Component implements 
> ImageObserver, MenuContainer,
>   */
>  static boolean isInc;
>  static int incRate;
> +
> +private static void initIncRate() {
> +String s = java.security.AccessController.doPrivileged(
> + new 
> GetPropertyAction("awt.image.incrementaldraw"));
> +isInc = (s == null || s.equals("true"));
> +
> +s = java.security.AccessController.doPrivileged(
> +  new GetPropertyAction("awt.image.redrawrate"));
> +incRate = (s != null) ? Integer.parseInt(s) : 100;
> +}
> +
>  static {
>  /* ensure that the necessary native libraries are loaded */
>  Toolkit.loadLibraries();
> @@ -625,14 +636,7 @@ public abstract class Component implements 
> ImageObserver, MenuContainer,
>  if (!GraphicsEnvironment.isHeadless()) {
>  initIDs();
>  }
> -
> -String s = java.security.AccessController.doPrivileged(
> -   new 
> GetPropertyAction("awt.image.incrementaldraw"));
> -isInc = (s == null || s.equals("true"));
> -
> -s = java.security.AccessController.doPrivileged(
> -new 
> GetPropertyAction("awt.image.redrawrate"));
> -incRate = (s != null) ? Integer.parseInt(s) : 100;
> +initIncRate();
>  }

Correct, there are ways to modify the code to make it more annotation-friendly. 
We thought about whether it's good to do it before adding the annotations or 
after it. Our decision now is to do it after because it will be more easy to 
see why it's necessary and we can take time to do them little by little. A new 
enhancement at https://bugs.openjdk.java.net/browse/JDK-8267432 is filed.

-

PR: https://git.openjdk.java.net/jdk/pull/4073

Re: RFR: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v10]

[Sandhya Viswanathan]

> 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 ops/ms 1.92
> Float128Vector.ASIN 57.15 117.95 ops/ms 2.06
> Float128Vector.ATAN 22.52 318.74 ops/ms 14.15
> Float128Vector.ATAN2 17.06 246.07 ops/ms 14.42
> 

Re: JEP draft: Scope Locals

[David Lloyd]

On Wed, May 12, 2021 at 9:43 AM Andrew Haley  wrote:

> There's been considerable discussion about scope locals on the loom-dev
> list,
> and it's now time to open this to a wider audience. This subject is
> important
> because. although scope locals were motivated by the the needs of Loom,
> they
> have many potential applications outside that project.
>

I didn't get a chance to mention earlier, but I think scope locals are
shaping up really great so far: simple yet powerful.  I know this will be a
really useful addition to the JDK.  There are several projects both within
and without Red Hat that I have authored or have contributed to which use a
very compatible pattern (that is, using a lexically constrained binding
strategy for context association) and are perfect candidates to switch over
to use this feature.

That said, I have one minor comment about the API. :-)

I think it would be really nice if the snapshot class could hold its
run/call method rather than making it a static method of `ScopeLocal`. This
reads more naturally to me (and is nicer to write):

var snap = ScopeLocal.snapshot();
return executor.submit(() -> snap.call(aCallable));

Than this:

var snap = ScopeLocal.snapshot();
return executor.submit(() -> ScopeLocal.callWithSnapshot(aCallable, snap
));

This kind of lexically bound contextual pattern can be found in several of
our existing projects (including those listed below [1] [2]).  In writing
these projects, we took it a step further than just Runnable and Callable
and added variants which accept `Consumer` plus a `T` to pass in, a
`Function` plus a `T` to pass in, and `BiConsumer` and `BiFunction`
variants as well which accept two arguments to pass in.  The practical
reason here is that we could then pass in a method handle with explicit
arguments rather than relying on lambda capture, which can be expensive in
certain circumstances.  The overall benefit vs cost of doing this is
probably debatable, but I thought the idea might be interesting at any rate.

Anyway that's it.  I love this feature and am excited to see it get into
the JDK!

[1] WildFly Elytron "Scoped" API, used by client and server authentication
contexts (WildFly's version of JAAS Subject):
https://github.com/wildfly-security/wildfly-elytron/blob/1.x/auth/server/base/src/main/java/org/wildfly/security/auth/server/Scoped.java
[2] WildFly Common "Contextual" API used for transaction, configuration,
and other context propagation:
https://github.com/wildfly/wildfly-common/blob/master/src/main/java/org/wildfly/common/context/Contextual.java

-- 
- DML • he/him

Re: [External] : Re: ReversibleCollection proposal

[Brian Goetz]

Has it ever been conceived to create an entire new API like how it was
done for Dates and Times? Obviously this would be a massive
undertaking, but it seems to me we've just about reached the limits of
how far we can stretch the current API.


"This code is a mess, we should throw it away and rewrite it"

   -- every developer

Don't confuse the volume of "I would rather do it this way" replies with 
the complexity of this issue; I think that's just the nature of the game 
("Being the biggest crime of the last 50 years, and everybody wanted to 
get in the newspaper story about it.")  Stuart's proposal is a measured, 
responsible, carefully-thought-through way to extend the framework we have.


In addition to "massive undertaking", and in addition to "if you think 
you can't get people to agree on something the size of a golf ball, try 
to get them to agree on something the size of Montana", there's another 
massive problem here: migration.  It's not just a matter of having a 
"better" Collections library; the collection interfaces we have 
(Collection, List, Map, Set) have found their way into the API of nearly 
every Java library.  Moving to Collections II would then force a 
migration on every one of those libraries (or consumer of those libraries.)

RFR: 8267056: tools/jpackage/share/RuntimePackageTest.java fails with NoSuchFileException

[Alexander Matveev]

For debug build on macOS, runtime which used for test fill be located in 
/path/jdk-17/fastdebug and /path/jdk-17 will not contain any other files except 
fastdebug and in this case our check to decide if we need to copy app or 
runtime will return /path/jdk-17 which is not correct. Fixed by skipping this 
check for runtime and only using it for app. Also, added ignoring .DS_Store to 
test which is needed if user used Finder to look inside runtime folder which 
will cause .DS_Store to be created and will cause test to fail, since this file 
is not being packaged.

-

Commit messages:
 - 8267056: tools/jpackage/share/RuntimePackageTest.java fails with 
NoSuchFileException

Changes: https://git.openjdk.java.net/jdk/pull/4120/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4120=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8267056
  Stats: 15 lines in 2 files changed: 5 ins; 0 del; 10 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4120.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4120/head:pull/4120

PR: https://git.openjdk.java.net/jdk/pull/4120

Re: JEP draft: Scope Locals

[David Lloyd]

On Wed, May 19, 2021 at 4:01 AM Andrew Haley  wrote:

> On 5/15/21 6:50 PM, Peter Levart wrote:
> > What if I wanted to create and start a thread that would be "pristine" -
> > not have any ScopeLocal value bound? Is this intentionally not allowed
> > in order to make sure that inheritable ScopeLocal(s) can't be cleared by
> > code that has no access to ScopeLocal instance(s)?
>
> That one is about to be changed by a revision to the JEP. There clearly
> is a need to control whether a newly-created thread inherits scope locals
> or not. For instance, an Executor might lazily create worker threads, and
> we don't want them to inherit scope locals from the thread that was
> running.
>

Turning this around, I would argue that there are few (or perhaps *no*)
cases where it would ever be desirable to inherit scope locals across
thread creation; in cases where this is explicitly desired, one can always
resume the snapshot from within the thread's Runnable. Was there a
particular use case this was meant to address?

-- 
- DML • he/him

Re: RFR: 8260517: implement Sealed Classes as a standard feature in Java [v8]

[Vicente Romero]

> Please review this PR that intents to make sealed classes a final feature in 
> Java. This PR contains compiler and VM changes. In line with similar PRs, 
> which has made preview features final, this one is mostly removing preview 
> related comments from APIs plus options in test cases. Please also review the 
> related [CSR](https://bugs.openjdk.java.net/browse/JDK-8265090)
> 
> Thanks
> 
> note: this PR is related to 
> [PR-3528](https://github.com/openjdk/jdk/pull/3528) and must be integrated 
> after it.

Vicente Romero 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 12 additional commits 
since the last revision:

 - Merge branch 'master' into JDK-8260517
 - Merge branch 'master' into JDK-8260517
 - restoring jdk.internal.javac.PreviewFeature.Feature.SEALED_CLASSES, it is 
needed by the build
 - Merge branch 'master' into JDK-8260517
 - Merge branch 'master' into JDK-8260517
 - updating comment after review feedback
 - removing javax.lang.model changes
 - Merge branch 'master' into JDK-8260517
 - Merge branch 'master' into JDK-8260517
 - fixing failing regression tests
 - ... and 2 more: https://git.openjdk.java.net/jdk/compare/cc4f43fb...d220bc20

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3526/files
  - new: https://git.openjdk.java.net/jdk/pull/3526/files/0208dcf0..d220bc20

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3526=07
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3526=06-07

  Stats: 40144 lines in 1123 files changed: 19391 ins; 13080 del; 7673 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3526.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3526/head:pull/3526

PR: https://git.openjdk.java.net/jdk/pull/3526

Re: RFR: 8267184: JEP 411: Add -Djava.security.manager=allow to tests calling System.setSecurityManager [v2]

[Sean Mullan]

On Tue, 18 May 2021 21:44:43 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 (~~serviceability~~, 
>> ~~hotspot-compiler~~, ~~i18n~~, ~~rmi~~, ~~javadoc~~, swing, 2d, 
>> ~~security~~, ~~hotspot-runtime~~, ~~nio~~, ~~xml~~, ~~beans~~, 
>> ~~core-libs~~, ~~net~~, ~~compiler~~, ~~hotspot-gc~~). 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.
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   adjust order of VM options

The changes to the security tests look good.

-

Marked as reviewed by mullan (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/4071

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

[Phil Race]

On Wed, 19 May 2021 18:38:39 GMT, Weijun Wang  wrote:

>> src/java.desktop/share/classes/java/awt/Component.java line 217:
>> 
>>> 215:  * @author  Sami Shaio
>>> 216:  */
>>> 217: @SuppressWarnings("removal")
>> 
>> Why is this placed on the *entire class* ??
>> This class is 10535 lines long and mentions AccessControl something like 8 
>> places it uses AccessController or AcessControlContext.
>
> This happens when a deprecated method is called inside a static block. The 
> annotation can only be added to a declaration and here it must be the whole 
> class. The call in this file is
> 
> s = java.security.AccessController.doPrivileged(
> new 
> GetPropertyAction("awt.image.redrawrate"));

That's a sad limitation of the annotation stuff then, but I don't think that it 
is insurmountable.
You can define a static private method to contain this and call it from the 
static initializer block.
Much better than applying the annotation to an entire class.

--- a/src/java.desktop/share/classes/java/awt/Component.java
+++ b/src/java.desktop/share/classes/java/awt/Component.java
@@ -618,6 +618,17 @@ public abstract class Component implements ImageObserver, 
MenuContainer,
  */
 static boolean isInc;
 static int incRate;
+
+private static void initIncRate() {
+String s = java.security.AccessController.doPrivileged(
+ new 
GetPropertyAction("awt.image.incrementaldraw"));
+isInc = (s == null || s.equals("true"));
+
+s = java.security.AccessController.doPrivileged(
+  new GetPropertyAction("awt.image.redrawrate"));
+incRate = (s != null) ? Integer.parseInt(s) : 100;
+}
+
 static {
 /* ensure that the necessary native libraries are loaded */
 Toolkit.loadLibraries();
@@ -625,14 +636,7 @@ public abstract class Component implements ImageObserver, 
MenuContainer,
 if (!GraphicsEnvironment.isHeadless()) {
 initIDs();
 }
-
-String s = java.security.AccessController.doPrivileged(
-   new 
GetPropertyAction("awt.image.incrementaldraw"));
-isInc = (s == null || s.equals("true"));
-
-s = java.security.AccessController.doPrivileged(
-new 
GetPropertyAction("awt.image.redrawrate"));
-incRate = (s != null) ? Integer.parseInt(s) : 100;
+initIncRate();
 }

-

PR: https://git.openjdk.java.net/jdk/pull/4073

Re: RFR: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v9]

[Paul Sandoz]

On Wed, 19 May 2021 03:37:11 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: 8266846: Add java.time.InstantSource [v3]

[Rémi Forax]

On Tue, 18 May 2021 23:18:42 GMT, Stephen Colebourne  
wrote:

>> 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

my bad

-

PR: https://git.openjdk.java.net/jdk/pull/4016

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

[Weijun Wang]

On Wed, 19 May 2021 18:44:06 GMT, Weijun Wang  wrote:

>> Similar as the one above, it's because of
>> 
>> static {
>> // Don't lazy-read because every app uses invalidate()
>> isJavaAwtSmartInvalidate = AccessController.doPrivileged(
>> new GetBooleanAction("java.awt.smartInvalidate"));
>> }
>
> We are thinking of more tweaks after this overall change to make the 
> annotation more precise. For example, in this case we can assign the 
> `doPrivileged` result to a local variable right at its declaration, and then 
> assign it to `isJavaAwtSmartInvalidate`. Some people might think this is 
> ugly. Such manual code changes need to done little by little to ease code 
> reviewing.

I know it's not easy to read the commit and that's why I make the 3rd commit 
totally automatic. Hopefully you have more confidence on the program than my 
hand. All annotations are added to the nearest declarations.

-

PR: https://git.openjdk.java.net/jdk/pull/4073

Re: RFR: 8266310: deadlock while loading the JNI code [v3]

[Jorn Vernee]

On Wed, 19 May 2021 16:29:33 GMT, Aleksei Voitylov  
wrote:

>> Please review this PR which fixes the deadlock in ClassLoader between the 
>> two lock objects - a lock object associated with the class being loaded, and 
>> the ClassLoader.loadedLibraryNames hash map, locked during the native 
>> library load operation.
>> 
>> Problem being fixed:
>> 
>> The initial reproducer demonstrated a deadlock between the JarFile/ZipFile 
>> and the hash map. That deadlock exists even when the ZipFile/JarFile lock is 
>> removed because there's another lock object in the class loader, associated 
>> with the name of the class being loaded. Such objects are stored in 
>> ClassLoader.parallelLockMap. The deadlock occurs when JNI_OnLoad() loads 
>> exactly the same class, whose signature is being verified in another thread.
>> 
>> Proposed fix:
>> 
>> The proposed patch suggests to get rid of locking loadedLibraryNames hash 
>> map and synchronize on each entry name, as it's done with class names in see 
>> ClassLoader.getClassLoadingLock(name) method.
>> 
>> The patch introduces nativeLibraryLockMap which holds the lock objects for 
>> each library name, and the getNativeLibraryLock() private method is used to 
>> lazily initialize the corresponding lock object. nativeLibraryContext was 
>> changed to ThreadLocal, so that in any concurrent thread it would have a 
>> NativeLibrary object on top of the stack, that's being currently 
>> loaded/unloaded in that thread. nativeLibraryLockMap accumulates the names 
>> of all native libraries loaded - in line with class loading code, it is not 
>> explicitly cleared.
>> 
>> Testing:  jtreg and jck testing with no regressions. A new regression test 
>> was developed.
>
> Aleksei Voitylov has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   fix trailing whitespace

src/java.base/share/classes/jdk/internal/loader/NativeLibraries.java line 262:

> 260: } finally {
> 261: releaseNativeLibraryLock(name);
> 262: }

The new locking scheme looks incorrect to me. It now seems possible for 2 
different class loaders in 2 different threads to load the same library (which 
should not be possible).

Thread 1:
  - acquires name lock
  - checks library name is already in `loadedLibraryNames` (it's not)
  - release name lock
  - start loading library
  
Then thread 2 comes in and does the same
  
Then again thread 1 finishes loading and:
 - acquires name lock
 - puts library name in `loadedLibraryNames`
 - puts library name in it's own `libraries`
 - release lock
 
Then thread 2 comes in and does the same again (although adding the name to 
`loadedLibraryNames` will silently return `false`).

It seems that this scenario is possible, and the library will be loaded by 2 
different class loaders, each with their own `lib` object. (If there's a race, 
there has to be a loser as well, which would need to discard their work when 
finding out they lost)

You might be able to stress this by checking if `loadedLibraryNames.add(name);` 
returns `true`, and throwing an exception if not.

I think a possible solution would be to claim the library name up front for a 
particular loader. Then 2 threads can still race to load the same library for 
the same class loader, but 2 threads with 2 different class loaders racing to 
load the library should not be possible.

Actually, you might be able to prevent a race on JNI_OnLoad altogether by 
claiming the library name for a particular thread upfront as well (e.g. using a 
`ConcurrentHashMap`).

-

PR: https://git.openjdk.java.net/jdk/pull/3976

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

[Weijun Wang]

On Wed, 19 May 2021 18:39:10 GMT, Weijun Wang  wrote:

>> src/java.desktop/share/classes/java/awt/Container.java line 97:
>> 
>>> 95:  * @since 1.0
>>> 96:  */
>>> 97: @SuppressWarnings("removal")
>> 
>> Same issue as with Component. a > 5,000 line file that uses AccessController 
>> in just 4 places.
>> 
>> Where else are you adding this to entire classes instead of the specific 
>> site ?
>
> Similar as the one above, it's because of
> 
> static {
> // Don't lazy-read because every app uses invalidate()
> isJavaAwtSmartInvalidate = AccessController.doPrivileged(
> new GetBooleanAction("java.awt.smartInvalidate"));
> }

We are thinking of more tweaks after this overall change to make the annotation 
more precise. For example, in this case we can assign the `doPrivileged` result 
to a local variable right at its declaration, and then assign it to 
`isJavaAwtSmartInvalidate`. Some people might think this is ugly. Such manual 
code changes need to done little by little to ease code reviewing.

-

PR: https://git.openjdk.java.net/jdk/pull/4073

Re: RFR: 8266936: Add a finalization JFR event [v2]

[Mandy Chung]

On Wed, 19 May 2021 17:57:11 GMT, Erik Gahlin  wrote:

> > I was also thinking if this event should be implemented in the VM in order 
> > to avoid some performance overhead such as object allocation. Erik, what is 
> > the benefit of implementing in in the VM?
> 
> No startup cost, no allocation and there are callbacks when a class gets 
> unloaded, so it's probably easier to clear any table where the statistics is 
> held.

Thanks for the confirmation.   This is performance sensitive area and so it's 
worth considering doing it in the VM.  In either case, performance measurement 
of the overhead will tell.

-

PR: https://git.openjdk.java.net/jdk/pull/4101

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

[Weijun Wang]

On Wed, 19 May 2021 18:26:25 GMT, Phil Race  wrote:

>> Weijun Wang has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   fixing awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java
>
> src/java.desktop/share/classes/java/awt/Component.java line 217:
> 
>> 215:  * @author  Sami Shaio
>> 216:  */
>> 217: @SuppressWarnings("removal")
> 
> Why is this placed on the *entire class* ??
> This class is 10535 lines long and mentions AccessControl something like 8 
> places it uses AccessController or AcessControlContext.

This happens when a deprecated method is called inside a static block. The 
annotation can only be added to a declaration and here it must be the whole 
class. The call in this file is

s = java.security.AccessController.doPrivileged(
new 
GetPropertyAction("awt.image.redrawrate"));

> src/java.desktop/share/classes/java/awt/Container.java line 97:
> 
>> 95:  * @since 1.0
>> 96:  */
>> 97: @SuppressWarnings("removal")
> 
> Same issue as with Component. a > 5,000 line file that uses AccessController 
> in just 4 places.
> 
> Where else are you adding this to entire classes instead of the specific site 
> ?

Similar as the one above, it's because of

static {
// Don't lazy-read because every app uses invalidate()
isJavaAwtSmartInvalidate = AccessController.doPrivileged(
new GetBooleanAction("java.awt.smartInvalidate"));
}

> test/jdk/java/awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java line 59:
> 
>> 57: ProcessCommunicator
>> 58: .executeChildProcess(Consumer.class, new 
>> String[0]);
>> 59: if (!"Hello".equals(processResults.getStdOut())) {
> 
> Who or what prompted this change ?

The child process is started with `-Djava.security.manager=allow` (after the 
other PR) and a warning will be printed to stderr. Therefore I move the message 
to stdout.

-

PR: https://git.openjdk.java.net/jdk/pull/4073

Re: RFR: 8266846: Add java.time.InstantSource [v3]

[Naoto Sato]

On Tue, 18 May 2021 23:18:42 GMT, Stephen Colebourne  
wrote:

>> 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

The Error was caused by the movement of the field `offset`, from inner 
`SystemClock` class to `Clock`.

-

PR: https://git.openjdk.java.net/jdk/pull/4016

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

[Phil Race]

On Wed, 19 May 2021 13:47:53 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.
>> 
>> Update: the deprecation annotations and javadoc tags, build, compiler, 
>> core-libs, hotspot, i18n, jmx, net, nio, security, and serviceability are 
>> reviewed. Rest are 2d, awt, beans, sound, and swing.
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   fixing awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java

src/java.desktop/share/classes/java/awt/Component.java line 217:

> 215:  * @author  Sami Shaio
> 216:  */
> 217: @SuppressWarnings("removal")

Why is this placed on the *entire class* ??
This class is 10535 lines long and mentions AccessControl something like 8 
places it uses AccessController or AcessControlContext.

src/java.desktop/share/classes/java/awt/Container.java line 97:

> 95:  * @since 1.0
> 96:  */
> 97: @SuppressWarnings("removal")

Same issue as with Component. a > 5,000 line file that uses AccessController in 
just 4 places.

Where else are you adding this to entire classes instead of the specific site ?

-

PR: https://git.openjdk.java.net/jdk/pull/4073

Re: RFR: 5023614: UUID needs methods to get most/leastSigBits and write to DataOutput [v3]

[Richard Fussenegger]

On Sat, 28 Nov 2020 10:12:10 GMT, Richard Fussenegger 
 wrote:

>> Made byte constructor public and changed the length assertion to an 
>> `IllegalArgumentException`, added a `getBytes` method that allows users to 
>> retrieve the raw bytes of the UUID, and created a new private constructor 
>> with an optimized construction for byte arrays that can set the version as 
>> desired and the variant to RFC 4122. Also changed the existing static 
>> factory methods to use the new constructor and removed the duplicate code 
>> from them where the variant and version is being set.
>> 
>> Report 
>> [5023614](https://bugs.java.com/bugdatabase/view_bug.do?bug_id=5023614) asks 
>> for more than what I provided and with different names. However, I believe 
>> that there is no value in providing methods to deal with `DataInput` and 
>> `DataOutput` because they would only encapsulate single method calls that 
>> the caller can directly write as well (e.g. `output.write(uuid.getBytes())` 
>> vs `uuid.write(output)`). Hence, I consider this change to satisfy the 
>> feature request.
>
> Richard Fussenegger has refreshed the contents of this pull request, and 
> previous commits have been removed. The incremental views will show 
> differences compared to the previous content of the PR.

@bridgekeeper I guess it will stay open for a little longer. 

-

PR: https://git.openjdk.java.net/jdk/pull/1465

Re: RFR: 6594730: UUID.getVersion() is only meaningful for Leach-Salz variant

[Richard Fussenegger]

On Thu, 26 Nov 2020 18:51:10 GMT, Richard Fussenegger 
 wrote:

> 6594730: UUID.getVersion() is only meaningful for Leach-Salz variant

@bridgekeeper I guess it will stay open for a little longer. 

-

PR: https://git.openjdk.java.net/jdk/pull/1467

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

[Phil Race]

On Wed, 19 May 2021 13:47:53 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.
>> 
>> Update: the deprecation annotations and javadoc tags, build, compiler, 
>> core-libs, hotspot, i18n, jmx, net, nio, security, and serviceability are 
>> reviewed. Rest are 2d, awt, beans, sound, and swing.
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   fixing awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java

This change is so large that github won't even display the list of files so I 
can't jump to where I want to go !
And when I try to scroll I get just a blank page.

-

PR: https://git.openjdk.java.net/jdk/pull/4073

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

[Phil Race]

On Wed, 19 May 2021 13:47:53 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.
>> 
>> Update: the deprecation annotations and javadoc tags, build, compiler, 
>> core-libs, hotspot, i18n, jmx, net, nio, security, and serviceability are 
>> reviewed. Rest are 2d, awt, beans, sound, and swing.
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   fixing awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java

test/jdk/java/awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java line 59:

> 57: ProcessCommunicator
> 58: .executeChildProcess(Consumer.class, new 
> String[0]);
> 59: if (!"Hello".equals(processResults.getStdOut())) {

Who or what prompted this change ?

-

PR: https://git.openjdk.java.net/jdk/pull/4073

Re: RFR: 8266846: Add java.time.InstantSource [v3]

[Rémi Forax]

On Tue, 18 May 2021 23:18:42 GMT, Stephen Colebourne  
wrote:

>> 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

It's a side effect of JEP 403, i believe

-

PR: https://git.openjdk.java.net/jdk/pull/4016

Re: RFR: 8266936: Add a finalization JFR event [v2]

[Erik Gahlin]

On Tue, 18 May 2021 22:41:06 GMT, Brent Christian  wrote:

>> Please review this enhancement to add a new JFR event, generated whenever a 
>> finalizer is run.
>> (The makeup is similar to the Deserialization event, 
>> [JDK-8261160](https://bugs.openjdk.java.net/browse/JDK-8261160).)
>> 
>> The event's only datum (beyond those common to all jfr events) is the class 
>> of the object that was finalized.
>> 
>> The Category for the event:
>> `"Java Virtual Machine" / "GC" / "Finalization"`
>> is what made sense to me, even though the event is generated from library 
>> code.
>> 
>> Along with the new regtest, I added a run mode to the basic finalizer test 
>> to enable jfr.
>> Automated testing looks good so far.
>> 
>> Thanks,
>> -Brent
>
> Brent Christian has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Test flag should be volatile

> I was also thinking if this event should be implemented in the VM in order to 
> avoid some performance overhead such as object allocation. Erik, what is the 
> benefit of implementing in in the VM?

No startup cost, no allocation and there are callbacks when a class gets 
unloaded, so it's probably easier to clear any table where the statistics is 
held.

-

PR: https://git.openjdk.java.net/jdk/pull/4101

Re: RFR: 8266846: Add java.time.InstantSource [v3]

[Naoto Sato]

On Tue, 18 May 2021 23:18:42 GMT, Stephen Colebourne  
wrote:

>> 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

Another test started failing with your fix.  Apparently, the following piece in 
java/time/test/java/time/TestClock_System.java throws 
ExceptionInInitializerError.

static {
try {
offsetField = 
Class.forName("java.time.Clock$SystemClock").getDeclaredField("offset");
offsetField.setAccessible(true);
} catch (ClassNotFoundException | NoSuchFieldException ex) {
throw new ExceptionInInitializerError(ex);
}
}

-

PR: https://git.openjdk.java.net/jdk/pull/4016

Re: RFR: 8203359: Container level resources events [v9]

[Erik Gahlin]

On Wed, 21 Apr 2021 22:38:32 GMT, Erik Gahlin  wrote:

>> Jaroslav Bachorik has refreshed the contents of this pull request, and 
>> previous commits have been removed. The incremental views will show 
>> differences compared to the previous content of the PR.
>
> I wonder if something similar to below could be added to 
> jdk.jfr.internal.Utils:
> 
> private static Metrics[] metrics;
> public static Metrics getMetrics() {
> if (metrics == null) {
> metrics = new Metrics[] { Metrics.systemMetrics() };
> }
> return metrics[0];
> }
> 
> public static boolean shouldSkipBytecode(String eventName, Class 
> superClass) {
> if (superClass != AbstractJDKEvent.class) {
> return false;
> }
> if (!eventName.startsWith("jdk.Container")) {
> return false;
> }
> return getMetrics() == null;
> }
> 
> Then we could add checks to 
> jdk.jfr.internal.JVMUpcalls::bytesForEagerInstrumentation(...)
> 
> eventName = ei.getEventName();
> if (Utils.shouldSkipBytecode(eventName, superClass))) {
> return oldBytes;
> }
> 
> and jdk.jfr.internal.JVMUpcalls:onRetransform(...)
> 
> if (jdk.internal.event.Event.class.isAssignableFrom(clazz) && 
> !Modifier.isAbstract(clazz.getModifiers())) {
> if (Utils.shouldSkipBytecode(clazz.getName(), clazz.getSuperclass())) 
> {
> return oldBytes;
> }
> 
> This way we would not pay for generating bytecode for events in a 
> non-container environment. 
> 
> Not sure if it works, but could perhaps make startup faster? We would still 
> pay for generating the event handlers during registration, but it's much 
> trickier to avoid since we need to store the event type somewhere.

> @egahlin Unfortunately, I had to make one late change in the periodic event 
> hook registration.
> If the events are registered conditionally only when running in a container 
> the event metadata are not correct and `TestDefaultConfigurations.java` test 
> will fail. When I register the hooks unconditionally, the metadata is 
> correctly generated and the test passes.
> I will hold off integration until I hear back from you whether this is 
> acceptable or I should try to find an alternative solution.

I think we should always register the metadata, even if you can't get the 
event. 

That's how we handle different GCs. Users must be able to explore events even 
if they can't use them. For example, you must be able to configure container 
events in JMC (with correct labels/descriptions) without actually connecting to 
a JVM running in a Docker container.

-

PR: https://git.openjdk.java.net/jdk/pull/3126

Re: RFR: 8266936: Add a finalization JFR event [v2]

[Mandy Chung]

On Wed, 19 May 2021 09:00:27 GMT, Alan Bateman  wrote:

> I wonder if there needs to be one event per finalized object?

I also concern with the performance overhead of one event per finalized object.

The primary goal of this JFR event is to help identifying the use of finalizers 
in existing libraries/applications and prepare them to migrate away from using 
finalization.  As well-known, the finalization mechanism is inherently 
problematic. 

> Perhaps a counter per class would be as useful, i.e. 
> jdk.FinalizationStatistics, and if it could be implemented in the VM, without 
> other implications, that would be great.

Therefore, a counter per class would be useful as it identifies the usage of 
finalizers while providing the number of objects per class pending for 
finalization (see `ReferenceQueue::enqueue` and `ReferenceQueue::reallyPoll` 
where it keeps track of the pending for finalization counter).

Another option is to go with a simple approach - just report the aggregated 
number of `Finalizer` objects per class which still meets the primary goal to 
identify what existing code uses finalizers and the counter gives the users an 
additional information how many finalizers are created.

BTW the number of finalizer invocation is not more useful than the number of 
`Finalizer` instances unless we provide both counters so that the users can 
determine the number of objects pending for finalization.
 
> Such an event could be enabled by default and provide much greater value than 
> an event that users would need to know about and configure themselves, which 
> 99,99% of all user will not do.

I agree an event enabled by default is more useful provided that the 
performance overhead is insignificant.

I was also thinking if this event should be implemented in the VM in order to 
avoid some performance overhead such as object allocation.  Erin, what is the 
benefit of implementing in in the VM?

-

PR: https://git.openjdk.java.net/jdk/pull/4101

Re: RFR: 8203359: Container level resources events [v9]

[Jaroslav Bachorik]

On Wed, 21 Apr 2021 22:38:32 GMT, Erik Gahlin  wrote:

>> Jaroslav Bachorik has refreshed the contents of this pull request, and 
>> previous commits have been removed. The incremental views will show 
>> differences compared to the previous content of the PR.
>
> I wonder if something similar to below could be added to 
> jdk.jfr.internal.Utils:
> 
> private static Metrics[] metrics;
> public static Metrics getMetrics() {
> if (metrics == null) {
> metrics = new Metrics[] { Metrics.systemMetrics() };
> }
> return metrics[0];
> }
> 
> public static boolean shouldSkipBytecode(String eventName, Class 
> superClass) {
> if (superClass != AbstractJDKEvent.class) {
> return false;
> }
> if (!eventName.startsWith("jdk.Container")) {
> return false;
> }
> return getMetrics() == null;
> }
> 
> Then we could add checks to 
> jdk.jfr.internal.JVMUpcalls::bytesForEagerInstrumentation(...)
> 
> eventName = ei.getEventName();
> if (Utils.shouldSkipBytecode(eventName, superClass))) {
> return oldBytes;
> }
> 
> and jdk.jfr.internal.JVMUpcalls:onRetransform(...)
> 
> if (jdk.internal.event.Event.class.isAssignableFrom(clazz) && 
> !Modifier.isAbstract(clazz.getModifiers())) {
> if (Utils.shouldSkipBytecode(clazz.getName(), clazz.getSuperclass())) 
> {
> return oldBytes;
> }
> 
> This way we would not pay for generating bytecode for events in a 
> non-container environment. 
> 
> Not sure if it works, but could perhaps make startup faster? We would still 
> pay for generating the event handlers during registration, but it's much 
> trickier to avoid since we need to store the event type somewhere.

@egahlin Unfortunately, I had to make one late change in the periodic event 
hook registration.
If the events are registered conditionally only when running in a container the 
event metadata are not correct and `TestDefaultConfigurations.java` test will 
fail. When I register the hooks unconditionally, the metadata is correctly 
generated and the test passes.
I will hold off integration until I hear back from you whether this is 
acceptable or I should try to find an alternative solution.

-

PR: https://git.openjdk.java.net/jdk/pull/3126

Re: RFR: 8266846: Add java.time.InstantSource [v3]

[Daniel Fuchs]

On Wed, 19 May 2021 08:41:08 GMT, Peter Levart  wrote:

>> This isn't my logic - it is existing code that has been moved. I'm not a fan 
>> of infinite retry loops as the can hang the system. But I'm happy to change 
>> it to work that way if there is a consensus to do so.
>
> I see the localOffset passed to VM.getNanoTimeAdjustment() has to be in the 
> range of current time +/- 2^32 seconds. This means the thread would have to 
> sleep for ~136 years before the exception is triggered. I think this is more 
> than enough. :-)

Exactly. You could hit the 1ns if the operating system clock was concurrently 
moved backward by a NTP adjustment of ~ 1024 seconds at the same time that the 
offset is taken but this would be extremely unlikely - I don't believe it 
deserves any infinite retry loop. That was discussed at the time the 
enhancement that provides sub-millisecond precision was proposed, and it was 
rejected in favor of a one time retry.

-

PR: https://git.openjdk.java.net/jdk/pull/4016

Re: RFR: 8267190: Optimize Vector API test operations

[Paul Sandoz]

On Fri, 14 May 2021 23:58:38 GMT, Sandhya Viswanathan 
 wrote:

> Vector API test operations (IS_DEFAULT, IS_FINITE, IS_INFINITE, IS_NAN and 
> IS_NEGATIVE) are computed in three steps:
> 1) reinterpreting the floating point vectors as integral vectors (int/long)
> 2) perform the test in integer domain to get a int/long mask
> 3) reinterpret the int/long mask as float/double mask
> Step 3) currently is very slow. It can be optimized by modifying the Java 
> code to utilize the existing reinterpret intrinsic.
> 
> For the VectorTestPerf attached to the JBS for JDK-8267190, the performance 
> improves as follows:
> 
> Base:
> Benchmark (size) Mode Cnt Score Error Units
> VectorTestPerf.IS_DEFAULT 1024 thrpt 5 223.156 ± 90.452 ops/ms
> VectorTestPerf.IS_FINITE 1024 thrpt 5 223.841 ± 91.685 ops/ms
> VectorTestPerf.IS_INFINITE 1024 thrpt 5 224.561 ± 83.890 ops/ms
> VectorTestPerf.IS_NAN 1024 thrpt 5 223.777 ± 70.629 ops/ms
> VectorTestPerf.IS_NEGATIVE 1024 thrpt 5 218.392 ± 79.806 ops/ms
> 
> With patch:
> Benchmark (size) Mode Cnt Score Error Units
> VectorTestPerf.IS_DEFAULT 1024 thrpt 5 8812.357 ± 40.477 ops/ms
> VectorTestPerf.IS_FINITE 1024 thrpt 5 7425.739 ± 296.622 ops/ms
> VectorTestPerf.IS_INFINITE 1024 thrpt 5 8932.730 ± 269.988 ops/ms
> VectorTestPerf.IS_NAN 1024 thrpt 5 8574.872 ± 498.649 ops/ms
> VectorTestPerf.IS_NEGATIVE 1024 thrpt 5 8838.400 ± 11.849 ops/ms
> 
> Best Regards,
> Sandhya

Tier 1 to 3 tests pass on supported platforms

-

PR: https://git.openjdk.java.net/jdk/pull/4039

Re: JEP draft: Scope Locals

[Peter Levart]

On 15/05/2021 19:50, Peter Levart wrote:
Another question: Suppose there are two inheritable ScopeLocal 
variables with bound values in scope (A and B) when I take a snapshot:


var snapshot = ScopeLocal.snapshot();

now I pass that snapshot to a thread which does the following:

ScopeLocal
    .where(C, "value for C")
    .run(() -> {
        System.out.printf("A:%s B:%s C:%s\n", A.isBound(), 
B.isBound(), C.isBound());

        ScopeLocal.runWithSnapshot​(() -> {
            System.out.printf("A:%s B:%s C:%s\n", A.isBound(), 
B.isBound(), C.isBound());

        }, snapshot);
        System.out.printf("A:%s B:%s C:%s\n", A.isBound(), 
B.isBound(), C.isBound());

    });

What would this code print?

...in other words, does runWithSnapshot replace the whole set of bound 
values or does it merge it with existing set? 



...let me answer this myself, after checking current implementation. The 
answer is: "It depends on whether C is an inheritable ScopeLocal or 
non-inheritable". If I understand the code correctly there are two 
independent sets of bindings: inheritable and non-inheritable. 
snapshot() retrieves the current inheritable set and runWithSnapshot​() 
replaces current inheriatable set with the snapshot for the execution of 
given Runnable and afterwards swaps previous inheritable set back.


So if C is inheritable, the output would be:

A:false B:false C:true
A:true B:true C:false
A:false B:false C:true

...but if C is non-inheritable, the output would be:

A:false B:false C:true
A:true B:true C:true
A:false B:false C:true

This seems consistent. In other words, non-inheritable bindings are 
never transferred from thread to thread automatically or by 
snapshot/runWithSnapshot. I can see that snapshot/runWithSnapshot was 
meant as a mechanism to "simulate" inheritance of bindings when 
execution is transferred from one thread to another which is not a newly 
started child thread.


Regards, Peter

Peter

Re: RFR: 8266851: Implement JEP 403: Strongly Encapsulate JDK Internals [v4]

[Mandy Chung]

On Tue, 18 May 2021 23:01:05 GMT, Mark Reinhold  wrote:

>> Please review this implementation of JEP 403 
>> (https://openjdk.java.net/jeps/403).
>> Alan Bateman is the original author of almost all of it.  Passes tiers 1-3 
>> on 
>> {linux,macos,windows}-x64 and {linux,macos}-aarch64.
>
> Mark Reinhold has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix up IllegalAccessTest

Marked as reviewed by mchung (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/4015

Re: RFR: 8266846: Add java.time.InstantSource [v2]

[Naoto Sato]

On Tue, 18 May 2021 23:22:49 GMT, Stephen Colebourne  
wrote:

>> test/jdk/java/time/test/java/time/TestInstantSource.java line 86:
>> 
>>> 84: assertEquals(test.instant(), instant);
>>> 85: assertSame(test.equals(InstantSource..fixed(instant));
>>> 86: assertEquals(test.hashCode(), 
>>> InstantSource..fixed(instant).hashCode());
>> 
>> Not sure this would compile.
>
> I was expecting the GitHub Action to pickup coding and test issues (since my 
> dev setup can't compile or run tests). But it seems it doesn't. do that. The 
> latest commit should at least compile as I copied the test class to another 
> IDE location, but I have no way of knowing if the tests pass.

GitHub Actions just verifies build and tier1 testing, while java.time tests are 
in tier2, so it won't catch the failure. The test case still does not compile 
without the `import java.time.InstantSource` statement. I've verified the test 
passed with that fix.

-

PR: https://git.openjdk.java.net/jdk/pull/4016

Re: RFR: 8266846: Add java.time.InstantSource [v3]

[Naoto Sato]

On Tue, 18 May 2021 23:18:42 GMT, Stephen Colebourne  
wrote:

>> 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

test/jdk/java/time/test/java/time/TestInstantSource.java line 33:

> 31: import java.time.Duration;
> 32: import java.time.Instant;
> 33: import java.time.ZoneId;

Needs `import java.time.InstantSource;`

-

PR: https://git.openjdk.java.net/jdk/pull/4016

Re: RFR: 8266310: deadlock while loading the JNI code [v2]

[Aleksei Voitylov]

On Wed, 19 May 2021 16:21:41 GMT, Aleksei Voitylov  
wrote:

>> Please review this PR which fixes the deadlock in ClassLoader between the 
>> two lock objects - a lock object associated with the class being loaded, and 
>> the ClassLoader.loadedLibraryNames hash map, locked during the native 
>> library load operation.
>> 
>> Problem being fixed:
>> 
>> The initial reproducer demonstrated a deadlock between the JarFile/ZipFile 
>> and the hash map. That deadlock exists even when the ZipFile/JarFile lock is 
>> removed because there's another lock object in the class loader, associated 
>> with the name of the class being loaded. Such objects are stored in 
>> ClassLoader.parallelLockMap. The deadlock occurs when JNI_OnLoad() loads 
>> exactly the same class, whose signature is being verified in another thread.
>> 
>> Proposed fix:
>> 
>> The proposed patch suggests to get rid of locking loadedLibraryNames hash 
>> map and synchronize on each entry name, as it's done with class names in see 
>> ClassLoader.getClassLoadingLock(name) method.
>> 
>> The patch introduces nativeLibraryLockMap which holds the lock objects for 
>> each library name, and the getNativeLibraryLock() private method is used to 
>> lazily initialize the corresponding lock object. nativeLibraryContext was 
>> changed to ThreadLocal, so that in any concurrent thread it would have a 
>> NativeLibrary object on top of the stack, that's being currently 
>> loaded/unloaded in that thread. nativeLibraryLockMap accumulates the names 
>> of all native libraries loaded - in line with class loading code, it is not 
>> explicitly cleared.
>> 
>> Testing:  jtreg and jck testing with no regressions. A new regression test 
>> was developed.
>
> Aleksei Voitylov has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   address review comments, add tests

Dear colleagues,

The updated PR addresses review comment regarding ThreadLocal as well as David' 
concern around the lock being held during JNI_OnLoad/JNI_OnUnload calls, and 
ensures all lock objects are deallocated. Multiple threads are allowed to enter 
NativeLibrary.load() to prevent any thread from locking while another thread 
loads a library. Before the update, there could be a class loading lock held by 
a parallel capable class loader, which can deadlock with the library loading 
lock. As proposed by David Holmes, the library loading lock was removed because 
dlopen/LoadLibrary are thread safe and they maintain internal reference 
counters on libraries. There's still a lock being held while a pair of 
containers are read/updated. It's not going to deadlock as there's no lock/wait 
operation performed while that lock is held. Multiple threads may create their 
own copies of NativeLibrary object and register it for auto unloading. 

Tests for auto unloading were added along with the PR update. There are now 3 
jtreg tests:
- one checks for deadlock, similar to the one proposed by Chris Hegarty
- two other tests are for library unload. 

The major side effect of that multiple threads are allowed to enter is that 
JNI_OnLoad/JNI_OnUnload may be called multiple (but same) number of times from 
concurrent threads. In particular, the number of calls to JNI_OnLoad must be 
equal to the number of calls to JNI_OnUnload after the relevant class loader is 
garbage collected. This may affect the behaviour that relies on specific order 
or the number of JNI_OnLoad/JNI_OnUnload calls. The current JNI specification 
does not mandate how many times JNI_OnLoad/JNI_OnUnload are called. Also, we 
could not locate tests in jck/jtreg/vmTestbase that would rely on the specific 
order or number of calls to JNI_OnLoad/JNI_OnUnload. 

Thank you Alan Bateman, David Holmes and Chris Hegarty for your valuable input.

-

PR: https://git.openjdk.java.net/jdk/pull/3976

Re: RFR: 8266310: deadlock while loading the JNI code [v3]

[Aleksei Voitylov]

> Please review this PR which fixes the deadlock in ClassLoader between the two 
> lock objects - a lock object associated with the class being loaded, and the 
> ClassLoader.loadedLibraryNames hash map, locked during the native library 
> load operation.
> 
> Problem being fixed:
> 
> The initial reproducer demonstrated a deadlock between the JarFile/ZipFile 
> and the hash map. That deadlock exists even when the ZipFile/JarFile lock is 
> removed because there's another lock object in the class loader, associated 
> with the name of the class being loaded. Such objects are stored in 
> ClassLoader.parallelLockMap. The deadlock occurs when JNI_OnLoad() loads 
> exactly the same class, whose signature is being verified in another thread.
> 
> Proposed fix:
> 
> The proposed patch suggests to get rid of locking loadedLibraryNames hash map 
> and synchronize on each entry name, as it's done with class names in see 
> ClassLoader.getClassLoadingLock(name) method.
> 
> The patch introduces nativeLibraryLockMap which holds the lock objects for 
> each library name, and the getNativeLibraryLock() private method is used to 
> lazily initialize the corresponding lock object. nativeLibraryContext was 
> changed to ThreadLocal, so that in any concurrent thread it would have a 
> NativeLibrary object on top of the stack, that's being currently 
> loaded/unloaded in that thread. nativeLibraryLockMap accumulates the names of 
> all native libraries loaded - in line with class loading code, it is not 
> explicitly cleared.
> 
> Testing:  jtreg and jck testing with no regressions. A new regression test 
> was developed.

Aleksei Voitylov has updated the pull request incrementally with one additional 
commit since the last revision:

  fix trailing whitespace

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3976/files
  - new: https://git.openjdk.java.net/jdk/pull/3976/files/a0b2163a..8f47d890

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3976=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3976=01-02

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3976.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3976/head:pull/3976

PR: https://git.openjdk.java.net/jdk/pull/3976

Re: RFR: 8266310: deadlock while loading the JNI code

[Aleksei Voitylov]

Hi David, Mandy,

In the updated PR I removed the lock held during the load/unload calls.
Our testing confirmed that without that removal the deadlock can easily
be reproduced, even without signed jars. Now the lock is only held to
prevent races during access to "libraries" and "loadedLibraryNames".

Thank you,
-Aleksei

On 14/05/2021 03:28, David Holmes wrote:
> On 14/05/2021 7:20 am, Mandy Chung wrote:
>>
>>
>> On 5/13/21 6:05 AM, David Holmes wrote:
>>> Not every problem has a solution :) If JNI_OnLoad has to execute
>>> atomically with respect to loading a library then there will always
>>> be a deadlock potential. The only complete solution would not hold a
>>> lock while JNI_OnLoad is executed, but that completely changes the
>>> semantics of native library loading. 
>>
>> I have been giving some thought on this issue.  I agree with David
>> that we should look into a solution not holding a lock while
>> JNI_OnLoad is executed.  It might be possible to put all threads that
>> trying to load the same native library that is being loaded to wait
>> and only one thread is loading it and executing JNI_OnLoad (e.g. add
>> a state in NativeLibrary to indicate it is being loaded, already
>> loaded, being unloaded).  I haven't had the cycle to think through it
>> in details though.
>
> Any mechanism that blocks threads from accessing the library while
> another thread is performing JNI_OnLoad, suffers from the same
> deadlock problem.
>
> David
>
>> Mandy

Withdrawn: 8258588: MD5 MessageDigest in java.util.UUID should be cached

[duke]

On Thu, 17 Dec 2020 13:36:17 GMT, PROgrm_JARvis 
 wrote:

> Please review this change moving lookup of MD5 digest in `java.lang.UUID` to 
> an internal holder class.

This pull request has been closed without being integrated.

-

PR: https://git.openjdk.java.net/jdk/pull/1821

Re: RFR: 8266310: deadlock while loading the JNI code [v2]

[Aleksei Voitylov]

> Please review this PR which fixes the deadlock in ClassLoader between the two 
> lock objects - a lock object associated with the class being loaded, and the 
> ClassLoader.loadedLibraryNames hash map, locked during the native library 
> load operation.
> 
> Problem being fixed:
> 
> The initial reproducer demonstrated a deadlock between the JarFile/ZipFile 
> and the hash map. That deadlock exists even when the ZipFile/JarFile lock is 
> removed because there's another lock object in the class loader, associated 
> with the name of the class being loaded. Such objects are stored in 
> ClassLoader.parallelLockMap. The deadlock occurs when JNI_OnLoad() loads 
> exactly the same class, whose signature is being verified in another thread.
> 
> Proposed fix:
> 
> The proposed patch suggests to get rid of locking loadedLibraryNames hash map 
> and synchronize on each entry name, as it's done with class names in see 
> ClassLoader.getClassLoadingLock(name) method.
> 
> The patch introduces nativeLibraryLockMap which holds the lock objects for 
> each library name, and the getNativeLibraryLock() private method is used to 
> lazily initialize the corresponding lock object. nativeLibraryContext was 
> changed to ThreadLocal, so that in any concurrent thread it would have a 
> NativeLibrary object on top of the stack, that's being currently 
> loaded/unloaded in that thread. nativeLibraryLockMap accumulates the names of 
> all native libraries loaded - in line with class loading code, it is not 
> explicitly cleared.
> 
> Testing:  jtreg and jck testing with no regressions. A new regression test 
> was developed.

Aleksei Voitylov has updated the pull request incrementally with one additional 
commit since the last revision:

  address review comments, add tests

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3976/files
  - new: https://git.openjdk.java.net/jdk/pull/3976/files/113ee0f3..a0b2163a

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3976=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3976=00-01

  Stats: 560 lines in 9 files changed: 441 ins; 41 del; 78 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3976.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3976/head:pull/3976

PR: https://git.openjdk.java.net/jdk/pull/3976

Re: jpackage java commands

[Michael Hall]

> On May 19, 2021, at 9:40 AM, Andy Herrick  wrote:
> 
> I don't think jlink will ever include the jdk development tools in a custom 
> runtime (using jlink on windows or mac without --strip-native-commands only 
> gives me java and keytool),

Mac gave me the 9 commands shown earlier. Something inconsistent involved?

> but you can include the whole jdk (less src.zip and the jmods directory) by 
> using --runtime-image  and pointing  to the JDK you 
> are running jpackage from (or you can take copy of that JDK, modify it in any 
> way you want, and point to that)).

I was trying a couple of these options for something unrelated last night. Just 
a quick look not extensive. But it seemed like if you copy —runtime-image you 
are limited in making some modular changes to that runtime. I also tried 
directly using jlink to make the runtime myself but that also errored. I didn’t 
make any effort to figure out the problem at that time. A linkage error of some 
sort in class initialization for my launcher class. 
Manually changing a copied runtime I didn’t try. You could then just copy the 
commands you need to the jpackage runtime if the application is unsigned? I 
used to do something similar for app’s that needed an embedded ‘java’ command 
available. But got scolded for doing so when I mentioned it on this list. 

If the functionality is critical to the app I will probably have to make one of 
those options work. Currently it isn’t critical. So I guess I could just add an 
availability check.

Thanks.

Withdrawn: 8260710: Inline and simplify String*::{coder, value, isLatin1} methods

[duke]

On Mon, 1 Feb 2021 13:11:52 GMT, Aleksey Shipilev  wrote:

> Since Compact Strings implementation, there are simple methods in String and 
> StringBuilders: `coder()`, `value()`, `isLatin1()`. They are mostly there to 
> capture `COMPACT_STRINGS` flag that would fold to "false" when compact 
> strings are disabled with `-XX:-CompactStrings`.
> 
> This was to guarantee the same performance when Compact String are 
> unimplemented and/or have surprising performance behaviors. Quite some time 
> had passed since JDK 9 release, and we can consider simplifying these. For 
> example, ignoring `COMPACT_STRINGS` and inlining the methods simplifies the 
> code. Some new code added to `String` after JDK 9 already has these methods 
> inlined, checking `coder` directly.
> 
> The major drawback is that supplying `-XX:-CompactStrings` would have to go 
> through the same `coder == UTF16` paths the "normal" Compact Strings are 
> going. It would not be folded to the most optimal form by the optimizer, 
> because `coder` field value is generally opaque to optimizers.
> 
> On the flip side, there is an advantage on paths that do not have optimizer 
> ready (yet), for example at startup. Hello World startup improves for about 
> 0.3 msec (or ~1.3%). The improvement is quite probably more visible on larger 
> workloads, i.e. large application servers. (In fact, this is where I spotted 
> this opportunity: OpenLiberty startup profiles).
> 
> Interpreter-only "Hello World":
> 
> 
>  Performance counter stats for 'taskset -c 13 build/baseline/bin/java 
> -Xms128m -Xmx128m -Xint Hello' (5000 runs):
> 
>  23.40 msec task-clock#0.933 CPUs utilized
> ( +-  0.01% )
> 86,133,190  cycles#3.680 GHz  
> ( +-  0.01% )
> 79,854,588  instructions  #0.93  insn per cycle   
> ( +-  0.00% )
> 
> 0.02507105 +- 0.0343 seconds time elapsed  ( +-  0.01% )
> 
>  Performance counter stats for 'taskset -c 13 
> build/linux-x86_64-server-release/images/jdk/bin/java -Xms128m -Xmx128m -Xint 
> Hello' (5000 runs):
> 
>  23.07 msec task-clock#0.935 CPUs utilized
> ( +-  0.01% )
> 84,877,118  cycles#3.679 GHz  
> ( +-  0.01% )
> 79,231,598  instructions  #0.93  insn per cycle   
> ( +-  0.00% )
> 
> 0.02466467 +- 0.0382 seconds time elapsed  ( +-  0.02% )
> 
> 
> 
> OpenLiberty startup:
> 
> 
>  Before: 2.296, 2.281, 2.291 (seconds)
>  After:  2.254, 2.267, 2.272 (seconds)
> 
> 
> Additional tests:
>  - [x] Linux `tier1` default (passes)
>  - [x] Linux `tier1` `-XX:-CompactStrings`

This pull request has been closed without being integrated.

-

PR: https://git.openjdk.java.net/jdk/pull/2334

Re: RFR: 8266835: Add a --validate option to the jar tool [v2]

[Jorn Vernee]

> This patch adds a `--validate` option to the jar tool which can be used to 
> validate a jar file that might be malformed. For instance, if a jar is a 
> multi-release jar, it is malformed if different versions expose different 
> APIs.
> 
> The implementation is straight forward since there already exists validation 
> logic that is run when creating or updating a jar. This patch just exposes 
> that logic directly under a new command line flag.
> 
> I've enhanced the existing ApiValidatorTest to also create malformed jars 
> using the zip file APIs (the jar tool does not output malformed jars) and run 
> them through `jar --validate`.
> 
> Note that while the jdk's jar tool does not output malformed jars, 
> third-party archiving tools might, or the jar could have been manually edited.
> 
> Some prior discussion here: 
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-May/077420.html
> 
> Testing: running jdk/tools/jar test suite locally, tier 1-3 (in progress), 
> manual testing.

Jorn Vernee has updated the pull request incrementally with one additional 
commit since the last revision:

  Update error message

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3971/files
  - new: https://git.openjdk.java.net/jdk/pull/3971/files/3f68ad6c..cbd6e81b

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3971=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3971=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3971.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3971/head:pull/3971

PR: https://git.openjdk.java.net/jdk/pull/3971

Re: RFR: 8203359: Container level resources events [v13]

[Severin Gehwolf]

On Wed, 19 May 2021 15:23:55 GMT, Jaroslav Bachorik  
wrote:

>> With this change it becomes possible to surface various cgroup level metrics 
>> (available via `jdk.internal.platform.Metrics`) as JFR events.
>> 
>> Only a subset of the metrics exposed by `jdk.internal.platform.Metrics` is 
>> turned into JFR events to start with.
>> * CPU related metrics
>> * Memory related metrics
>> * I/O related metrics
>> 
>> For each of those subsystems a configuration data will be emitted as well. 
>> The initial proposal is to emit the configuration data events at least once 
>> per chunk and the metrics values at 30 seconds interval. 
>> By using these values the emitted events seem to contain useful information 
>> without increasing overhead (the metrics values are read from `/proc` 
>> filesystem so that should not be done too frequently).
>
> Jaroslav Bachorik has refreshed the contents of this pull request, and 
> previous commits have been removed. The incremental views will show 
> differences compared to the previous content of the PR. The pull request 
> contains one new commit since the last revision:
> 
>   Fix event metadata

LGTM

-

Marked as reviewed by sgehwolf (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3126

Re: RFR: 8203359: Container level resources events [v12]

[Jaroslav Bachorik]

On Wed, 19 May 2021 14:11:40 GMT, Jaroslav Bachorik  
wrote:

>> With this change it becomes possible to surface various cgroup level metrics 
>> (available via `jdk.internal.platform.Metrics`) as JFR events.
>> 
>> Only a subset of the metrics exposed by `jdk.internal.platform.Metrics` is 
>> turned into JFR events to start with.
>> * CPU related metrics
>> * Memory related metrics
>> * I/O related metrics
>> 
>> For each of those subsystems a configuration data will be emitted as well. 
>> The initial proposal is to emit the configuration data events at least once 
>> per chunk and the metrics values at 30 seconds interval. 
>> By using these values the emitted events seem to contain useful information 
>> without increasing overhead (the metrics values are read from `/proc` 
>> filesystem so that should not be done too frequently).
>
> Jaroslav Bachorik has updated the pull request with a new target base due to 
> a merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains 11 additional 
> commits since the last revision:
> 
>  - Fix event metadata
>  - Small fixes
>  - Remove trailing spaces
>  - Doh
>  - Report container type and register events conditionally
>  - Remove unused test files
>  - Initial test support for JFR container events
>  - Update the JFR control files
>  - Split off the CPU throttling metrics
>  - Formatting spaces
>  - ... and 1 more: 
> https://git.openjdk.java.net/jdk/compare/a32c4b70...c3fa274c

Thanks for the review!
I've fixed the outstanding test failures and the patch is in its final form.

-

PR: https://git.openjdk.java.net/jdk/pull/3126

Re: RFR: 8203359: Container level resources events [v13]

[Jaroslav Bachorik]

> With this change it becomes possible to surface various cgroup level metrics 
> (available via `jdk.internal.platform.Metrics`) as JFR events.
> 
> Only a subset of the metrics exposed by `jdk.internal.platform.Metrics` is 
> turned into JFR events to start with.
> * CPU related metrics
> * Memory related metrics
> * I/O related metrics
> 
> For each of those subsystems a configuration data will be emitted as well. 
> The initial proposal is to emit the configuration data events at least once 
> per chunk and the metrics values at 30 seconds interval. 
> By using these values the emitted events seem to contain useful information 
> without increasing overhead (the metrics values are read from `/proc` 
> filesystem so that should not be done too frequently).

Jaroslav Bachorik has refreshed the contents of this pull request, and previous 
commits have been removed. The incremental views will show differences compared 
to the previous content of the PR. The pull request contains one new commit 
since the last revision:

  Fix event metadata

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3126/files
  - new: https://git.openjdk.java.net/jdk/pull/3126/files/c3fa274c..e0f5855b

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3126=12
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3126=11-12

  Stats: 8 lines in 1 file changed: 8 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3126.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3126/head:pull/3126

PR: https://git.openjdk.java.net/jdk/pull/3126

Integrated: 8267321: Use switch expression for VarHandle$AccessMode lookup

[Claes Redestad]

On Tue, 18 May 2021 20:59:49 GMT, Claes Redestad  wrote:

> Using a switch expression instead of a (read-only) static `HashMap` reduces 
> initialization overhead of `VarHandle$AccessMode`. This gets loaded earlier 
> after JDK-8265079, so it started showing up in a few lambda startup tests.
> 
> This also obsoletes a jtreg test that only verified that this map was 
> optimally sized.

This pull request has now been integrated.

Changeset: 9760dba7
Author:Claes Redestad 
URL:   
https://git.openjdk.java.net/jdk/commit/9760dba71c07cf7b0df16590b3e84e23ad587621
Stats: 90 lines in 2 files changed: 31 ins; 56 del; 3 mod

8267321: Use switch expression for VarHandle$AccessMode lookup

Reviewed-by: jvernee

-

PR: https://git.openjdk.java.net/jdk/pull/4102

Re: RFR: 8267321: Use switch expression for VarHandle$AccessMode lookup

[Claes Redestad]

On Wed, 19 May 2021 14:31:48 GMT, Jorn Vernee  wrote:

> LGTM. I assume existing tests verify that all values of the enum are covered 
> by the switch?

Thanks! Yes, the method with the switch is called when linking a VH, and the 
tests should be exhaustive in that regard.

-

PR: https://git.openjdk.java.net/jdk/pull/4102

Re: jpackage java commands

[Andy Herrick]

I don't think jlink will ever include the jdk development tools in a 
custom runtime (using jlink on windows or mac without 
--strip-native-commands only gives me java and keytool), but you can 
include the whole jdk (less src.zip and the jmods directory) by using 
--runtime-image  and pointing  to the JDK you 
are running jpackage from (or you can take copy of that JDK, modify it 
in any way you want, and point to that)).


/Andy

On 5/19/2021 10:00 AM, Michael Hall wrote:

I added
--jlink-options '--strip-debug --no-man-pages --no-header-files’
to my invocation, avoiding --strip-native-commands,  so that the native java 
commands are not stripped. I believe currently recommended jpackage way to do 
this.
I get the following commands in my embedded JDK

javajdb jshell  rmidserialver
javac   jrunscript  keytool rmiregistry

The installed jdk bin directory has 29 different commands including ones like 
jpackage, jlink, javap, jcmd…

What if I want to run one of those from the application? I can exec them but 
the version might not match the embedded?

Not a current problem that I know of, I will use the installed for now.

   

Re: RFR: 8267321: Use switch expression for VarHandle$AccessMode lookup

[Jorn Vernee]

On Tue, 18 May 2021 20:59:49 GMT, Claes Redestad  wrote:

> Using a switch expression instead of a (read-only) static `HashMap` reduces 
> initialization overhead of `VarHandle$AccessMode`. This gets loaded earlier 
> after JDK-8265079, so it started showing up in a few lambda startup tests.
> 
> This also obsoletes a jtreg test that only verified that this map was 
> optimally sized.

LGTM. I assume existing tests verify that all values of the enum are covered by 
the switch?

-

Marked as reviewed by jvernee (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/4102

Re: RFR: 8266835: Add a --validate option to the jar tool

[Jorn Vernee]

On Tue, 11 May 2021 10:51:18 GMT, Jorn Vernee  wrote:

> This patch adds a `--validate` option to the jar tool which can be used to 
> validate a jar file that might be malformed. For instance, if a jar is a 
> multi-release jar, it is malformed if different versions expose different 
> APIs.
> 
> The implementation is straight forward since there already exists validation 
> logic that is run when creating or updating a jar. This patch just exposes 
> that logic directly under a new command line flag.
> 
> I've enhanced the existing ApiValidatorTest to also create malformed jars 
> using the zip file APIs (the jar tool does not output malformed jars) and run 
> them through `jar --validate`.
> 
> Note that while the jdk's jar tool does not output malformed jars, 
> third-party archiving tools might, or the jar could have been manually edited.
> 
> Some prior discussion here: 
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-May/077420.html
> 
> Testing: running jdk/tools/jar test suite locally, tier 1-3 (in progress), 
> manual testing.

CSR here: https://bugs.openjdk.java.net/browse/JDK-8267402

-

PR: https://git.openjdk.java.net/jdk/pull/3971

RFR: 8266835: Add a --validate option to the jar tool

[Jorn Vernee]

This patch adds a `--validate` option to the jar tool which can be used to 
validate a jar file that might be malformed. For instance, if a jar is a 
multi-release jar, it is malformed if different versions expose different APIs.

The implementation is straight forward since there already exists validation 
logic that is run when creating or updating a jar. This patch just exposes that 
logic directly under a new command line flag.

I've enhanced the existing ApiValidatorTest to also create malformed jars using 
the zip file APIs (the jar tool does not output malformed jars) and run them 
through `jar --validate`.

Note that while the jdk's jar tool does not output malformed jars, third-party 
archiving tools might, or the jar could have been manually edited.

Some prior discussion here: 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-May/077420.html

Testing: running jdk/tools/jar test suite locally, tier 1-3 (in progress), 
manual testing.

-

Commit messages:
 - - Remove spurious test imports
 - Add tests
 - Change validate option wording to not mention multi-release jars
 - Add jar --validate

Changes: https://git.openjdk.java.net/jdk/pull/3971/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3971=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8266835
  Stats: 146 lines in 4 files changed: 114 ins; 4 del; 28 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3971.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3971/head:pull/3971

PR: https://git.openjdk.java.net/jdk/pull/3971

Re: RFR: 8203359: Container level resources events [v12]

[Jaroslav Bachorik]

> With this change it becomes possible to surface various cgroup level metrics 
> (available via `jdk.internal.platform.Metrics`) as JFR events.
> 
> Only a subset of the metrics exposed by `jdk.internal.platform.Metrics` is 
> turned into JFR events to start with.
> * CPU related metrics
> * Memory related metrics
> * I/O related metrics
> 
> For each of those subsystems a configuration data will be emitted as well. 
> The initial proposal is to emit the configuration data events at least once 
> per chunk and the metrics values at 30 seconds interval. 
> By using these values the emitted events seem to contain useful information 
> without increasing overhead (the metrics values are read from `/proc` 
> filesystem so that should not be done too frequently).

Jaroslav Bachorik has updated the pull request with a new target base due to a 
merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains 11 additional commits 
since the last revision:

 - Fix event metadata
 - Small fixes
 - Remove trailing spaces
 - Doh
 - Report container type and register events conditionally
 - Remove unused test files
 - Initial test support for JFR container events
 - Update the JFR control files
 - Split off the CPU throttling metrics
 - Formatting spaces
 - ... and 1 more: https://git.openjdk.java.net/jdk/compare/2f8f5ce3...c3fa274c

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3126/files
  - new: https://git.openjdk.java.net/jdk/pull/3126/files/fddd1727..c3fa274c

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3126=11
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3126=10-11

  Stats: 638229 lines in 6576 files changed: 92464 ins; 530978 del; 14787 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3126.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3126/head:pull/3126

PR: https://git.openjdk.java.net/jdk/pull/3126

Re: RFR: 8267110: Update java.util to use instanceof pattern variable [v2]

[Patrick Concannon]

> Hi,
> 
> Could someone please review my code for updating the code in the `java.util` 
> package to make use of the `instanceof` pattern variable?
> 
> Kind regards,
> Patrick

Patrick Concannon has updated the pull request with a new target base due to a 
merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains three additional 
commits since the last revision:

 - 8267110: Reverted changes in java/util/Formatter as primitive to boxed types 
may have semantic/performance implications
 - Merge remote-tracking branch 'origin/master' into JDK-8267110
 - 8267110: Update java.util to use instanceof pattern variable

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4088/files
  - new: https://git.openjdk.java.net/jdk/pull/4088/files/0ed0bd99..cd99dc49

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4088=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4088=00-01

  Stats: 4003 lines in 175 files changed: 2590 ins; 890 del; 523 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4088.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4088/head:pull/4088

PR: https://git.openjdk.java.net/jdk/pull/4088

jpackage java commands

[Michael Hall]

I added 
--jlink-options '--strip-debug --no-man-pages --no-header-files’
to my invocation, avoiding --strip-native-commands,  so that the native java 
commands are not stripped. I believe currently recommended jpackage way to do 
this.
I get the following commands in my embedded JDK

javajdb jshell  rmidserialver
javac   jrunscript  keytool rmiregistry

The installed jdk bin directory has 29 different commands including ones like 
jpackage, jlink, javap, jcmd…

What if I want to run one of those from the application? I can exec them but 
the version might not match the embedded?

Not a current problem that I know of, I will use the installed for now.

  

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

[Weijun Wang]

> 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.
> 
> Update: the deprecation annotations and javadoc tags, build, compiler, 
> core-libs, hotspot, i18n, jmx, net, nio, security, and serviceability are 
> reviewed. Rest are 2d, awt, beans, sound, and swing.

Weijun Wang has updated the pull request incrementally with one additional 
commit since the last revision:

  fixing awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4073/files
  - new: https://git.openjdk.java.net/jdk/pull/4073/files/bb73093a..c4221b5f

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4073=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4073=01-02

  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 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

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v2]

[Weijun Wang]

On Tue, 18 May 2021 21:21:45 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.
>> 
>> Update: the deprecation annotations and javadoc tags, build, compiler, 
>> core-libs, hotspot, i18n, jmx, net, nio, security, and serviceability are 
>> reviewed. Rest are 2d, awt, beans, sound, and swing.
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   feedback from Sean, Phil and Alan

A new commit fixing `awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java` 
test in tier4. The test compares stderr output with a known value but we have a 
new warning written to stderr now. It's now using stdout instead. @prrace, 
Please confirm this is acceptable. Thanks.

-

PR: https://git.openjdk.java.net/jdk/pull/4073

Re: RFR: 8266851: Implement JEP 403: Strongly Encapsulate JDK Internals [v4]

[Harold Seigel]

On Tue, 18 May 2021 23:01:05 GMT, Mark Reinhold  wrote:

>> Please review this implementation of JEP 403 
>> (https://openjdk.java.net/jeps/403).
>> Alan Bateman is the original author of almost all of it.  Passes tiers 1-3 
>> on 
>> {linux,macos,windows}-x64 and {linux,macos}-aarch64.
>
> Mark Reinhold has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix up IllegalAccessTest

Hotspot changes look good.
Thanks, Harold

-

Marked as reviewed by hseigel (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/4015

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v23]

[Maurizio Cimadamore]

> 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 VaList test
  Remove unused code in Utils

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3699/files
  - new: https://git.openjdk.java.net/jdk/pull/3699/files/f6051daf..f5668ffc

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3699=22
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3699=21-22

  Stats: 6 lines in 2 files changed: 0 ins; 5 del; 1 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: 8203359: Container level resources events [v11]

[Jaroslav Bachorik]

On Wed, 19 May 2021 10:00:04 GMT, Severin Gehwolf  wrote:

>> Jaroslav Bachorik has updated the pull request with a new target base due to 
>> a merge or a rebase. The pull request now contains 10 commits:
>> 
>>  - Small fixes
>>  - Remove trailing spaces
>>  - Doh
>>  - Report container type and register events conditionally
>>  - Remove unused test files
>>  - Initial test support for JFR container events
>>  - Update the JFR control files
>>  - Split off the CPU throttling metrics
>>  - Formatting spaces
>>  - 8203359: Container level resources events
>
> src/jdk.jfr/share/classes/jdk/jfr/events/ContainerMemoryUsageEvent.java line 
> 48:
> 
>> 46: @Description("(attempts per second * 1000), if enabled, that the 
>> operating system tries to satisfy a memory request for any " +
>> 47:  "process in the current container when no free memory 
>> is readily available.")
>> 48: public double memoryPressure;
> 
> Should this `memoryPressure` field go from `ContainerMemoryUsageEvent` class? 
> It's not set anywhere is it? would be cgroup v1 only api so I'm not sure it 
> should be there for a generic event like this.

Yes. Removing.

-

PR: https://git.openjdk.java.net/jdk/pull/3126

Re: RFR: 8203359: Container level resources events [v11]

[Severin Gehwolf]

On Mon, 3 May 2021 13:16:06 GMT, Jaroslav Bachorik  
wrote:

>> With this change it becomes possible to surface various cgroup level metrics 
>> (available via `jdk.internal.platform.Metrics`) as JFR events.
>> 
>> Only a subset of the metrics exposed by `jdk.internal.platform.Metrics` is 
>> turned into JFR events to start with.
>> * CPU related metrics
>> * Memory related metrics
>> * I/O related metrics
>> 
>> For each of those subsystems a configuration data will be emitted as well. 
>> The initial proposal is to emit the configuration data events at least once 
>> per chunk and the metrics values at 30 seconds interval. 
>> By using these values the emitted events seem to contain useful information 
>> without increasing overhead (the metrics values are read from `/proc` 
>> filesystem so that should not be done too frequently).
>
> Jaroslav Bachorik has updated the pull request with a new target base due to 
> a merge or a rebase. The pull request now contains 10 commits:
> 
>  - Small fixes
>  - Remove trailing spaces
>  - Doh
>  - Report container type and register events conditionally
>  - Remove unused test files
>  - Initial test support for JFR container events
>  - Update the JFR control files
>  - Split off the CPU throttling metrics
>  - Formatting spaces
>  - 8203359: Container level resources events

Changes requested by sgehwolf (Reviewer).

src/jdk.jfr/share/classes/jdk/jfr/events/ContainerMemoryUsageEvent.java line 48:

> 46: @Description("(attempts per second * 1000), if enabled, that the 
> operating system tries to satisfy a memory request for any " +
> 47:  "process in the current container when no free memory is 
> readily available.")
> 48: public double memoryPressure;

Should this `memoryPressure` field go from `ContainerMemoryUsageEvent` class? 
It's not set anywhere is it? would be cgroup v1 only api so I'm not sure it 
should be there for a generic event like this.

-

PR: https://git.openjdk.java.net/jdk/pull/3126

Integrated: 8267357: build breaks with -Werror option on micro benchmark added for JDK-8256973

[Jatin Bhateja]

On Wed, 19 May 2021 08:20:13 GMT, Jatin Bhateja  wrote:

> Relevant declarations modified and tested with -Werror, no longer see 
> unchecked conversion warnings.
> 
> Kindly review and approve.

This pull request has now been integrated.

Changeset: 88b11423
Author:Jatin Bhateja 
URL:   
https://git.openjdk.java.net/jdk/commit/88b114235c5716ea43c55a9c4bc886bf5bcf4b42
Stats: 10 lines in 1 file changed: 0 ins; 1 del; 9 mod

8267357: build breaks with -Werror option on micro benchmark added for 
JDK-8256973

Reviewed-by: jiefu, neliasso, thartmann

-

PR: https://git.openjdk.java.net/jdk/pull/4108

Re: RFR: 8267357: build breaks with -Werror option on micro benchmark added for JDK-8256973

[Tobias Hartmann]

On Wed, 19 May 2021 08:20:13 GMT, Jatin Bhateja  wrote:

> Relevant declarations modified and tested with -Werror, no longer see 
> unchecked conversion warnings.
> 
> Kindly review and approve.

Marked as reviewed by thartmann (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/4108

Re: [External] : Re: JEP draft: Scope Locals

[Alan Bateman]

On 19/05/2021 10:15, Andrew Haley wrote:

:
Yes, that's true. I think that what you have in mind is a more elaborate
mechanism than what is proposed here, which does no more than bind values
to names over a scope. There needs to be more discussion in the JEP of
what this proposal isn't intended to do, and how we might cope with
mutability of a scope local's values.

Also just to add that the closable-resource in the example is being used 
on concurrent threads so it already has to cope with attempted usage 
after close or async close (close while in use). Also once we combine 
this with SC then we can be sure that the child task has completed 
before closing the resource.


-Alan

RE: New java.util.Strings class

[Alberto Otero Rodríguez]

Hi,

I have made some changes to the Strings class I proposed in my previous message.

The changes are:

  *   Use str.isEmpty() instead of EMPTY_STRING.equals(str)
  *   Create methods using str.strip() to check a String is not Whitespace

You can see the new code here:
https://github.com/Aliuken/Java-Strings/blob/main/Strings.java

With those changes, the following annotations could also be created for method 
arguments and return types:
- @NonNullNorWhiteSpace
- @NonNullNorWhiteSpaceElse(defaultValue)
- @NonNullNorWhiteSpaceElseGet(class::supplierMethod)

I didn't have any response to the previous message.

Please, take this one in consideration.

Regards,

Alberto Otero Rodríguez.


De: Alberto Otero Rodríguez
Enviado: lunes, 17 de mayo de 2021 14:58
Para: core-libs-dev@openjdk.java.net 
Asunto: 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: 8266936: Add a finalization JFR event [v2]

[Chris Hegarty]

On Tue, 18 May 2021 22:41:06 GMT, Brent Christian  wrote:

>> Please review this enhancement to add a new JFR event, generated whenever a 
>> finalizer is run.
>> (The makeup is similar to the Deserialization event, 
>> [JDK-8261160](https://bugs.openjdk.java.net/browse/JDK-8261160).)
>> 
>> The event's only datum (beyond those common to all jfr events) is the class 
>> of the object that was finalized.
>> 
>> The Category for the event:
>> `"Java Virtual Machine" / "GC" / "Finalization"`
>> is what made sense to me, even though the event is generated from library 
>> code.
>> 
>> Along with the new regtest, I added a run mode to the basic finalizer test 
>> to enable jfr.
>> Automated testing looks good so far.
>> 
>> Thanks,
>> -Brent
>
> Brent Christian has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Test flag should be volatile

My 0.02$ ;-)  I really like the idea of a jdk.FinalizationStatistics event. The 
devil is (as always) in the details.

Multiple concerns have been raised above, which benefit from being separated 
out:

1) Raising an event per invocation of each individual finalizer may be costly 
when JFR is enabled (since there could be an extremely large number of these), 
as well as a little unwieldy for a human observer. Aggregating invocations of 
finalizers and reporting periodically seems like a nice solution to this.

2) A jdk.FinalizationStatistics event that provides an aggregate count of *all* 
finalizer invocations seems most straightforward, but less useful. A 
jdk.FinalizationStatistics event that provides per-class invocation metrics 
seems more useful, but at the expense of a more complex event structure. Maybe 
model jdk.FinalizationStatistics as a tuple of Class and long (count) - 
periodically committing multiple jdk.FinalizationStatistics events, one event 
per class? ( or was the thought to somehow aggregate all these per-class 
metrics into a single jdk.FinalizationStatistics event? )

3) If we keep the currently proposed semantic - capturing actual 
invocation/queueing counts (rather than registrations), then I see no reason 
why the implementation of a jdk.FinalizationStatistics needs to be in the JVM. 
The metrics can be captured in Java code and reported in a similar way to the 
container metrics (being proposed in [PR 3126][PR3126]). Surely, this would be 
more straightforward to implement and maintain, no?

4) The startup cost of JFR. I dunno enough about this, but what I do know is 
that a handler needs to be spun per Java-event, so maybe this has some bearing 
on the decision of no.3 above?

[PR3126]: https://github.com/openjdk/jdk/pull/3126

-

PR: https://git.openjdk.java.net/jdk/pull/4101

Re: [External] : Re: JEP draft: Scope Locals

[Andrew Haley]

On 5/18/21 3:19 AM, Mike Rettig wrote:
> With the current proposal I can't simply flush and close at the
> end of the scope because there might be a child scope that is still active.

Yes, that's true. I think that what you have in mind is a more elaborate
mechanism than what is proposed here, which does no more than bind values
to names over a scope. There needs to be more discussion in the JEP of
what this proposal isn't intended to do, and how we might cope with
mutability of a scope local's values.

-- 
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. 
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

Re: RFR: 8266936: Add a finalization JFR event [v2]

[Alan Bateman]

On Wed, 19 May 2021 07:50:55 GMT, Erik Gahlin  wrote:

> I wonder if there needs to be one event per finalized object?
> 
> Perhaps a counter per class would be as useful, i.e. 
> jdk.FinalizationStatistics, and if it could be implemented in the VM, without 
> other implications, that would be great.
> 
> Such an event could be enabled by default and provide much greater value than 
> an event that users would need to know about and configure themselves, which 
> 99,99% of all user will not do.

So some statistics or periodic event that N instances of class C were 
registered or queued? I think you are right that this would be a lot more 
valuable.

-

PR: https://git.openjdk.java.net/jdk/pull/4101

Re: RFR: 8267357: build breaks with -Werror option on micro benchmark added for JDK-8256973

[Nils Eliasson]

On Wed, 19 May 2021 08:20:13 GMT, Jatin Bhateja  wrote:

> Relevant declarations modified and tested with -Werror, no longer see 
> unchecked conversion warnings.
> 
> Kindly review and approve.

Looks good.

-

Marked as reviewed by neliasso (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/4108

Re: JEP draft: Scope Locals

[Andrew Haley]

On 5/15/21 6:50 PM, Peter Levart wrote:
> What if I wanted to create and start a thread that would be "pristine" - 
> not have any ScopeLocal value bound? Is this intentionally not allowed 
> in order to make sure that inheritable ScopeLocal(s) can't be cleared by 
> code that has no access to ScopeLocal instance(s)?

That one is about to be changed by a revision to the JEP. There clearly
is a need to control whether a newly-created thread inherits scope locals
or not. For instance, an Executor might lazily create worker threads, and
we don't want them to inherit scope locals from the thread that was
running.

-- 
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. 
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

Re: RFR: 8267357: build breaks with -Werror option on micro benchmark added for JDK-8256973

[Jie Fu]

On Wed, 19 May 2021 08:20:13 GMT, Jatin Bhateja  wrote:

> Relevant declarations modified and tested with -Werror, no longer see 
> unchecked conversion warnings.
> 
> Kindly review and approve.

LGTM

-

Marked as reviewed by jiefu (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/4108

Re: RFR: 8266846: Add java.time.InstantSource [v3]

[Peter Levart]

On Tue, 18 May 2021 23:14:53 GMT, Stephen Colebourne  
wrote:

>> src/java.base/share/classes/java/time/Clock.java line 487:
>> 
>>> 485: // it more unlikely to hit the 1ns in the future condition.
>>> 486: localOffset = System.currentTimeMillis()/1000 - 1024;
>>> 487: 
>> 
>> Is it possible that after a fresh localOffset is retrieved, the thread is 
>> preempted and when it is scheduled again after a pause, the 
>> getNanoTimeAdjustment below returns -1 ? Would it help if instead of 
>> throwing exception, there was an infinite retry loop?
>
> This isn't my logic - it is existing code that has been moved. I'm not a fan 
> of infinite retry loops as the can hang the system. But I'm happy to change 
> it to work that way if there is a consensus to do so.

I see the localOffset passed to VM.getNanoTimeAdjustment() has to be in the 
range of current time +/- 2^32 seconds. This means the thread would have to 
sleep for ~136 years before the exception is triggered. I think this is more 
than enough. :-)

-

PR: https://git.openjdk.java.net/jdk/pull/4016

RFR: 8267357: build breaks with -Werror option on micro benchmark added for JDK-8256973

[Jatin Bhateja]

Relevant declarations modified and tested with -Werror, no longer see unchecked 
conversion warnings.

Kindly review and approve.

-

Commit messages:
 - 8267357: build breaks with -Werror option on micro benchmark added for 
JDK-8256973

Changes: https://git.openjdk.java.net/jdk/pull/4108/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4108=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8267357
  Stats: 10 lines in 1 file changed: 0 ins; 1 del; 9 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4108.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4108/head:pull/4108

PR: https://git.openjdk.java.net/jdk/pull/4108

Re: RFR: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v9]

[Vladimir Ivanov]

On Wed, 19 May 2021 03:37:11 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: 8262891: Compiler implementation for Pattern Matching for switch (Preview) [v4]

[Jan Lahoda]

> 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 one additional 
commit since the last revision:

  Fixing various error-related bugs.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3863/files
  - new: https://git.openjdk.java.net/jdk/pull/3863/files/5fa8005a..0875377b

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3863=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3863=02-03

  Stats: 117 lines in 6 files changed: 94 ins; 13 del; 10 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: 8266936: Add a finalization JFR event [v2]

[Erik Gahlin]

On Tue, 18 May 2021 22:41:06 GMT, Brent Christian  wrote:

>> Please review this enhancement to add a new JFR event, generated whenever a 
>> finalizer is run.
>> (The makeup is similar to the Deserialization event, 
>> [JDK-8261160](https://bugs.openjdk.java.net/browse/JDK-8261160).)
>> 
>> The event's only datum (beyond those common to all jfr events) is the class 
>> of the object that was finalized.
>> 
>> The Category for the event:
>> `"Java Virtual Machine" / "GC" / "Finalization"`
>> is what made sense to me, even though the event is generated from library 
>> code.
>> 
>> Along with the new regtest, I added a run mode to the basic finalizer test 
>> to enable jfr.
>> Automated testing looks good so far.
>> 
>> Thanks,
>> -Brent
>
> Brent Christian has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Test flag should be volatile

I wonder if there needs to be one event per finalized object?

Perhaps a counter per class would be as useful, i.e. 
jdk.FinalizationStatistics, and if it could be implemented in the VM, without 
other implications, that would be great. 

Such an event could be enabled by default and provide much greater value than 
an event that users would need to know about and configure themselves, which 
99,99% of all user will not do.

-

PR: https://git.openjdk.java.net/jdk/pull/4101

Re: RFR: 8266936: Add a finalization JFR event [v2]

[Alan Bateman]

On Wed, 19 May 2021 06:20:59 GMT, Erik Gahlin  wrote:

> This looks useful, but I am worried about the performance impact:
> 
> * The added allocation for every object that is finalized.
> * The event being enabled in the default configuration.
> 
> The default configuration must be safe to use even in pathological cases, 
> i.e. an application with lots of objects being finalized. It's probably too 
> much overhead (or number of events) for the profile configuration as well.
> 
> There is also the added startup cost of JFR. One event may not matter that 
> much, but they all add up. We need to put in place a mechanism that doesn't 
> rely on bytecode instrumentation at runtime. There is an enhancement for 
> that, but it requires some engineering first.

I'm a bit worried by this too. Does it need to be enabled by default? Can we 
put a static final instance of FinalizerEvent in that class that can be used to 
test if the event is enabled so that it doesn't create a FinalizerEvent when 
disabled?

Is it worth exploring doing have an event in the VM, in register_finalizer, 
instead?

-

PR: https://git.openjdk.java.net/jdk/pull/4101

Re: RFR: 8266851: Implement JEP 403: Strongly Encapsulate JDK Internals [v4]

[Alan Bateman]

On Tue, 18 May 2021 23:01:05 GMT, Mark Reinhold  wrote:

>> Please review this implementation of JEP 403 
>> (https://openjdk.java.net/jeps/403).
>> Alan Bateman is the original author of almost all of it.  Passes tiers 1-3 
>> on 
>> {linux,macos,windows}-x64 and {linux,macos}-aarch64.
>
> Mark Reinhold has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix up IllegalAccessTest

Marked as reviewed by alanb (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/4015

Re: RFR: 8266936: Add a finalization JFR event [v2]

[Erik Gahlin]

On Tue, 18 May 2021 22:41:06 GMT, Brent Christian  wrote:

>> Please review this enhancement to add a new JFR event, generated whenever a 
>> finalizer is run.
>> (The makeup is similar to the Deserialization event, 
>> [JDK-8261160](https://bugs.openjdk.java.net/browse/JDK-8261160).)
>> 
>> The event's only datum (beyond those common to all jfr events) is the class 
>> of the object that was finalized.
>> 
>> The Category for the event:
>> `"Java Virtual Machine" / "GC" / "Finalization"`
>> is what made sense to me, even though the event is generated from library 
>> code.
>> 
>> Along with the new regtest, I added a run mode to the basic finalizer test 
>> to enable jfr.
>> Automated testing looks good so far.
>> 
>> Thanks,
>> -Brent
>
> Brent Christian has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Test flag should be volatile

This looks useful, but I am worried about the performance impact:

- The added allocation for every object that is finalized. 
- The event being enabled in the default configuration. 

The default configuration must be safe to use even in pathological cases, i.e. 
an application with lots of objects being finalized. It's probably too much 
overhead (or number of events) for the profile configuration as well.

There is also the added startup cost of JFR. One event may not matter that 
much, but they all add up. We need to put in place a mechanism that doesn't 
rely on bytecode instrumentation at runtime. There is an enhancement for that, 
but it requires some engineering first.

-

PR: https://git.openjdk.java.net/jdk/pull/4101

Re: RFR: 8263202: Update Hebrew/Indonesian/Yiddish ISO 639 language codes to current [v2]

[Joe Wang]

On Tue, 18 May 2021 23:35:12 GMT, Naoto Sato  wrote:

>> test/jdk/java/util/Locale/LocaleTest.java line 683:
>> 
>>> 681:  * @bug 4052404 4778440 8263202
>>> 682:  */
>>> 683: public void TestChangedISO639Codes() {
>> 
>> Could probably be simplified with a DataProvider.
>
> That would be nice, but the test is not testng based, and it would be an 
> entire test rewrite which I would not do it at this time.

I see. Good old test is still good.

-

PR: https://git.openjdk.java.net/jdk/pull/4069