Re: RFR: 8266054: VectorAPI rotate operation optimization [v5]

[Jatin Bhateja]

On Sat, 8 May 2021 15:40:53 GMT, Paul Sandoz  wrote:

> Looks good. Someone from the HotSpot side needs to review related changes.
> 
> The way i read the perf numbers is that on non AVX512 systems the numbers are 
> in the noise (no worse, no better), with significant improvement on AVX512.

Hi @PaulSandoz, thanks for your suggestions and review, yes AVX2 speedup is an 
artifact of the code re-organization, since C2 now directly dismantles the 
rotates thus this offer better in-lining behavior compared to existing 
implementation and this is what show up in the performance data.

-

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

Re: RFR: 8266784: java/text/Collator/RuleBasedCollatorTest.java fails with jtreg 6

[Joe Wang]

On Mon, 10 May 2021 23:12:04 GMT, Naoto Sato  wrote:

> Please review this test case fix for the upcoming jtreg 6. The test was using 
> `@BeforeGroups` annotation, and the behavior of it has changed in TestNG 7.1 
> so that it is only issued when the test was configured with filtering. 
> Changed to use `@BeforeClass` instead.

Marked as reviewed by joehw (Reviewer).

-

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

Re: RFR: 8266456: Replace direct TKit.run() calls with jdk.jpackage.test.Annotations.Test annotation [v2]

[Alexander Matveev]

On Sat, 8 May 2021 01:44:46 GMT, Alexander Matveev  wrote:

>> - Replaced direct TKit.run() calls with Test annotation.
>>  - Increased timeout for SigningPackageTest from default to 360 due to 
>> timeout. This is regression from JDK-8248904 due to changes done in signing 
>> and --remove-signature adds additional time since it is run per file.
>>  - Fixed issue with jtreg.SkippedException which caused test to fail instead 
>> of being skipped, since it was wrapped in ExceptionBox.
>
> Alexander Matveev has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8266456: Replace direct TKit.run() calls with 
> jdk.jpackage.test.Annotations.Test annotation [v2]

Changed to check for RuntimeException as Alexey suggested. Also, added missing 
throw at line 158 in Functional.java.

-

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

Re: RFR: 8266456: Replace direct TKit.run() calls with jdk.jpackage.test.Annotations.Test annotation [v3]

[Alexander Matveev]

> - Replaced direct TKit.run() calls with Test annotation.
>  - Increased timeout for SigningPackageTest from default to 360 due to 
> timeout. This is regression from JDK-8248904 due to changes done in signing 
> and --remove-signature adds additional time since it is run per file.
>  - Fixed issue with jtreg.SkippedException which caused test to fail instead 
> of being skipped, since it was wrapped in ExceptionBox.

Alexander Matveev has updated the pull request incrementally with one 
additional commit since the last revision:

  8266456: Replace direct TKit.run() calls with 
jdk.jpackage.test.Annotations.Test annotation [v3]

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3911/files
  - new: https://git.openjdk.java.net/jdk/pull/3911/files/b382561f..523a2c65

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

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

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

RFR: 8266857: PipedOutputStream.sink should be volatile

[Liam Miller-Cushon]

8266857: PipedOutputStream.sink should be volatile

-

Commit messages:
 - 8266857: PipedOutputStream.sink should be volatile

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

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

RFR: 8266784: java/text/Collator/RuleBasedCollatorTest.java fails with jtreg 6

[Naoto Sato]

Please review this test case fix for the upcoming jtreg 6. The test was using 
`@BeforeGroups` annotation, and the behavior of it has changed in TestNG 7.1 so 
that it is only issued when the test was configured with filtering. Changed to 
use `@BeforeClass` instead.

-

Commit messages:
 - 8266784: java/text/Collator/RuleBasedCollatorTest.java fails with jtreg 6

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

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

Re: Review CSR JDK-8266760: Remove sun.misc.Unsafe::defineAnonymousClass

[Mandy Chung]

Thank you all.

Mandy

On 5/10/21 2:27 PM, Paul Sandoz wrote:

Looks good, I took the liberty of making some minor edits, mostly fixing some 
typos.

Paul.


On May 10, 2021, at 12:26 PM, Mandy Chung  wrote:

Hidden classes were added in Java SE 15. Class data support was added in 16. 
`sun.misc.Unsafe::defineAnonymousClass` was deprecated in JDK 15 and deprecated 
for terminally removal in JDK 16.

I propose to remove `sun.misc.Unsafe::defineAnonymousClass` from JDK 17:
CSR: https://bugs.openjdk.java.net/browse/JDK-8266760

Comments?

Mandy

Re: RFR: 8266317: Vector API enhancements [v3]

[Paul Sandoz]

