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

[Erik Gahlin]

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

Looks good, but if there are test issues they should be fixed.

-

Marked as reviewed by egahlin (Reviewer).

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

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

[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: 8224158: assertion related to NPE at DynamicCallSiteDesc::withArgs should be reworded

[Vicente Romero]

On Mon, 17 May 2021 16:53:24 GMT, Vicente Romero  wrote:

> This is a very small patch that is just rewording the spec for 
> DynamicCallSiteDesc::withArgs. Basically adding that NPE can also be thrown 
> if the content of the argument is `null`
> 
> TIA for the review

ping

-

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

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

[Sandhya Viswanathan]

On Wed, 19 May 2021 00:58:15 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: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v8]

[Vladimir Kozlov]

On Wed, 19 May 2021 00:58:15 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: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v7]

[Sandhya Viswanathan]

On Wed, 19 May 2021 00:26:48 GMT, Vladimir Kozlov  wrote:

>> Sandhya Viswanathan has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   jcheck fixes
>
> This is much much better! Thank you for changing it. I am only asking now to 
> add comment explaining names.

@vnkozlov I have added comments explaining naming convention. Please let me 
know if this looks ok.

-

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

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

[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: 8267190: Optimize Vector API test operations

[Vladimir Kozlov]

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

Change in vectorIntrinsics.cpp seems fine. I did not look on Java code.

-

Marked as reviewed by kvn (Reviewer).

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

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

[Vladimir Kozlov]

On Tue, 18 May 2021 23:59:28 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: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v7]

[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: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v6]

[Sandhya Viswanathan]

On Tue, 18 May 2021 23:43:13 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: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v6]

