Re: RFR: 8264208: Console charset API [v4]

2021-04-12 Thread Joe Wang
On Mon, 12 Apr 2021 23:01:24 GMT, Naoto Sato  wrote:

>> Please review the changes for the subject issue.  This has been suggested in 
>> a recent discussion thread for the JEP 400 
>> [[1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-March/075214.html)].
>>  A CSR has also been drafted, and comments are welcome 
>> [[2](https://bugs.openjdk.java.net/browse/JDK-8264209)].
>
> Naoto Sato has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Reverted PrintStream changes

src/java.base/share/classes/java/lang/System.java line 2020:

> 2018: setIn0(new BufferedInputStream(fdIn));
> 2019: setOut0(newPrintStream(fdOut, cs));
> 2020: setErr0(newPrintStream(fdErr, cs));

It was getting from sun.stdout.encoding or sun.stderr.encoding. After the 
change, when there is no console, it would be set with 
Charset.defaultCharset(). Would that be an incompatible change?

-

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


Re: Conflict API definitions of Math.pow(x, 0.5) and Math.sqrt(x) for x={-0.0, Double.NEGATIVE_INFINITY}(Internet mail)

2021-04-12 Thread 傅杰
Hi Joe,

Thanks for your nice sharing.

Very glad to know that the Java library implementation of pow does delegate to 
sqrt while respecting the relevant special cases.
This implementation [1] has set a good example for us which means it's safe to 
replace pow(x, 0.5) with sqrt(x) for all x > 0.0.

Thanks
Best regards,
Jie

[1] 
https://github.com/openjdk/jdk/blob/d84a7e55be40eae57b6c322694d55661a5053a55/src/java.base/share/classes/java/lang/FdLibm.java#L366


On 2021/4/13, 2:21 AM, "Joe Darcy"  wrote:

Hello,

Adding some additional context, more recent versions of the IEEE 754 
standard have given explicit recommendations for math library function 
definitions, including pow. The Java definitions of those methods long 
predate the IEEE 754 coverage and there are a small number of 
differences, including in the pow special cases. These differences are 
now described more explicitly in the JDK 17 specs as of

JDK-8240632: Note differences between IEEE 754-2019 math lib special 
cases and java.lang.Math

The difference in question for pow is:

>  * @apiNote
>  * The special cases definitions of this method differ from the
>  * special case definitions of the IEEE 754 recommended {@code
>  * pow} operation for {@code 1.0} raised to an infinite
>  * power. This method treats such cases as indeterminate and
>  * specifies a NaN is returned. The IEEE 754 specification treats
>  * the infinite power as a large integer (large-magnitude
>  * floating-point numbers are numerically integers, specifically
>  * even integers) and therefore specifies {@code 1.0} be returned.

There are no plans to align the Java definition of pow with the IEEE 754 
definition in these few cases.

Note that the Java library implementation of pow does delegate to sqrt 
while respecting the relevant special cases:

> } else if (y == 0.5) {
> if (x >= -Double.MAX_VALUE) // Handle x == -infinity later
> return Math.sqrt(x + 0.0); // Add 0.0 to properly 
> handle x == -0.0

https://github.com/openjdk/jdk/blob/d84a7e55be40eae57b6c322694d55661a5053a55/src/java.base/share/classes/java/lang/FdLibm.java#L366

Mathematically, the sqrt function is about as pleasant a function as you 
can be asked to approximate in floating-point arithmetic. The function 
is smooth, doesn't overflow or underflow, and has a simple 
Newton-iteration that can be expressed in basic arithmetic operations. 
The pow function doesn't enjoy these properties and has a larger design 
space for what a reasonable floating-point approximation could be.

HTH,

-Joe

On 4/12/2021 6:39 AM, jiefu(傅杰) wrote:
> Hi Raffaello,
>
> Thanks for your execllent analysis.
> I agree with you now.
> And I'll close the optimization PR [1] tomorrow if there is no objections.
>
> Thanks.
> Best regards,
> Jie
>
> [1] https://github.com/openjdk/jdk/pull/3404/
>
>
> On 2021/4/12, 9:06 PM, "Raffaello Giulietti" 
 wrote:
>
>  Hi Jie,
>  
>  I don't think that changing the spec of Math.pow() to be misaligned 
with
>  IEEE 754 would be a wise option. IEEE is much more pervasive than 
Java.
>  There are many aspects in IEEE that might be seen as questionable, 
but
>  at least it is a widely adopted standard.
>  
>  AFAIU, the only reason you would like to "optimize" the special case 
of
>  y = 0.5 in pow(x, y) to return sqrt(x) is for performance, more 
accuracy
>  and some kind of consistency.
>  
>  But then, why not a special case for y = 0.25 as sqrt(sqrt(x))?
>  And what about y = 0.75? Should this be translated to 
sqrt(sqrt(pow(x, 3)))?
>  What about y = 1.0 / 3.0? Should this become cbrt(x)?
>  And why not consider y = 2.0 / 3.0 in a special rule: cbrt(x * x)?
>  
>  You see, the special cases can quickly become unmanageable. Also,
>  special rules would produce results which are "discontinuous" with
>  nearby exponents, like y = 0.5001.
>  
>  That's probably why IEEE doesn't propose translation rules for finite
>  numerical exponents that are not integers, except when x is a 
special value.
>  
>  
>  Greetings
>  Raffaello
>  
>  
>  
>  On 2021-04-12 13:44, jiefu(傅杰) wrote:
>  > Hi Andrew H, Andrew D, and Raffaello,
>  >
>  > Thank you all for your kind reply and helpful comments.
>  >
>  > Now I got where the rules come from.
>  > But I don't think the IEEE standars are reasonable to specify 
conflits rules.
>  > Maybe, these computations should be open to 

Re: RFR: 8265029: Preserve SIZED characteristics on slice operations (skip, limit) [v2]

2021-04-12 Thread Paul Sandoz
On Sun, 11 Apr 2021 07:14:07 GMT, Tagir F. Valeev  wrote:

>> With the introduction of `toList()`, preserving the SIZED characteristics in 
>> more cases becomes more important. This patch preserves SIZED on `skip()` 
>> and `limit()` operations, so now every combination of 
>> `map/mapToX/boxed/asXyzStream/skip/limit/sorted` preserves size, and 
>> `toList()`, `toArray()` and `count()` may benefit from this. E. g., 
>> `LongStream.range(0, 10_000_000_000L).skip(1).count()` returns result 
>> instantly with this patch.
>> 
>> Some microbenchmarks added that confirm the reduced memory allocation in 
>> `toList()` and `toArray()` cases. Before patch:
>> 
>> ref.SliceToList.seq_baseline:·gc.alloc.rate.norm1  
>> thrpt   10   40235,534 ± 0,984B/op
>> ref.SliceToList.seq_limit:·gc.alloc.rate.norm   1  
>> thrpt   10  106431,101 ± 0,198B/op
>> ref.SliceToList.seq_skipLimit:·gc.alloc.rate.norm   1  
>> thrpt   10  106544,977 ± 1,983B/op
>> value.SliceToArray.seq_baseline:·gc.alloc.rate.norm 1  
>> thrpt   10   40121,878 ± 0,247B/op
>> value.SliceToArray.seq_limit:·gc.alloc.rate.norm1  
>> thrpt   10  106317,693 ± 1,083B/op
>> value.SliceToArray.seq_skipLimit:·gc.alloc.rate.norm1  
>> thrpt   10  106430,954 ± 0,136B/op
>> 
>> 
>> After patch:
>> 
>> ref.SliceToList.seq_baseline:·gc.alloc.rate.norm1  
>> thrpt   10  40235,648 ± 1,354B/op
>> ref.SliceToList.seq_limit:·gc.alloc.rate.norm   1  
>> thrpt   10  40355,784 ± 1,288B/op
>> ref.SliceToList.seq_skipLimit:·gc.alloc.rate.norm   1  
>> thrpt   10  40476,032 ± 2,855B/op
>> value.SliceToArray.seq_baseline:·gc.alloc.rate.norm 1  
>> thrpt   10  40121,830 ± 0,308B/op
>> value.SliceToArray.seq_limit:·gc.alloc.rate.norm1  
>> thrpt   10  40242,554 ± 0,443B/op
>> value.SliceToArray.seq_skipLimit:·gc.alloc.rate.norm1  
>> thrpt   10  40363,674 ± 1,576B/op
>> 
>> 
>> Time improvements are less exciting. It's likely that inlining and 
>> vectorizing dominate in these tests over array allocations and unnecessary 
>> copying. Still, I notice a significant improvement in SliceToArray.seq_limit 
>> case (2x) and mild improvement (+12..16%) in other slice tests. No 
>> significant change in parallel execution time, though its performance is 
>> much less stable and I didn't run enough tests.
>> 
>> Before patch:
>> 
>> Benchmark (size)   Mode  Cnt  Score Error  
>> Units
>> ref.SliceToList.par_baseline   1  thrpt   30  14876,723 ±  99,770  
>> ops/s
>> ref.SliceToList.par_limit  1  thrpt   30  14856,841 ± 215,089  
>> ops/s
>> ref.SliceToList.par_skipLimit  1  thrpt   30   9555,818 ± 991,335  
>> ops/s
>> ref.SliceToList.seq_baseline   1  thrpt   30  23732,290 ± 444,162  
>> ops/s
>> ref.SliceToList.seq_limit  1  thrpt   30  14894,040 ± 176,496  
>> ops/s
>> ref.SliceToList.seq_skipLimit  1  thrpt   30  10646,929 ±  36,469  
>> ops/s
>> value.SliceToArray.par_baseline1  thrpt   30  25093,141 ± 376,402  
>> ops/s
>> value.SliceToArray.par_limit   1  thrpt   30  24798,889 ± 760,762  
>> ops/s
>> value.SliceToArray.par_skipLimit   1  thrpt   30  16456,310 ± 926,882  
>> ops/s
>> value.SliceToArray.seq_baseline1  thrpt   30  69669,787 ± 494,562  
>> ops/s
>> value.SliceToArray.seq_limit   1  thrpt   30  21097,081 ± 117,338  
>> ops/s
>> value.SliceToArray.seq_skipLimit   1  thrpt   30  15522,871 ± 112,557  
>> ops/s
>> 
>> 
>> After patch:
>> 
>> Benchmark (size)   Mode  Cnt  Score  Error  
>> Units
>> ref.SliceToList.par_baseline   1  thrpt   30  14793,373 ±   64,905  
>> ops/s
>> ref.SliceToList.par_limit  1  thrpt   30  13301,024 ± 1300,431  
>> ops/s
>> ref.SliceToList.par_skipLimit  1  thrpt   30  11131,698 ± 1769,932  
>> ops/s
>> ref.SliceToList.seq_baseline   1  thrpt   30  24101,048 ±  263,528  
>> ops/s
>> ref.SliceToList.seq_limit  1  thrpt   30  16872,168 ±   76,696  
>> ops/s
>> ref.SliceToList.seq_skipLimit  1  thrpt   30  11953,253 ±  105,231  
>> ops/s
>> value.SliceToArray.par_baseline1  thrpt   30  25442,442 ±  455,554  
>> ops/s
>> value.SliceToArray.par_limit   1  thrpt   30  23111,730 ± 2246,086  
>> ops/s
>> value.SliceToArray.par_skipLimit   1  thrpt   30  17980,750 ± 2329,077  
>> ops/s
>> value.SliceToArray.seq_baseline1  thrpt   30  66512,898 ± 1001,042  
>> ops/s
>> value.SliceToArray.seq_limit   1  thrpt   30  41792,549 ± 1085,547  
>> ops/s
>> value.SliceToArray.seq_skipLimit   1  thrpt   30  18007,613 ±  141,716  
>> ops/s
>> 
>> 
>> I also 

Re: RFR: 8265029: Preserve SIZED characteristics on slice operations (skip, limit) [v2]

2021-04-12 Thread Paul Sandoz
On Sun, 11 Apr 2021 07:14:07 GMT, Tagir F. Valeev  wrote:

>> With the introduction of `toList()`, preserving the SIZED characteristics in 
>> more cases becomes more important. This patch preserves SIZED on `skip()` 
>> and `limit()` operations, so now every combination of 
>> `map/mapToX/boxed/asXyzStream/skip/limit/sorted` preserves size, and 
>> `toList()`, `toArray()` and `count()` may benefit from this. E. g., 
>> `LongStream.range(0, 10_000_000_000L).skip(1).count()` returns result 
>> instantly with this patch.
>> 
>> Some microbenchmarks added that confirm the reduced memory allocation in 
>> `toList()` and `toArray()` cases. Before patch:
>> 
>> ref.SliceToList.seq_baseline:·gc.alloc.rate.norm1  
>> thrpt   10   40235,534 ± 0,984B/op
>> ref.SliceToList.seq_limit:·gc.alloc.rate.norm   1  
>> thrpt   10  106431,101 ± 0,198B/op
>> ref.SliceToList.seq_skipLimit:·gc.alloc.rate.norm   1  
>> thrpt   10  106544,977 ± 1,983B/op
>> value.SliceToArray.seq_baseline:·gc.alloc.rate.norm 1  
>> thrpt   10   40121,878 ± 0,247B/op
>> value.SliceToArray.seq_limit:·gc.alloc.rate.norm1  
>> thrpt   10  106317,693 ± 1,083B/op
>> value.SliceToArray.seq_skipLimit:·gc.alloc.rate.norm1  
>> thrpt   10  106430,954 ± 0,136B/op
>> 
>> 
>> After patch:
>> 
>> ref.SliceToList.seq_baseline:·gc.alloc.rate.norm1  
>> thrpt   10  40235,648 ± 1,354B/op
>> ref.SliceToList.seq_limit:·gc.alloc.rate.norm   1  
>> thrpt   10  40355,784 ± 1,288B/op
>> ref.SliceToList.seq_skipLimit:·gc.alloc.rate.norm   1  
>> thrpt   10  40476,032 ± 2,855B/op
>> value.SliceToArray.seq_baseline:·gc.alloc.rate.norm 1  
>> thrpt   10  40121,830 ± 0,308B/op
>> value.SliceToArray.seq_limit:·gc.alloc.rate.norm1  
>> thrpt   10  40242,554 ± 0,443B/op
>> value.SliceToArray.seq_skipLimit:·gc.alloc.rate.norm1  
>> thrpt   10  40363,674 ± 1,576B/op
>> 
>> 
>> Time improvements are less exciting. It's likely that inlining and 
>> vectorizing dominate in these tests over array allocations and unnecessary 
>> copying. Still, I notice a significant improvement in SliceToArray.seq_limit 
>> case (2x) and mild improvement (+12..16%) in other slice tests. No 
>> significant change in parallel execution time, though its performance is 
>> much less stable and I didn't run enough tests.
>> 
>> Before patch:
>> 
>> Benchmark (size)   Mode  Cnt  Score Error  
>> Units
>> ref.SliceToList.par_baseline   1  thrpt   30  14876,723 ±  99,770  
>> ops/s
>> ref.SliceToList.par_limit  1  thrpt   30  14856,841 ± 215,089  
>> ops/s
>> ref.SliceToList.par_skipLimit  1  thrpt   30   9555,818 ± 991,335  
>> ops/s
>> ref.SliceToList.seq_baseline   1  thrpt   30  23732,290 ± 444,162  
>> ops/s
>> ref.SliceToList.seq_limit  1  thrpt   30  14894,040 ± 176,496  
>> ops/s
>> ref.SliceToList.seq_skipLimit  1  thrpt   30  10646,929 ±  36,469  
>> ops/s
>> value.SliceToArray.par_baseline1  thrpt   30  25093,141 ± 376,402  
>> ops/s
>> value.SliceToArray.par_limit   1  thrpt   30  24798,889 ± 760,762  
>> ops/s
>> value.SliceToArray.par_skipLimit   1  thrpt   30  16456,310 ± 926,882  
>> ops/s
>> value.SliceToArray.seq_baseline1  thrpt   30  69669,787 ± 494,562  
>> ops/s
>> value.SliceToArray.seq_limit   1  thrpt   30  21097,081 ± 117,338  
>> ops/s
>> value.SliceToArray.seq_skipLimit   1  thrpt   30  15522,871 ± 112,557  
>> ops/s
>> 
>> 
>> After patch:
>> 
>> Benchmark (size)   Mode  Cnt  Score  Error  
>> Units
>> ref.SliceToList.par_baseline   1  thrpt   30  14793,373 ±   64,905  
>> ops/s
>> ref.SliceToList.par_limit  1  thrpt   30  13301,024 ± 1300,431  
>> ops/s
>> ref.SliceToList.par_skipLimit  1  thrpt   30  11131,698 ± 1769,932  
>> ops/s
>> ref.SliceToList.seq_baseline   1  thrpt   30  24101,048 ±  263,528  
>> ops/s
>> ref.SliceToList.seq_limit  1  thrpt   30  16872,168 ±   76,696  
>> ops/s
>> ref.SliceToList.seq_skipLimit  1  thrpt   30  11953,253 ±  105,231  
>> ops/s
>> value.SliceToArray.par_baseline1  thrpt   30  25442,442 ±  455,554  
>> ops/s
>> value.SliceToArray.par_limit   1  thrpt   30  23111,730 ± 2246,086  
>> ops/s
>> value.SliceToArray.par_skipLimit   1  thrpt   30  17980,750 ± 2329,077  
>> ops/s
>> value.SliceToArray.seq_baseline1  thrpt   30  66512,898 ± 1001,042  
>> ops/s
>> value.SliceToArray.seq_limit   1  thrpt   30  41792,549 ± 1085,547  
>> ops/s
>> value.SliceToArray.seq_skipLimit   1  thrpt   30  18007,613 ±  141,716  
>> ops/s
>> 
>> 
>> I also 

Re: RFR: 8264208: Console charset API [v2]

2021-04-12 Thread Naoto Sato
On Mon, 12 Apr 2021 21:12:08 GMT, Naoto Sato  wrote:

>> src/java.base/share/classes/java/io/Console.java line 397:
>> 
>>> 395: /**
>>> 396:  * Returns the {@link java.nio.charset.Charset Charset} object 
>>> used in
>>> 397:  * this {@code Console}.
>> 
>> The Console is a singleton and the existing methods use "the console" so I 
>> think we should do the same here.
>> 
>> We'll need to add to the description of the System.{in,out,err} fields, I 
>> don't mind if we do it as part of this PR or another issue.
>
> Javadoc updated. Instead of modifying System.out/err (System.in is not 
> affected, as it does not convert b2c), I modified PrintStream javadoc.

Reverted the changes to PrintStream, as it defaults to 
Charset.defaultCharset(). Added descriptions to System.out/err instead (and 
modified the impl).

-

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


Re: RFR: 8264208: Console charset API [v4]

2021-04-12 Thread Naoto Sato
> Please review the changes for the subject issue.  This has been suggested in 
> a recent discussion thread for the JEP 400 
> [[1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-March/075214.html)].
>  A CSR has also been drafted, and comments are welcome 
> [[2](https://bugs.openjdk.java.net/browse/JDK-8264209)].

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

  Reverted PrintStream changes

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3419/files
  - new: https://git.openjdk.java.net/jdk/pull/3419/files/c172d0a1..68db1a79

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

  Stats: 50 lines in 2 files changed: 8 ins; 9 del; 33 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3419.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3419/head:pull/3419

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


Re: RFR: 8264806: Remove the experimental JIT compiler [v2]

2021-04-12 Thread Vladimir Kozlov
On Sun, 11 Apr 2021 10:25:47 GMT, Doug Simon  wrote:

>> Vladimir Kozlov has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains three additional 
>> commits since the last revision:
>> 
>>  - Restore Graal Builder image makefile
>>  - Merge latest changes from branch 'JDK-8264805' into JDK-8264806
>>  - 8264806: Remove the experimental JIT compiler
>
> We would definitely like to be able to continue testing of GraalVM with the 
> JDK set of jtreg tests. So keeping `Compiler::isGraalEnabled()` working like 
> it does today is important.

@dougxc  I restored Compiler::isGraalEnabled().

-

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


Re: RFR: 8264806: Remove the experimental JIT compiler [v3]

2021-04-12 Thread Vladimir Kozlov
> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related to 
> Java-based JIT compiler (Graal) from JDK:
> 
> - `jdk.internal.vm.compiler` — the Graal compiler 
> - `jdk.internal.vm.compiler.management` — Graal's `MBean`
> - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests
> 
> Remove Graal related code in makefiles.
> 
> Note, next two `module-info.java` files are preserved so that the JVMCI 
> module `jdk.internal.vm.ci` continues to build:
> 
> src/jdk.internal.vm.compiler/share/classes/module-info.java
> src/jdk.internal.vm.compiler.management/share/classes/module-info.java
> 
> 
> @AlanBateman suggested that we can avoid it by using Module API to export 
> packages at runtime . It requires changes in GraalVM's JVMCI too so I will 
> file followup RFE to implement it.
> 
> Tested hs-tier1-4

Vladimir Kozlov has updated the pull request incrementally with one additional 
commit since the last revision:

  Restore Compiler::isGraalEnabled()

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3421/files
  - new: https://git.openjdk.java.net/jdk/pull/3421/files/a246aaa6..9d6bd42c

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

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

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


Re: RFR: 8263157: [macos]: java.library.path is being set incorrectly

2021-04-12 Thread Alexey Semenyuk
On Mon, 12 Apr 2021 20:18:46 GMT, Alexander Matveev  
wrote:

> This is regression from JDK-8242302. Fixed by setting java.library.path to 
> same values as it was before JDK-8242302.

Marked as reviewed by asemenyuk (Reviewer).

-

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


Re: RFR: 8214761: Bug in parallel Kahan summation implementation

2021-04-12 Thread Ian Graves
On Mon, 12 Apr 2021 19:45:24 GMT, Ian Graves  wrote:

> Fixes a bug where the compensated sum should be negated when added together 
> in the merge step of a given collector. This impacts accuracy of parallel 
> summations with Double streams and creates larger deviations from a standard 
> sequential (ie non-parallel) compensated summation.

Duplicate of https://github.com/openjdk/jdk/pull/2988

-

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


Withdrawn: 8214761: Bug in parallel Kahan summation implementation

2021-04-12 Thread Ian Graves
On Mon, 12 Apr 2021 19:45:24 GMT, Ian Graves  wrote:

> Fixes a bug where the compensated sum should be negated when added together 
> in the merge step of a given collector. This impacts accuracy of parallel 
> summations with Double streams and creates larger deviations from a standard 
> sequential (ie non-parallel) compensated summation.

This pull request has been closed without being integrated.

-

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


Re: RFR: 8264208: Console charset API [v2]

2021-04-12 Thread Naoto Sato
On Sun, 11 Apr 2021 13:44:05 GMT, Alan Bateman  wrote:

>> Naoto Sato has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Reflected the review comments.
>
> src/java.base/share/classes/java/io/Console.java line 397:
> 
>> 395: /**
>> 396:  * Returns the {@link java.nio.charset.Charset Charset} object used 
>> in
>> 397:  * this {@code Console}.
> 
> The Console is a singleton and the existing methods use "the console" so I 
> think we should do the same here.
> 
> We'll need to add to the description of the System.{in,out,err} fields, I 
> don't mind if we do it as part of this PR or another issue.

Javadoc updated. Instead of modifying System.out/err (System.in is not 
affected, as it does not convert b2c), I modified PrintStream javadoc.

-

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


Re: RFR: 8264208: Console charset API [v3]

2021-04-12 Thread Naoto Sato
> Please review the changes for the subject issue.  This has been suggested in 
> a recent discussion thread for the JEP 400 
> [[1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-March/075214.html)].
>  A CSR has also been drafted, and comments are welcome 
> [[2](https://bugs.openjdk.java.net/browse/JDK-8264209)].

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

  Reflecting the review comments.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3419/files
  - new: https://git.openjdk.java.net/jdk/pull/3419/files/8fd8f6e6..c172d0a1

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

  Stats: 33 lines in 2 files changed: 3 ins; 0 del; 30 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3419.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3419/head:pull/3419

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


RFR: 8214761: Bug in parallel Kahan summation implementation

2021-04-12 Thread Ian Graves
Fixes a bug where the compensated sum should be negated when added together in 
the merge step of a given collector. This impacts accuracy of parallel 
summations with Double streams and creates larger deviations from a standard 
sequential (ie non-parallel) compensated summation.

-

Commit messages:
 - Fixing a compensated/Kahan summation bug that increase error

Changes: https://git.openjdk.java.net/jdk/pull/3442/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3442=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8214761
  Stats: 16 lines in 3 files changed: 10 ins; 0 del; 6 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3442.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3442/head:pull/3442

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


RFR: 8263157: [macos]: java.library.path is being set incorrectly

2021-04-12 Thread Alexander Matveev
This is regression from JDK-8242302. Fixed by setting java.library.path to same 
values as it was before JDK-8242302.

-

Commit messages:
 - 8263157: [macos]: java.library.path is being set incorrectly

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

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


Integrated: 8263763: Synthetic constructor parameters of enum are not considered for annotation indices

2021-04-12 Thread Rafael Winterhalter
On Thu, 18 Mar 2021 21:03:20 GMT, Rafael Winterhalter 
 wrote:

> 8263763: The constructor of an enumeration prefixes with two synthetic 
> arguments for constant name and ordinal index. For this reason, the 
> constructor annotations need to be shifted two indices to the right, such 
> that the annotation indices match with the parameter indices.

This pull request has now been integrated.

Changeset: 9dd96257
Author:Rafael Winterhalter 
Committer: Joe Darcy 
URL:   https://git.openjdk.java.net/jdk/commit/9dd96257
Stats: 73 lines in 4 files changed: 64 ins; 0 del; 9 mod

8263763: Synthetic constructor parameters of enum are not considered for 
annotation indices

Reviewed-by: darcy, jfranck

-

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


Re: RFR: 8264806: Remove the experimental JIT compiler [v2]

2021-04-12 Thread Erik Joelsson
On Mon, 12 Apr 2021 17:18:36 GMT, Vladimir Kozlov  wrote:

>> make/common/Modules.gmk line 68:
>> 
>>> 66: 
>>> 67: # Filter out Graal specific modules
>>> 68: MODULES_FILTER += jdk.internal.vm.compiler
>> 
>> If we are unconditionally filtering out these modules, then why leave the 
>> module-info.java files in at all?
>
> We filter out because we can't build Graal anymore. But we need these 
> module-info.java files because JVMCI's module-info.java references them:
> https://github.com/openjdk/jdk/blob/master/src/jdk.internal.vm.ci/share/classes/module-info.java#L26
> 
> Otherwise we can't build JVMCI which we continue to support.
> 
> I filed followup RFE to implement Alan's suggestion to use Module  API which 
> will allow to remove these files later:
> https://bugs.openjdk.java.net/browse/JDK-8265091

Right, I thought I saw something about modules that Alan commented on, but 
couldn't find it. All good then.

-

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


Re: RFR: 8264806: Remove the experimental JIT compiler [v2]

2021-04-12 Thread Erik Joelsson
On Fri, 9 Apr 2021 22:26:40 GMT, Vladimir Kozlov  wrote:

>> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related 
>> to Java-based JIT compiler (Graal) from JDK:
>> 
>> - `jdk.internal.vm.compiler` — the Graal compiler 
>> - `jdk.internal.vm.compiler.management` — Graal's `MBean`
>> - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests
>> 
>> Remove Graal related code in makefiles.
>> 
>> Note, next two `module-info.java` files are preserved so that the JVMCI 
>> module `jdk.internal.vm.ci` continues to build:
>> 
>> src/jdk.internal.vm.compiler/share/classes/module-info.java
>> src/jdk.internal.vm.compiler.management/share/classes/module-info.java
>> 
>> 
>> @AlanBateman suggested that we can avoid it by using Module API to export 
>> packages at runtime . It requires changes in GraalVM's JVMCI too so I will 
>> file followup RFE to implement it.
>> 
>> Tested hs-tier1-4
>
> Vladimir Kozlov has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains three additional 
> commits since the last revision:
> 
>  - Restore Graal Builder image makefile
>  - Merge latest changes from branch 'JDK-8264805' into JDK-8264806
>  - 8264806: Remove the experimental JIT compiler

Build changes look good.

-

Marked as reviewed by erikj (Reviewer).

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


Re: RFR: 8228988: AnnotationParser throws NullPointerException on incompatible member type

2021-04-12 Thread Rafael Winterhalter
On Fri, 5 Feb 2021 21:24:34 GMT, Rafael Winterhalter  
wrote:

> When a class is compiled against a version of an annotation that is later 
> loaded in an incompatible manner where an enum-typed member is changed into 
> an annotation or vice versa, the reflection API currently throws a 
> `NullPointerException` upon accessing the member. Instead an 
> `AnnotationTypeMismatchException` should be thrown.
> 
> This change adjusts the parsing to trigger the correct exception.

Sure, I created a [CSR for the 
change](https://bugs.openjdk.java.net/browse/JDK-8265097). Are those picked up 
automatically or is there something more I should do to have it reviewed?

-

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


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

2021-04-12 Thread Erik Gahlin
On Fri, 2 Apr 2021 12:33:03 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 incrementally with one 
> additional commit since the last revision:
> 
>   Remove trailing spaces

src/jdk.jfr/share/classes/jdk/jfr/internal/instrument/JDKEvents.java line 163:

> 161: private static void initializeContainerEvents() {
> 162: containerMetrics = Container.metrics();
> 163: if (containerMetrics != null) {

I understand this will reduce startup time, but it's contrary to how we treat 
other events. 

We register events, even if they can't be used. We want users to see what 
events are available (and their metadata) and use JMC recording wizard or other 
means to configure a .jfc file without actually being connected to a 
containerized process. We want the same events to minimize (subtle) platform 
dependent bugs.

I think we should try to find other means to reduce the startup time. It's 
better to have consistent behaviour, but an initial implementation than isn't 
as performant, than inconsistent behavior and somewhat faster implementation.

At some point we will need to address the startup cost of registering Java 
events anyway. For example, we could generate metadata at build time in a 
binary format, similar to what we already do with native events. Could even be 
the same file. Then we can have hundreds of Java events without the cost of 
reflection and unnecessary class loading at startup. We could add a simple 
check so that bytecode for the container events (commit() etc) are not 
generated unless in a container environment. A couple of (cached) checks in 
JVMUpcalls may be sufficient to prevent instrumentation cost.

src/jdk.jfr/share/conf/jfr/default.jfc line 1051:

> 1049:   false
> 1050: 
> 1051:   true

I don't think we should create "flag" for "Container Events". Instead we should 
treat them like CPU and other OS events, always on. Since JFR can be used 
outside a container, it seems wrong to have this as an option.

-

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


Re: RFR: 8048199: Replace anonymous inner classes with lambdas, where applicable, in JNDI [v2]

2021-04-12 Thread Chris Hegarty
On Mon, 12 Apr 2021 14:09:55 GMT, Conor Cleary  wrote:

>> ### Description
>> This fix is part of a previous effort to both cleanup/modernise JNDI code, 
>> the details of which can be seen in 
>> [JDK-8048091](https://bugs.openjdk.java.net/browse/JDK-8048091). A number 
>> JNDI methods under `java.naming` use Anonymous Inner Classes in cases where 
>> only a single object unique to the requirements of the method is used. The 
>> issues these occurrences of AICs cause are highlighted below.
>> 
>> - AICs, in the cases concerned with this fix, are used where only one 
>> operation is required. While AICs can be useful for more complex 
>> implementations (using interfaces, multiple methods needed, local fields 
>> etc.), Lambdas are better suited here as they result in a more readable and 
>> concise solution.
>> 
>> ### Fixes
>> - Where applicable, occurrences of AICs were replaced with lambdas to 
>> address the issues above resulting primarily in more readable/concise code.
>
> Conor Cleary has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Update copyright headers
>  - Tidied up lambdas

Marked as reviewed by chegar (Reviewer).

-

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


Re: Conflict API definitions of Math.pow(x, 0.5) and Math.sqrt(x) for x={-0.0, Double.NEGATIVE_INFINITY}(Internet mail)

2021-04-12 Thread Joe Darcy

Hello,

Adding some additional context, more recent versions of the IEEE 754 
standard have given explicit recommendations for math library function 
definitions, including pow. The Java definitions of those methods long 
predate the IEEE 754 coverage and there are a small number of 
differences, including in the pow special cases. These differences are 
now described more explicitly in the JDK 17 specs as of


JDK-8240632: Note differences between IEEE 754-2019 math lib special 
cases and java.lang.Math


The difference in question for pow is:


 * @apiNote
 * The special cases definitions of this method differ from the
 * special case definitions of the IEEE 754 recommended {@code
 * pow} operation for {@code 1.0} raised to an infinite
 * power. This method treats such cases as indeterminate and
 * specifies a NaN is returned. The IEEE 754 specification treats
 * the infinite power as a large integer (large-magnitude
 * floating-point numbers are numerically integers, specifically
 * even integers) and therefore specifies {@code 1.0} be returned.


There are no plans to align the Java definition of pow with the IEEE 754 
definition in these few cases.


Note that the Java library implementation of pow does delegate to sqrt 
while respecting the relevant special cases:



    } else if (y == 0.5) {
    if (x >= -Double.MAX_VALUE) // Handle x == -infinity later
    return Math.sqrt(x + 0.0); // Add 0.0 to properly 
handle x == -0.0

https://github.com/openjdk/jdk/blob/d84a7e55be40eae57b6c322694d55661a5053a55/src/java.base/share/classes/java/lang/FdLibm.java#L366

Mathematically, the sqrt function is about as pleasant a function as you 
can be asked to approximate in floating-point arithmetic. The function 
is smooth, doesn't overflow or underflow, and has a simple 
Newton-iteration that can be expressed in basic arithmetic operations. 
The pow function doesn't enjoy these properties and has a larger design 
space for what a reasonable floating-point approximation could be.


HTH,

-Joe

On 4/12/2021 6:39 AM, jiefu(傅杰) wrote:

Hi Raffaello,

Thanks for your execllent analysis.
I agree with you now.
And I'll close the optimization PR [1] tomorrow if there is no objections.

Thanks.
Best regards,
Jie

[1] https://github.com/openjdk/jdk/pull/3404/


On 2021/4/12, 9:06 PM, "Raffaello Giulietti"  
wrote:

 Hi Jie,
 
 I don't think that changing the spec of Math.pow() to be misaligned with

 IEEE 754 would be a wise option. IEEE is much more pervasive than Java.
 There are many aspects in IEEE that might be seen as questionable, but
 at least it is a widely adopted standard.
 
 AFAIU, the only reason you would like to "optimize" the special case of

 y = 0.5 in pow(x, y) to return sqrt(x) is for performance, more accuracy
 and some kind of consistency.
 
 But then, why not a special case for y = 0.25 as sqrt(sqrt(x))?

 And what about y = 0.75? Should this be translated to sqrt(sqrt(pow(x, 
3)))?
 What about y = 1.0 / 3.0? Should this become cbrt(x)?
 And why not consider y = 2.0 / 3.0 in a special rule: cbrt(x * x)?
 
 You see, the special cases can quickly become unmanageable. Also,

 special rules would produce results which are "discontinuous" with
 nearby exponents, like y = 0.5001.
 
 That's probably why IEEE doesn't propose translation rules for finite

 numerical exponents that are not integers, except when x is a special 
value.
 
 
 Greetings

 Raffaello
 
 
 
 On 2021-04-12 13:44, jiefu(傅杰) wrote:

 > Hi Andrew H, Andrew D, and Raffaello,
 >
 > Thank you all for your kind reply and helpful comments.
 >
 > Now I got where the rules come from.
 > But I don't think the IEEE standars are reasonable to specify conflits 
rules.
 > Maybe, these computations should be open to be implementation dependent.
 >
 > (If it's possible) I really hope the special cases of Math.pow(x, 0.5) 
can be aligned with Math.sqrt(x) in Java.
 > We already allow some plausible behaviors to be different with the IEEE 
recommendations for some special cases, right?
 > And in that case, we can replace pow(x, 0.5) with sqrt(x) safely.
 >
 > Thanks.
 > Best regards,
 > Jie
 >
 >
 > On 2021/4/12, 6:40 PM, "Raffaello Giulietti" 
 wrote:
 >
 >  Hi Jie,
 >
 >  the behavior you report is the one specified by the standard IEEE 
754.
 >  Java follows this standard as closely as it can.
 >
 >  The standard says that
 >  * squareRoot(-0) = -0
 >  * squareRoot(-∞) = NaN
 >
 >  Also, the standard has a long lists of special cases for pow(x, y),
 >  among them:
 >  * pow(±0, y) is +0 for finite y > 0 and not an odd integer
 >  * pow(-∞, y) is +∞ for finite y > 0 and not an odd 

Re: RFR: 8048199: Replace anonymous inner classes with lambdas, where applicable, in JNDI [v2]

2021-04-12 Thread Roger Riggs
On Mon, 12 Apr 2021 14:09:55 GMT, Conor Cleary  wrote:

>> ### Description
>> This fix is part of a previous effort to both cleanup/modernise JNDI code, 
>> the details of which can be seen in 
>> [JDK-8048091](https://bugs.openjdk.java.net/browse/JDK-8048091). A number 
>> JNDI methods under `java.naming` use Anonymous Inner Classes in cases where 
>> only a single object unique to the requirements of the method is used. The 
>> issues these occurrences of AICs cause are highlighted below.
>> 
>> - AICs, in the cases concerned with this fix, are used where only one 
>> operation is required. While AICs can be useful for more complex 
>> implementations (using interfaces, multiple methods needed, local fields 
>> etc.), Lambdas are better suited here as they result in a more readable and 
>> concise solution.
>> 
>> ### Fixes
>> - Where applicable, occurrences of AICs were replaced with lambdas to 
>> address the issues above resulting primarily in more readable/concise code.
>
> Conor Cleary has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Update copyright headers
>  - Tidied up lambdas

Marked as reviewed by rriggs (Reviewer).

-

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


Re: RFR: 8264806: Remove the experimental JIT compiler [v2]

2021-04-12 Thread Vladimir Kozlov
On Mon, 12 Apr 2021 16:18:32 GMT, Erik Joelsson  wrote:

>> Vladimir Kozlov has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains three additional 
>> commits since the last revision:
>> 
>>  - Restore Graal Builder image makefile
>>  - Merge latest changes from branch 'JDK-8264805' into JDK-8264806
>>  - 8264806: Remove the experimental JIT compiler
>
> make/common/Modules.gmk line 68:
> 
>> 66: 
>> 67: # Filter out Graal specific modules
>> 68: MODULES_FILTER += jdk.internal.vm.compiler
> 
> If we are unconditionally filtering out these modules, then why leave the 
> module-info.java files in at all?

We filter out because we can't build Graal anymore. But we need these 
module-info.java files because JVMCI's module-info.java references them:
https://github.com/openjdk/jdk/blob/master/src/jdk.internal.vm.ci/share/classes/module-info.java#L26

Otherwise we can't build JVMCI which we continue to support.

I filed followup RFE to implement Alan's suggestion to use Module  API which 
will allow to remove these files later:
https://bugs.openjdk.java.net/browse/JDK-8265091

-

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


Re: RFR: 8048199: Replace anonymous inner classes with lambdas, where applicable, in JNDI [v2]

2021-04-12 Thread Aleksei Efimov
On Mon, 12 Apr 2021 14:09:55 GMT, Conor Cleary  wrote:

>> ### Description
>> This fix is part of a previous effort to both cleanup/modernise JNDI code, 
>> the details of which can be seen in 
>> [JDK-8048091](https://bugs.openjdk.java.net/browse/JDK-8048091). A number 
>> JNDI methods under `java.naming` use Anonymous Inner Classes in cases where 
>> only a single object unique to the requirements of the method is used. The 
>> issues these occurrences of AICs cause are highlighted below.
>> 
>> - AICs, in the cases concerned with this fix, are used where only one 
>> operation is required. While AICs can be useful for more complex 
>> implementations (using interfaces, multiple methods needed, local fields 
>> etc.), Lambdas are better suited here as they result in a more readable and 
>> concise solution.
>> 
>> ### Fixes
>> - Where applicable, occurrences of AICs were replaced with lambdas to 
>> address the issues above resulting primarily in more readable/concise code.
>
> Conor Cleary has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Update copyright headers
>  - Tidied up lambdas

The change looks good to me with one small suggestion:

src/java.naming/share/classes/javax/naming/ldap/StartTlsRequest.java line 223:

> 221:  */
> 222: private final ClassLoader getContextClassLoader() {
> 223: PrivilegedAction pa = () -> 
> Thread.currentThread().getContextClassLoader();

We can use here an instance method reference to beautify code a little bit more:
```PrivilegedAction pa = 
Thread.currentThread()::getContextClassLoader;```

-

Marked as reviewed by aefimov (Committer).

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


Re: RFR: 8264208: Console charset API [v2]

2021-04-12 Thread Naoto Sato

Hi Bernd,

On 4/9/21 5:21 PM, Bernd Eckenfels wrote:

Hello,

I like the API, it is useful, however not enough to replace the defaultCharset 
once the Change to UTF8 is done. You still need a way to query the platforms 
file encoding (especially on Windows).


Initially I thought it would be beneficial to provide the method that 
returns so-called `platform` charset, but I am not so sure introducing 
it. The reason is that once JEP 400 is enabled, that method only serves 
to migrate the old apps in the new environment. And that's where the 
`COMPAT` system property would be utilized. If those apps have luxury to 
make source code changes, I would recommend migrating the code by giving 
the charset argument to the failing FileReader or alike.




Also I wonder if the Javadoc needs to discuss platform aspects of console, 
especially System.out and LANG on unix vs. windows.


I will add some descriptions to System.out/err in relation to Console, 
but how they map to platform's settings (LANG on Unix/System locale on 
Windows) is an implementation detail, and I don't think it should be be 
described in the spec.


Naoto



Gruss
Bernd
--
http://bernd.eckenfels.net

Von: security-dev  im Auftrag von Naoto Sato 

Gesendet: Friday, April 9, 2021 11:06:00 PM
An: core-libs-dev@openjdk.java.net ; 
security-...@openjdk.java.net 
Betreff: Re: RFR: 8264208: Console charset API [v2]


Please review the changes for the subject issue.  This has been suggested in a 
recent discussion thread for the JEP 400 
[[1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-March/075214.html)].
 A CSR has also been drafted, and comments are welcome 
[[2](https://bugs.openjdk.java.net/browse/JDK-8264209)].


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

   Reflected the review comments.

-

Changes:
   - all: https://git.openjdk.java.net/jdk/pull/3419/files
   - new: https://git.openjdk.java.net/jdk/pull/3419/files/d6db04bb..8fd8f6e6

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

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

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



Re: RFR: 8264806: Remove the experimental JIT compiler [v2]

2021-04-12 Thread Erik Joelsson
On Fri, 9 Apr 2021 22:26:40 GMT, Vladimir Kozlov  wrote:

>> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related 
>> to Java-based JIT compiler (Graal) from JDK:
>> 
>> - `jdk.internal.vm.compiler` — the Graal compiler 
>> - `jdk.internal.vm.compiler.management` — Graal's `MBean`
>> - `test/hotspot/jtreg/compiler/graalunit` — Graal's unit tests
>> 
>> Remove Graal related code in makefiles.
>> 
>> Note, next two `module-info.java` files are preserved so that the JVMCI 
>> module `jdk.internal.vm.ci` continues to build:
>> 
>> src/jdk.internal.vm.compiler/share/classes/module-info.java
>> src/jdk.internal.vm.compiler.management/share/classes/module-info.java
>> 
>> 
>> @AlanBateman suggested that we can avoid it by using Module API to export 
>> packages at runtime . It requires changes in GraalVM's JVMCI too so I will 
>> file followup RFE to implement it.
>> 
>> Tested hs-tier1-4
>
> Vladimir Kozlov has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains three additional 
> commits since the last revision:
> 
>  - Restore Graal Builder image makefile
>  - Merge latest changes from branch 'JDK-8264805' into JDK-8264806
>  - 8264806: Remove the experimental JIT compiler

make/common/Modules.gmk line 68:

> 66: 
> 67: # Filter out Graal specific modules
> 68: MODULES_FILTER += jdk.internal.vm.compiler

If we are unconditionally filtering out these modules, then why leave the 
module-info.java files in at all?

-

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


Re: RFR: 8048199: Replace anonymous inner classes with lambdas, where applicable, in JNDI [v2]

2021-04-12 Thread Daniel Fuchs
On Mon, 12 Apr 2021 14:09:55 GMT, Conor Cleary  wrote:

>> ### Description
>> This fix is part of a previous effort to both cleanup/modernise JNDI code, 
>> the details of which can be seen in 
>> [JDK-8048091](https://bugs.openjdk.java.net/browse/JDK-8048091). A number 
>> JNDI methods under `java.naming` use Anonymous Inner Classes in cases where 
>> only a single object unique to the requirements of the method is used. The 
>> issues these occurrences of AICs cause are highlighted below.
>> 
>> - AICs, in the cases concerned with this fix, are used where only one 
>> operation is required. While AICs can be useful for more complex 
>> implementations (using interfaces, multiple methods needed, local fields 
>> etc.), Lambdas are better suited here as they result in a more readable and 
>> concise solution.
>> 
>> ### Fixes
>> - Where applicable, occurrences of AICs were replaced with lambdas to 
>> address the issues above resulting primarily in more readable/concise code.
>
> Conor Cleary has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Update copyright headers
>  - Tidied up lambdas

LGTM

src/java.naming/share/classes/com/sun/jndi/ldap/LdapPoolManager.java line 400:

> 398: private static final String getProperty(final String propName, final 
> String defVal) {
> 399: PrivilegedAction pa = () -> System.getProperty(propName, 
> defVal);
> 400: return AccessController.doPrivileged(pa);

Hmmm... This is not strictly equivalent but will work because java.naming is 
loaded by the boot loader and has the permission to read all system properties. 
I guess the code on the left-hand side was written at a time where JNDI was 
still in a stand-alone library?

-

Marked as reviewed by dfuchs (Reviewer).

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


Re: RFR: 8265061: Simplify MethodHandleNatives::canBeCalledVirtual

2021-04-12 Thread Mandy Chung
On Mon, 12 Apr 2021 10:45:20 GMT, Claes Redestad  wrote:

> Desugaring the single-case switch in MethodHandleNatives::canBeCalledVirtual 
> is a cleanup and minimal startup improvement on apps spinning up 
> MethodHandles.

Marked as reviewed by mchung (Reviewer).

-

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


What is the status of JEP draft: "Predictable regex performance"?

2021-04-12 Thread Raffaello Giulietti

Hello,

I'm just curious about the status of the important project described by 
"JEP draft: Predictable regex performance" [1].


Afaiu, there are mentions about:
* Using the native re2 library [2] via JNI, with the cons that usage 
becomes kind of unnatural, like "closing" the pattern to release native 
resources.

* Possibly porting re2 to Java, but it seems rather big, several kLOC.
* Adopting/adapting the incomplete and unmaintained re2j [3] Java 
implementation of re2.
* Using the sregex project [4], but it's unclear to me what makes it 
substantially different from re2.


Less clear is whether the project is still exploring various 
alternatives in addition to the above ones or has started to converge 
towards a preferred choice.



Greetings
Raffaello



[1] https://openjdk.java.net/jeps/8260688
[2] https://github.com/google/re2
[3] https://github.com/rapid7/re2-java
[4] https://github.com/openresty/sregex


Re: RFR: 8048199: Replace anonymous inner classes with lambdas, where applicable, in JNDI [v2]

2021-04-12 Thread Conor Cleary
On Fri, 9 Apr 2021 16:30:05 GMT, Roger Riggs  wrote:

>> Thanks for the suggestion Roger, I think the `privilegedGetProperty(prop, 
>> default)` for the `getProperty()` method looks great. 
>> 
>> WRT to using it for `getInt()` and `getLong()`, I think its reasonable to 
>> use other means for these methods in the interest of consistency due to, as 
>> you pointed out, only `int` being supported. Would you think? Or would it be 
>> better to use the same means in all 3 methods?
>
> Its a slippery slope that might require a bit more investigation to check the 
> expected value sizes to see if a change to the number of bits in the value 
> for each property might break something.  Technical debt can take you down a 
> rabbit hole. Quickest to just convert to the lamba as you proposed.

I have stuck with the lambda conversion solution in the [current 
changes](https://github.com/openjdk/jdk/pull/3416/commits/1746840eaa9a8de34d89d73d993e2f86aa0d0ed3),
 however WRT to Alan's previous feedback on creating a PrivilegedAction 
explicitly as it makes everything a bit more readable.

-

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


Re: RFR: 8048199: Replace anonymous inner classes with lambdas, where applicable, in JNDI [v2]

2021-04-12 Thread Conor Cleary
> ### Description
> This fix is part of a previous effort to both cleanup/modernise JNDI code, 
> the details of which can be seen in 
> [JDK-8048091](https://bugs.openjdk.java.net/browse/JDK-8048091). A number 
> JNDI methods under `java.naming` use Anonymous Inner Classes in cases where 
> only a single object unique to the requirements of the method is used. The 
> issues these occurrences of AICs cause are highlighted below.
> 
> - AICs, in the cases concerned with this fix, are used where only one 
> operation is required. While AICs can be useful for more complex 
> implementations (using interfaces, multiple methods needed, local fields 
> etc.), Lambdas are better suited here as they result in a more readable and 
> concise solution.
> 
> ### Fixes
> - Where applicable, occurrences of AICs were replaced with lambdas to address 
> the issues above resulting primarily in more readable/concise code.

Conor Cleary has updated the pull request incrementally with two additional 
commits since the last revision:

 - Update copyright headers
 - Tidied up lambdas

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3416/files
  - new: https://git.openjdk.java.net/jdk/pull/3416/files/f0c2238a..840ea35c

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

  Stats: 29 lines in 5 files changed: 1 ins; 7 del; 21 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3416.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3416/head:pull/3416

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


Re: Conflict API definitions of Math.pow(x, 0.5) and Math.sqrt(x) for x={-0.0, Double.NEGATIVE_INFINITY}(Internet mail)

2021-04-12 Thread 傅杰
Hi Raffaello,

Thanks for your execllent analysis.
I agree with you now.
And I'll close the optimization PR [1] tomorrow if there is no objections.

Thanks.
Best regards,
Jie

[1] https://github.com/openjdk/jdk/pull/3404/


On 2021/4/12, 9:06 PM, "Raffaello Giulietti"  
wrote:

Hi Jie,

I don't think that changing the spec of Math.pow() to be misaligned with 
IEEE 754 would be a wise option. IEEE is much more pervasive than Java. 
There are many aspects in IEEE that might be seen as questionable, but 
at least it is a widely adopted standard.

AFAIU, the only reason you would like to "optimize" the special case of 
y = 0.5 in pow(x, y) to return sqrt(x) is for performance, more accuracy 
and some kind of consistency.

But then, why not a special case for y = 0.25 as sqrt(sqrt(x))?
And what about y = 0.75? Should this be translated to sqrt(sqrt(pow(x, 3)))?
What about y = 1.0 / 3.0? Should this become cbrt(x)?
And why not consider y = 2.0 / 3.0 in a special rule: cbrt(x * x)?

You see, the special cases can quickly become unmanageable. Also, 
special rules would produce results which are "discontinuous" with 
nearby exponents, like y = 0.5001.

That's probably why IEEE doesn't propose translation rules for finite 
numerical exponents that are not integers, except when x is a special value.


Greetings
Raffaello



On 2021-04-12 13:44, jiefu(傅杰) wrote:
> Hi Andrew H, Andrew D, and Raffaello,
> 
> Thank you all for your kind reply and helpful comments.
> 
> Now I got where the rules come from.
> But I don't think the IEEE standars are reasonable to specify conflits 
rules.
> Maybe, these computations should be open to be implementation dependent.
> 
> (If it's possible) I really hope the special cases of Math.pow(x, 0.5) 
can be aligned with Math.sqrt(x) in Java.
> We already allow some plausible behaviors to be different with the IEEE 
recommendations for some special cases, right?
> And in that case, we can replace pow(x, 0.5) with sqrt(x) safely.
> 
> Thanks.
> Best regards,
> Jie
> 
> 
> On 2021/4/12, 6:40 PM, "Raffaello Giulietti" 
 wrote:
> 
>  Hi Jie,
>  
>  the behavior you report is the one specified by the standard IEEE 
754.
>  Java follows this standard as closely as it can.
>  
>  The standard says that
>  * squareRoot(-0) = -0
>  * squareRoot(-∞) = NaN
>  
>  Also, the standard has a long lists of special cases for pow(x, y),
>  among them:
>  * pow(±0, y) is +0 for finite y > 0 and not an odd integer
>  * pow(-∞, y) is +∞ for finite y > 0 and not an odd integer
>  
>  Thus, the conflicts you observe originate in following the standard, 
not
>  by special Java rules.
>  
>  Unfortunately, the IEEE standard does not explain the reasons for the
>  special rules. Some are obvious, some are not.
>  
>  
>  HTH
>  Raffaello
>  
>  
>  > Hi all,
>  >
>  > I found Math.pow(x, 0.5) and Math.sqrt(x) would compute different 
values as the following:
>  > ```
>  > Math.pow(-0.0, 0.5) = 0.0
>  > Math.sqrt(-0.0) = -0.0
>  >
>  > Math.pow(Double.NEGATIVE_INFINITY, 0.5) = Infinity
>  > Math.sqrt(Double.NEGATIVE_INFINITY) = NaN
>  > ```
>  >
>  > The reason is that both of pow and sqrt have special rules for 
these computations.
>  > For example, this rule [1] specifies Math.pow(-0.0, 0.5) must be 
0.0.
>  > And this one [2] specifies Math.sqrt(-0.0) must be -0.0.
>  > And we do have rules for Math.pow(Double.NEGATIVE_INFINITY, 0.5) = 
Infinity and Math.sqrt(Double.NEGATIVE_INFINITY) = NaN too.
>  >
>  > I think most people will be confused by these rules because from 
the view of mathematics, Math.pow(x, 0.5) should be equal to Math.sqrt(x).
>  >
>  > So why Java creates conflict special rules for them?
>  > Is it possible to let Math.pow(-0.0, 0.5) = -0.0 and 
Math.pow(Double.NEGATIVE_INFINITY, 0.5) = NaN also be allowed?
>  >
>  > I came across this problem when I was trying to optimize pow(x, 
0.5) with sqrt(x).
>  > If pow(x, 0.5)'s two special rules can be aligned with sqrt(x), 
then pow(x, 0.5)'s performance can be improved by 7x~14x [3].
>  >
>  > Thanks.
>  > Best regards,
>  > Jie
>  
>  
> 





Re: Conflict API definitions of Math.pow(x, 0.5) and Math.sqrt(x) for x={-0.0, Double.NEGATIVE_INFINITY}(Internet mail)

2021-04-12 Thread Raffaello Giulietti

Hi Jie,

I don't think that changing the spec of Math.pow() to be misaligned with 
IEEE 754 would be a wise option. IEEE is much more pervasive than Java. 
There are many aspects in IEEE that might be seen as questionable, but 
at least it is a widely adopted standard.


AFAIU, the only reason you would like to "optimize" the special case of 
y = 0.5 in pow(x, y) to return sqrt(x) is for performance, more accuracy 
and some kind of consistency.


But then, why not a special case for y = 0.25 as sqrt(sqrt(x))?
And what about y = 0.75? Should this be translated to sqrt(sqrt(pow(x, 3)))?
What about y = 1.0 / 3.0? Should this become cbrt(x)?
And why not consider y = 2.0 / 3.0 in a special rule: cbrt(x * x)?

You see, the special cases can quickly become unmanageable. Also, 
special rules would produce results which are "discontinuous" with 
nearby exponents, like y = 0.5001.


That's probably why IEEE doesn't propose translation rules for finite 
numerical exponents that are not integers, except when x is a special value.



Greetings
Raffaello



On 2021-04-12 13:44, jiefu(傅杰) wrote:

Hi Andrew H, Andrew D, and Raffaello,

Thank you all for your kind reply and helpful comments.

Now I got where the rules come from.
But I don't think the IEEE standars are reasonable to specify conflits rules.
Maybe, these computations should be open to be implementation dependent.

(If it's possible) I really hope the special cases of Math.pow(x, 0.5) can be 
aligned with Math.sqrt(x) in Java.
We already allow some plausible behaviors to be different with the IEEE 
recommendations for some special cases, right?
And in that case, we can replace pow(x, 0.5) with sqrt(x) safely.

Thanks.
Best regards,
Jie


On 2021/4/12, 6:40 PM, "Raffaello Giulietti"  
wrote:

 Hi Jie,
 
 the behavior you report is the one specified by the standard IEEE 754.

 Java follows this standard as closely as it can.
 
 The standard says that

 * squareRoot(-0) = -0
 * squareRoot(-∞) = NaN
 
 Also, the standard has a long lists of special cases for pow(x, y),

 among them:
 * pow(±0, y) is +0 for finite y > 0 and not an odd integer
 * pow(-∞, y) is +∞ for finite y > 0 and not an odd integer
 
 Thus, the conflicts you observe originate in following the standard, not

 by special Java rules.
 
 Unfortunately, the IEEE standard does not explain the reasons for the

 special rules. Some are obvious, some are not.
 
 
 HTH

 Raffaello
 
 
 > Hi all,

 >
 > I found Math.pow(x, 0.5) and Math.sqrt(x) would compute different values 
as the following:
 > ```
 > Math.pow(-0.0, 0.5) = 0.0
 > Math.sqrt(-0.0) = -0.0
 >
 > Math.pow(Double.NEGATIVE_INFINITY, 0.5) = Infinity
 > Math.sqrt(Double.NEGATIVE_INFINITY) = NaN
 > ```
 >
 > The reason is that both of pow and sqrt have special rules for these 
computations.
 > For example, this rule [1] specifies Math.pow(-0.0, 0.5) must be 0.0.
 > And this one [2] specifies Math.sqrt(-0.0) must be -0.0.
 > And we do have rules for Math.pow(Double.NEGATIVE_INFINITY, 0.5) = 
Infinity and Math.sqrt(Double.NEGATIVE_INFINITY) = NaN too.
 >
 > I think most people will be confused by these rules because from the 
view of mathematics, Math.pow(x, 0.5) should be equal to Math.sqrt(x).
 >
 > So why Java creates conflict special rules for them?
 > Is it possible to let Math.pow(-0.0, 0.5) = -0.0 and 
Math.pow(Double.NEGATIVE_INFINITY, 0.5) = NaN also be allowed?
 >
 > I came across this problem when I was trying to optimize pow(x, 0.5) 
with sqrt(x).
 > If pow(x, 0.5)'s two special rules can be aligned with sqrt(x), then 
pow(x, 0.5)'s performance can be improved by 7x~14x [3].
 >
 > Thanks.
 > Best regards,
 > Jie
 
 



Re: RFR: 8263763: Synthetic constructor parameters of enum are not considered for annotation indices [v3]

2021-04-12 Thread Joel Borggrén-Franck
On Sat, 10 Apr 2021 20:22:56 GMT, Rafael Winterhalter 
 wrote:

>> 8263763: The constructor of an enumeration prefixes with two synthetic 
>> arguments for constant name and ordinal index. For this reason, the 
>> constructor annotations need to be shifted two indices to the right, such 
>> that the annotation indices match with the parameter indices.
>
> Rafael Winterhalter has refreshed the contents of this pull request, and 
> previous commits have been removed. The incremental views will show 
> differences compared to the previous content of the PR. The pull request 
> contains one new commit since the last revision:
> 
>   8263763: The constructor of an enumeration prefixes with two synthetic 
> arguments for constant name and ordinal index. For this reason, the 
> constructor annotations need to be shifted two indices to the right, such 
> that the annotation indices match with the parameter indices.

Marked as reviewed by jfranck (Reviewer).

-

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


Re: RFR: 8265061: Simplify MethodHandleNatives::canBeCalledVirtual

2021-04-12 Thread Jorn Vernee
On Mon, 12 Apr 2021 10:45:20 GMT, Claes Redestad  wrote:

> Desugaring the single-case switch in MethodHandleNatives::canBeCalledVirtual 
> is a cleanup and minimal startup improvement on apps spinning up 
> MethodHandles.

Marked as reviewed by jvernee (Committer).

-

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


Re: Conflict API definitions of Math.pow(x, 0.5) and Math.sqrt(x) for x={-0.0, Double.NEGATIVE_INFINITY}(Internet mail)

2021-04-12 Thread 傅杰
Hi Andrew H, Andrew D, and Raffaello,

Thank you all for your kind reply and helpful comments.

Now I got where the rules come from.
But I don't think the IEEE standars are reasonable to specify conflits rules.
Maybe, these computations should be open to be implementation dependent.

(If it's possible) I really hope the special cases of Math.pow(x, 0.5) can be 
aligned with Math.sqrt(x) in Java.
We already allow some plausible behaviors to be different with the IEEE 
recommendations for some special cases, right?
And in that case, we can replace pow(x, 0.5) with sqrt(x) safely.

Thanks.
Best regards,
Jie


On 2021/4/12, 6:40 PM, "Raffaello Giulietti"  
wrote:

Hi Jie,

the behavior you report is the one specified by the standard IEEE 754. 
Java follows this standard as closely as it can.

The standard says that
* squareRoot(-0) = -0
* squareRoot(-∞) = NaN

Also, the standard has a long lists of special cases for pow(x, y), 
among them:
* pow(±0, y) is +0 for finite y > 0 and not an odd integer
* pow(-∞, y) is +∞ for finite y > 0 and not an odd integer

Thus, the conflicts you observe originate in following the standard, not 
by special Java rules.

Unfortunately, the IEEE standard does not explain the reasons for the 
special rules. Some are obvious, some are not.


HTH
Raffaello


> Hi all,
> 
> I found Math.pow(x, 0.5) and Math.sqrt(x) would compute different values 
as the following:
> ```
> Math.pow(-0.0, 0.5) = 0.0
> Math.sqrt(-0.0) = -0.0
> 
> Math.pow(Double.NEGATIVE_INFINITY, 0.5) = Infinity
> Math.sqrt(Double.NEGATIVE_INFINITY) = NaN
> ```
> 
> The reason is that both of pow and sqrt have special rules for these 
computations.
> For example, this rule [1] specifies Math.pow(-0.0, 0.5) must be 0.0.
> And this one [2] specifies Math.sqrt(-0.0) must be -0.0.
> And we do have rules for Math.pow(Double.NEGATIVE_INFINITY, 0.5) = 
Infinity and Math.sqrt(Double.NEGATIVE_INFINITY) = NaN too.
> 
> I think most people will be confused by these rules because from the view 
of mathematics, Math.pow(x, 0.5) should be equal to Math.sqrt(x).
> 
> So why Java creates conflict special rules for them?
> Is it possible to let Math.pow(-0.0, 0.5) = -0.0 and 
Math.pow(Double.NEGATIVE_INFINITY, 0.5) = NaN also be allowed?
> 
> I came across this problem when I was trying to optimize pow(x, 0.5) with 
sqrt(x).
> If pow(x, 0.5)'s two special rules can be aligned with sqrt(x), then 
pow(x, 0.5)'s performance can be improved by 7x~14x [3].
> 
> Thanks.
> Best regards,
> Jie





Integrated: 8264827: Large mapped buffer/segment crash the VM when calling isLoaded

2021-04-12 Thread Chris Hegarty
On Wed, 7 Apr 2021 15:45:30 GMT, Chris Hegarty  wrote:

> Avoid overflow when calculating the number of pages for large mapped segment 
> sizes.

This pull request has now been integrated.

Changeset: 3c9858dd
Author:Chris Hegarty 
URL:   https://git.openjdk.java.net/jdk/commit/3c9858dd
Stats: 36 lines in 5 files changed: 23 ins; 0 del; 13 mod

8264827: Large mapped buffer/segment crash the VM when calling isLoaded

Reviewed-by: alanb, mcimadamore

-

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


RFR: 8265061: Simplify MethodHandleNatives::canBeCalledVirtual

2021-04-12 Thread Claes Redestad
Desugaring the single-case switch in MethodHandleNatives::canBeCalledVirtual is 
a cleanup and minimal startup improvement on apps spinning up MethodHandles.

-

Commit messages:
 - Remove switch in canBeCalledVirtual

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

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


Re: Conflict API definitions of Math.pow(x, 0.5) and Math.sqrt(x) for x={-0.0, Double.NEGATIVE_INFINITY}

2021-04-12 Thread Raffaello Giulietti

Hi Jie,

the behavior you report is the one specified by the standard IEEE 754. 
Java follows this standard as closely as it can.


The standard says that
* squareRoot(-0) = -0
* squareRoot(-∞) = NaN

Also, the standard has a long lists of special cases for pow(x, y), 
among them:

* pow(±0, y) is +0 for finite y > 0 and not an odd integer
* pow(-∞, y) is +∞ for finite y > 0 and not an odd integer

Thus, the conflicts you observe originate in following the standard, not 
by special Java rules.


Unfortunately, the IEEE standard does not explain the reasons for the 
special rules. Some are obvious, some are not.



HTH
Raffaello



Hi all,

I found Math.pow(x, 0.5) and Math.sqrt(x) would compute different values as the 
following:
```
Math.pow(-0.0, 0.5) = 0.0
Math.sqrt(-0.0) = -0.0

Math.pow(Double.NEGATIVE_INFINITY, 0.5) = Infinity
Math.sqrt(Double.NEGATIVE_INFINITY) = NaN
```

The reason is that both of pow and sqrt have special rules for these 
computations.
For example, this rule [1] specifies Math.pow(-0.0, 0.5) must be 0.0.
And this one [2] specifies Math.sqrt(-0.0) must be -0.0.
And we do have rules for Math.pow(Double.NEGATIVE_INFINITY, 0.5) = Infinity and 
Math.sqrt(Double.NEGATIVE_INFINITY) = NaN too.

I think most people will be confused by these rules because from the view of 
mathematics, Math.pow(x, 0.5) should be equal to Math.sqrt(x).

So why Java creates conflict special rules for them?
Is it possible to let Math.pow(-0.0, 0.5) = -0.0 and 
Math.pow(Double.NEGATIVE_INFINITY, 0.5) = NaN also be allowed?

I came across this problem when I was trying to optimize pow(x, 0.5) with 
sqrt(x).
If pow(x, 0.5)'s two special rules can be aligned with sqrt(x), then pow(x, 
0.5)'s performance can be improved by 7x~14x [3].

Thanks.
Best regards,
Jie


Re: RFR: 8264827: Large mapped buffer/segment crash the VM when calling isLoaded [v2]

2021-04-12 Thread Maurizio Cimadamore
On Mon, 12 Apr 2021 08:31:51 GMT, Chris Hegarty  wrote:

>> Avoid overflow when calculating the number of pages for large mapped segment 
>> sizes.
>
> Chris Hegarty has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Skip testing on 32-bit systems

Looks good - thanks for fixing.

-

Marked as reviewed by mcimadamore (Reviewer).

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


Re: Conflict API definitions of Math.pow(x, 0.5) and Math.sqrt(x) for x={-0.0, Double.NEGATIVE_INFINITY}

2021-04-12 Thread Andrew Dinn

On 12/04/2021 07:51, jiefu(傅杰) wrote:


I think most people will be confused by these rules because from the
view of mathematics, Math.pow(x, 0.5) should be equal to
Math.sqrt(x).


This is already a confused situation from the point of view of 
mathematics. Consider these two expressions:


Math.sqrt(-0.0) * Math.sqrt(-0.0)

Math.pow(-0.0, 0.5) * Math.pow(-0.0, 0.5)

It doesn't matter whether the functions sqrt and pow compute -0.0 or 0.0 
as the value here. Either result will fail to satisfy the equality


  f(x) * f(x) == x

The problem is that computation has already diverged from mathematical 
expectation by introducing the value -0.0. So, Java (and other 
languages) have to make up a rule here.



So why Java creates conflict special rules for them? Is it possible
to let Math.pow(-0.0, 0.5) = -0.0 and
Math.pow(Double.NEGATIVE_INFINITY, 0.5) = NaN also be allowed?


It might well match expectations better if both functions were to 
generate the same value here. However, expectations have already been 
set by libc:


$ cat > sqrt.c << END
#include 
#include 
int main(int argc, char **argv) {
printf("sqrt(-0.F) = %f\n", sqrt(-0.F));
printf("pow(-0.F, 0.5) = %f\n", pow(-0.F, 0.5));
}
END
$ make sqrt
cc sqrt.c   -o sqrt
$ ./sqrt
sqrt(-0.F) = -0.00
pow(-0.F, 0.5) = 0.00

I have no idea why these specific results were made up for C but Java 
really ought to follow them.


regards,


Andrew Dinn
---
Red Hat Distinguished Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill



Re: RFR: 8264827: Large mapped buffer/segment crash the VM when calling isLoaded [v2]

2021-04-12 Thread Chris Hegarty
> Avoid overflow when calculating the number of pages for large mapped segment 
> sizes.

Chris Hegarty has updated the pull request incrementally with one additional 
commit since the last revision:

  Skip testing on 32-bit systems

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3378/files
  - new: https://git.openjdk.java.net/jdk/pull/3378/files/1816b200..1dbe4a63

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

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

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


Re: RFR: 8265039: Adjust javadoc for ByteArray*Stream and InputStream

2021-04-12 Thread Сергей Цыпанов
On Mon, 12 Apr 2021 02:40:27 GMT, Yi Yang  wrote:

>> Hello,
>> 
>> to avoid cases detected in 
>> [https://github.com/openjdk/jdk/pull/2992](https://github.com/openjdk/jdk/pull/2992)
>>  I propose to modify JavaDoc of  `ByteArray*Stream` to explicitly mention 
>> redundancy of wrapping with `BufferedInputStream`.
>> 
>> As of `InputStream` I think in notation `It is never correct to use the 
>> return value of this method to allocate a buffer intended to hold all data 
>> in this stream.` the word 'never' should be replaced with 'usually': apart 
>> from `ByteArrayInputStream` e.g. `FileInputStream.available()` often returns 
>> the count of remaining bytes (the only exclusion I'm aware of is the files 
>> located under `/proc/`) and indeed this count can be used to allocate a 
>> buffer to read all the bytes in one call.
>> 
>> Consider benchmark
>> 
>> 
>> @BenchmarkMode(Mode.AverageTime)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> public class ByteArrayInputStreamBenchmark {
>> 
>>   @Benchmark
>>   public void read(Data data, Blackhole bh) {
>> int value;
>> var in = data.bais;
>> while ((value = in.read()) != -1) {
>>   bh.consume(value);
>> }
>>   }
>> 
>>   @Benchmark
>>   public void readBuffered(Data data, Blackhole bh) throws IOException {
>> int value;
>> var in = new BufferedInputStream(data.bais);
>> while ((value = in.read()) != -1) {
>>   bh.consume(value);
>> }
>>   }
>> 
>>   @Benchmark
>>   public Object readAllBytes(Data data) {
>> var in = data.bais;
>> return in.readAllBytes();
>>   }
>> 
>>   @Benchmark
>>   public Object readAllBytesBuffered(Data data) throws IOException {
>> var in = data.bais;
>> return new BufferedInputStream(in).readAllBytes();
>>   }
>> 
>>   @State(Scope.Thread)
>>   public static class Data {
>> 
>> @Param({"8", "128", "512", "1024"})
>> private int length;
>> 
>> private byte[] bytes;
>> private ByteArrayInputStream bais;
>> 
>> @Setup(Level.Iteration)
>> public void setUp() {
>>   bytes = new byte[length];
>>   ThreadLocalRandom.current().nextBytes(bytes);
>> }
>> 
>> @Setup(Level.Invocation)
>> public void setUpBais() {
>>   bais = new ByteArrayInputStream(bytes);
>> }
>>   }
>> }
>> 
>> 
>> giving
>> 
>> 
>>  (length)   Score Error   
>> Units
>> read8  55.737 ±   0.431   
>> ns/op
>> read  128 533.172 ±   1.613   
>> ns/op
>> read  5122066.238 ±  23.989   
>> ns/op
>> read 10244335.570 ±  20.137   
>> ns/op
>> 
>> readBuffered8 521.936 ±   2.454   
>> ns/op
>> readBuffered  128 971.617 ± 100.469   
>> ns/op
>> readBuffered  5122284.472 ± 251.390   
>> ns/op
>> readBuffered 10244168.598 ±  77.980   
>> ns/op
>> 
>> readAllBytes8  34.850 ±   0.072   
>> ns/op
>> readAllBytes  128  36.751 ±   0.133   
>> ns/op
>> readAllBytes  512  45.304 ±   0.699   
>> ns/op
>> readAllBytes 1024  61.790 ±   0.386   
>> ns/op
>> 
>> readAllBytesBuffered8 870.454 ±   4.406   
>> ns/op
>> readAllBytesBuffered  128 910.176 ±  32.258   
>> ns/op
>> readAllBytesBuffered  512 896.155 ±   6.005   
>> ns/op
>> readAllBytesBuffered 1024 965.596 ±  29.225   
>> ns/op
>> 
>> read:·gc.alloc.rate.norm8  32.006 ±   0.001
>> B/op
>> read:·gc.alloc.rate.norm  128  32.007 ±   0.004
>> B/op
>> read:·gc.alloc.rate.norm  512  32.010 ±   0.010
>> B/op
>> read:·gc.alloc.rate.norm 1024  32.011 ±   0.008
>> B/op
>> 
>> readBuffered:·gc.alloc.rate.norm88280.354 ±   0.016
>> B/op
>> readBuffered:·gc.alloc.rate.norm  1288240.484 ±   0.015
>> B/op
>> readBuffered:·gc.alloc.rate.norm  5128240.599 ±   0.056
>> B/op
>> readBuffered:·gc.alloc.rate.norm 10248280.978 ±   0.024
>> B/op
>> 
>> readAllBytes:·gc.alloc.rate.norm8  56.008 ±   0.001
>> B/op
>> readAllBytes:·gc.alloc.rate.norm  128 176.017 ±   0.001
>> B/op
>> readAllBytes:·gc.alloc.rate.norm  512 560.035 ±   0.002
>> B/op
>> readAllBytes:·gc.alloc.rate.norm 10241072.057 ±   0.002
>> B/op
>> 
>> readAllBytesBuffered:·gc.alloc.rate.norm8   16512.660 ±   0.026
>> B/op
>> readAllBytesBuffered:·gc.alloc.rate.norm  128   

RFR: 8265039: Adjust javadoc for ByteArray*Stream and InputStream

2021-04-12 Thread Сергей Цыпанов
Hello,

to avoid cases detected in 
[https://github.com/openjdk/jdk/pull/2992](https://github.com/openjdk/jdk/pull/2992)
 I propose to modify JavaDoc of  `ByteArray*Stream` to explicitly mention 
redundancy of wrapping with `BufferedInputStream`.

As of `InputStream` I think in notation `It is never correct to use the return 
value of this method to allocate a buffer intended to hold all data in this 
stream.` the word 'never' should be replaced with 'usually': apart from 
`ByteArrayInputStream` e.g. `FileInputStream.available()` often returns the 
count of remaining bytes (the only exclusion I'm aware of is the files located 
under `/proc/`) and indeed this count can be used to allocate a buffer to read 
all the bytes in one call.

Consider benchmark


@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public class ByteArrayInputStreamBenchmark {

  @Benchmark
  public void read(Data data, Blackhole bh) {
int value;
var in = data.bais;
while ((value = in.read()) != -1) {
  bh.consume(value);
}
  }

  @Benchmark
  public void readBuffered(Data data, Blackhole bh) throws IOException {
int value;
var in = new BufferedInputStream(data.bais);
while ((value = in.read()) != -1) {
  bh.consume(value);
}
  }

  @Benchmark
  public Object readAllBytes(Data data) {
var in = data.bais;
return in.readAllBytes();
  }

  @Benchmark
  public Object readAllBytesBuffered(Data data) throws IOException {
var in = data.bais;
return new BufferedInputStream(in).readAllBytes();
  }

  @State(Scope.Thread)
  public static class Data {

@Param({"8", "128", "512", "1024"})
private int length;

private byte[] bytes;
private ByteArrayInputStream bais;

@Setup(Level.Iteration)
public void setUp() {
  bytes = new byte[length];
  ThreadLocalRandom.current().nextBytes(bytes);
}

@Setup(Level.Invocation)
public void setUpBais() {
  bais = new ByteArrayInputStream(bytes);
}
  }
}


giving


 (length)   Score Error   Units
read8  55.737 ±   0.431   ns/op
read  128 533.172 ±   1.613   ns/op
read  5122066.238 ±  23.989   ns/op
read 10244335.570 ±  20.137   ns/op

readBuffered8 521.936 ±   2.454   ns/op
readBuffered  128 971.617 ± 100.469   ns/op
readBuffered  5122284.472 ± 251.390   ns/op
readBuffered 10244168.598 ±  77.980   ns/op

readAllBytes8  34.850 ±   0.072   ns/op
readAllBytes  128  36.751 ±   0.133   ns/op
readAllBytes  512  45.304 ±   0.699   ns/op
readAllBytes 1024  61.790 ±   0.386   ns/op

readAllBytesBuffered8 870.454 ±   4.406   ns/op
readAllBytesBuffered  128 910.176 ±  32.258   ns/op
readAllBytesBuffered  512 896.155 ±   6.005   ns/op
readAllBytesBuffered 1024 965.596 ±  29.225   ns/op

read:·gc.alloc.rate.norm8  32.006 ±   0.001B/op
read:·gc.alloc.rate.norm  128  32.007 ±   0.004B/op
read:·gc.alloc.rate.norm  512  32.010 ±   0.010B/op
read:·gc.alloc.rate.norm 1024  32.011 ±   0.008B/op

readBuffered:·gc.alloc.rate.norm88280.354 ±   0.016B/op
readBuffered:·gc.alloc.rate.norm  1288240.484 ±   0.015B/op
readBuffered:·gc.alloc.rate.norm  5128240.599 ±   0.056B/op
readBuffered:·gc.alloc.rate.norm 10248280.978 ±   0.024B/op

readAllBytes:·gc.alloc.rate.norm8  56.008 ±   0.001B/op
readAllBytes:·gc.alloc.rate.norm  128 176.017 ±   0.001B/op
readAllBytes:·gc.alloc.rate.norm  512 560.035 ±   0.002B/op
readAllBytes:·gc.alloc.rate.norm 10241072.057 ±   0.002B/op

readAllBytesBuffered:·gc.alloc.rate.norm8   16512.660 ±   0.026B/op
readAllBytesBuffered:·gc.alloc.rate.norm  128   16632.684 ±   0.008B/op
readAllBytesBuffered:·gc.alloc.rate.norm  512   17016.694 ±   0.017B/op
readAllBytesBuffered:·gc.alloc.rate.norm 1024   17528.748 ±   0.012B/op

-

Commit messages:
 - Adjust javadoc for ByteArray*Stream

Changes: https://git.openjdk.java.net/jdk/pull/3341/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3341=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8265039
  Stats: 20 lines in 3 files changed: 7 ins; 0 del; 13 mod
  Patch: 

Re: RFR: 8265039: Adjust javadoc for ByteArray*Stream and InputStream

2021-04-12 Thread Yi Yang
On Mon, 5 Apr 2021 08:37:15 GMT, Сергей Цыпанов 
 wrote:

> Hello,
> 
> to avoid cases detected in 
> [https://github.com/openjdk/jdk/pull/2992](https://github.com/openjdk/jdk/pull/2992)
>  I propose to modify JavaDoc of  `ByteArray*Stream` to explicitly mention 
> redundancy of wrapping with `BufferedInputStream`.
> 
> As of `InputStream` I think in notation `It is never correct to use the 
> return value of this method to allocate a buffer intended to hold all data in 
> this stream.` the word 'never' should be replaced with 'usually': apart from 
> `ByteArrayInputStream` e.g. `FileInputStream.available()` often returns the 
> count of remaining bytes (the only exclusion I'm aware of is the files 
> located under `/proc/`) and indeed this count can be used to allocate a 
> buffer to read all the bytes in one call.
> 
> Consider benchmark
> 
> 
> @BenchmarkMode(Mode.AverageTime)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class ByteArrayInputStreamBenchmark {
> 
>   @Benchmark
>   public void read(Data data, Blackhole bh) {
> int value;
> var in = data.bais;
> while ((value = in.read()) != -1) {
>   bh.consume(value);
> }
>   }
> 
>   @Benchmark
>   public void readBuffered(Data data, Blackhole bh) throws IOException {
> int value;
> var in = new BufferedInputStream(data.bais);
> while ((value = in.read()) != -1) {
>   bh.consume(value);
> }
>   }
> 
>   @Benchmark
>   public Object readAllBytes(Data data) {
> var in = data.bais;
> return in.readAllBytes();
>   }
> 
>   @Benchmark
>   public Object readAllBytesBuffered(Data data) throws IOException {
> var in = data.bais;
> return new BufferedInputStream(in).readAllBytes();
>   }
> 
>   @State(Scope.Thread)
>   public static class Data {
> 
> @Param({"8", "128", "512", "1024"})
> private int length;
> 
> private byte[] bytes;
> private ByteArrayInputStream bais;
> 
> @Setup(Level.Iteration)
> public void setUp() {
>   bytes = new byte[length];
>   ThreadLocalRandom.current().nextBytes(bytes);
> }
> 
> @Setup(Level.Invocation)
> public void setUpBais() {
>   bais = new ByteArrayInputStream(bytes);
> }
>   }
> }
> 
> 
> giving
> 
> 
>  (length)   Score Error   
> Units
> read8  55.737 ±   0.431   
> ns/op
> read  128 533.172 ±   1.613   
> ns/op
> read  5122066.238 ±  23.989   
> ns/op
> read 10244335.570 ±  20.137   
> ns/op
> 
> readBuffered8 521.936 ±   2.454   
> ns/op
> readBuffered  128 971.617 ± 100.469   
> ns/op
> readBuffered  5122284.472 ± 251.390   
> ns/op
> readBuffered 10244168.598 ±  77.980   
> ns/op
> 
> readAllBytes8  34.850 ±   0.072   
> ns/op
> readAllBytes  128  36.751 ±   0.133   
> ns/op
> readAllBytes  512  45.304 ±   0.699   
> ns/op
> readAllBytes 1024  61.790 ±   0.386   
> ns/op
> 
> readAllBytesBuffered8 870.454 ±   4.406   
> ns/op
> readAllBytesBuffered  128 910.176 ±  32.258   
> ns/op
> readAllBytesBuffered  512 896.155 ±   6.005   
> ns/op
> readAllBytesBuffered 1024 965.596 ±  29.225   
> ns/op
> 
> read:·gc.alloc.rate.norm8  32.006 ±   0.001
> B/op
> read:·gc.alloc.rate.norm  128  32.007 ±   0.004
> B/op
> read:·gc.alloc.rate.norm  512  32.010 ±   0.010
> B/op
> read:·gc.alloc.rate.norm 1024  32.011 ±   0.008
> B/op
> 
> readBuffered:·gc.alloc.rate.norm88280.354 ±   0.016
> B/op
> readBuffered:·gc.alloc.rate.norm  1288240.484 ±   0.015
> B/op
> readBuffered:·gc.alloc.rate.norm  5128240.599 ±   0.056
> B/op
> readBuffered:·gc.alloc.rate.norm 10248280.978 ±   0.024
> B/op
> 
> readAllBytes:·gc.alloc.rate.norm8  56.008 ±   0.001
> B/op
> readAllBytes:·gc.alloc.rate.norm  128 176.017 ±   0.001
> B/op
> readAllBytes:·gc.alloc.rate.norm  512 560.035 ±   0.002
> B/op
> readAllBytes:·gc.alloc.rate.norm 10241072.057 ±   0.002
> B/op
> 
> readAllBytesBuffered:·gc.alloc.rate.norm8   16512.660 ±   0.026
> B/op
> readAllBytesBuffered:·gc.alloc.rate.norm  128   16632.684 ±   0.008
> B/op
> readAllBytesBuffered:·gc.alloc.rate.norm  512   17016.694 ±   0.017
> B/op
> 

Conflict API definitions of Math.pow(x, 0.5) and Math.sqrt(x) for x={-0.0, Double.NEGATIVE_INFINITY}

2021-04-12 Thread 傅杰
Hi all,

I found Math.pow(x, 0.5) and Math.sqrt(x) would compute different values as the 
following:
```
Math.pow(-0.0, 0.5) = 0.0
Math.sqrt(-0.0) = -0.0

Math.pow(Double.NEGATIVE_INFINITY, 0.5) = Infinity
Math.sqrt(Double.NEGATIVE_INFINITY) = NaN
```

The reason is that both of pow and sqrt have special rules for these 
computations.
For example, this rule [1] specifies Math.pow(-0.0, 0.5) must be 0.0.
And this one [2] specifies Math.sqrt(-0.0) must be -0.0.
And we do have rules for Math.pow(Double.NEGATIVE_INFINITY, 0.5) = Infinity and 
Math.sqrt(Double.NEGATIVE_INFINITY) = NaN too.

I think most people will be confused by these rules because from the view of 
mathematics, Math.pow(x, 0.5) should be equal to Math.sqrt(x).

So why Java creates conflict special rules for them?
Is it possible to let Math.pow(-0.0, 0.5) = -0.0 and 
Math.pow(Double.NEGATIVE_INFINITY, 0.5) = NaN also be allowed?

I came across this problem when I was trying to optimize pow(x, 0.5) with 
sqrt(x).
If pow(x, 0.5)'s two special rules can be aligned with sqrt(x), then pow(x, 
0.5)'s performance can be improved by 7x~14x [3].

Thanks.
Best regards,
Jie

[1] 
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/lang/Math.java#L644
[2] 
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/lang/Math.java#L385
[3] https://github.com/openjdk/jdk/pull/3404/