> This PR contains API and implementation changes for [JEP-414 Vector API 
> (Second Incubator)](https://openjdk.java.net/jeps/414), in preparation for 
> when targeted.
> 
> Enhancements are made to the API for the support of operations on characters, 
> such as for UTF-8 character decoding. Specifically, methods for 
> loading/storing a `short` vector from/to a `char[]` array, and new vector 
> comparison operators for unsigned comparisons with integral vectors. The x64 
> implementation is enhanced to supported unsigned comparisons.
> 
> Enhancements are made to the API for loading/storing a `byte` vector from/to 
> a `boolean[]` array.
> 
> The testing of loads/stores can be expanded for scatter/gather, but before 
> doing that i think some refactoring of the tests is required to reposition 
> tests in the right classes. I would like to do that work after integration of 
> the PR.

Paul Sandoz has updated the pull request incrementally with one additional 
commit since the last revision:

  JavaDoc refs for unsigned operators.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3803/files
  - new: https://git.openjdk.java.net/jdk/pull/3803/files/93b093c1..aa6bb717

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

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

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

Integrated: 8265128: [REDO] Optimize Vector API slice and unslice operations

[Sandhya Viswanathan]

On Thu, 29 Apr 2021 21:29:03 GMT, Sandhya Viswanathan 
 wrote:

> All the slice and unslice variants that take more than one argument can 
> benefit from already intrinsic methods on similar lines as slice(origin) and 
> unslice(origin).
> 
> Changes include:
>  * Rewrite Vector API slice/unslice using already intrinsic methods
>  * Fix in library_call.cpp:inline_preconditions_checkIndex() to not modify 
> control if intrinsification fails
>  * Vector API conversion tests thresholds adjustment
>  
> Base Performance:
> Benchmark (size) Mode Cnt Score Error Units
> TestSlice.vectorSliceOrigin 1024 thrpt 5 11763.372 ± 254.580 ops/ms
> TestSlice.vectorSliceOriginVector 1024 thrpt 5 599.286 ± 326.770 ops/ms
> TestSlice.vectorSliceUnsliceOrigin 1024 thrpt 5 6627.601 ± 22.060 ops/ms
> TestSlice.vectorSliceUnsliceOriginVector 1024 thrpt 5 401.858 ± 220.340 ops/ms
> TestSlice.vectorSliceUnsliceOriginVectorPart 1024 thrpt 5 421.993 ± 231.703 
> ops/ms
> 
> Performance with patch:
> Benchmark (size) Mode Cnt Score Error Units
> TestSlice.vectorSliceOrigin 1024 thrpt 5 11792.091 ± 37.296 ops/ms
> TestSlice.vectorSliceOriginVector 1024 thrpt 5 8388.174 ± 115.886 ops/ms
> TestSlice.vectorSliceUnsliceOrigin 1024 thrpt 5 6662.159 ± 8.203 ops/ms
> TestSlice.vectorSliceUnsliceOriginVector 1024 thrpt 5 5206.300 ± 43.637 ops/ms
> TestSlice.vectorSliceUnsliceOriginVectorPart 1024 thrpt 5 5194.278 ± 13.376 
> ops/ms

This pull request has now been integrated.

Changeset: 23446f1f
Author:Sandhya Viswanathan 
URL:   
https://git.openjdk.java.net/jdk/commit/23446f1f5ee087376bc1ab89413a011fc52bde1f
Stats: 881 lines in 43 files changed: 172 ins; 518 del; 191 mod

8265128: [REDO] Optimize Vector API slice and unslice operations

Reviewed-by: psandoz, vlivanov

-

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

Re: Would lambda expressions be meaningful annotation properties?

[Brian Goetz]

Yes, this has been considered at some length.  The summary verdict is:

 - Method references for static/unbound methods seem like reasonable 
constant literals to put in annotations, not unlike class literals.

 - Lambdas, on the other hand: absolutely, positively, not.

To actually get to method refs in annotations, there's a lot of tedious 
but not terribly hard work at a number of levels: JVMS, JLS, reflection, 
plus of course JVM runtime and javac.  Given the relatively broad cost, 
it hasn't been too high on the priority list, but it is a 
reasonable-to-have feature.


On 5/10/2021 5:40 PM, Rafael Winterhalter wrote:

Hello, I was wondering if there was ever any consideration of allowing
(stateless) lambda expressions as annotation members. As an example, this
would make the following lambda expression possible:

@interface MyAnnotation {
   Supplier value();
}

In many enterprise applications, this would be a very helpful addition and
replace a rather complicated pattern with little type-safety. For example,
in Spring, beans can be registered conditionally. Today, this requires
declaring a class that implements the Condition interface which then can be
supplied as an argument to the Conditional annotation:

public class MyCondition implements Condition {
   @Override
   public boolean matches(ConditionContext context, AnnotatedTypeMetadata
metadata) {
 return MyCode.isEnabled(context, metadata)
   }
}

which is then used in:

@Conditional(MyCondition.class) @Bean
public MyBean myBean() { ... }

By allowing stateless lambda expressions as annotation members, this could
be simplified to:

@Conditional(MyCode::isEnabled) @Bean
public MyBean myBean() { ... }

Another example where this would improve code readability a lot would be
bean validation frameworks, where custom constraints could be moved
directly to a property:

class MyBean {
   @Valid(value -> value != null && MyCode.validate(value))
   String property;
}

I observe such patterns regularly and therefore, I was curious if such a
feature was ever discussed or if this would be considered for a future
version. I understand that the intention of annotations is to provide
declarative metadata which can be processed also for unloaded code, but I
still feel like this would be a useful extension. The lambda expression
would be implicitly stateless, but of course they represent code that
requires class loading and would therefore be not necessarily meaningful to
annotation processors or other static code processing tools. If this would
be a feature to consider and only not a priority, I would be happy to
contribute, given I could get some help around the formalities of such a
process.

Thanks, Rafael

Re: RFR: 8265128: [REDO] Optimize Vector API slice and unslice operations [v4]

[Sandhya Viswanathan]

On Mon, 10 May 2021 18:31:30 GMT, Sandhya Viswanathan 
 wrote:

>> All the slice and unslice variants that take more than one argument can 
>> benefit from already intrinsic methods on similar lines as slice(origin) and 
>> unslice(origin).
>> 
>> Changes include:
>>  * Rewrite Vector API slice/unslice using already intrinsic methods
>>  * Fix in library_call.cpp:inline_preconditions_checkIndex() to not modify 
>> control if intrinsification fails
>>  * Vector API conversion tests thresholds adjustment
>>  
>> Base Performance:
>> Benchmark (size) Mode Cnt Score Error Units
>> TestSlice.vectorSliceOrigin 1024 thrpt 5 11763.372 ± 254.580 ops/ms
>> TestSlice.vectorSliceOriginVector 1024 thrpt 5 599.286 ± 326.770 ops/ms
>> TestSlice.vectorSliceUnsliceOrigin 1024 thrpt 5 6627.601 ± 22.060 ops/ms
>> TestSlice.vectorSliceUnsliceOriginVector 1024 thrpt 5 401.858 ± 220.340 
>> ops/ms
>> TestSlice.vectorSliceUnsliceOriginVectorPart 1024 thrpt 5 421.993 ± 231.703 
>> ops/ms
>> 
>> Performance with patch:
>> Benchmark (size) Mode Cnt Score Error Units
>> TestSlice.vectorSliceOrigin 1024 thrpt 5 11792.091 ± 37.296 ops/ms
>> TestSlice.vectorSliceOriginVector 1024 thrpt 5 8388.174 ± 115.886 ops/ms
>> TestSlice.vectorSliceUnsliceOrigin 1024 thrpt 5 6662.159 ± 8.203 ops/ms
>> TestSlice.vectorSliceUnsliceOriginVector 1024 thrpt 5 5206.300 ± 43.637 
>> ops/ms
>> TestSlice.vectorSliceUnsliceOriginVectorPart 1024 thrpt 5 5194.278 ± 13.376 
>> ops/ms
>
> Sandhya Viswanathan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   library_call.cpp changes not needed after Objects.checkIndex arguments fixed

Thanks a lot for the review Paul and Vladimir.
Vladimir, I will submit a separate patch for 
LibraryCallKit::inline_preconditions_checkIndex fix.

-

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

Re: RFR: 8266317: Vector API enhancements [v2]

[Vladimir Ivanov]

On Mon, 10 May 2021 21:37:25 GMT, Paul Sandoz  wrote:

>> This PR contains API and implementation changes for [JEP-414 Vector API 
>> (Second Incubator)](https://openjdk.java.net/jeps/414), in preparation for 
>> when targeted.
>> 
>> Enhancements are made to the API for the support of operations on 
>> characters, such as for UTF-8 character decoding. Specifically, methods for 
>> loading/storing a `short` vector from/to a `char[]` array, and new vector 
>> comparison operators for unsigned comparisons with integral vectors. The x64 
>> implementation is enhanced to supported unsigned comparisons.
>> 
>> Enhancements are made to the API for loading/storing a `byte` vector from/to 
>> a `boolean[]` array.
>> 
>> The testing of loads/stores can be expanded for scatter/gather, but before 
>> doing that i think some refactoring of the tests is required to reposition 
>> tests in the right classes. I would like to do that work after integration 
>> of the PR.
>
> Paul Sandoz has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Rename method.

Marked as reviewed by vlivanov (Reviewer).

-

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

Would lambda expressions be meaningful annotation properties?

[Rafael Winterhalter]

Hello, I was wondering if there was ever any consideration of allowing
(stateless) lambda expressions as annotation members. As an example, this
would make the following lambda expression possible:

@interface MyAnnotation {
  Supplier value();
}

In many enterprise applications, this would be a very helpful addition and
replace a rather complicated pattern with little type-safety. For example,
in Spring, beans can be registered conditionally. Today, this requires
declaring a class that implements the Condition interface which then can be
supplied as an argument to the Conditional annotation:

public class MyCondition implements Condition {
  @Override
  public boolean matches(ConditionContext context, AnnotatedTypeMetadata
metadata) {
return MyCode.isEnabled(context, metadata)
  }
}

which is then used in:

@Conditional(MyCondition.class) @Bean
public MyBean myBean() { ... }

By allowing stateless lambda expressions as annotation members, this could
be simplified to:

@Conditional(MyCode::isEnabled) @Bean
public MyBean myBean() { ... }

Another example where this would improve code readability a lot would be
bean validation frameworks, where custom constraints could be moved
directly to a property:

class MyBean {
  @Valid(value -> value != null && MyCode.validate(value))
  String property;
}

I observe such patterns regularly and therefore, I was curious if such a
feature was ever discussed or if this would be considered for a future
version. I understand that the intention of annotations is to provide
declarative metadata which can be processed also for unloaded code, but I
still feel like this would be a useful extension. The lambda expression
would be implicitly stateless, but of course they represent code that
requires class loading and would therefore be not necessarily meaningful to
annotation processors or other static code processing tools. If this would
be a feature to consider and only not a priority, I would be happy to
contribute, given I could get some help around the formalities of such a
process.

Thanks, Rafael

Re: RFR: 8266317: Vector API enhancements [v2]

[Paul Sandoz]

> This PR contains API and implementation changes for [JEP-414 Vector API 
> (Second Incubator)](https://openjdk.java.net/jeps/414), in preparation for 
> when targeted.
> 
> Enhancements are made to the API for the support of operations on characters, 
> such as for UTF-8 character decoding. Specifically, methods for 
> loading/storing a `short` vector from/to a `char[]` array, and new vector 
> comparison operators for unsigned comparisons with integral vectors. The x64 
> implementation is enhanced to supported unsigned comparisons.
> 
> Enhancements are made to the API for loading/storing a `byte` vector from/to 
> a `boolean[]` array.
> 
> The testing of loads/stores can be expanded for scatter/gather, but before 
> doing that i think some refactoring of the tests is required to reposition 
> tests in the right classes. I would like to do that work after integration of 
> the PR.

Paul Sandoz has updated the pull request incrementally with one additional 
commit since the last revision:

  Rename method.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3803/files
  - new: https://git.openjdk.java.net/jdk/pull/3803/files/de210a88..93b093c1

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

  Stats: 7 lines in 7 files changed: 0 ins; 0 del; 7 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3803.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3803/head:pull/3803

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

Re: RFR: 8266317: Vector API enhancements [v2]

[Paul Sandoz]

On Mon, 10 May 2021 21:05:02 GMT, Vladimir Ivanov  wrote:

>> Paul Sandoz has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Rename method.
>
> src/hotspot/cpu/aarch64/aarch64.ad line 2440:
> 
>> 2438: }
>> 2439: 
>> 2440: bool Matcher::supports_unsigned_vector_comparison(int vlen, BasicType 
>> bt) {
> 
> `Matcher::supports_vector_comparison_unsigned()` looks a better fit.

I renamed the method (see commit).

-

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

Re: Review CSR JDK-8266760: Remove sun.misc.Unsafe::defineAnonymousClass

[Paul Sandoz]

Looks good, I took the liberty of making some minor edits, mostly fixing some 
typos.

Paul.

> On May 10, 2021, at 12:26 PM, Mandy Chung  wrote:
> 
> Hidden classes were added in Java SE 15. Class data support was added in 16. 
> `sun.misc.Unsafe::defineAnonymousClass` was deprecated in JDK 15 and 
> deprecated for terminally removal in JDK 16.
> 
> I propose to remove `sun.misc.Unsafe::defineAnonymousClass` from JDK 17:
> CSR: https://bugs.openjdk.java.net/browse/JDK-8266760
> 
> Comments?
> 
> Mandy

Re: RFR: 8266317: Vector API enhancements

[Vladimir Ivanov]

On Thu, 29 Apr 2021 21:13:38 GMT, Paul Sandoz  wrote:

> This PR contains API and implementation changes for [JEP-414 Vector API 
> (Second Incubator)](https://openjdk.java.net/jeps/414), in preparation for 
> when targeted.
> 
> Enhancements are made to the API for the support of operations on characters, 
> such as for UTF-8 character decoding. Specifically, methods for 
> loading/storing a `short` vector from/to a `char[]` array, and new vector 
> comparison operators for unsigned comparisons with integral vectors. The x64 
> implementation is enhanced to supported unsigned comparisons.
> 
> Enhancements are made to the API for loading/storing a `byte` vector from/to 
> a `boolean[]` array.
> 
> The testing of loads/stores can be expanded for scatter/gather, but before 
> doing that i think some refactoring of the tests is required to reposition 
> tests in the right classes. I would like to do that work after integration of 
> the PR.

Looks good.

src/hotspot/cpu/aarch64/aarch64.ad line 2440:

> 2438: }
> 2439: 
> 2440: bool Matcher::supports_unsigned_vector_comparison(int vlen, BasicType 
> bt) {

`Matcher::supports_vector_comparison_unsigned()` looks a better fit.

-

Marked as reviewed by vlivanov (Reviewer).

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

Re: RFR: 8265128: [REDO] Optimize Vector API slice and unslice operations [v4]

[Vladimir Ivanov]

On Mon, 10 May 2021 18:31:30 GMT, Sandhya Viswanathan 
 wrote:

>> All the slice and unslice variants that take more than one argument can 
>> benefit from already intrinsic methods on similar lines as slice(origin) and 
>> unslice(origin).
>> 
>> Changes include:
>>  * Rewrite Vector API slice/unslice using already intrinsic methods
>>  * Fix in library_call.cpp:inline_preconditions_checkIndex() to not modify 
>> control if intrinsification fails
>>  * Vector API conversion tests thresholds adjustment
>>  
>> Base Performance:
>> Benchmark (size) Mode Cnt Score Error Units
>> TestSlice.vectorSliceOrigin 1024 thrpt 5 11763.372 ± 254.580 ops/ms
>> TestSlice.vectorSliceOriginVector 1024 thrpt 5 599.286 ± 326.770 ops/ms
>> TestSlice.vectorSliceUnsliceOrigin 1024 thrpt 5 6627.601 ± 22.060 ops/ms
>> TestSlice.vectorSliceUnsliceOriginVector 1024 thrpt 5 401.858 ± 220.340 
>> ops/ms
>> TestSlice.vectorSliceUnsliceOriginVectorPart 1024 thrpt 5 421.993 ± 231.703 
>> ops/ms
>> 
>> Performance with patch:
>> Benchmark (size) Mode Cnt Score Error Units
>> TestSlice.vectorSliceOrigin 1024 thrpt 5 11792.091 ± 37.296 ops/ms
>> TestSlice.vectorSliceOriginVector 1024 thrpt 5 8388.174 ± 115.886 ops/ms
>> TestSlice.vectorSliceUnsliceOrigin 1024 thrpt 5 6662.159 ± 8.203 ops/ms
>> TestSlice.vectorSliceUnsliceOriginVector 1024 thrpt 5 5206.300 ± 43.637 
>> ops/ms
>> TestSlice.vectorSliceUnsliceOriginVectorPart 1024 thrpt 5 5194.278 ± 13.376 
>> ops/ms
>
> Sandhya Viswanathan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   library_call.cpp changes not needed after Objects.checkIndex arguments fixed

Looks good.

PS: I still think there's a problem with 
`LibraryCallKit::inline_preconditions_checkIndex`: it shouldn't bail out 
intrinsification in the middle of the process leaving a partially constructed 
graph behind. I don't see why short-circuiting the logic once the path is dead 
(`if (stopped()) return true;`) won't work. But that's a topic for another fix.

-

Marked as reviewed by vlivanov (Reviewer).

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

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

[Maurizio Cimadamore]

On Fri, 30 Apr 2021 17:20:21 GMT, Mandy Chung  wrote:

>>> I think the implementation does not support that. I will also need to look 
>>> into how this impacts JDK-8266010. As I suggest earlier, I'm fine to do 
>>> this as a follow up after integration.
>> 
>> I've added `@CS` in the interface methods too. I've also added a stronger 
>> test which creates method handles in one module (which doesn't have native 
>> access) and then calls them from another module (which does NOT have native 
>> access enabled), and make sure that all call fails as expected (e.g. that 
>> caller context is that of the lookup module).
>> 
>> Surprisingly, CheckCSMs does NOT fail if both interface and implementation 
>> methods are `@CS` annotated - but if only interface is annotated, the test 
>> fails. This seems odd - since I can't find any logic in the test which check 
>> for overriding. Is the test accidentally passing?
>
>> I've added `@CS` in the interface methods too. I've also added a stronger 
>> test which creates method handles in one module (which doesn't have native 
>> access) and then calls them from another module (which does NOT have native 
>> access enabled), and make sure that all call fails as expected (e.g. that 
>> caller context is that of the lookup module).
> 
> Thanks.
>  
>> Surprisingly, CheckCSMs does NOT fail if both interface and implementation 
>> methods are `@CS` annotated - but if only interface is annotated, the test 
>> fails. This seems odd - since I can't find any logic in the test which check 
>> for overriding. Is the test accidentally passing?
> 
> I create JDK-8266383 and I will look into that.

Thanks for the review @mlchung - hopefully looks better now!

-

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

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

[Maurizio Cimadamore]

> This PR contains the API and implementation changes for JEP-412 [1]. A more 
> detailed description of such changes, to avoid repetitions during the review 
> process, is included as a separate comment.
> 
> [1] - https://openjdk.java.net/jeps/412

Maurizio Cimadamore has updated the pull request incrementally with one 
additional commit since the last revision:

  * Remove unused imports
  * Fix broken javadoc after removal of @throws clauses
  * Remove other `@CallerSensitive` annotations from `AbstractCLinker`

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3699/files
  - new: https://git.openjdk.java.net/jdk/pull/3699/files/8de9da36..6701654c

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

  Stats: 24 lines in 7 files changed: 0 ins; 23 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3699.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3699/head:pull/3699

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

Re: Review CSR JDK-8266760: Remove sun.misc.Unsafe::defineAnonymousClass

[Remi Forax]

- Mail original -
> De: "mandy chung" 
> À: "core-libs-dev" , "valhalla-dev" 
> 
> Envoyé: Lundi 10 Mai 2021 21:26:33
> Objet: Review CSR JDK-8266760: Remove sun.misc.Unsafe::defineAnonymousClass

> Hidden classes were added in Java SE 15. Class data support was added in
> 16. `sun.misc.Unsafe::defineAnonymousClass` was deprecated in JDK 15 and
> deprecated for terminally removal in JDK 16.
> 
> I propose to remove `sun.misc.Unsafe::defineAnonymousClass` from JDK 17:
> CSR: https://bugs.openjdk.java.net/browse/JDK-8266760
> 
> Comments?

Do it.

I will be brave and not cry too much :)

> 
> Mandy

Rémi

Review CSR JDK-8266760: Remove sun.misc.Unsafe::defineAnonymousClass

[Mandy Chung]

Hidden classes were added in Java SE 15. Class data support was added in 
16. `sun.misc.Unsafe::defineAnonymousClass` was deprecated in JDK 15 and 
deprecated for terminally removal in JDK 16.


I propose to remove `sun.misc.Unsafe::defineAnonymousClass` from JDK 17:
CSR: https://bugs.openjdk.java.net/browse/JDK-8266760

Comments?

Mandy

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

[Mandy Chung]

On Mon, 10 May 2021 18:15:01 GMT, Maurizio Cimadamore  
wrote:

>> This PR contains the API and implementation changes for JEP-412 [1]. A more 
>> detailed description of such changes, to avoid repetitions during the review 
>> process, is included as a separate comment.
>> 
>> [1] - https://openjdk.java.net/jeps/412
>
> Maurizio Cimadamore has updated the pull request with a new target base due 
> to a merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains 23 additional 
> commits since the last revision:
> 
>  - Merge branch 'master' into JEP-412
>  - Remove redundant checks for --enable-native-access
>  - Fix issue in snippet in package-info
>  - Replace uses of -Djdk.foreign.restricted (useless now) with 
> --enable-native-access
>  - Fix message string in Reflection::ensureNativeAccess
>  - Tweak comment in Module::enableNativeAccess
>  - Tweak code some more
>  - Use uniform naming convention for implementation metods in Module
>  - Remove IllegalNativeAccessChecker
>  - Remove redundant initializer in Module
>  - ... and 13 more: 
> https://git.openjdk.java.net/jdk/compare/5a35ba03...8de9da36

The caller-sensitive cleanup change looks good.

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java line 
148:

> 146:  * @throws IllegalArgumentException in the case of a method type and 
> function descriptor mismatch.
> 147:  * {@code --enable-native-access} is either absent, or does not 
> mention the module name {@code M}, or
> 148:  * {@code ALL-UNNAMED} in case {@code M} is an unnamed module.

line 147-148 are leftover from ICE and they should be removed.

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java line 
209:

> 207:  * than the thread owning {@code scope}.
> 208:  * {@code --enable-native-access} is either absent, or does not 
> mention the module name {@code M}, or
> 209:  * {@code ALL-UNNAMED} in case {@code M} is an unnamed module.

line 208-209 are also left-over from the last patch.

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/AbstractCLinker.java
 line 44:

> 42: @CallerSensitive
> 43: public final MethodHandle downcallHandle(Addressable symbol, 
> MethodType type, FunctionDescriptor function) {
> 44: Reflection.ensureNativeAccess(Reflection.getCallerClass());

`downcallHandle` method is no longer caller-sensitive.  `@CallerSensitive` and 
`Reflection.ensureNativeAccess` are no longer needed.Same for the 4-arg 
`downcallHandle` method.

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/LibrariesHelper.java
 line 38:

> 36: import jdk.internal.loader.NativeLibrary;
> 37: import jdk.internal.reflect.CallerSensitive;
> 38: import jdk.internal.reflect.Reflection;

I think these 2 imports are no longer used.

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/aarch64/AArch64Linker.java
 line 37:

> 35: import jdk.internal.foreign.abi.UpcallStubs;
> 36: import jdk.internal.reflect.CallerSensitive;
> 37: import jdk.internal.reflect.Reflection;

These 2 imports are no longer needed.  Same for other CLinker implementation 
classes.

-

Marked as reviewed by mchung (Reviewer).

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

Re: RFR: 8265128: [REDO] Optimize Vector API slice and unslice operations [v4]

[Paul Sandoz]

On Mon, 10 May 2021 18:31:30 GMT, Sandhya Viswanathan 
 wrote:

>> All the slice and unslice variants that take more than one argument can 
>> benefit from already intrinsic methods on similar lines as slice(origin) and 
>> unslice(origin).
>> 
>> Changes include:
>>  * Rewrite Vector API slice/unslice using already intrinsic methods
>>  * Fix in library_call.cpp:inline_preconditions_checkIndex() to not modify 
>> control if intrinsification fails
>>  * Vector API conversion tests thresholds adjustment
>>  
>> Base Performance:
>> Benchmark (size) Mode Cnt Score Error Units
>> TestSlice.vectorSliceOrigin 1024 thrpt 5 11763.372 ± 254.580 ops/ms
>> TestSlice.vectorSliceOriginVector 1024 thrpt 5 599.286 ± 326.770 ops/ms
>> TestSlice.vectorSliceUnsliceOrigin 1024 thrpt 5 6627.601 ± 22.060 ops/ms
>> TestSlice.vectorSliceUnsliceOriginVector 1024 thrpt 5 401.858 ± 220.340 
>> ops/ms
>> TestSlice.vectorSliceUnsliceOriginVectorPart 1024 thrpt 5 421.993 ± 231.703 
>> ops/ms
>> 
>> Performance with patch:
>> Benchmark (size) Mode Cnt Score Error Units
>> TestSlice.vectorSliceOrigin 1024 thrpt 5 11792.091 ± 37.296 ops/ms
>> TestSlice.vectorSliceOriginVector 1024 thrpt 5 8388.174 ± 115.886 ops/ms
>> TestSlice.vectorSliceUnsliceOrigin 1024 thrpt 5 6662.159 ± 8.203 ops/ms
>> TestSlice.vectorSliceUnsliceOriginVector 1024 thrpt 5 5206.300 ± 43.637 
>> ops/ms
>> TestSlice.vectorSliceUnsliceOriginVectorPart 1024 thrpt 5 5194.278 ± 13.376 
>> ops/ms
>
> Sandhya Viswanathan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   library_call.cpp changes not needed after Objects.checkIndex arguments fixed

I don't claim to understand the HotSpot details, but Java changes still look 
good.

-

Marked as reviewed by psandoz (Reviewer).

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

Integrated: 8265208: [JEP-356] : SplittableRandom and SplittableGenerators - splits() methods does not throw NullPointerException when source is null

[Jim Laskey]

On Wed, 14 Apr 2021 17:33:30 GMT, Jim Laskey  wrote:

> Add check for null.

This pull request has now been integrated.

Changeset: 0cc7833f
Author:Jim Laskey 
URL:   
https://git.openjdk.java.net/jdk/commit/0cc7833f3d84971dd03a9a620585152a6debb40e
Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod

8265208: [JEP-356] : SplittableRandom and SplittableGenerators - splits() 
methods does not throw NullPointerException when source is null

Reviewed-by: rriggs

-

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

Re: RFR: 8265128: [REDO] Optimize Vector API slice and unslice operations [v3]

[Sandhya Viswanathan]

On Fri, 30 Apr 2021 23:34:15 GMT, Paul Sandoz  wrote:

>>> @PaulSandoz would it be possible for you to run this through your testing?
>> 
>> Started, will report back when done.
>
>> > @PaulSandoz would it be possible for you to run this through your testing?
>> 
>> Started, will report back when done.
> 
> Tier 1 to 3 tests all pass on build profiles linux-x64 linux-aarch64 
> macosx-x64 windows-x64

@PaulSandoz After we fixed the Objects.checkIndex arguments per your review 
comment, the changes in library_call.cpp are no more needed so I have backed 
them out.  That leaves only changes in vector api code which you have reviewed. 
Please let me know if it is ok to push.

-

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

Re: RFR: 8265128: [REDO] Optimize Vector API slice and unslice operations [v4]

[Sandhya Viswanathan]

> All the slice and unslice variants that take more than one argument can 
> benefit from already intrinsic methods on similar lines as slice(origin) and 
> unslice(origin).
> 
> Changes include:
>  * Rewrite Vector API slice/unslice using already intrinsic methods
>  * Fix in library_call.cpp:inline_preconditions_checkIndex() to not modify 
> control if intrinsification fails
>  * Vector API conversion tests thresholds adjustment
>  
> Base Performance:
> Benchmark (size) Mode Cnt Score Error Units
> TestSlice.vectorSliceOrigin 1024 thrpt 5 11763.372 ± 254.580 ops/ms
> TestSlice.vectorSliceOriginVector 1024 thrpt 5 599.286 ± 326.770 ops/ms
> TestSlice.vectorSliceUnsliceOrigin 1024 thrpt 5 6627.601 ± 22.060 ops/ms
> TestSlice.vectorSliceUnsliceOriginVector 1024 thrpt 5 401.858 ± 220.340 ops/ms
> TestSlice.vectorSliceUnsliceOriginVectorPart 1024 thrpt 5 421.993 ± 231.703 
> ops/ms
> 
> Performance with patch:
> Benchmark (size) Mode Cnt Score Error Units
> TestSlice.vectorSliceOrigin 1024 thrpt 5 11792.091 ± 37.296 ops/ms
> TestSlice.vectorSliceOriginVector 1024 thrpt 5 8388.174 ± 115.886 ops/ms
> TestSlice.vectorSliceUnsliceOrigin 1024 thrpt 5 6662.159 ± 8.203 ops/ms
> TestSlice.vectorSliceUnsliceOriginVector 1024 thrpt 5 5206.300 ± 43.637 ops/ms
> TestSlice.vectorSliceUnsliceOriginVectorPart 1024 thrpt 5 5194.278 ± 13.376 
> ops/ms

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

  library_call.cpp changes not needed after Objects.checkIndex arguments fixed

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3804/files
  - new: https://git.openjdk.java.net/jdk/pull/3804/files/94f184ef..14439667

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

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

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

Re: RFR: 8265208: [JEP-356] : SplittableRandom and SplittableGenerators - splits() methods does not throw NullPointerException when source is null [v2]

[Roger Riggs]

On Mon, 10 May 2021 18:17:30 GMT, Jim Laskey  wrote:

>> Add check for null.
>
> Jim Laskey has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now contains four commits:
> 
>  - Merge branch 'master' into 8265208
>  - Check for null source
>  - Make makeXXXSpliterator final
>  - Move makeXXXSpilterator from public (@hidden) to protected. No API change.

Marked as reviewed by rriggs (Reviewer).

-

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

Re: RFR: 8265208: [JEP-356] : SplittableRandom and SplittableGenerators - splits() methods does not throw NullPointerException when source is null [v2]

[Jim Laskey]

> Add check for null.

Jim Laskey has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now contains four commits:

 - Merge branch 'master' into 8265208
 - Check for null source
 - Make makeXXXSpliterator final
 - Move makeXXXSpilterator from public (@hidden) to protected. No API change.

-

Changes: https://git.openjdk.java.net/jdk/pull/3495/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3495=01
  Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3495.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3495/head:pull/3495

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

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

[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 with a new target base due to 
a merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains 23 additional commits 
since the last revision:

 - Merge branch 'master' into JEP-412
 - Remove redundant checks for --enable-native-access
 - Fix issue in snippet in package-info
 - Replace uses of -Djdk.foreign.restricted (useless now) with 
--enable-native-access
 - Fix message string in Reflection::ensureNativeAccess
 - Tweak comment in Module::enableNativeAccess
 - Tweak code some more
 - Use uniform naming convention for implementation metods in Module
 - Remove IllegalNativeAccessChecker
 - Remove redundant initializer in Module
 - ... and 13 more: https://git.openjdk.java.net/jdk/compare/bb4038eb...8de9da36

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3699/files
  - new: https://git.openjdk.java.net/jdk/pull/3699/files/3aaeb09f..8de9da36

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

  Stats: 33173 lines in 668 files changed: 19837 ins; 7268 del; 6068 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: 8264777: Overload optimized FileInputStream::readAllBytes [v7]

[Brian Burkhalter]

> Please consider this request to override the `java.io.InputStream` methods 
> `readAllBytes()` and `readNBytes(int)` in `FileInputStream` with more 
> performant implementations. The method overrides attempt to read all 
> requested bytes into a single array of the required size rather than 
> composing the result from a sequence of smaller arrays. An example of the 
> performance improvements is as follows.
> 
> **readAllBytes**
> Before
> 
> Benchmark(length)   Mode  Cnt  Score  
>Error  Units
> ReadAllBytes.readAllBytesFileInputStream  100  thrpt   20   2412.031 
> ±   7.309  ops/s
> ReadAllBytes.readAllBytesFileInputStream 1000  thrpt   20212.181 
> ±   0.369  ops/s
> ReadAllBytes.readAllBytesFileInputStream1  thrpt   20 19.860 
> ±   0.048  ops/s
> ReadAllBytes.readAllBytesFileInputStream   10  thrpt   20  1.298 
> ±   0.183  ops/s
> 
> After
> 
> Benchmark(length)   Mode  Cnt  Score  
>Error  Units
> ReadAllBytes.readAllBytesFileInputStream  100  thrpt   20   8218.473 
> ±  43.425  ops/s
> ReadAllBytes.readAllBytesFileInputStream 1000  thrpt   20302.781 
> ±   1.273  ops/s
> ReadAllBytes.readAllBytesFileInputStream1  thrpt   20 22.461 
> ±   0.084  ops/s
> ReadAllBytes.readAllBytesFileInputStream   10  thrpt   20  1.502 
> ±   0.003  ops/s
> 
> 
> **readNBytes**
> 
> `N = file_size / 2`
> 
> Before
> 
> Benchmark(length)   Mode  Cnt  Score  
>Error  Units
> ReadAllBytes.readHalfBytesFileInputStream 100  thrpt   20   5585.500 
> ± 153.353  ops/s
> ReadAllBytes.readHalfBytesFileInputStream1000  thrpt   20436.164 
> ±   1.104  ops/s
> ReadAllBytes.readHalfBytesFileInputStream   1  thrpt   20 40.167 
> ±   0.141  ops/s
> ReadAllBytes.readHalfBytesFileInputStream  10  thrpt   20  3.291 
> ±   0.211  ops/s
> 
> 
> After
> 
> Benchmark(length)   Mode  Cnt  Score  
>Error  Units
> ReadAllBytes.readHalfBytesFileInputStream 100  thrpt   20  15463.210 
> ±  66.045  ops/s
> ReadAllBytes.readHalfBytesFileInputStream1000  thrpt   20561.752 
> ±   0.951  ops/s
> ReadAllBytes.readHalfBytesFileInputStream   1  thrpt   20 45.043 
> ±   0.102  ops/s
> ReadAllBytes.readHalfBytesFileInputStream  10  thrpt   20  4.629 
> ±   0.035  ops/s

Brian Burkhalter has updated the pull request incrementally with one additional 
commit since the last revision:

  8264777: Handle readNBytes(0): src/test

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3845/files
  - new: https://git.openjdk.java.net/jdk/pull/3845/files/5272d5ef..4fae0209

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

  Stats: 6 lines in 2 files changed: 5 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3845.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3845/head:pull/3845

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

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

[Maurizio Cimadamore]

> This PR contains the API and implementation changes for JEP-412 [1]. A more 
> detailed description of such changes, to avoid repetitions during the review 
> process, is included as a separate comment.
> 
> [1] - https://openjdk.java.net/jeps/412

Maurizio Cimadamore has updated the pull request incrementally with one 
additional commit since the last revision:

  Remove redundant checks for --enable-native-access

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3699/files
  - new: https://git.openjdk.java.net/jdk/pull/3699/files/1ce6366a..3aaeb09f

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

  Stats: 64 lines in 7 files changed: 0 ins; 63 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3699.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3699/head:pull/3699

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

RFR: CSR JDK-8266553 Technical corrections to java/util/random/package-info.java

[Jim Laskey]

Could I get a CSR review. Technical clarification in the javadoc.

Cheers,

-- Jim


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

PR: https://github.com/openjdk/jdk/pull/3881 

Bug: https://bugs.openjdk.java.net/browse/JDK-8266552 

Integrated: 8266603: jpackage: Add missing copyright file in Java runtime .deb installers

[Alexey Semenyuk]

On Thu, 6 May 2021 01:22:41 GMT, Alexey Semenyuk  wrote:

> jpackage should create copyright file in /usr/share/doc directory tree when 
> building .deb package for Java runtime with installation directory in /usr 
> directory tree.
> 
> jpackage creates share/doc/copyright file in installation directory for apps 
> installed outside of /usr tree.
> 
> jpackage creates /usr/share/doc/${package_name}/copyright file for apps 
> installed in /usr tree .
> 
> jpackage doesn't create copyright file at all for Java runtime. The reason 
> for this behavior was that jpackage should not place additional files in 
> installation directory of Java runtime. However when installing Java runtime 
> or app in /usr tree, copyright file will be placed outside of installation 
> directory. Thus copyright file should be always created if package 
> installation directory is inside of /usr tree.

This pull request has now been integrated.

Changeset: c8b74474
Author:Alexey Semenyuk 
URL:   
https://git.openjdk.java.net/jdk/commit/c8b744743bd54a00a4f7bf1e852d454fcd942abd
Stats: 27 lines in 2 files changed: 24 ins; 1 del; 2 mod

8266603: jpackage: Add missing copyright file in Java runtime .deb installers

Reviewed-by: almatvee, herrick

-

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

Re: RFR: 8266603: jpackage: Add missing copyright file in Java runtime .deb installers [v2]

[Andy Herrick]

On Fri, 7 May 2021 19:57:17 GMT, Alexey Semenyuk  wrote:

>> jpackage should create copyright file in /usr/share/doc directory tree when 
>> building .deb package for Java runtime with installation directory in /usr 
>> directory tree.
>> 
>> jpackage creates share/doc/copyright file in installation directory for apps 
>> installed outside of /usr tree.
>> 
>> jpackage creates /usr/share/doc/${package_name}/copyright file for apps 
>> installed in /usr tree .
>> 
>> jpackage doesn't create copyright file at all for Java runtime. The reason 
>> for this behavior was that jpackage should not place additional files in 
>> installation directory of Java runtime. However when installing Java runtime 
>> or app in /usr tree, copyright file will be placed outside of installation 
>> directory. Thus copyright file should be always created if package 
>> installation directory is inside of /usr tree.
>
> Alexey Semenyuk has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains two commits:
> 
>  - trailing whitespaces removed
>  - 8266603: jpackage: Add missing copyright file in Java runtime .deb 
> installers

Marked as reviewed by herrick (Reviewer).

-

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

Integrated: 8266774: System property values for stdout/err on Windows UTF-8

[Naoto Sato]

On Fri, 7 May 2021 22:46:08 GMT, Naoto Sato  wrote:

> Please review this small fix to Windows system property init code. This is 
> leftover from the support for `Console.charset()` method, where it lacked to 
> initialize `sun.stdout/err.encoding` to `UTF-8` for the code page `cp65001`.  
> The fix has been manually verified, but no automated test case is provided, 
> as automatically setting `UTF-8` for the system locale on Windows test 
> machine seems not possible.

This pull request has now been integrated.

Changeset: c494efc5
Author:Naoto Sato 
URL:   
https://git.openjdk.java.net/jdk/commit/c494efc5b5d9a142fceff600285fd4c8c883e795
Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod

8266774: System property values for stdout/err on Windows UTF-8

Reviewed-by: bpb, alanb

-

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

Re: RFR: 8266456: Replace direct TKit.run() calls with jdk.jpackage.test.Annotations.Test annotation [v2]

[Alexey Semenyuk]

On Fri, 7 May 2021 23:07:02 GMT, Alexander Matveev  wrote:

>> test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Functional.java line 161:
>> 
>>> 159: }
>>> 160: 
>>> 161: if 
>>> (throwable.getClass().getName().equals("jtreg.SkippedException")) {
>> 
>> Would it make sense to have check: `if (throwable instanceof Runnable)`?
>
> Not sure. I do not think it will work. SkippedException extends 
> RuntimeException, so not sure why we need to check it with Runnable.

My point is that if the exception is Runnable, there is no need to wrap it in 
ExceptionBox instance.

-

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

Re: RFR: 8266456: Replace direct TKit.run() calls with jdk.jpackage.test.Annotations.Test annotation [v2]

[Alexey Semenyuk]

On Sat, 8 May 2021 01:44:46 GMT, Alexander Matveev  wrote:

>> - Replaced direct TKit.run() calls with Test annotation.
>>  - Increased timeout for SigningPackageTest from default to 360 due to 
>> timeout. This is regression from JDK-8248904 due to changes done in signing 
>> and --remove-signature adds additional time since it is run per file.
>>  - Fixed issue with jtreg.SkippedException which caused test to fail instead 
>> of being skipped, since it was wrapped in ExceptionBox.
>
> Alexander Matveev has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8266456: Replace direct TKit.run() calls with 
> jdk.jpackage.test.Annotations.Test annotation [v2]

Changes requested by asemenyuk (Reviewer).

-

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

Integrated: JDK-8266797: Fix for 8266610 breaks the build on macos

[Vyom Tewari]

On Mon, 10 May 2021 06:34:36 GMT, Vyom Tewari  wrote:

> this change will  include  the below headers files to Linux only. 
> #include 
> #include 

This pull request has now been integrated.

Changeset: b823b3ef
Author:Vyom Tewari 
URL:   
https://git.openjdk.java.net/jdk/commit/b823b3ef2912c4c3b8412dff6ff4e9af81c5b910
Stats: 5 lines in 1 file changed: 3 ins; 0 del; 2 mod

8266797: Fix for 8266610 breaks the build on macos

Reviewed-by: dholmes, jdv

-

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

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

[Alan Bateman]

On Mon, 10 May 2021 10:31:40 GMT, Aleksey Shipilev  wrote:

>> No, change is specific to Linux. Please 
>> see(https://github.com/openjdk/jdk/commit/69b96f9a1b4a959c6b86f41c2259d9e4d47c8ede).
>
> OK, so what you are saying is that the path that references `S_ISBLK` is 
> protected by `BLKGETSIZE64` that is guaranteed to be undefined on OS X? If 
> so, this is (awkwardly) fine.

I think Vymon took the code from FileDispatcherImpl_size0, which has been 
compiling on macOS without issues. I don't mind if we fix the issue with this 
PR or just back it out and fix it with another PR.

-

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

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

[Alan Bateman]

On Mon, 10 May 2021 06:34:36 GMT, Vyom Tewari  wrote:

> this change will  include  the below headers files to Linux only. 
> #include 
> #include 

src/java.base/unix/native/libjava/io_util_md.c line 256:

> 254: return -1;
> 255: }
> 256: #if defined(__linux__) && defined(BLKGETSIZE64)

Looks okay although would be good to fix the indentation at L256 before you 
integrate.

-

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

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

[Vyom Tewari]

On Mon, 10 May 2021 09:50:44 GMT, Aleksey Shipilev  wrote:

>> this change will  include  the below headers files to Linux only. 
>> #include 
>> #include 
>
> src/java.base/unix/native/libjava/io_util_md.c line 39:
> 
>> 37: #if defined(__linux__)
>> 38: #include 
>> 39: #include 
> 
> Does Mac OS need `sys/stat.h`, though?

No, change is specific to Linux. Please 
see(https://github.com/openjdk/jdk/commit/69b96f9a1b4a959c6b86f41c2259d9e4d47c8ede).

-

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

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

[Vyom Tewari]

On Mon, 10 May 2021 11:26:30 GMT, Alan Bateman  wrote:

>> this change will  include  the below headers files to Linux only. 
>> #include 
>> #include 
>
> src/java.base/unix/native/libjava/io_util_md.c line 256:
> 
>> 254: return -1;
>> 255: }
>> 256: #if defined(__linux__) && defined(BLKGETSIZE64)
> 
> Looks okay although would be good to fix the indentation at L256 before you 
> integrate.

done fixed the indentation

-

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

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

[Vyom Tewari]

On Mon, 10 May 2021 11:09:07 GMT, Vyom Tewari  wrote:

>> I am not the Mac expert, but i checked on my MacOS Laptop and i did not 
>> found "BLKGETSIZE64" defined in sys/stat.h.
>> As Alan already told that i  took the code from FileDispatcherImpl.size0 and 
>> followed the same pattern.
>
> I will fix in this PR only. i will do the changes for only "io_util_md.c".

updated the PR as suggested.

-

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

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

[Vyom Tewari]

On Mon, 10 May 2021 10:59:37 GMT, Vyom Tewari  wrote:

>> I think Vymon took the code from FileDispatcherImpl_size0, which has been 
>> compiling on macOS without issues. I don't mind if we fix the issue with 
>> this PR or just back it out and fix it with another PR.
>
> I am not the Mac expert, but i checked on my MacOS Laptop and i did not found 
> "BLKGETSIZE64" defined in sys/stat.h.
> As Alan already told that i  took the code from FileDispatcherImpl.size0 and 
> followed the same pattern.

I will fix in this PR only. i will do the changes for only "io_util_md.c".

-

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

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

[Aleksey Shipilev]

On Mon, 10 May 2021 10:27:34 GMT, Vyom Tewari  wrote:

>> src/java.base/unix/native/libjava/io_util_md.c line 39:
>> 
>>> 37: #if defined(__linux__)
>>> 38: #include 
>>> 39: #include 
>> 
>> Does Mac OS need `sys/stat.h`, though?
>
> No, change is specific to Linux. Please 
> see(https://github.com/openjdk/jdk/commit/69b96f9a1b4a959c6b86f41c2259d9e4d47c8ede).

OK, so what you are saying is that the path that references `S_ISBLK` is 
protected by `BLKGETSIZE64` that is guaranteed to be undefined on OS X? If so, 
this is (awkwardly) fine.

-

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

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

[Vyom Tewari]

On Mon, 10 May 2021 10:44:10 GMT, Alan Bateman  wrote:

>> OK, so what you are saying is that the path that references `S_ISBLK` is 
>> protected by `BLKGETSIZE64` that is guaranteed to be undefined on OS X? If 
>> so, this is (awkwardly) fine.
>
> I think Vymon took the code from FileDispatcherImpl_size0, which has been 
> compiling on macOS without issues. I don't mind if we fix the issue with this 
> PR or just back it out and fix it with another PR.

I am not the Mac expert, but i checked on my MacOS Laptop and i did not found 
"BLKGETSIZE64" defined in sys/stat.h.
As Alan already told that i  took the code from FileDispatcherImpl.size0 and 
followed the same pattern.

-

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

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

[David Holmes]

On Mon, 10 May 2021 08:46:15 GMT, Vyom Tewari  wrote:

> Original fix was intended to Linux specific. Current Fix will not impact 
> other platforms.

Your original fix failed to be Linux specific, so given you never actually 
tested it on other platforms you don't know whether another part of the fix is 
also not actually Linux-specific.

At a minimum ensure you have GitHub actions enabled so that your fixes get some 
basic cross-platform testing before they are integrated. Thanks.

-

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

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

[Vyom Tewari]

On Mon, 10 May 2021 11:21:34 GMT, Vyom Tewari  wrote:

>>> Hi Vyom,
>>> 
>>> That fixes the include file problem but as this was obviously not tested on 
>>> non-Linux have you checked that the rest of the fix for 8266610 works okay 
>>> on non-Linux?
>>> 
>>> Thanks,
>>> David
>> 
>> Hi David,
>> 
>> Original fix  was intended to  Linux specific. Current  Fix  will not impact 
>> other platforms. 
>> 
>> There is existing test (java/nio/channels/FileChannel/BlockDeviceSize.java) 
>> which test the new code but currently it is silently passing without running 
>> the test. This test requires root access.
>> 
>> Please see the bug( https://bugs.openjdk.java.net/browse/JDK-8150539) 
>> 
>> Thanks,
>> Vyom
>
>> @vyommani how are you going to test this?
> 
> I tested locally at my Linux environment. we have a test 
> "java/nio/channels/FileChannel/BlockDeviceSize.java" but  it requires root 
> privileged , i ran this as well as a root.  Please  have a look into 
> this(https://bugs.openjdk.java.net/browse/JDK-8150539).

> @vyommani You can start tier1 CI build to make sure build passes with this 
> fix.

can you please do let me know how to  start tier1 CI build ?

-

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

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

[Vyom Tewari]

On Mon, 10 May 2021 06:34:36 GMT, Vyom Tewari  wrote:

> this change will  include  the below headers files to Linux only. 
> #include 
> #include 

somehow tests not working for me. 
https://github.com/vyommani/jdk/actions/runs/827989185

-

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

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

[Vyom Tewari]

On Mon, 10 May 2021 13:01:45 GMT, Jayathirth D V  wrote:

> In internal CI with fix tier 1 builds are running fine.

Ok i will integrate my changes now.

-

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

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

[Vyom Tewari]

On Mon, 10 May 2021 08:46:15 GMT, Vyom Tewari  wrote:

>> Hi Vyom,
>> 
>> That fixes the include file problem but as this was obviously not tested on 
>> non-Linux have you checked that the rest of the fix for 8266610 works okay 
>> on non-Linux?
>> 
>> Thanks,
>> David
>
>> Hi Vyom,
>> 
>> That fixes the include file problem but as this was obviously not tested on 
>> non-Linux have you checked that the rest of the fix for 8266610 works okay 
>> on non-Linux?
>> 
>> Thanks,
>> David
> 
> Hi David,
> 
> Original fix  was intended to  Linux specific. Current  Fix  will not impact 
> other platforms. 
> 
> There is existing test (java/nio/channels/FileChannel/BlockDeviceSize.java) 
> which test the new code but currently it is silently passing without running 
> the test. This test requires root access.
> 
> Please see the bug( https://bugs.openjdk.java.net/browse/JDK-8150539) 
> 
> Thanks,
> Vyom

> @vyommani how are you going to test this?

I tested locally at my Linux environment. we have a test 
"java/nio/channels/FileChannel/BlockDeviceSize.java" but  it requires root 
privileged , i ran this as well as a root.  Please  have a look into 
this(https://bugs.openjdk.java.net/browse/JDK-8150539).

-

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

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

[Pavel Rappo]

On Mon, 10 May 2021 06:34:36 GMT, Vyom Tewari  wrote:

> this change will  include  the below headers files to Linux only. 
> #include 
> #include 

The fix to 8266610 caused a build failure on macOS. What is your plan to make 
sure this won't happen with this change?

I would recommend enabling [GitHub 
Actions](https://docs.github.com/en/github/administering-a-repository/disabling-or-limiting-github-actions-for-a-repository)
 for the "vyommani/jdk" repo.

I have run this PR through our CI: the tier1 tests are fine on all supported 
platforms. Please integrate this PR as soon as possible. 

Separately, please sort out the way you test your PRs. Read the error message 
that the failed GitHub Actions gave you:
> Prerequisites
> Actions jobs for this repository have been disabled by GitHub staff. If you 
> are the repository owner, you can contact support via github.com/contact for 
> more information.

-

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

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

[Pavel Rappo]

On Mon, 10 May 2021 08:46:15 GMT, Vyom Tewari  wrote:

>> Hi Vyom,
>> 
>> That fixes the include file problem but as this was obviously not tested on 
>> non-Linux have you checked that the rest of the fix for 8266610 works okay 
>> on non-Linux?
>> 
>> Thanks,
>> David
>
>> Hi Vyom,
>> 
>> That fixes the include file problem but as this was obviously not tested on 
>> non-Linux have you checked that the rest of the fix for 8266610 works okay 
>> on non-Linux?
>> 
>> Thanks,
>> David
> 
> Hi David,
> 
> Original fix  was intended to  Linux specific. Current  Fix  will not impact 
> other platforms. 
> 
> There is existing test (java/nio/channels/FileChannel/BlockDeviceSize.java) 
> which test the new code but currently it is silently passing without running 
> the test. This test requires root access.
> 
> Please see the bug( https://bugs.openjdk.java.net/browse/JDK-8150539) 
> 
> Thanks,
> Vyom

@vyommani how are you going to test this?

-

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

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

[Jayathirth D V]

On Mon, 10 May 2021 11:21:34 GMT, Vyom Tewari  wrote:

>>> Hi Vyom,
>>> 
>>> That fixes the include file problem but as this was obviously not tested on 
>>> non-Linux have you checked that the rest of the fix for 8266610 works okay 
>>> on non-Linux?
>>> 
>>> Thanks,
>>> David
>> 
>> Hi David,
>> 
>> Original fix  was intended to  Linux specific. Current  Fix  will not impact 
>> other platforms. 
>> 
>> There is existing test (java/nio/channels/FileChannel/BlockDeviceSize.java) 
>> which test the new code but currently it is silently passing without running 
>> the test. This test requires root access.
>> 
>> Please see the bug( https://bugs.openjdk.java.net/browse/JDK-8150539) 
>> 
>> Thanks,
>> Vyom
>
>> @vyommani how are you going to test this?
> 
> I tested locally at my Linux environment. we have a test 
> "java/nio/channels/FileChannel/BlockDeviceSize.java" but  it requires root 
> privileged , i ran this as well as a root.  Please  have a look into 
> this(https://bugs.openjdk.java.net/browse/JDK-8150539).

@vyommani You can start tier1 CI build to make sure build passes with this fix.

-

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

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

[Vyom Tewari]

On Mon, 10 May 2021 07:39:44 GMT, David Holmes  wrote:

> Hi Vyom,
> 
> That fixes the include file problem but as this was obviously not tested on 
> non-Linux have you checked that the rest of the fix for 8266610 works okay on 
> non-Linux?
> 
> Thanks,
> David

Hi David,

Original fix  was intended to  Linux specific. Current  Fix  will not impact 
other platforms. 

There is existing test (java/nio/channels/FileChannel/BlockDeviceSize.java) 
which test the new code but currently it is silently passing without running 
the test. This test requires root access.

Please see the bug( https://bugs.openjdk.java.net/browse/JDK-8150539) 

Thanks,
Vyom

-

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

RFR: 8266791: Annotation property which is compiled as an array property but changed to a single element throws NullPointerException

[Rafael Winterhalter]

During annotation parsing, the parser assumes that a declared property is of an 
array type if the parsed annotation property is defined as an array. In this 
case, the component type is `null`, and a `NullPointerException` is thrown. 
This change discovers the mismatch and throws an 
`AnnotationTypeMismatchException`.

-

Commit messages:
 - 8266791: Annotation property which is compiled as an array property but 
changed to a single element throws NullPointerException
 - 8266766: Arrays of types that cannot be an annotation member do not yield 
exceptions

Changes: https://git.openjdk.java.net/jdk/pull/3940/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3940=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8266791
  Stats: 218 lines in 3 files changed: 216 ins; 1 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3940.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3940/head:pull/3940

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

Re: RFR: 8266791: Annotation property which is compiled as an array property but changed to a single element throws NullPointerException

[Rafael Winterhalter]

On Sun, 9 May 2021 21:59:29 GMT, Rafael Winterhalter  
wrote:

> During annotation parsing, the parser assumes that a declared property is of 
> an array type if the parsed annotation property is defined as an array. In 
> this case, the component type is `null`, and a `NullPointerException` is 
> thrown. This change discovers the mismatch and throws an 
> `AnnotationTypeMismatchException`.

I implemented this change on top of 
https://bugs.openjdk.java.net/browse/JDK-8266766 to avoid replicating most of 
that pull requests changes.

-

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

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

[Aleksey Shipilev]

On Mon, 10 May 2021 06:34:36 GMT, Vyom Tewari  wrote:

> this change will  include  the below headers files to Linux only. 
> #include 
> #include 

MacOS builds fail with GHA since recently. So enabling GHA would serve a basic 
test for this to work.

src/java.base/unix/native/libjava/io_util_md.c line 39:

> 37: #if defined(__linux__)
> 38: #include 
> 39: #include 

Does Mac OS need `sys/stat.h`, though?

-

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

RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

[Vyom Tewari]

this change will  include  the below headers files to Linux only. 
#include 
#include 

-

Commit messages:
 - fixed the indentation
 - added the additional check so that modified code get executed on linux only.
 - JDK-8266797: Fix for 8266610 breaks the build on macos

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

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

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

[Jayathirth D V]

On Mon, 10 May 2021 06:34:36 GMT, Vyom Tewari  wrote:

> this change will  include  the below headers files to Linux only. 
> #include 
> #include 

Marked as reviewed by jdv (Reviewer).

In internal CI with fix tier 1 builds are running fine.

-

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

Re: RFR: JDK-8266797: Fix for 8266610 breaks the build on macos

[David Holmes]

On Mon, 10 May 2021 06:34:36 GMT, Vyom Tewari  wrote:

> this change will  include  the below headers files to Linux only. 
> #include 
> #include 

Hi Vyom,

That fixes the include file problem but as this was obviously not tested on 
non-Linux have you checked that the rest of the fix for 8266610 works okay on 
non-Linux?

Thanks,
David

The /test functionality doesn't work. You need to either test locally or using 
GitHub actions.

-

Marked as reviewed by dholmes (Reviewer).

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

Re: [External] : Re: Proposal to add JavaScript platform to jpackage

[Kevin Rushforth]

Regardless of whether you integrate into jpackage or provide a new 
"jspackage" tool, this presupposes the capability of running a set of 
Java class files on top of a JavaScript engine. Pointing to a 
third-party library that happens to implement this isn't sufficient to 
define this new platform. Given that you are targeting a platform / 
runtime environment that isn't part of the JDK, then I don't see why the 
packaging tool for that platform should be part of the JDK.


-- Kevin


On 5/8/2021 8:47 AM, Andrew Oliver wrote:
Much of the feedback (public and private) from this proposal has 
focused on integration with jpackage.  There is concern that jpackage 
is strictly for platforms where the OpenJDK runtime is bundled with 
the class files in an installer for distribution on that platform.  I 
have a couple of questions for this list:


1. Is it the intent that jpackage _only_ be used for platforms where 
the OpenJDK runtime is bundled, in whole or in part?  The descriptions 
from the jpackage docs mentions taking a Java run-time image as input, 
but then says it produces an application that "includes all the 
necessary dependencies": "The |jpackage| tool will take as input a 
Java application and a Java run-time image, and produce a Java 
application image that includes all the necessary dependencies. It 
will be able to produce a native package in a platform-specific 
format, such as an exe on Windows or a dmg on macOS."


In this case, the Java application will be converted to JavaScript, 
and the required code to meet JLS semantics will be provided.  Since 
the runtime binary (java.exe) isn't needed, it is not a necessary 
dependency and is omitted.  The resulting native package is provided 
in a platform-specific format (WAR or ZIP), as appropriate for a web 
application.  To me, this certainly meets the letter of the jpackage 
description, but perhaps not the spirit.  I'm interested in more 
feedback on this point.


2. If the answer to (1) above is 'yes', jpackage is only suitable if 
you plan to bundle OpenJDK VM code, then I will propose a new tool, 
separate from jpackage.  The tentative name is "jspackage".


The jspackage command will take as input a Java application, and 
produce a web application that includes all of the necessary 
dependencies.  It will produce a cross-platform web application bundle 
in either WAR or ZIP format.



On Mon, Apr 26, 2021 at 5:39 AM Kevin Rushforth 
mailto:kevin.rushfo...@oracle.com>> wrote:


Without commenting on the value proposition of what you propose to
do, I
am fairly certain that jpackage is not the way to do it. The job of
jpackage is to take an application, bundle it with a Java Runtime,
and
create a native package / installer from it. What you are describing
goes far beyond that. You are describing a new capability of the JDK
that would take Java bytecode and compile it to run it on top of a
JavaScript engine.

> jpackage will use a JavaScript AOT compiler (TeaVM) to convert
the Java
> code to JavaScript, with the main class compiled to a JavaScript
method
> called 'main()'.

This is a good indicator that your proposal isn't simply targeting
a new
platform that already exists, and for which there is a Java
runtime that
supports running on this platform.

-- Kevin


On 4/25/2021 5:10 PM, Andrew Oliver wrote:
> While I agree it is a somewhat different platform than Linux,
Mac, or
> Windows, I do think the web is a platform worth targeting.  And
when seen
> through just a slightly different lens, it is more like the
others than it
> might first seem:
>
> On the platform:
> * It is difficult for users to run Java bytecode or JARs directly
> * Bytecode needs some form of transformation to operate efficiently
> * Packaging into a platform-specific format is required for easy
> distribution
> * Without a packager tool, developers have to surmount significant
> obstacles to distribute on the platform, reducing the appeal and
adoption
> of Java
>
> Yes, there are maven and gradle plugins available to allow Java
to target
> the JavaScript platform.  ( For example,
> https://teavm.org/docs/intro/getting-started.html


)
>
> However, for many users a browser-friendly solution with a small
number of
> dependencies is going to be the only option.  Take, for example,
new users,
> students, and educational settings.  In many cases, programming
assignments
> are required to be posted on the web.  If the JDK could target
> self-contained web applications, as per this proposal, students
could
> easily post their assignments for the whole class to see.  This
would be
> much 

JDK Dynalink only works with unconditionally exported packages

[Thiago Henrique Hupner]

Hi all.

I've been testing the JDK Dynalink recently and
I think I've found a bug.

The class jdk.dynalink.beans.CheckRestrictedPackage checks
if a package is restricted.

So, I have a class that has some static methods. But to Dynalink find the
class,
the class needs to be public. OK,
I've changed my class to be public. However, it needs that
the package of the class is exported in the module too.

I think this is a little too restrictive. I don't want to expose
to my users some internal implementation.

The main problem is that the method
CheckRestrictedPackage::isRestrictedClass
does the following check:

// ...
final String pkgName = name.substring(0, i);
final Module module = clazz.getModule();
if (module != null && !module.isExported(pkgName)) {
return true;
 }
// ...

I have a feeling that this check should be changed to something like

// ...
final String pkgName = name.substring(0, i);
final Module module = clazz.getModule();
if (module != null && !module.isOpen(pkgName,
CheckRestrictedPackage.class.getModule())) {
return true;
 }
// ...

In this way, my code can only "opens" a single package to the jdk.dynalink
module,
not having to unconditionally export a package.

>From what I could understand, in Nashorn, the generated classes
were defined in the unnamed module, that always exports all the packages,
so the access always worked.

The code can be found here:
https://github.com/openjdk/jdk/blob/master/src/jdk.dynalink/share/classes/jdk/dynalink/beans/CheckRestrictedPackage.java#L94

Best regards.

Thiago

Re: RFR: 8265448: (zipfs): Reduce read contention in ZipFileSystem [v5]

[Lance Andersen]

On Fri, 7 May 2021 03:10:32 GMT, Jason Zaugg  wrote:

>> If the given Path represents a file, use the overload of read defined
>> in FileChannel that accepts an explicit position and avoid serializing
>> reads.
>> 
>> Note: The underlying NIO implementation is not required to implement
>> FileChannel.read(ByteBuffer, long) concurrently; Windows still appears
>> to lock, as it returns true for NativeDispatcher.needsPositionLock.
>> 
>> 
>> On MacOS X, the enclosed benchmark improves from:
>> 
>> 
>> BenchmarkMode  Cnt   Score   Error  Units
>> ZipFileSystemBenchmark.read  avgt   10  75.311 ? 3.301  ms/op
>> 
>> 
>> To:
>> 
>> 
>> BenchmarkMode  Cnt   Score   Error  Units
>> ZipFileSystemBenchmark.read  avgt   10  12.520 ? 0.875  ms/op
>
> Jason Zaugg has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   [zipfs] Add missing check-failed exception to position/read test
>   
>   This appears to have been omitted when this test was added.
>   To avoid false error reports, the condition must deal with the
>   edge case of zero-length entries, for which read will return -1.

Mach5 jdk-tier1, jdk-tier, jdk-tier3 completed successfully

-

Marked as reviewed by lancea (Reviewer).

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

Re: Collection::getAny discussion

[Remi Forax]

- Mail original -
> De: "Stephen Colebourne" 
> À: "core-libs-dev" 
> Envoyé: Vendredi 30 Avril 2021 23:15:45
> Objet: Re: Collection::getAny discussion

> On Fri, 30 Apr 2021 at 19:50, Stuart Marks  wrote:
>> You're asking for something that's somewhat different, which you called the
>> "find
>> the first element when there is only one" problem. Here, there's a 
>> precondition
>> that
>> the collection have a single element. (It's not clear to me what should 
>> happen
>> if
>> the collection has zero or more than one element.)
> 
> I think any get() or getAny() method on Collection is semantically
> equivalent to iterator.next(). I'm not sure there is another viable
> option.

Thinking a little more about conflating "first" and "any".
I wonder if we have not already cross the Rubicon on that matter,

If we have a HashSet or any collections and using Stream.findFirst()
  var collection = new HashSet<>(...); 
  var result = collection.stream().findFirst().orElseThrow();

We will get the result of collection.iterator().next()

So adding a default method getFirst() on Collection that returns the result of 
iterator().next() is pretty coherent, no ?

[...]

> 
> Stephen

Rémi

Re: [External] : Re: ReversibleCollection proposal

[forax]

- Mail original -
> De: "dfranken jdk" 
> À: "Remi Forax" , "Stuart Marks" 
> Cc: "core-libs-dev" 
> Envoyé: Dimanche 9 Mai 2021 12:13:58
> Objet: Re: [External] : Re: ReversibleCollection proposal

> When I thought about Collection's role in the hierarchy, it seemed to
> me that 'Collection' is an interface for describing how elements are
> stored in a cardboard box (we can and and remove them) and that we
> might need a different, yet related, interface to describe how to
> retrieve the items from the box. This way we are not tied to the
> Collection hierarchy and we could have one Set implementation which is
> ordered and another Set implementation which is not and they would both
> still implement Collection, but only one would implement the interface.

So you want to model ReversibleSet as Set + Reversible,
Reversible being an interface with a small number of method specific to the 
fact that the implementation is reversible. 

This does not work well for two reasons,
- there is no syntax to represent the union of types in Java,
   Set & Reversible set = ...
  is not a valid syntax. You can use a type variable with several bounds 
instead but it does work nicely with the rest of the language (overidding, 
overloading, etc).

- having public methods that takes an interface with few methods is a common 
design error.
  Let suppose you have a method foo that only prints the elements of a 
collection, in that case you may want to type the first parameter as Iterable 
or Collection.
  But requirements change an now you want to prints the elements sorted, oops, 
you have to change the signature of the public method which may be something 
not possible
  depending how many "clients" this method has.
  Providing interfaces with a small number of access methods will lead to this 
kind of issue. 

> 
> Imagine an interface like 'java.lang.OrderedEnumerable` if you will
> with methods such as 'first(), last(), etc', then each implementation
> would be free to choose to implement the interface or not. I also
> thought about 'OrderedIterable', but there would be too much confusion
> with 'Iterable', but I think these are related too. Retrieving items is
> an iteration problem, not a storage problem.

The problem is that is you multiply the number of interfaces to access the 
elements, you add the dilemma of choice in the mix.
The first iteration of the Scala Collection were like this, too many 
interfaces, at least for my taste. 

> 
> While I would love to see the Collection hierarchy redesigned to also
> allow for ImmutableCollection which for instance would not have an
> `add(T e)` method, I don't think we can simply do something like that
> without breaking too many things.

Dear Santa, i want an interface BuilderCollection that only allows add() but no 
remove()/clear(), because if you can not remove elements, then all the 
iterators can implement the snapshot at the beginning semantics,
so no ConcurrentModificationException anymore. For me, being able to 
snapshot/freeze a collection is better than an ImmutableCollection, because it 
can be immutable when you want. 

Anyway, it's not gonna to happen, there is so many ways to slice an onion and 
each have pros and cons and providing all the possible interfaces is a good.way 
to make something simple complex.

> 
> So in short, separating retrieval aspects from storage aspects with a
> different interface might be the way to go.
> 
> Kind regards,
> 
> Dave Franken
> 

regards,
Rémi

> On Wed, 2021-05-05 at 12:41 +0200, fo...@univ-mlv.fr wrote:
>> - Mail original -
>> > De: "Stuart Marks" 
>> > À: "Remi Forax" 
>> > Cc: "core-libs-dev" 
>> > Envoyé: Mercredi 5 Mai 2021 02:00:03
>> > Objet: Re: [External] : Re: ReversibleCollection proposal
>> 
>> > On 5/1/21 5:57 AM, fo...@univ-mlv.fr wrote:
>> > > I suppose the performance issue comes from the fact that
>> > > traversing a
>> > > LinkedHahSet is slow because it's a linked list ?
>> > > 
>> > > You can replace a LinkedHashSet by a List + Set, the List keeps
>> > > the values in
>> > > order, the Set avoids duplicates.
>> > > 
>> > > Using a Stream, it becomes
>> > >    Stream.of(getItemsFromSomeplace(), getItemsFromAnotherPlace(),
>> > >    getItemsFromSomeplaceElse())
>> > >     .flatMap(List::stream)
>> > >     .distinct()  // use a Set internally
>> > >     .collect(toList());
>> > 
>> > The problem with any example is that simplifying assumptions are
>> > necessary for
>> > showing the example, but those assumptions enable it to be easily
>> > picked apart.
>> > Of
>> > course, the example isn't just a particular example; it is a
>> > *template* for a
>> > whole
>> > space of possible examples. Consider the possibility that the items
>> > processing
>> > client needs to do some intermediate processing on the first group
>> > of items
>> > before
>> > adding the other groups. This might not be possible to do using
>> > streams. Use
>> > your
>> > imagination.
>> > 
>> > > I