[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: 8263202: Update Hebrew/Indonesian/Yiddish ISO 639 language codes to current [v2]

[Naoto Sato]

> Please review the changes to the subject issue. java.util.Locale class has a 
> long-standing issue for those obsolete ISO 639 languages where its 
> normalization ends up in the obsolete codes. This change intends to flip the 
> normalization towards the current codes, providing a system property for 
> compatibility behavior. ResourceBundle class is also modified to load either 
> obsolete/current bundles. For more detail, take a look at the CSR.

Naoto Sato has updated the pull request incrementally with one additional 
commit since the last revision:

  Locale's class description modification

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4069/files
  - new: https://git.openjdk.java.net/jdk/pull/4069/files/dee95bf1..0f76ac45

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

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

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

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

[Naoto Sato]

On Tue, 18 May 2021 22:22:06 GMT, Joe Wang  wrote:

>> Naoto Sato has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Locale's class description modification
>
> src/java.base/share/classes/java/util/Locale.java line 462:
> 
>> 460:  * backward compatible forms.
>> 461:  *
>> 462:  * The APIs added in 1.7 map between the old and new language codes,
> 
> This paragraph needs a rewrite as well it seems, esp. the part that states 
> "getLanguage and toString reflect the old code" is no longer true.

Good catch! In fact, I had modified this paragraph in my preliminary fix, but 
it slipped away somehow along the fix. Corrected the PR and CSR as well.

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

-

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

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

[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 [v2]

[Stephen Colebourne]

On Tue, 18 May 2021 21:04:18 GMT, Naoto Sato  wrote:

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

-

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

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

[Stephen Colebourne]

On Sun, 16 May 2021 07:39:21 GMT, Peter Levart  wrote:

>> Stephen Colebourne has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8266846: Add java.time.InstantSource
>
> src/java.base/share/classes/java/time/Clock.java line 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.

-

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

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

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

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

  Stats: 12 lines in 3 files changed: 1 ins; 3 del; 8 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: 8266851: Implement JEP 403: Strongly Encapsulate JDK Internals [v3]

[Mark Reinhold]

On Fri, 14 May 2021 08:54:23 GMT, Alan Bateman  wrote:

>> Mark Reinhold has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Remove now-unnecessary uses and mentions of the --illegal-access option
>
> test/jdk/tools/launcher/modules/illegalaccess/IllegalAccessTest.java line 26:
> 
>> 24: /**
>> 25:  * @test
>> 26:  * @requires vm.compMode != "Xcomp"
> 
> I think `@requires vm.compMode != "Xcomp" was added because the test took too 
> long with -Xcomp. I think it can be dropped now.

Dropped.

> test/jdk/tools/launcher/modules/illegalaccess/IllegalAccessTest.java line 37:
> 
>> 35: 
>> 36: /**
>> 37:  * Basic test of --illegal-access=value to deny or permit access to JDK 
>> internals.
> 
> The comment should probably be updated as the test no longer tests denying or 
> permitting access.

Fixed.

-

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

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

[Mark Reinhold]

> 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

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4015/files
  - new: https://git.openjdk.java.net/jdk/pull/4015/files/1c14998e..cde0adbb

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

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

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

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

[Joe Wang]

On Mon, 17 May 2021 16:55:35 GMT, Naoto Sato  wrote:

> Please review the changes to the subject issue. java.util.Locale class has a 
> long-standing issue for those obsolete ISO 639 languages where its 
> normalization ends up in the obsolete codes. This change intends to flip the 
> normalization towards the current codes, providing a system property for 
> compatibility behavior. ResourceBundle class is also modified to load either 
> obsolete/current bundles. For more detail, take a look at the CSR.

src/java.base/share/classes/java/util/Locale.java line 462:

> 460:  * backward compatible forms.
> 461:  *
> 462:  * The APIs added in 1.7 map between the old and new language codes,

This paragraph needs a rewrite as well it seems, esp. the part that states 
"getLanguage and toString reflect the old code" is no longer true.

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.

-

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

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

[Brent Christian]

> 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

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4101/files
  - new: https://git.openjdk.java.net/jdk/pull/4101/files/200268ab..e0ef383b

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

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

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

Re: RFR: 8266936: Add a finalization JFR event

[Brent Christian]

On Tue, 18 May 2021 21:40:57 GMT, Istvan Neuwirth 
 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
>
> test/jdk/jdk/jfr/event/gc/finalization/TestFinalizerEvent.java line 48:
> 
>> 46: 
>> 47: public class TestFinalizerEvent {
>> 48: static boolean finalizerRun = false;
> 
> Should not this be a `volatile` field? (Or alternatively, using 
> `AtomicBoolean`). As the `finalize` is invoked on the finalizer thread, the 
> other thread executing the test might not see the change.

Good idea, thanks.

-

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

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

[Weijun Wang]

> 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

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4071/files
  - new: https://git.openjdk.java.net/jdk/pull/4071/files/900c28c0..bfa955ad

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

  Stats: 59 lines in 18 files changed: 8 ins; 6 del; 45 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4071.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4071/head:pull/4071

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

Re: RFR: 8266936: Add a finalization JFR event

[Istvan Neuwirth]

On Tue, 18 May 2021 20:55:10 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

test/jdk/jdk/jfr/event/gc/finalization/TestFinalizerEvent.java line 48:

> 46: 
> 47: public class TestFinalizerEvent {
> 48: static boolean finalizerRun = false;

Should not this be a `volatile` field? (Or alternatively, using 
`AtomicBoolean`). As the `finalize` is invoked on the finalizer thread, the 
other thread executing the test might not see the change.

-

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

RFR: 8266936: Add a finalization JFR event

[Brent Christian]

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

-

Commit messages:
 - Fix jcheck some more
 - Fix jcheck
 - Merge branch 'master' into 8266936
 - Add jfr mode to test/jdk/java/lang/ref/FinalizeOverride.java
 - Fix spacing in new regtest
 - Remove extraneous comments from new regtest
 - Removed unintentional changes from FinalizeOverride.java
 - Actually check in new regtest
 - Update TestActiveSettingEvent.java
 - Update TestDefaultConfigurations.java and .jfc files
 - ... and 2 more: https://git.openjdk.java.net/jdk/compare/e6705c0e...200268ab

Changes: https://git.openjdk.java.net/jdk/pull/4101/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4101=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8266936
  Stats: 191 lines in 11 files changed: 190 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4101.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4101/head:pull/4101

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

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

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

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

  feedback from Sean, Phil and Alan

-

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

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

  Stats: 9 lines in 3 files changed: 4 ins; 1 del; 4 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

[Weijun Wang]

On Tue, 18 May 2021 18:38:52 GMT, Weijun Wang  wrote:

>> src/java.base/share/classes/java/security/AccessController.java line 877:
>> 
>>> 875: @CallerSensitive
>>> 876: public static  T doPrivileged(PrivilegedExceptionAction 
>>> action,
>>> 877:  @SuppressWarnings("removal") 
>>> AccessControlContext context, Permission... perms)
>> 
>> you might find it easier if you put the Permissions parameter on a new line. 
>> There are several places in AccessController where the same thing happens.
>
> I'll try to update my auto-annotating program.

Turns out this only happens in this class:

$ rg '^\s*@SuppressWarnings("removal").*?,.'
src/java.base/share/classes/java/security/AccessController.java:449:
@SuppressWarnings("removal") AccessControlContext context, Permission... perms) 
{
src/java.base/share/classes/java/security/AccessController.java:514:
@SuppressWarnings("removal") AccessControlContext context, Permission... perms) 
{
src/java.base/share/classes/java/security/AccessController.java:877:
 @SuppressWarnings("removal") AccessControlContext 
context, Permission... perms)

I'll fix them manually.

-

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

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

[Claes Redestad]

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.

-

Commit messages:
 - Remove obsolete test
 - Transform AccessMode name map into switch expression

Changes: https://git.openjdk.java.net/jdk/pull/4102/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4102=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8267321
  Stats: 90 lines in 2 files changed: 31 ins; 56 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4102.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4102/head:pull/4102

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

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

[Naoto Sato]

On Mon, 17 May 2021 17:19:19 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

src/java.base/share/classes/java/time/Instant.java line 275:

> 273:  */
> 274: public static Instant now() {
> 275: return Clock.currentInstant();

Copyright year should change to 2021 for this change.

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

> 57: assertTrue(Math.abs(testMillis - millis) < 1000);
> 58: assertTrue(Math.abs(testInstantMillis - millis) < 1000);
> 59: assertSame(test.equals(InstantSource.system());

')' is missing. Couple other locations.

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.

-

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

Re: RFR: 8264859: Implement Context-Specific Deserialization Filters [v2]

[Roger Riggs]

> JEP 415: Context-specific Deserialization Filters extends the deserialization 
> filtering mechanisms with more flexible and customizable protections against 
> malicious deserialization.  See JEP 415: https://openjdk.java.net/jeps/415.
> The `java.io.ObjectInputFilter` and `java.io.ObjectInputStream` classes are 
> extended with additional
> configuration mechanisms and filter utilities.
> 
> javadoc for `ObjectInputFilter`, `ObjectInputFilter.Config`, and 
> `ObjectInputStream`:
> 
> http://cr.openjdk.java.net/~rriggs/filter-factory/java.base/java/io/ObjectInputFilter.html

Roger Riggs has updated the pull request incrementally with one additional 
commit since the last revision:

  Add utilities to reject undecided results, allow unlimited limits, and 
improve robustness of examples

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3996/files
  - new: https://git.openjdk.java.net/jdk/pull/3996/files/5414d505..3ec309f3

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

  Stats: 562 lines in 4 files changed: 351 ins; 59 del; 152 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3996.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3996/head:pull/3996

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

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

Changes look good, some minor comments. I shall run it through tier 1 to 3 
tests and report back.

src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Byte128Vector.java 
line 609:

> 607: @ForceInline
> 608: final 
> 609: VectorMask defaultMaskReinterpret(VectorSpecies dsp) {

Since this method is only called by `cast` we can make this method private and 
accept an argument of `AbstractSpecies`. Further, the length check is 
duplicated by `cast` so we could turn it into an assert. Extra bonus points for 
converting the statement switch into an expression switch.

-

Marked as reviewed by psandoz (Reviewer).

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

Re: RFR: 8153490: Cannot setBytes() if incoming buffer's length is bigger than number of elements we want to insert. [v3]

[Lance Andersen]

On Mon, 17 May 2021 14:39:05 GMT, Mitsuru Kariya 
 wrote:

>> Fix `SerialBlob.setBytes(long pos, byte[] bytes, int offset, int length)` in 
>> the following cases:
>> 
>> 1. `pos - 1 + bytes.length - offset > this.length() && pos - 1 + length <= 
>> this.length()`
>>The original implementation throws `ArrayIndexOutOfBoundsException` but 
>> this case should end successfully.
>>(test31)
>> 
>> 2. `pos - 1 + length > this.length()`
>>The original implementation throws `ArrayIndexOutOfBoundsException` but 
>> this case should end successfully. *1
>>(test32)
>> 
>> 3. `pos == this.length() + 1`
>>The original implementation throws `SerialException` but this case should 
>> end successfully. *2
>>(test33)
>> 
>> 4. `length < 0`
>>The original implementation throws `ArrayIndexOutOfBoundsException` but 
>> this case should throw `SerialException`.
>>(test34)
>> 
>> 5. `offset + length > Integer.MAX_VALUE`
>>The original implementation throws `ArrayIndexOutOfBoundsException` (or 
>> `OutOfMemoryError` in most cases) but this case should throw 
>> `SerialException`.
>>(test35)
>> 
>> Additionally, fix `SerialClob.setString(long pos, String str, int offset, 
>> int length)` in the following cases:
>> 
>> 1. `offset > str.length()`
>>The original implementaion throws `StringIndexOutOfBoundsException` but 
>> this case should throw `SerialException`.
>>(test39)
>> 
>> 2. `pos - 1 + str.length() - offset > this.length() && pos - 1 + length <= 
>> this.length()`
>>The original implementation throws `ArrayIndexOutOfBoundsException` but 
>> this case should end successfully.
>>(test32)
>> 
>> 3. `pos - 1 + length > this.length()`
>>The original implementaion throws `SerialException` but this case should 
>> end successfully. *3
>>(test40)
>> 
>> 4. `pos == this.length() + 1`
>>The original implementaion throws `SerialException` but this case should 
>> end successfully. *4
>>(test41)
>> 
>> 5. `length < 0`
>>The original implementation throws `StringIndexOutOfBoundsException` but 
>> this case should throw `SerialException`.
>>(test42)
>> 
>> 6. `offset + length > Integer.MAX_VALUE`
>>The original implementation throws `ArrayIndexOutOfBoundsException` (or 
>> `OutOfMemoryError` in most cases) but this case should throw 
>> `SerialException`.
>>(test43)
>> 
>> 
>> The javadoc has also been modified according to the above.
>> 
>> *1 The documentation of `Blob.setBytes()` says, "If the end of the Blob 
>> value is reached while writing the array of bytes, then the length of the 
>> Blob value will be increased to accommodate the extra bytes."
>> 
>> *2 The documentation of `Blob.setBytes()` says, "If the value specified for 
>> pos is greater than the length+1 of the BLOB value then the behavior is 
>> undefined."
>>So, it should work correctly when pos == length+1 of the BLOB value.
>> 
>> *3 The documentation of `Clob.setString()` says, "If the end of the Clob 
>> value is eached while writing the given string, then the length of the Clob 
>> value will be increased to accommodate the extra characters."
>> 
>> *4 The documentation of `Clob.setString()` says, "If the value specified for 
>> pos is greater than the length+1 of the CLOB value then the behavior is 
>> undefined."
>>So, it should work correctly when pos == length+1 of the CLOB value.
>
> Mitsuru Kariya has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix for length + offset > Integer.MAX_VALUE case

I have run the JCK tests in addition to to the JTREG Tess to validate there are 
no additional failures due to these changes

-

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

Re: RFR: 8153490: Cannot setBytes() if incoming buffer's length is bigger than number of elements we want to insert. [v3]

[Lance Andersen]

On Mon, 17 May 2021 14:39:05 GMT, Mitsuru Kariya 
 wrote:

>> Fix `SerialBlob.setBytes(long pos, byte[] bytes, int offset, int length)` in 
>> the following cases:
>> 
>> 1. `pos - 1 + bytes.length - offset > this.length() && pos - 1 + length <= 
>> this.length()`
>>The original implementation throws `ArrayIndexOutOfBoundsException` but 
>> this case should end successfully.
>>(test31)
>> 
>> 2. `pos - 1 + length > this.length()`
>>The original implementation throws `ArrayIndexOutOfBoundsException` but 
>> this case should end successfully. *1
>>(test32)
>> 
>> 3. `pos == this.length() + 1`
>>The original implementation throws `SerialException` but this case should 
>> end successfully. *2
>>(test33)
>> 
>> 4. `length < 0`
>>The original implementation throws `ArrayIndexOutOfBoundsException` but 
>> this case should throw `SerialException`.
>>(test34)
>> 
>> 5. `offset + length > Integer.MAX_VALUE`
>>The original implementation throws `ArrayIndexOutOfBoundsException` (or 
>> `OutOfMemoryError` in most cases) but this case should throw 
>> `SerialException`.
>>(test35)
>> 
>> Additionally, fix `SerialClob.setString(long pos, String str, int offset, 
>> int length)` in the following cases:
>> 
>> 1. `offset > str.length()`
>>The original implementaion throws `StringIndexOutOfBoundsException` but 
>> this case should throw `SerialException`.
>>(test39)
>> 
>> 2. `pos - 1 + str.length() - offset > this.length() && pos - 1 + length <= 
>> this.length()`
>>The original implementation throws `ArrayIndexOutOfBoundsException` but 
>> this case should end successfully.
>>(test32)
>> 
>> 3. `pos - 1 + length > this.length()`
>>The original implementaion throws `SerialException` but this case should 
>> end successfully. *3
>>(test40)
>> 
>> 4. `pos == this.length() + 1`
>>The original implementaion throws `SerialException` but this case should 
>> end successfully. *4
>>(test41)
>> 
>> 5. `length < 0`
>>The original implementation throws `StringIndexOutOfBoundsException` but 
>> this case should throw `SerialException`.
>>(test42)
>> 
>> 6. `offset + length > Integer.MAX_VALUE`
>>The original implementation throws `ArrayIndexOutOfBoundsException` (or 
>> `OutOfMemoryError` in most cases) but this case should throw 
>> `SerialException`.
>>(test43)
>> 
>> 
>> The javadoc has also been modified according to the above.
>> 
>> *1 The documentation of `Blob.setBytes()` says, "If the end of the Blob 
>> value is reached while writing the array of bytes, then the length of the 
>> Blob value will be increased to accommodate the extra bytes."
>> 
>> *2 The documentation of `Blob.setBytes()` says, "If the value specified for 
>> pos is greater than the length+1 of the BLOB value then the behavior is 
>> undefined."
>>So, it should work correctly when pos == length+1 of the BLOB value.
>> 
>> *3 The documentation of `Clob.setString()` says, "If the end of the Clob 
>> value is eached while writing the given string, then the length of the Clob 
>> value will be increased to accommodate the extra characters."
>> 
>> *4 The documentation of `Clob.setString()` says, "If the value specified for 
>> pos is greater than the length+1 of the CLOB value then the behavior is 
>> undefined."
>>So, it should work correctly when pos == length+1 of the CLOB value.
>
> Mitsuru Kariya has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix for length + offset > Integer.MAX_VALUE case

Overall the changes look reasonable.  As mentioned in the comments, a CSR will 
be required due to some of the wordsmithing cleanup

src/java.sql.rowset/share/classes/javax/sql/rowset/serial/SerialBlob.java line 
306:

> 304:  *
> 305:  * @param pos the position in the SQL BLOB value at 
> which
> 306:  * to start writing. The first position is 1;

When updating the javadoc to use @code, please update all instances for 
consistency

src/java.sql.rowset/share/classes/javax/sql/rowset/serial/SerialBlob.java line 
308:

> 306:  * to start writing. The first position is 1;
> 307:  * must not be less than 1 nor greater than
> 308:  * the length+1 of this {@code SerialBlob} object.

Changes such as this require a CSR.  I think I have convinced myself that it is 
OK to move forward with the CSR.

src/java.sql.rowset/share/classes/javax/sql/rowset/serial/SerialBlob.java line 
313:

> 311:  * @return the number of bytes written
> 312:  * @throws SerialException if there is an error accessing the
> 313:  * {@code BLOB} value; or if an invalid position is set;

Even though this addresses a typo, this will require a CSR

-

Changes requested by lancea (Reviewer).

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

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

[Weijun Wang]

On Tue, 18 May 2021 17:36:55 GMT, Alan Bateman  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.
>
> src/java.base/share/classes/java/security/AccessController.java line 877:
> 
>> 875: @CallerSensitive
>> 876: public static  T doPrivileged(PrivilegedExceptionAction 
>> action,
>> 877:  @SuppressWarnings("removal") 
>> AccessControlContext context, Permission... perms)
> 
> you might find it easier if you put the Permissions parameter on a new line. 
> There are several places in AccessController where the same thing happens.

My rule is that a new line is added if there's only whitespaces before the 
insert position and the previous line does not end with a comma. In this case, 
unless I move `Permission... perms` onto a new line that an annotation on a new 
line looks like covering both `context` and `perms`.

-

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

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

[Peter Levart]

On Mon, 17 May 2021 17:19:19 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

Changes requested by plevart (Reviewer).

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?

-

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

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

[Mandy Chung]

On Mon, 17 May 2021 18:23:41 GMT, Weijun Wang  wrote:

> Please review this implementation of [JEP 
> 411](https://openjdk.java.net/jeps/411).
> 
> The code change is divided into 3 commits. Please review them one by one.
> 
> 1. 
> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1
>  The essential change for this JEP, including the `@Deprecate` annotations 
> and spec change. It also update the default value of the 
> `java.security.manager` system property to "disallow", and necessary test 
> change following this update.
> 2. 
> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66
>  Manual changes to several files so that the next commit can be generated 
> programatically.
> 3. 
> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950
>  Automatic changes to other source files to avoid javac warnings on 
> deprecation for removal
> 
> The 1st and 2nd commits should be reviewed carefully. The 3rd one is 
> generated programmatically, see the comment below for more details. If you 
> are only interested in a portion of the 3rd commit and would like to review 
> it as a separate file, please comment here and I'll generate an individual 
> webrev.
> 
> Due to the size of this PR, no attempt is made to update copyright years for 
> any file to minimize unnecessary merge conflict.
> 
> Furthermore, since the default value of `java.security.manager` system 
> property is now "disallow", most of the tests calling 
> `System.setSecurityManager()` need to launched with 
> `-Djava.security.manager=allow`. This is covered in a different PR at 
> https://github.com/openjdk/jdk/pull/4071.

Marked as reviewed by mchung (Reviewer).

-

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

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

[Alan Bateman]

On Mon, 17 May 2021 18:23:41 GMT, Weijun Wang  wrote:

> Please review this implementation of [JEP 
> 411](https://openjdk.java.net/jeps/411).
> 
> The code change is divided into 3 commits. Please review them one by one.
> 
> 1. 
> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1
>  The essential change for this JEP, including the `@Deprecate` annotations 
> and spec change. It also update the default value of the 
> `java.security.manager` system property to "disallow", and necessary test 
> change following this update.
> 2. 
> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66
>  Manual changes to several files so that the next commit can be generated 
> programatically.
> 3. 
> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950
>  Automatic changes to other source files to avoid javac warnings on 
> deprecation for removal
> 
> The 1st and 2nd commits should be reviewed carefully. The 3rd one is 
> generated programmatically, see the comment below for more details. If you 
> are only interested in a portion of the 3rd commit and would like to review 
> it as a separate file, please comment here and I'll generate an individual 
> webrev.
> 
> Due to the size of this PR, no attempt is made to update copyright years for 
> any file to minimize unnecessary merge conflict.
> 
> Furthermore, since the default value of `java.security.manager` system 
> property is now "disallow", most of the tests calling 
> `System.setSecurityManager()` need to launched with 
> `-Djava.security.manager=allow`. This is covered in a different PR at 
> https://github.com/openjdk/jdk/pull/4071.

Marked as reviewed by alanb (Reviewer).

src/java.base/share/classes/java/security/AccessController.java line 877:

> 875: @CallerSensitive
> 876: public static  T doPrivileged(PrivilegedExceptionAction action,
> 877:  @SuppressWarnings("removal") 
> AccessControlContext context, Permission... perms)

you might find it easier if you put the Permissions parameter on a new line. 
There are several places in AccessController where the same thing happens.

-

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

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

[Mandy Chung]

On Mon, 17 May 2021 17:51:36 GMT, Weijun Wang  wrote:

> Please review the test changes for [JEP 
> 411](https://openjdk.java.net/jeps/411).
> 
> With JEP 411 and the default value of `-Djava.security.manager` becoming 
> `disallow`, tests calling `System.setSecurityManager()`  need 
> `-Djava.security.manager=allow` when launched. This PR covers such changes 
> for tier1 to tier3 (except for the JCK tests).
> 
> To make it easier to focus your review on the tests in your area, this PR is 
> divided into multiple commits for different areas (~~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.

I reviewed non-client areas.  Looks okay.

-

Marked as reviewed by mchung (Reviewer).

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

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

[Joe Wang]

On Mon, 17 May 2021 18:23:41 GMT, Weijun Wang  wrote:

> Please review this implementation of [JEP 
> 411](https://openjdk.java.net/jeps/411).
> 
> The code change is divided into 3 commits. Please review them one by one.
> 
> 1. 
> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1
>  The essential change for this JEP, including the `@Deprecate` annotations 
> and spec change. It also update the default value of the 
> `java.security.manager` system property to "disallow", and necessary test 
> change following this update.
> 2. 
> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66
>  Manual changes to several files so that the next commit can be generated 
> programatically.
> 3. 
> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950
>  Automatic changes to other source files to avoid javac warnings on 
> deprecation for removal
> 
> The 1st and 2nd commits should be reviewed carefully. The 3rd one is 
> generated programmatically, see the comment below for more details. If you 
> are only interested in a portion of the 3rd commit and would like to review 
> it as a separate file, please comment here and I'll generate an individual 
> webrev.
> 
> Due to the size of this PR, no attempt is made to update copyright years for 
> any file to minimize unnecessary merge conflict.
> 
> Furthermore, since the default value of `java.security.manager` system 
> property is now "disallow", most of the tests calling 
> `System.setSecurityManager()` need to launched with 
> `-Djava.security.manager=allow`. This is covered in a different PR at 
> https://github.com/openjdk/jdk/pull/4071.

java.xml changes look good. Thanks.

-

Marked as reviewed by joehw (Reviewer).

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

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

[Naoto Sato]

On Tue, 18 May 2021 10:37:21 GMT, Patrick Concannon  
wrote:

> 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

Classes in the i18n area look good.

-

Marked as reviewed by naoto (Reviewer).

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

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

[Naoto Sato]

On Mon, 17 May 2021 18:23:41 GMT, Weijun Wang  wrote:

> Please review this implementation of [JEP 
> 411](https://openjdk.java.net/jeps/411).
> 
> The code change is divided into 3 commits. Please review them one by one.
> 
> 1. 
> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1
>  The essential change for this JEP, including the `@Deprecate` annotations 
> and spec change. It also update the default value of the 
> `java.security.manager` system property to "disallow", and necessary test 
> change following this update.
> 2. 
> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66
>  Manual changes to several files so that the next commit can be generated 
> programatically.
> 3. 
> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950
>  Automatic changes to other source files to avoid javac warnings on 
> deprecation for removal
> 
> The 1st and 2nd commits should be reviewed carefully. The 3rd one is 
> generated programmatically, see the comment below for more details. If you 
> are only interested in a portion of the 3rd commit and would like to review 
> it as a separate file, please comment here and I'll generate an individual 
> webrev.
> 
> Due to the size of this PR, no attempt is made to update copyright years for 
> any file to minimize unnecessary merge conflict.
> 
> Furthermore, since the default value of `java.security.manager` system 
> property is now "disallow", most of the tests calling 
> `System.setSecurityManager()` need to launched with 
> `-Djava.security.manager=allow`. This is covered in a different PR at 
> https://github.com/openjdk/jdk/pull/4071.

Reviewed i18n/java.time/charset. They all look good.

-

Marked as reviewed by naoto (Reviewer).

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

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

[Chris Hegarty]

On Mon, 17 May 2021 18:23:41 GMT, Weijun Wang  wrote:

> Please review this implementation of [JEP 
> 411](https://openjdk.java.net/jeps/411).
> 
> The code change is divided into 3 commits. Please review them one by one.
> 
> 1. 
> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1
>  The essential change for this JEP, including the `@Deprecate` annotations 
> and spec change. It also update the default value of the 
> `java.security.manager` system property to "disallow", and necessary test 
> change following this update.
> 2. 
> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66
>  Manual changes to several files so that the next commit can be generated 
> programatically.
> 3. 
> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950
>  Automatic changes to other source files to avoid javac warnings on 
> deprecation for removal
> 
> The 1st and 2nd commits should be reviewed carefully. The 3rd one is 
> generated programmatically, see the comment below for more details. If you 
> are only interested in a portion of the 3rd commit and would like to review 
> it as a separate file, please comment here and I'll generate an individual 
> webrev.
> 
> Due to the size of this PR, no attempt is made to update copyright years for 
> any file to minimize unnecessary merge conflict.
> 
> Furthermore, since the default value of `java.security.manager` system 
> property is now "disallow", most of the tests calling 
> `System.setSecurityManager()` need to launched with 
> `-Djava.security.manager=allow`. This is covered in a different PR at 
> https://github.com/openjdk/jdk/pull/4071.

The changes in the net area look fine.

-

Marked as reviewed by chegar (Reviewer).

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

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

[Joe Darcy]

On Mon, 17 May 2021 18:23:41 GMT, Weijun Wang  wrote:

> Please review this implementation of [JEP 
> 411](https://openjdk.java.net/jeps/411).
> 
> The code change is divided into 3 commits. Please review them one by one.
> 
> 1. 
> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1
>  The essential change for this JEP, including the `@Deprecate` annotations 
> and spec change. It also update the default value of the 
> `java.security.manager` system property to "disallow", and necessary test 
> change following this update.
> 2. 
> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66
>  Manual changes to several files so that the next commit can be generated 
> programatically.
> 3. 
> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950
>  Automatic changes to other source files to avoid javac warnings on 
> deprecation for removal
> 
> The 1st and 2nd commits should be reviewed carefully. The 3rd one is 
> generated programmatically, see the comment below for more details. If you 
> are only interested in a portion of the 3rd commit and would like to review 
> it as a separate file, please comment here and I'll generate an individual 
> webrev.
> 
> Due to the size of this PR, no attempt is made to update copyright years for 
> any file to minimize unnecessary merge conflict.
> 
> Furthermore, since the default value of `java.security.manager` system 
> property is now "disallow", most of the tests calling 
> `System.setSecurityManager()` need to launched with 
> `-Djava.security.manager=allow`. This is covered in a different PR at 
> https://github.com/openjdk/jdk/pull/4071.

Marked as reviewed by darcy (Reviewer).

-

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

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

[Sean Mullan]

On Tue, 18 May 2021 15:19:21 GMT, Alan Bateman  wrote:

>> It includes both:
>> ![Screen Shot 2021-05-18 at 8 41 11 
>> AM](https://user-images.githubusercontent.com/35072269/118652730-dfb35400-b7b4-11eb-83ee-92be9136fea2.jpg)
>
> Thanks for checking, I assumed that was the case so wondering if it should be 
> dropped from the deprecation text to avoid the repeated sentence.

My feeling is that it is better to be more specific than the generic removal 
text as this is the most significant class that is being deprecated for 
removal. Also, this says "the Security Manager" (note the space between) which 
really encompasses more than the `java.lang.SecurityManager` API and which is 
explained more completely in the JEP.

-

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

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

[Alan Bateman]

On Tue, 18 May 2021 12:42:08 GMT, Sean Mullan  wrote:

>> src/java.base/share/classes/java/lang/SecurityManager.java line 315:
>> 
>>> 313:  *
>>> 314:  * @since   1.0
>>> 315:  * @deprecated The Security Manager is deprecated and subject to 
>>> removal in a
>> 
>> Javadoc will prefix, in bold, "Deprecated, for removal: This API element is 
>> subject to removal in a future version". I'm just wondering if the sentence 
>> will be repeated here, can you check?
>
> It includes both:
> ![Screen Shot 2021-05-18 at 8 41 11 
> AM](https://user-images.githubusercontent.com/35072269/118652730-dfb35400-b7b4-11eb-83ee-92be9136fea2.jpg)

Thanks for checking, I assumed that was the case so wondering if it should be 
dropped from the deprecation text to avoid the repeated sentence.

-

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

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

[Weijun Wang]

On Tue, 18 May 2021 05:48:56 GMT, Alan Bateman  wrote:

> The changes look okay but a bit inconsistent on where -Djava...=allow is 
> inserted for tests that already set other system properties or other 
> parameters. Not a correctness issue, just looks odd in several places, e.g.
> 
> test/jdk/java/lang/System/LoggerFinder/BaseLoggerFinderTest/BaseLoggerFinderTest.java
>  - the tests sets the system properties after -Xbootclasspath/a: but the 
> change means it sets properties before and after.
> 
> test/jdk/java/lang/Class/getResource/ResourcesTest.java - you've added it in 
> the middle of the module and class path parameters.
> 
> For uses using ProcessTools then it seems to be very random.

I've updated the 3 cases you mentioned and will go through more. Yes it looks 
good to group system property settings together. Thanks.

-

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

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

[Weijun Wang]

On Tue, 18 May 2021 11:12:00 GMT, Daniel Fuchs  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.
>
> test/jdk/javax/management/remote/mandatory/notif/NotificationEmissionTest.java
>  line 34:
> 
>> 32:  * @run clean NotificationEmissionTest
>> 33:  * @run build NotificationEmissionTest
>> 34:  * @run main NotificationEmissionTest 1
> 
> This test case (NotificationEmissionTest 1) calls 
> `System.setProperty("java.security.policy", policyFile);` - even though it 
> doesn't call System.setSecurityManager(); Should the @run command line for 
> test case 1 be modified as well?

Or maybe the policy setting line should only be called when a security manager 
is set? IIRC jtreg is able to restore system properties even in agentvm mode. 
So maybe this really doesn't matter. We just want to modify as little as 
possible in this PR.

-

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

Re: RFC: Add a --validate option to the jar tool

[Jorn Vernee]

I'll go ahead with this enhancement, and make sure the wording is open 
to adding validation logic not related to multi-release jars in the future.


Jorn

On 12/05/2021 14:47, Jorn Vernee wrote:

On 12/05/2021 14:41, Alan Bateman wrote:

On 12/05/2021 11:58, Jorn Vernee wrote:

Hi,

I see that the jar tool has validation logic for multi-release jars 
that is triggered when creating or updating a jar that has a 
versioned file as an input. I wanted to ask what people think about 
the idea of exposing this validation logic directly under a 
--validate command line flag as well.


Malformed multi-release jars can cause compilation and runtime 
problems because the API exposed by the jar is different across 
different Java versions. Exposing the validation logic directly 
would allow for easy validation of jar files that are suspected of 
being malformed multi-release jars.


The validation logic is already cleanly separated into a separate 
Validator class in the source code, and adding a command line flag 
that exposes it directly is a relatively minimal change [1]. It 
seems like maybe the intent in the past was also to expose this 
validation logic directly?


What's the history here? Any opinions about exposing this validation 
logic through a command line option?


I think it could be useful, esp. if Maven or Gradle plugins could 
make use of it.


There is other validation that could be done. JDK-8207339 is one 
example where it would be useful to identify a JAR file with a 
services configuration file that disagrees with the module 
definition. I bring it up because a general --validate option could 
do more than the fingerprint check. If the intention is to limit it 
to MR JAR features then I think it will need a different and more 
specific option name.
Potentially doing other validation seems like a good idea to me as 
well. It seems like the validation logic could be expanded further in 
the future. I brought up multi-release jars because AFAICS that's the 
only thing the existing validation logic concerns itself with, but I 
don't see any reason why validation that doesn't relate to 
multi-release jars couldn't be added as well (and I guess I chose the 
name --validate for that reason :) )


Jorn

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

[Sean Mullan]

On Tue, 18 May 2021 06:31:06 GMT, Alan Bateman  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.
>
> src/java.base/share/classes/java/lang/SecurityManager.java line 315:
> 
>> 313:  *
>> 314:  * @since   1.0
>> 315:  * @deprecated The Security Manager is deprecated and subject to 
>> removal in a
> 
> Javadoc will prefix, in bold, "Deprecated, for removal: This API element is 
> subject to removal in a future version". I'm just wondering if the sentence 
> will be repeated here, can you check?

It includes both:
![Screen Shot 2021-05-18 at 8 41 11 
AM](https://user-images.githubusercontent.com/35072269/118652730-dfb35400-b7b4-11eb-83ee-92be9136fea2.jpg)

-

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

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

[Doug Lea]

On Tue, 18 May 2021 10:37:21 GMT, Patrick Concannon  
wrote:

> 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

Because we still make jdk11-compatible test-release java.util.concurrent jars, 
we are not big fans of adding non-essential differences to openjdk version. But 
if there is some advantage to doing this, we'll cope.

-

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

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

[Alan Bateman]

On Tue, 18 May 2021 10:37:21 GMT, Patrick Concannon  
wrote:

> 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

You may need to coordinate with @DougLea on the changes to j.u.concurrent.

-

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

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

[Daniel Fuchs]

On Mon, 17 May 2021 17:51:36 GMT, Weijun Wang  wrote:

> Please review the test changes for [JEP 
> 411](https://openjdk.java.net/jeps/411).
> 
> With JEP 411 and the default value of `-Djava.security.manager` becoming 
> `disallow`, tests calling `System.setSecurityManager()`  need 
> `-Djava.security.manager=allow` when launched. This PR covers such changes 
> for tier1 to tier3 (except for the JCK tests).
> 
> To make it easier to focus your review on the tests in your area, this PR is 
> divided into multiple commits for different areas. Mostly the rule is the 
> same as how Skara adds labels, but there are several small tweaks:
> 
> 1. When a file is covered by multiple labels, only one is chosen. I make my 
> best to avoid putting too many tests into `core-libs`. If a file is covered 
> by `core-libs` and another label, I categorized it into the other label.
> 2. If a file is in `core-libs` but contains `/xml/` in its name, it's in the 
> `xml` commit.
> 3. If a file is in `core-libs` but contains `/rmi/` in its name, it's in the 
> `rmi` commit.
> 4. One file not covered by any label -- 
> `test/jdk/com/sun/java/accessibility/util/8051626/Bug8051626.java` -- is in 
> the `swing` commit.
> 
> Due to the size of this PR, no attempt is made to update copyright years for 
> all files to minimize unnecessary merge conflict.
> 
> Please note that this PR can be integrated before the source changes for JEP 
> 411, as the possible values of this system property was already defined long 
> time ago in JDK 9.
> 
> Most of the change in this PR is a simple adding of 
> `-Djava.security.manager=allow` to the `@run main/othervm` line. Sometimes it 
> was not `othervm` and we add one. Sometimes there's no `@run` at all and we 
> add the line.
> 
> There are several tests that launch another Java process that needs to call 
> the `System.setSecurityManager()` method, and the system property is added to 
> `ProcessBuilder`, `ProcessTools`, or the java command line (if the test is a 
> shell test).
> 
> 3 langtools tests are added into problem list due to 
> [JDK-8265611](https://bugs.openjdk.java.net/browse/JDK-8265611).
> 
> 2 SQL tests are moved because they need different options on the `@run` line 
> but they are inside a directory that has a `TEST.properties`:
> 
> rename test/jdk/java/sql/{testng/test/sql/othervm => 
> permissionTests}/DriverManagerPermissionsTests.java (93%)
> rename test/jdk/javax/sql/{testng/test/rowset/spi => 
> permissionTests}/SyncFactoryPermissionsTests.java (95%)
>  ```
> 
> The source change for JEP 411 is at https://github.com/openjdk/jdk/pull/4073.

I looked at serviceability, net, and corelibs. The changes look good to me - 
though I have one question about the NotificationEmissionTest. I believe that 
regardless of whether the new property is needed, test case 1 should be run in 
/othervm too.

test/jdk/javax/management/remote/mandatory/notif/NotificationEmissionTest.java 
line 34:

> 32:  * @run clean NotificationEmissionTest
> 33:  * @run build NotificationEmissionTest
> 34:  * @run main NotificationEmissionTest 1

This test case (NotificationEmissionTest 1) calls 
`System.setProperty("java.security.policy", policyFile);` - even though it 
doesn't call System.setSecurityManager(); Should the @run command line for test 
case 1 be modified as well?

-

Marked as reviewed by dfuchs (Reviewer).

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

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

[Lance Andersen]

On Tue, 18 May 2021 10:37:21 GMT, Patrick Concannon  
wrote:

> 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

Changes look good.

-

Marked as reviewed by lancea (Reviewer).

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

RFR: 8267110: Update java.util to use instanceof pattern variable

[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

-

Commit messages:
 - 8267110: Update java.util to use instanceof pattern variable

Changes: https://git.openjdk.java.net/jdk/pull/4088/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4088=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8267110
  Stats: 304 lines in 35 files changed: 1 ins; 144 del; 159 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

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

[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 two 
additional commits since the last revision:

 - Merge pull request #11 from JornVernee/JEP-412-MXCSR
   
   Add MXCSR save and restore to upcall stubs for non-windows platforms
 - Add MXCSR save and restore to upcall stubs for non-windows platforms

-

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

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

  Stats: 32 lines in 1 file changed: 26 ins; 1 del; 5 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: 8262891: Compiler implementation for Pattern Matching for switch (Preview) [v3]

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

  Avoiding fall-through from the total case to a synthetic default but changing 
total patterns to default.

-

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

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

  Stats: 22 lines in 2 files changed: 21 ins; 0 del; 1 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: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal

[Alan Bateman]

On Mon, 17 May 2021 18:23:41 GMT, Weijun Wang  wrote:

> Please review this implementation of [JEP 
> 411](https://openjdk.java.net/jeps/411).
> 
> The code change is divided into 3 commits. Please review them one by one.
> 
> 1. 
> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1
>  The essential change for this JEP, including the `@Deprecate` annotations 
> and spec change. It also update the default value of the 
> `java.security.manager` system property to "disallow", and necessary test 
> change following this update.
> 2. 
> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66
>  Manual changes to several files so that the next commit can be generated 
> programatically.
> 3. 
> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950
>  Automatic changes to other source files to avoid javac warnings on 
> deprecation for removal
> 
> The 1st and 2nd commits should be reviewed carefully. The 3rd one is 
> generated programmatically, see the comment below for more details. If you 
> are only interested in a portion of the 3rd commit and would like to review 
> it as a separate file, please comment here and I'll generate an individual 
> webrev.
> 
> Due to the size of this PR, no attempt is made to update copyright years for 
> any file to minimize unnecessary merge conflict.
> 
> Furthermore, since the default value of `java.security.manager` system 
> property is now "disallow", most of the tests calling 
> `System.setSecurityManager()` need to launched with 
> `-Djava.security.manager=allow`. This is covered in a different PR at 
> https://github.com/openjdk/jdk/pull/4071.

src/java.base/share/classes/java/lang/SecurityManager.java line 315:

> 313:  *
> 314:  * @since   1.0
> 315:  * @deprecated The Security Manager is deprecated and subject to removal 
> in a

Javadoc will prefix, in bold, "Deprecated, for removal: This API element is 
subject to removal in a future version". I'm just wondering if the sentence 
will be repeated here, can you check?

-

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