Re: jpackage bugs

[David Holmes]

Hi Michael,

On 17/04/2021 10:57 am, Michael Hall wrote:

Is there anyway to get a simple/test reference type application available that 
could be used in reproducing bugs?

I had two I think potentially serious bugs that were basically not addressed 
for problems in reproducing.

JDK-8263156 : [macos]: OS X application signing concerns - a sealed resource is 
missing or invalid
https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8263156 


The command to reproduce was provided. The error appears to be in files 
included in the embedded JDK not being signed. So apparently not having to do 
with anything of mine. (Mentioned I now see in the comments).

As I indicate this is not a serious error for me since I am not submitting the 
app to the Mac App Store but I believe this would get the app Apple rejected 
for anyone who is attempting that. A show stopper for a major use case. It 
seems too bad to simply close it because I missed an email asking for a 
reproduce.


Note the bug referenced is closed as "incomplete" - that is a temporary 
state while awaiting additional information  (usually from the 
submitter). If we never hear back from the submitter then it will be 
closed with a different (more terminal) state. If we do hear back then 
the bug gets reopened.


Cheers,
David


With a reference application I could demonstrate the error against would 
eliminate the need to provide a separate reproducible test case. Quite sized 
for the application in question. Such an application is actually mentioned in 
the bug report comments. Could such a application be made available for 
download or user reproducing - the jpackage command and arguments?

I have looked a little bit at if to see if I can figure out how to sign the 
embedded jdk files. So far only accomplishing that I can no longer simply use 
my name for signing but have to use my fully qualified security identity.

The question now seems to be what is in fact the difference between mine and 
the, unavailable to me, reference application as to these files verifying as 
correctly signed.

A second bug
JDK-8263154 : [macos]: OS X DMG builds have errors and end up incorrect

I thought a fix for this was all set to go in and was pulled. It was apparently 
determined that the problem only applies if the —install-dir parameter is used 
for DMG’s. Where it really makes no sense. My use apparently held over from 
when I was just creating the app.I thought this had somehow also in some way 
regressed to not reproducible. I still think a fairly simple change to the 
AppleScript as was originally planned would resolve the issue independently of 
parameters. The default DMG build I would think should _always_ indicate 
installation to the Applications folder.

With —install-dir this remains a reproducible bug for me at 17-ea.

If a reference build was provided somewhere I might have picked up on the 
parameter difference sooner.

Re: 8252827: Caching Integer.toString just like Integer.valueOf

[David Holmes]

On 17/04/2021 4:54 am, Raffaello Giulietti wrote:
I guess the reporter meant to limit the cache range similarly to the one 
used for valueOf().


I have no clue about the benefit/cost ratio for the proposed String 
cache. It really depends on usage, workload, etc. One can easily imagine 
both extreme scenarios but it's hard to tell how the average one would 
look.


My post is only about either solving the issue by implementing the 
cache, which I can contribute to; or closing it because of lack of 
real-world need or interest.


Caching for the sake of caching is not an objective in itself. Unless 
the caching can be shown to solve a real problem, and the strategy for 
managing the cache is well-defined, then I would just close the 
enhancement request. (Historically whether an issue we don't have any 
firm plans to address is just left open "forever" or closed, depends 
very much on who does the bug triaging in that area. :) )


Cheers,
David



Greetings
Raffaello


On 2021-04-16 20:36, Roger Riggs wrote:

Hi,

Is there any way to quantify the savings?
And what technique can be applied to limit the size of the cache.
The size of the small integer cache is somewhat arbitrary.

Regards, Roger

On 4/16/21 12:48 PM, Raffaello Giulietti wrote:

Hello,

does the enhancement proposed in [1] make sense, both today and when 
wrappers will be migrated to primitive classes?

If so, it should be rather simple to add it and I could prepare a PR.
If not, the issue might perhaps be closed.


Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8252827

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

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

 - Changed shell based test into java based
 - Added link to Charset#defaultChaset() in InputStreamReader.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3419/files
  - new: https://git.openjdk.java.net/jdk/pull/3419/files/083f6180..238dbb42

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

  Stats: 88 lines in 3 files changed: 27 ins; 51 del; 10 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: 8265029: Preserve SIZED characteristics on slice operations (skip, limit) [v5]

[Tagir F . Valeev]

> 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 modernized SliceOps a little bit, using switch expression (with no 
> explicit default!) and diamonds on anonymous classes.

Tagir F. Valeev has updated the pull request 

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

[Tagir F . Valeev]

> 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 modernized SliceOps a little bit, using switch expression (with no 
> explicit default!) and diamonds on anonymous classes.

Tagir F. Valeev has updated the pull request 

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

[Tagir F . Valeev]

On Sat, 17 Apr 2021 01:55:26 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) [v3]

[Tagir F . Valeev]

> 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 modernized SliceOps a little bit, using switch expression (with no 
> explicit default!) and diamonds on anonymous classes.

Tagir F. Valeev has updated the pull request 

jpackage bugs

[Michael Hall]

Is there anyway to get a simple/test reference type application available that 
could be used in reproducing bugs?

I had two I think potentially serious bugs that were basically not addressed 
for problems in reproducing.

JDK-8263156 : [macos]: OS X application signing concerns - a sealed resource is 
missing or invalid
https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8263156 


The command to reproduce was provided. The error appears to be in files 
included in the embedded JDK not being signed. So apparently not having to do 
with anything of mine. (Mentioned I now see in the comments).

As I indicate this is not a serious error for me since I am not submitting the 
app to the Mac App Store but I believe this would get the app Apple rejected 
for anyone who is attempting that. A show stopper for a major use case. It 
seems too bad to simply close it because I missed an email asking for a 
reproduce. 

With a reference application I could demonstrate the error against would 
eliminate the need to provide a separate reproducible test case. Quite sized 
for the application in question. Such an application is actually mentioned in 
the bug report comments. Could such a application be made available for 
download or user reproducing - the jpackage command and arguments?

I have looked a little bit at if to see if I can figure out how to sign the 
embedded jdk files. So far only accomplishing that I can no longer simply use 
my name for signing but have to use my fully qualified security identity. 

The question now seems to be what is in fact the difference between mine and 
the, unavailable to me, reference application as to these files verifying as 
correctly signed.

A second bug 
JDK-8263154 : [macos]: OS X DMG builds have errors and end up incorrect

I thought a fix for this was all set to go in and was pulled. It was apparently 
determined that the problem only applies if the —install-dir parameter is used 
for DMG’s. Where it really makes no sense. My use apparently held over from 
when I was just creating the app.I thought this had somehow also in some way 
regressed to not reproducible. I still think a fairly simple change to the 
AppleScript as was originally planned would resolve the issue independently of 
parameters. The default DMG build I would think should _always_ indicate 
installation to the Applications folder. 

With —install-dir this remains a reproducible bug for me at 17-ea.

If a reference build was provided somewhere I might have picked up on the 
parameter difference sooner.

Re: RFR: 8265237: String.join and StringJoiner can be improved further [v2]

[Peter Levart]

On Fri, 16 Apr 2021 19:05:25 GMT, Roger Riggs  wrote:

>> Peter Levart has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Add String.join benchmark method to StringJoinerBenchmark and adjust some 
>> parameters to cover bigger range
>
> src/java.base/share/classes/java/lang/String.java line 3254:
> 
>> 3252: 
>> 3253: byte[] value = StringConcatHelper.newArray(((long) icoder << 
>> 32) | llen);
>> 3254: int off = 0;
> 
> StringConcatHelper.newArray() can double the length (based on the coder) and 
> it is then truncated to 32 bits when passed to 
> UNSAFE.allocatlUnitializedArray.
> The test of length above only ensures llen can be truncated to 32 bits 
> without loss of data.

I thought about that, yes. And I think we have to do the check for the doubled 
length before calling the newArray. I checked the StringJoinerTest and it only 
deals with ascii strings unfortunately. Will have to add a test for that too...

> src/java.base/share/classes/java/lang/String.java line 3256:
> 
>> 3254: int off = 0;
>> 3255: prefix.getBytes(value, off, coder); off += prefix.length();
>> 3256: for (int i = 0; i < size; i++) {
> 
> Can you save a branch inside the loop by handling element 0 outside the loop 
> and
> then do the loop for the rest?

Thanks, I'll do that and then re-test to see if there's any improvement.

-

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

Re: RFR: 8263154: [macos] DMG builds have finder errors

[Alexander Matveev]

On Thu, 15 Apr 2021 01:24:04 GMT, Alexander Matveev  
wrote:

> - Issue was reproducible when install-dir points to some invalid location.
>  - Fixed by defaulting DMG drag and drop location to /Applications folder and 
> --install-dir will be ignored with warning for DMG.
>  - I do not see any way to support other valid, but uncommon locations for 
> drag and drop. For example: /Users/USERNAME/Applications is not possible to 
> support since user name is not known. /usr/bin requires root privileges and 
> should contain symbolic links. Locations which does not exist also not 
> possible to support, since DMG cannot create paths. So 
> /Applications/MyCompany is not possible for DMG.

Should we always show warning messages, especially that output will be 
different from what user expected? I can switch to Log.verbose(), but I still 
think it should be displayed always.

-

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

Integrated: 8265375: Bootcycle builds fail with StackOverflowError in cldrconverter

[Naoto Sato]

On Fri, 16 Apr 2021 21:10:42 GMT, Naoto Sato  wrote:

> Please review the fix to the tier4 build failure. The piece of code that made 
> into `CLDRLocaleProviderAdapter.java` was also needed in the build tool 
> counterpart (`CLDRConverter`).

This pull request has now been integrated.

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

8265375: Bootcycle builds fail with StackOverflowError in cldrconverter

Reviewed-by: joehw

-

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

Re: RFR: 8265375: Bootcycle builds fail with StackOverflowError in cldrconverter [v2]

[Naoto Sato]

> Please review the fix to the tier4 build failure. The piece of code that made 
> into `CLDRLocaleProviderAdapter.java` was also needed in the build tool 
> counterpart (`CLDRConverter`).

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

  Copyright year update.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3553/files
  - new: https://git.openjdk.java.net/jdk/pull/3553/files/91326786..e54cb6ec

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

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

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

Re: ReversibleCollection proposal

[Donald Raab]

Hi Stuart,

We should be cautious when adding new APIs to existing interfaces. There may be 
libraries which extend JDK types and already have reversed(), toReversed() or 
asReversed() APIs and corresponding interfaces.

There are OrderedIterable and ReversibleIterable interfaces and asReversed() 
and toReversed() methods in the Eclipse Collections API. 

JavaDoc:
https://www.eclipse.org/collections/javadoc/10.4.0/org/eclipse/collections/api/ordered/OrderedIterable.html
https://www.eclipse.org/collections/javadoc/10.4.0/org/eclipse/collections/api/ordered/ReversibleIterable.html

Code Browser:
https://code.yawk.at/org.eclipse.collections/eclipse-collections-api/10.4.0/org/eclipse/collections/api/ordered/OrderedIterable.java
https://code.yawk.at/org.eclipse.collections/eclipse-collections-api/10.4.0/org/eclipse/collections/api/ordered/ReversibleIterable.java

OrderedIterable and ReversibleIterable appear in the mind map for RichIterable 
and have corresponding primitive versions in the mind map for PrimitiveIterable.

https://medium.com/oracledevs/visualizing-eclipse-collections-646dad9533a9?source=friends_link=3370a5e8bb5a516e6b5d7040f7d0955b
 

There are methods asReversed() and toReversed() on ReversibleIterable().

https://code.yawk.at/org.eclipse.collections/eclipse-collections-api/10.4.0/org/eclipse/collections/api/ordered/ReversibleIterable.java#org.eclipse.collections.api.ordered.ReversibleIterable%23asReversed%28%29
https://code.yawk.at/org.eclipse.collections/eclipse-collections-api/10.4.0/org/eclipse/collections/api/ordered/ReversibleIterable.java#org.eclipse.collections.api.ordered.ReversibleIterable%23toReversed%28%29

The toReversed() method is co-variantly overridden in subtypes of 
ReversibleIterable. 

https://code.yawk.at/org.eclipse.collections/eclipse-collections-api/10.4.0/org/eclipse/collections/api/list/ListIterable.java#org.eclipse.collections.api.list.ListIterable%23toReversed%28%29

The asReversed() method returns a ReverseIterable which is lazy.

https://code.yawk.at/org.eclipse.collections/eclipse-collections/9.2.0/org/eclipse/collections/impl/lazy/ReverseIterable.java

Thanks,
Don



> On Apr 16, 2021, at 1:40 PM, Stuart Marks  wrote:
> 
> This is a proposal to add a ReversibleCollection interface to the Collections 
> Framework. I'm looking for comments on overall design before I work on 
> detailed specifications and tests. Please send such comments as replies on 
> this email thread.
> 
> Here's a link to a draft PR that contains the code diffs. It's prototype 
> quality, but it should be good enough to build and try out:
> 
>https://github.com/openjdk/jdk/pull/3533
> 
> And here's a link to a class diagram showing the proposed additions:
> 
> https://cr.openjdk.java.net/~smarks/ReversibleCollection/ReversibleCollectionDiagram.pdf
> 
> Thanks,
> 
> s'marks
> 
> 
> # Ordering and Reversibility
> 
> 
> A long-standing concept that's been missing from collections is that of the 
> positioning, sequencing, or arrangement of elements as a structural property 
> of a collection. (This is sometimes called the "iteration order" of a 
> collection.) For example, a HashSet is not ordered, but a List is. This 
> concept is mostly not manifested in the collections API.
> 
> Iterating a collection produces elements one after another in *some* 
> sequence. The concept of "ordered" determines whether this sequence is 
> defined or whether it's a coincidence of implementation. What does "having an 
> order" mean? It implies that there is a first element and that each element 
> has a successor. Since collections have a finite number of elements, it 
> further implies that there is a last element that has no successor. However, 
> it is difficult to discern whether a collection has a defined order. HashSet 
> generally iterates its elements in the same undefined order, and you can't 
> actually tell that it's not a defined order.
> 
> Streams do have a notion of ordering ("encounter order") and this is 
> preserved, where appropriate, through the stream pipeline. It's possible to 
> detect this by testing whether its spliterator has the ORDERED 
> characteristic. Any collection with a defined order will have a spliterator 
> with this characteristic. However, this is quite a roundabout way to 
> determine whether a collection has a defined order. Furthermore, knowing this 
> doesn't enable any additional operations. It only provides constraints on the 
> stream's implementations (keeping the elements in order) and provides 
> stronger semantics for certain operations. For example, findFirst() on an 
> unordered stream is the same as findAny(), but actually finds the first 
> element if the stream is ordered.
> 
> The concept of ordering is thus present in the system but is surfaced only in 
> a fairly indirect way. We can strengthen abstraction of ordering by making a 
> few observations and considering their implications.
> 
> Given that there is a first element and a last element, the 

Re: Enhancement proposal regarding bufferization of InputStream

[Сергей Цыпанов]

Hi Roger,

> Are you taking into account that for many reads the data is not copied
> into the local buffer.
> See the comments in BufferedInputStream.read1: 277:280?

sure, I'm aware of this optimization. The buffer however is always allocated as 
BufferedInputStream instantiated,
waisting by default 8 kB of memory.

If I take a benchmark [1] measuring costs of reading metadata of a simple 
Spring bean it demonstrates [2]
measurable imrovement when buffering is dropped:

original
   Mode  Cnt  Score Error   Units
read   avgt  100122.041 ±   1.286   us/op
read:·gc.alloc.rate.norm   avgt  100  50795.798 ±  13.941B/op  
<<

patched
   Mode  Cnt  Score Error   Units
read   avgt  100119.524 ±   1.171   us/op
read:·gc.alloc.rate.norm   avgt  100  42635.578 ±  10.866B/op <<

So in this case it's about 3 microseconds and 8 KB per bean.

When I measured the impact of change [2] on Spring Boot application I got 
start-up time improved
from 760 milliseconds to 718 milliseconds a memory consumption decrease from 
48,09 MB to 47,47 MB.


Also see https://github.com/openjdk/jdk/pull/2992

1. 
https://github.com/stsypanov/ovn/blob/master/src/main/java/com/tsypanov/ovn/MetadataReaderBenchmark.java
2. https://github.com/spring-projects/spring-framework/pull/24946


Regards,
Sergey Tsypanov

> Hi Sergey,
> 
> Are you taking into account that for many reads the data is not copied
> into the local buffer.
> See the comments in BufferedInputStream.read1: 277:280?
> 
> How much is the slowdown, when BufferedInputStreams are chained?
> 
> Thanks, Roger
> 
> On 4/15/21 7:08 AM, Pavel Rappo wrote:
> 
>>> On 15 Apr 2021, at 08:33, Сергей Цыпанов  wrote:
>>>
>>> Hello,
>>>
>>> buffering with j.i.BufferedInputStream is a handy way to improve 
>>> performance of IO operations.
>>> However in many cases buffering is redundant. Consider this code snippet 
>>> from Spring Framework:
>>>
>>> static ClassReader getClassReader(Resource rsc) throws Exception {
>>> try (var is = new BufferedInputStream(rsc.getInputStream())) {
>>> return new ClassReader(is);
>>> }
>>> }
>>>
>>> Interface Resource has lots of implementations some of them return buffered 
>>> InputStream,
>>> others don't, but all of them get wrapped with BufferedInputStream.
>>>
>>> Apart from obvious misuses like BufferedInputStream cascade such wrapping 
>>> is not necessary,
>>> e.g. when we read a huge file using FileInputStream.readAllBytes(),
>>> in others cases it's harmful e.g. when we read a small (comparing to the 
>>> default
>>> size of buffer in BufferedInputStream) file with readAllBytes() or
>>> when we read from ByteArrayInputStream which is kind of buffered one by its 
>>> nature.
>>>
>>> I think an instance of InputStream should decide itself whether it requires 
>>> buffering,
>>> so I suggest to add a couple of methods into j.i.InputStream:
>>>
>>> // subclasses can override this
>>> protected boolean needsBuffer() {
>>> return true;
>>> }
>>>
>>> public InputStream buffered() {
>>> return needsBuffer() ? new BufferedInputStream(this) : this;
>>> }
>>>
>>> this allows subclasses of InputStream to override needsBuffer() to declare 
>>> buffering redundancy.
>>> Let's imagine we've overridden needsBuffer() in BufferedInputStream:
>>>
>>> public class BufferedInputStream {
>>> @Override
>>> public boolean needsBuffer() {
>>> return true;
>>> }
>>> }
>>>
>>> then the code we've mentioned above should be rewritten as
>>>
>>> static ClassReader getClassReader(Resource rsc) throws Exception {
>>> try (var is = rsc.getInputStream().buffered()) {
>>> return new ClassReader(is);
>>> }
>>> }
>>>
>>> preventing cascade of BufferedInputStreams.
>>
>> When I read this part
>>
>>> There are also cases when the need for buffering depends on the way how we 
>>> read from InputStream:
>>>
>>> new FileInputStream(file).buffered().readAllBytes() // buffering is 
>>> redundant
>>
>> my knee-jerk reaction was that a better solution likely lies with 
>> introducing a marker interface and selectively implementing it as opposed to 
>> adding two new methods to the existing class and selectively overriding 
>> them. Let's call this interface java.io.Buffered: Bufferred is to 
>> InputStream as RandomAccess is to List.
>>
>> Just to be clear: I'm not proposing to actually do this. It's just a thought.
>>
>> -Pavel
>>
>>> var data = new DataInputStream(new FileInputStream(file).buffered())
>>> for (int i = 0; i < 1000; i++) {
>>> data.readInt(); // readInt() does 4 calls to InputStream.read() so 
>>> buffering is needed
>>> }
>>>
>>> here if FileInputStream.needsBuffer() is overridden and returns false 
>>> (assuming most of reads from it are bulk)
>>> then we won't have buffering for DataInputStream. To avoid this we can also
>>> add InputStream.buffered(boolean enforceBuffering) to have 

Re: RFR: 8265375: Bootcycle builds fail with StackOverflowError in cldrconverter

[Joe Wang]

On Fri, 16 Apr 2021 21:10:42 GMT, Naoto Sato  wrote:

> Please review the fix to the tier4 build failure. The piece of code that made 
> into `CLDRLocaleProviderAdapter.java` was also needed in the build tool 
> counterpart (`CLDRConverter`).

Marked as reviewed by joehw (Reviewer).

-

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

RFR: 8265375: Bootcycle builds fail with StackOverflowError in cldrconverter

[Naoto Sato]

Please review the fix to the tier4 build failure. The piece of code that made 
into `CLDRLocaleProviderAdapter.java` was also needed in the build tool 
counterpart (`CLDRConverter`).

-

Commit messages:
 - 8265375: Bootcycle builds fail with StackOverflowError in cldrconverter

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

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

Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxilary classes [v2]

[Rafael Winterhalter]

> To allow agents the definition of auxiliary classes, an API is needed to 
> allow this. Currently, this is often achieved by using `sun.misc.Unsafe` or 
> `jdk.internal.misc.Unsafe` ever since the `defineClass` method was removed 
> from `sun.misc.Unsafe`.

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:

  8200559: Java agents doing instrumentation need a means to define auxiliary 
classes

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3546/files
  - new: https://git.openjdk.java.net/jdk/pull/3546/files/362efc82..2ef21598

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

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

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

Integrated: 8265358: ProblemList jdk/jshell/ToolBasicTest.java on macOS-aarch64

[Daniel D . Daugherty]

On Fri, 16 Apr 2021 18:07:01 GMT, Daniel D. Daugherty  
wrote:

> A set of trivial ProblemListing for macos-aarch64 Tier2 test failures:
> 
> - JDK-8265358 ProblemList jdk/jshell/ToolBasicTest.java on macOS-aarch64
> - JDK-8265361 ProblemList a few compiler/whitebox tests on macos-aarch64
> - JDK-8265363 ProblemList java/net/Socket/UdpSocket.java on macos-aarch64
> - JDK-8265368 ProblemList 3 java/net/httpclient/websocket tests on 
> macos-aarch64
> - JDK-8265370 ProblemList java/net/MulticastSocket/Promiscuous.java on 
> macos-aarch64

This pull request has now been integrated.

Changeset: 888d80b5
Author:Daniel D. Daugherty 
URL:   https://git.openjdk.java.net/jdk/commit/888d80b5
Stats: 13 lines in 3 files changed: 13 ins; 0 del; 0 mod

8265358: ProblemList jdk/jshell/ToolBasicTest.java on macOS-aarch64
8265361: ProblemList a few compiler/whitebox tests on macos-aarch64
8265363: ProblemList java/net/Socket/UdpSocket.java on macos-aarch64
8265368: ProblemList 3 java/net/httpclient/websocket tests on macos-aarch64
8265370: ProblemList java/net/MulticastSocket/Promiscuous.java on macos-aarch64

Reviewed-by: bpb, mikael

-

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

Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxilary classes

[Rafael Winterhalter]

I have never seen a need for a non-agent to define a class in a
non-existing package. Injection is typically required if you want to work
with package-private types or methods which is really only relevant for the
Spring framework, but even there I do not think it's such a big area that
it cannot be addressed. Non-agent code generation is typically the use of
proxies where the code generation framework is sharing a class loader with
the user code and there is normally a way to weave it. Agents on the other
hand have to deal with unknown class loader hierarchies and it can be
necessary to inject code into a common class loader parent without
instrumenting classes in this loader or even knowing any classes of this
loader. It's really agents where this is required and therefore
Instrumentation is a good target for such an addition.

I have never heard about a 'discussion [that] will eventually lead into
putting at least some restrictions on agents loaded into a running VM' and
as a heavy user and someone who has helped to write a long row of
commercial agents and followed them into their use in production and who
has seen how helpful they are in reducing deployment complexity, I can only
hope that you will change your mind on this. Dynamically attached agents
must already run as the same user as the target process. If you are
concerned about agents 'illegally intruding' your Java process, I'd say you
have bigger security issues at hand and it is already possible to disable
dynamic attachment if you want to avoid it. Dynamic attachment has become
pretty much the default approach for a lot of Java tooling in production
environments. It has proven to be very convenient if a for example a
tracing tool is scanning Java processes to attach to them, rather than
requiring the deployment operators to be explicitly set up command line
arguments which are easily forgotten if they only need to be added in some
environments. This reduction in complexity for operations has literally
saved millions of dollars to Java users and Oracle customers. This makes
Java a popular choice, especially compared to languages such as Go where
this is naturally not possible. It's easy and there is no record of this
feature doing any harm. I would not see any good reason to restrict this by
default.

That said, even if it was restricted in the future, this would mean that
some of the Instrumentation API methods will throw exceptions in the
future. There would not be much difference if an introduced defineClass
method would do the same.

Am Fr., 16. Apr. 2021 um 19:31 Uhr schrieb Alan Bateman <
alan.bate...@oracle.com>:

> On 16/04/2021 17:40, Rafael Winterhalter wrote:
> > :
> > I will try to make my case on the mailing list. I hoped this could get
> resolved within the release of Java 17 as this would make it possible to
> write agents without use of Unsafe API to support Java 17 and later. Since
> agents often are supplementary to a broad range of Java applications, the
> LTS release will likely be an important support boundary for years and
> years to come.
> >
> "are supplementary to a board range of Java applications" is part of the
> concern with the proposal. If possible, it would be good if the write-up
> could separate the requirements for injection/instrumentation by
> frameworks at runtime from the requirements of tool agents. If the
> requirements cover testing time and mocking then it would useful to
> separate those too.
>
> Just to add to Rémi's comment: For frameworks/libraries, the
> Lookup.defineClass and defineHiddenClass APIs are to define classes in
> the same run-time package as the Lookup class. There isn't any API for
> libraries/frameworks to define class in a "new run-time package".
> There's a chunky project there. Part of it is the Lookup API itself,
> part of is that there isn't an exposed way to extend the set of packages
> in a named module. Mandy has done some exploration on this topic and may
> be able to say a bit more about this.
>
> On Java agents, then I think the discussion will eventually lead into
> putting at least some restrictions on agents loaded into a running VM.
> Agents started on the command line with -javaagent are all-powerful but
> maybe agents loaded into a target VM get a restricted Instrumentation
> object that cannot redefine modules or retransform classes in named
> modules. The narrower requirement for agents doing load time
> instrumentation to define auxiliary classes in the same package as the
> class being loaded fits with the intent of the original API and I don't
> think is controversial.
>
> -Alan
>

RFC: 8200559: Java agents doing instrumentation need a means to define auxiliary classes

[Rafael Winterhalter]

I am trying to revive issue 8200559 (
https://bugs.openjdk.java.net/browse/JDK-8200559) which was briefly
discussed on this mailing list over three years ago (
http://mail.openjdk.java.net/pipermail/jdk-dev/2018-January/000405.html). I
am hopeful that a solution could be reached prior to releasing Java 17.
With the LTS label, JVMs of that version are likely to be seen for quite
some time and without a fix, many Java agents will still rely on unsafe API
to support that version. With the issue resolved, agents that aim to
support Java 17 as a baseline could be implemented using only official API
which I consider a desirable milestone. This would also allow me to remove
use of unsafe API from a range of popular open source projects that I
maintain once Java 17 becomes their baseline. I have proactively added a
pull request (https://github.com/openjdk/jdk/pull/3546) with what I
consider to be the most complete and satisfactory solution for agent
developers. I implemented this patch to test potential migration for my own
and customer projects and can confirm that this would indeed make any
unsafe API obsolete.

I had already lined out why APIs such as MethodHandles.Lookup::defineClass
do not work for ClassFileTransformers in my original posting. I will try to
avoid reiterating on my original argument. As a result of this discussion,
an API was proposed where the ClassFileTransformer would be overloaded with
an additional parameter that receives an instance of ClassDefiner as its
argument. The instance would allow you to define a class but assert that
the auxiliary class is located in the same package as the class under
transformation. For example, if a class:

package p;
class C {
  void m(A a) {
a.callback(new Value());
  }
}

would be instrumented to become:

package p;
class C {
  void m(A a) {
a.callback(new Replacement()); // changed
  }
}

then ClassDefiner would assert that the auxiliary class Replacement is also
defined in package 'p'. In my eyes, this is not a very satisfying solution.
In some cases, a Java agent is for example using existing classes to carry
additional state between two instrumented classes. For instance, class
Replacement might be used as a carrier of additional state to pass to class
A which is itself instrumented as follows:

package p2;
class A {
  void callback(Value v) {
if (v instanceof Replacement) {
  // read data of v and process
}
  }
}

This example is far from constructed, but a common and effective strategy
to implement tracing, security monitoring or code intelligence.

It is of course possible to implement this pattern by using the
package-restricted ClassDefiner. It is however rather costly and
complicated to implement. A ClassFileTransformer is a simple callback that
is not in control of the class loading order but is only invoked whenever a
class is loaded for the first time. The first class to be loaded might of
course be either C or A, it might not even be deterministic for the same
application. If the class Replacement was required to live in the package
of the class that is currently instrumented, Replacement would need to be
defined in either p or p2, depending on the materialized class loading
order. This would require some form of state management in the class file
transformer to keep track of the package that one would need to use when
instrumenting the second class. Due to parallel class loading and with
constellations that involve even more classes, this can get quite
complicated. Using a stable package name is however not only a convenience.
The name of a package can affect reflection-heavy frameworks, debugging
opportunities, logs, stacktraces and more.

Furthermore, one should consider the costs of migrating an existing agent
in the light that an agent can easily open jdk.internal.misc.Unsafe by
using the instrumentation API or use a class file transformer to erase the
explicit package assertion of ClassDefiner. Current agents often use
sun.misc.Unsafe or jdk.internal.misc.Unsafe to invoke the defineClass
method of these types. This API is very performant and easy to use. To some
degree, it is even possible to emulate the Unsafe API using ClassDefiner by
strategically retransforming classes in the right package. The following
code allows to define a class using a witness class by using a
non-operational class file transformer:

static Class defineClass(Instrumentation inst, final Class witness,
final byte[] classFile) throws Exception {
  AtomicReference> ref = new AtomicReference<>();
  ClassFileTransformer definer = new ClassFileTransformer() {
@Override
public byte[] transform(ClassDefiner classDefiner,
Module module,
ClassLoader loader,
String className,
Class classBeingRedefined,
ProtectionDomain protectionDomain,
byte[] classfileBuffer) {
  if 

Re: RFR: 8265358: ProblemList jdk/jshell/ToolBasicTest.java on macOS-aarch64 [v2]

[Daniel D . Daugherty]

On Fri, 16 Apr 2021 19:42:06 GMT, Brian Burkhalter  wrote:

>> Daniel D. Daugherty has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Remove changes for JDK-8265366 at @fguallini's request.
>
> Marked as reviewed by bpb (Reviewer).

@bplb and @vidmik - Thanks for the reviews.

I'm removing the changes for JDK-8265366 at @fguallini's request.

-

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

Re: RFR: 8265358: ProblemList jdk/jshell/ToolBasicTest.java on macOS-aarch64 [v2]

[Mikael Vidstedt]

On Fri, 16 Apr 2021 20:12:45 GMT, Daniel D. Daugherty  
wrote:

>> A set of trivial ProblemListing for macos-aarch64 Tier2 test failures:
>> 
>> - JDK-8265358 ProblemList jdk/jshell/ToolBasicTest.java on macOS-aarch64
>> - JDK-8265361 ProblemList a few compiler/whitebox tests on macos-aarch64
>> - JDK-8265363 ProblemList java/net/Socket/UdpSocket.java on macos-aarch64
>> - JDK-8265368 ProblemList 3 java/net/httpclient/websocket tests on 
>> macos-aarch64
>> - JDK-8265370 ProblemList java/net/MulticastSocket/Promiscuous.java on 
>> macos-aarch64
>
> Daniel D. Daugherty has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Remove changes for JDK-8265366 at @fguallini's request.

Marked as reviewed by mikael (Reviewer).

-

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

Re: RFR: 8265358: ProblemList jdk/jshell/ToolBasicTest.java on macOS-aarch64 [v2]

[Daniel D . Daugherty]

> A set of trivial ProblemListing for macos-aarch64 Tier2 test failures:
> 
> - JDK-8265358 ProblemList jdk/jshell/ToolBasicTest.java on macOS-aarch64
> - JDK-8265361 ProblemList a few compiler/whitebox tests on macos-aarch64
> - JDK-8265363 ProblemList java/net/Socket/UdpSocket.java on macos-aarch64
> - JDK-8265368 ProblemList 3 java/net/httpclient/websocket tests on 
> macos-aarch64
> - JDK-8265370 ProblemList java/net/MulticastSocket/Promiscuous.java on 
> macos-aarch64

Daniel D. Daugherty has updated the pull request incrementally with one 
additional commit since the last revision:

  Remove changes for JDK-8265366 at @fguallini's request.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3549/files
  - new: https://git.openjdk.java.net/jdk/pull/3549/files/5fb3de6a..7f245f4e

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

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

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

Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxilary classes

[Rafael Winterhalter]

On Fri, 16 Apr 2021 13:44:16 GMT, Rafael Winterhalter 
 wrote:

> To allow agents the definition of auxiliary classes, an API is needed to 
> allow this. Currently, this is often achieved by using `sun.misc.Unsafe` or 
> `jdk.internal.misc.Unsafe` ever since the `defineClass` method was removed 
> from `sun.misc.Unsafe`.

I have never seen a need for a non-agent to define a class in a
non-existing package. Injection is typically required if you want to work
with package-private types or methods which is really only relevant for the
Spring framework, but even there I do not think it's such a big area that
it cannot be addressed. Non-agent code generation is typically the use of
proxies where the code generation framework is sharing a class loader with
the user code and there is normally a way to weave it. Agents on the other
hand have to deal with unknown class loader hierarchies and it can be
necessary to inject code into a common class loader parent without
instrumenting classes in this loader or even knowing any classes of this
loader. It's really agents where this is required and therefore
Instrumentation is a good target for such an addition.

I have never heard about a 'discussion [that] will eventually lead into
putting at least some restrictions on agents loaded into a running VM' and
as a heavy user and someone who has helped to write a long row of
commercial agents and followed them into their use in production and who
has seen how helpful they are in reducing deployment complexity, I can only
hope that you will change your mind on this. Dynamically attached agents
must already run as the same user as the target process. If you are
concerned about agents 'illegally intruding' your Java process, I'd say you
have bigger security issues at hand and it is already possible to disable
dynamic attachment if you want to avoid it. Dynamic attachment has become
pretty much the default approach for a lot of Java tooling in production
environments. It has proven to be very convenient if a for example a
tracing tool is scanning Java processes to attach to them, rather than
requiring the deployment operators to be explicitly set up command line
arguments which are easily forgotten if they only need to be added in some
environments. This reduction in complexity for operations has literally
saved millions of dollars to Java users and Oracle customers. This makes
Java a popular choice, especially compared to languages such as Go where
this is naturally not possible. It's easy and there is no record of this
feature doing any harm. I would not see any good reason to restrict this by
default.

That said, even if it was restricted in the future, this would mean that
some of the Instrumentation API methods will throw exceptions in the
future. There would not be much difference if an introduced defineClass
method would do the same.

Am Fr., 16. Apr. 2021 um 19:33 Uhr schrieb mlbridge[bot] <
***@***.***>:

> *Mailing list message from Alan Bateman ***@***.***> on
> core-libs-dev ***@***.***>:*
>
> On 16/04/2021 17:40, Rafael Winterhalter wrote:
>
> :
> I will try to make my case on the mailing list. I hoped this could get
> resolved within the release of Java 17 as this would make it possible to
> write agents without use of Unsafe API to support Java 17 and later. Since
> agents often are supplementary to a broad range of Java applications, the
> LTS release will likely be an important support boundary for years and
> years to come.
>
> "are supplementary to a board range of Java applications" is part of the
> concern with the proposal. If possible, it would be good if the write-up
> could separate the requirements for injection/instrumentation by
> frameworks at runtime from the requirements of tool agents. If the
> requirements cover testing time and mocking then it would useful to
> separate those too.
>
> Just to add to R?mi's comment: For frameworks/libraries, the
> Lookup.defineClass and defineHiddenClass APIs are to define classes in
> the same run-time package as the Lookup class. There isn't any API for
> libraries/frameworks to define class in a "new run-time package".
> There's a chunky project there. Part of it is the Lookup API itself,
> part of is that there isn't an exposed way to extend the set of packages
> in a named module. Mandy has done some exploration on this topic and may
> be able to say a bit more about this.
>
> On Java agents, then I think the discussion will eventually lead into
> putting at least some restrictions on agents loaded into a running VM.
> Agents started on the command line with -javaagent are all-powerful but
> maybe agents loaded into a target VM get a restricted Instrumentation
> object that cannot redefine modules or retransform classes in named
> modules. The narrower requirement for agents doing load time
> instrumentation to define auxiliary classes in the same package as the
> class being loaded fits with the intent of the original API and I don't
> think is 

Integrated: 8262744: Formatter '%g' conversion uses wrong format for BigDecimal rounding up to limits

[Ian Graves]

On Tue, 6 Apr 2021 20:34:52 GMT, Ian Graves  wrote:

> This fixes a bug where the formatting code for `%g` flags incorrectly tries 
> to round `BigDecimal` after determining whether it should be a decimal 
> numeric format or a scientific numeric format. The solution rounds before 
> determining the correct format.

This pull request has now been integrated.

Changeset: 0bdc3e7a
Author:Ian Graves 
Committer: Roger Riggs 
URL:   https://git.openjdk.java.net/jdk/commit/0bdc3e7a
Stats: 66 lines in 2 files changed: 62 ins; 1 del; 3 mod

8262744: Formatter '%g' conversion uses wrong format for BigDecimal rounding up 
to limits

Reviewed-by: rriggs, bpb

-

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

Re: RFR: 8262744: Formatter '%g' conversion uses wrong format for BigDecimal rounding up to limits [v3]

[Ian Graves]

On Fri, 16 Apr 2021 16:08:53 GMT, Ian Graves  wrote:

>> This fixes a bug where the formatting code for `%g` flags incorrectly tries 
>> to round `BigDecimal` after determining whether it should be a decimal 
>> numeric format or a scientific numeric format. The solution rounds before 
>> determining the correct format.
>
> Ian Graves has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Inlining some single use variables

That's correct. Passes `java.math` tests. Ran it again, just to be sure.

-

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

Re: RFR: 8265358: ProblemList jdk/jshell/ToolBasicTest.java on macOS-aarch64

[Mikael Vidstedt]

On Fri, 16 Apr 2021 18:07:01 GMT, Daniel D. Daugherty  
wrote:

> A set of trivial ProblemListing for macos-aarch64 Tier2 test failures:
> 
> - JDK-8265358 ProblemList jdk/jshell/ToolBasicTest.java on macOS-aarch64
> - JDK-8265361 ProblemList a few compiler/whitebox tests on macos-aarch64
> - JDK-8265363 ProblemList java/net/Socket/UdpSocket.java on macos-aarch64
> - JDK-8265366 ProblemList 2 javax/net/ssl/DTLS tests on macos-aarch64
> - JDK-8265368 ProblemList 3 java/net/httpclient/websocket tests on 
> macos-aarch64
> - JDK-8265370 ProblemList java/net/MulticastSocket/Promiscuous.java on 
> macos-aarch64

Marked as reviewed by mikael (Reviewer).

-

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

Re: RFR: 8265358: ProblemList jdk/jshell/ToolBasicTest.java on macOS-aarch64

[Brian Burkhalter]

On Fri, 16 Apr 2021 18:07:01 GMT, Daniel D. Daugherty  
wrote:

> A set of trivial ProblemListing for macos-aarch64 Tier2 test failures:
> 
> - JDK-8265358 ProblemList jdk/jshell/ToolBasicTest.java on macOS-aarch64
> - JDK-8265361 ProblemList a few compiler/whitebox tests on macos-aarch64
> - JDK-8265363 ProblemList java/net/Socket/UdpSocket.java on macos-aarch64
> - JDK-8265366 ProblemList 2 javax/net/ssl/DTLS tests on macos-aarch64
> - JDK-8265368 ProblemList 3 java/net/httpclient/websocket tests on 
> macos-aarch64
> - JDK-8265370 ProblemList java/net/MulticastSocket/Promiscuous.java on 
> macos-aarch64

Marked as reviewed by bpb (Reviewer).

-

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

RFR: 8265358: ProblemList jdk/jshell/ToolBasicTest.java on macOS-aarch64

[Daniel D . Daugherty]

A set of trivial ProblemListing for macos-aarch64 Tier2 test failures:

- JDK-8265358 ProblemList jdk/jshell/ToolBasicTest.java on macOS-aarch64
- JDK-8265361 ProblemList a few compiler/whitebox tests on macos-aarch64
- JDK-8265363 ProblemList java/net/Socket/UdpSocket.java on macos-aarch64
- JDK-8265366 ProblemList 2 javax/net/ssl/DTLS tests on macos-aarch64
- JDK-8265368 ProblemList 3 java/net/httpclient/websocket tests on macos-aarch64
- JDK-8265370 ProblemList java/net/MulticastSocket/Promiscuous.java on 
macos-aarch64

-

Commit messages:
 - 8265370: ProblemList java/net/MulticastSocket/Promiscuous.java on 
macos-aarch64
 - 8265368: ProblemList 3 java/net/httpclient/websocket tests on macos-aarch64
 - 8265366 ProblemList 2 javax/net/ssl/DTLS tests on macos-aarch64
 - 8265363: ProblemList java/net/Socket/UdpSocket.java on macos-aarch64
 - 8265361: ProblemList a few compiler/whitebox tests on macos-aarch64
 - 8265358: ProblemList jdk/jshell/ToolBasicTest.java on macOS-aarch64

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

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

Re: RFR: 8265237: String.join and StringJoiner can be improved further [v2]

[Roger Riggs]

On Thu, 15 Apr 2021 19:26:48 GMT, Peter Levart  wrote:

>> While JDK-8148937 improved StringJoiner class by replacing internal use of 
>> getChars that copies out characters from String elements into a char[] array 
>> with StringBuilder which is somehow more optimal, the improvement was 
>> marginal in speed (0% ... 10%) and mainly for smaller strings, while GC was 
>> reduced by about 50% in average per operation.
>> Initial attempt to tackle that issue was more involved, but was later 
>> discarded because it was apparently using too much internal String details 
>> in code that lives outside String and outside java.lang package.
>> But there is another way to package such "intimate" code - we can put it 
>> into String itself and just call it from StringJoiner.
>> This PR is an attempt at doing just that. It introduces new package-private 
>> method in `java.lang.String` which is then used from both pubic static 
>> `String.join` methods as well as from `java.util.StringJoiner` (via 
>> SharedSecrets). The improvements can be seen by running the following JMH 
>> benchmark:
>> 
>> https://gist.github.com/plevart/86ac7fc6d4541dbc08256cde544019ce
>> 
>> The comparative results are here:
>> 
>> https://jmh.morethan.io/?gist=7eb421cf7982456a2962269137f71c15
>> 
>> The jmh-result.json files are here:
>> 
>> https://gist.github.com/plevart/7eb421cf7982456a2962269137f71c15
>> 
>> Improvement in speed ranges from 8% (for small strings) to 200% (for long 
>> strings), while creation of garbage has been further reduced to an almost 
>> garbage-free operation.
>> 
>> So WDYT?
>
> Peter Levart has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add String.join benchmark method to StringJoinerBenchmark and adjust some 
> parameters to cover bigger range

Look very good.

src/java.base/share/classes/java/lang/String.java line 3254:

> 3252: 
> 3253: byte[] value = StringConcatHelper.newArray(((long) icoder << 
> 32) | llen);
> 3254: int off = 0;

StringConcatHelper.newArray() can double the length (based on the coder) and it 
is then truncated to 32 bits when passed to UNSAFE.allocatlUnitializedArray.
The test of length above only ensures llen can be truncated to 32 bits without 
loss of data.

src/java.base/share/classes/java/lang/String.java line 3256:

> 3254: int off = 0;
> 3255: prefix.getBytes(value, off, coder); off += prefix.length();
> 3256: for (int i = 0; i < size; i++) {

Can you save a branch inside the loop by handling element 0 outside the loop and
then do the loop for the rest?

-

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

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

[Naoto Sato]

On Fri, 16 Apr 2021 18:15:41 GMT, Roger Riggs  wrote:

>> Naoto Sato has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Modified javadocs per suggestions.
>
> src/java.base/share/classes/java/io/InputStreamReader.java line 48:
> 
>> 46:  *  For top efficiency, consider wrapping an InputStreamReader within 
>> a
>> 47:  * BufferedReader.  For example:
>> 48:  *
> 
> Oddly, none of the reference in this class to the default charset are links 
> to Charset.defaultCharset().
> That would be a useful addition, either in the class javadoc or in the 1-arg 
> constructor that uses the default charset.

Thanks, Roger. Both are good suggestions. Will incorporate them into the next 
iteration.

-

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

Re: 8252827: Caching Integer.toString just like Integer.valueOf

[Raffaello Giulietti]

I guess the reporter meant to limit the cache range similarly to the one 
used for valueOf().


I have no clue about the benefit/cost ratio for the proposed String 
cache. It really depends on usage, workload, etc. One can easily imagine 
both extreme scenarios but it's hard to tell how the average one would look.


My post is only about either solving the issue by implementing the 
cache, which I can contribute to; or closing it because of lack of 
real-world need or interest.



Greetings
Raffaello


On 2021-04-16 20:36, Roger Riggs wrote:

Hi,

Is there any way to quantify the savings?
And what technique can be applied to limit the size of the cache.
The size of the small integer cache is somewhat arbitrary.

Regards, Roger

On 4/16/21 12:48 PM, Raffaello Giulietti wrote:

Hello,

does the enhancement proposed in [1] make sense, both today and when 
wrappers will be migrated to primitive classes?

If so, it should be rather simple to add it and I could prepare a PR.
If not, the issue might perhaps be closed.


Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8252827

Re: RFR: 4926314: Optimize Reader.read(CharBuffer) [v9]

[Brian Burkhalter]

On Sat, 13 Mar 2021 14:28:25 GMT, Philippe Marschall 
 wrote:

>> Implement three optimiztations for Reader.read(CharBuffer)
>> 
>> * Add a code path for heap buffers in Reader#read to use the backing array 
>> instead of allocating a new one.
>> * Change the code path for direct buffers in Reader#read to limit the 
>> intermediate allocation to `TRANSFER_BUFFER_SIZE`.
>> * Implement `InputStreamReader#read(CharBuffer)` and delegate to 
>> `StreamDecoder`.
>> * Implement `StreamDecoder#read(CharBuffer)` and avoid buffer allocation.
>
> Philippe Marschall has updated the pull request with a new target base due to 
> a merge or a rebase. The pull request now contains 15 commits:
> 
>  - Merge master
>  - Fix bug in CharArrayReader and add unit test
>  - Clean up unit tests
>  - Revert off-heap code path
>  - Replace c-style array declarations
>  - Share work buffer between #skip and #read
>  - Update copyright year
>  - Implement review comment
>  - Revert StreamDecoder changes
>  - Implement CharArrayReader#read(CharBuffer)
>  - ... and 5 more: 
> https://git.openjdk.java.net/jdk/compare/d339320e...c4c859e0

Approved modulo my "as they may" remarks on three comment lines in the tests.

-

Marked as reviewed by bpb (Reviewer).

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

Re: RFR: 4926314: Optimize Reader.read(CharBuffer) [v9]

[Alan Bateman]

On Sat, 13 Mar 2021 14:28:25 GMT, Philippe Marschall 
 wrote:

>> Implement three optimiztations for Reader.read(CharBuffer)
>> 
>> * Add a code path for heap buffers in Reader#read to use the backing array 
>> instead of allocating a new one.
>> * Change the code path for direct buffers in Reader#read to limit the 
>> intermediate allocation to `TRANSFER_BUFFER_SIZE`.
>> * Implement `InputStreamReader#read(CharBuffer)` and delegate to 
>> `StreamDecoder`.
>> * Implement `StreamDecoder#read(CharBuffer)` and avoid buffer allocation.
>
> Philippe Marschall has updated the pull request with a new target base due to 
> a merge or a rebase. The pull request now contains 15 commits:
> 
>  - Merge master
>  - Fix bug in CharArrayReader and add unit test
>  - Clean up unit tests
>  - Revert off-heap code path
>  - Replace c-style array declarations
>  - Share work buffer between #skip and #read
>  - Update copyright year
>  - Implement review comment
>  - Revert StreamDecoder changes
>  - Implement CharArrayReader#read(CharBuffer)
>  - ... and 5 more: 
> https://git.openjdk.java.net/jdk/compare/d339320e...c4c859e0

Thanks for rolling back to the direct buffer case, the updated implementation 
changes look okay.

-

Marked as reviewed by alanb (Reviewer).

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

Re: 8252827: Caching Integer.toString just like Integer.valueOf

[Roger Riggs]

Hi,

Is there any way to quantify the savings?
And what technique can be applied to limit the size of the cache.
The size of the small integer cache is somewhat arbitrary.

Regards, Roger

On 4/16/21 12:48 PM, Raffaello Giulietti wrote:

Hello,

does the enhancement proposed in [1] make sense, both today and when 
wrappers will be migrated to primitive classes?

If so, it should be rather simple to add it and I could prepare a PR.
If not, the issue might perhaps be closed.


Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8252827

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

[Roger Riggs]

On Thu, 15 Apr 2021 18:29:17 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:
> 
>   Modified javadocs per suggestions.

For the test, can it be re-written in Java.  
The direction has been to avoid creating new shell tests as they are fragile.
There are test utilities in ProcessTool to make launching and checking for 
output very easy.

src/java.base/share/classes/java/io/InputStreamReader.java line 48:

> 46:  *  For top efficiency, consider wrapping an InputStreamReader within a
> 47:  * BufferedReader.  For example:
> 48:  *

Oddly, none of the reference in this class to the default charset are links to 
Charset.defaultCharset().
That would be a useful addition, either in the class javadoc or in the 1-arg 
constructor that uses the default charset.

-

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

Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxilary classes

[forax]

> De: "Rafael Winterhalter" 
> À: "Remi Forax" 
> Cc: "Rafael Winterhalter" , "core-libs-dev"
> , "serviceability-dev"
> 
> Envoyé: Vendredi 16 Avril 2021 18:41:26
> Objet: Re: RFR: 8200559: Java agents doing instrumentation need a means to
> define auxilary classes

> This would be a problem, however. Since there is currently no official way of
> defining a class, and since Java agents do not control the class loading 
> order,
> if a class was loaded for the first time, you could for example not add a 
> field
> with a type of an auxiliary class that you had planned to inject. A class 
> being
> loaded is normally the first opportunity for a Java agent and if no witness
> class is available at this point, using a method handle is no option. Since it
> is difficult to know if such a witness class is available in the general case,
> it would also add quite a performance and managerial toll on agent authors. I
> would hope that an API equally convenient to today's unsafe options could be
> added.

I was thinking about adding a dummy class in the package you want to load 
classes. 

Anyway, you can also call ClassLoader.defineClass directly. Put the code that 
calls defineClass in a module and use Instrumentation.redefineModule() to open 
java.base to this module. 

Rémi 

> Am Fr., 16. Apr. 2021 um 18:35 Uhr schrieb < [ mailto:fo...@univ-mlv.fr |
> fo...@univ-mlv.fr ] >:

>>> De: "Rafael Winterhalter" < [ mailto:rafael@gmail.com | 
>>> rafael@gmail.com
>>> ] >
>>> À: "Remi Forax" < [ mailto:fo...@univ-mlv.fr | fo...@univ-mlv.fr ] >
>>> Cc: "Rafael Winterhalter" < [ mailto:winterhal...@openjdk.java.net |
>>> winterhal...@openjdk.java.net ] >, "core-libs-dev" < [
>>> mailto:core-libs-dev@openjdk.java.net | core-libs-dev@openjdk.java.net ] >,
>>> "serviceability-dev" < [ mailto:serviceability-...@openjdk.java.net |
>>> serviceability-...@openjdk.java.net ] >
>>> Envoyé: Vendredi 16 Avril 2021 18:27:46
>>> Objet: Re: RFR: 8200559: Java agents doing instrumentation need a means to
>>> define auxilary classes

>>> Not by my understanding. A suitable lookup requires a loaded class for the
>>> package. A Java agent might however not provide a handle for a class that is
>>> not yet loaded. Or how would you suggest to approach this ?

>> yes, you need a witness class in the package you want to define a new class.
>> Apart if you load classes in the unamed module, you can not load a class in a
>> non existing package anyway (apart if you generate your own module-info),
>> so you need at least a dummy class to define a package, so you can use it to 
>> get
>> a Lookup.

>> Rémi

>>> Am Fr., 16. Apr. 2021 um 16:21 Uhr schrieb Remi Forax < [
>>> mailto:fo...@univ-mlv.fr | fo...@univ-mlv.fr ] >:

 - Mail original -
> De: "Rafael Winterhalter" < [ mailto:winterhal...@openjdk.java.net |
 > winterhal...@openjdk.java.net ] >
> À: "core-libs-dev" < [ mailto:core-libs-dev@openjdk.java.net |
> core-libs-dev@openjdk.java.net ] >, "serviceability-dev" < [
> mailto:serviceability-...@openjdk.java.net |
 > serviceability-...@openjdk.java.net ] >
 > Envoyé: Vendredi 16 Avril 2021 15:52:07
> Objet: RFR: 8200559: Java agents doing instrumentation need a means to 
> define
 > auxilary classes

 > To allow agents the definition of auxiliary classes, an API is needed to 
 > allow
 > this. Currently, this is often achieved by using `sun.misc.Unsafe` or
 > `jdk.internal.misc.Unsafe` ever since the `defineClass` method was 
 > removed from
 > `sun.misc.Unsafe`.

 You can already use Lookup.defineClass() + privateLookupIn() +
 Instrumentation.redefineModule() for that ?

 Rémi


 > -

 > Commit messages:
 > - 8200559: Java agents doing instrumentation need a means to define 
 > auxiliary
 > classes

> Changes: [ https://git.openjdk.java.net/jdk/pull/3546/files |
 > https://git.openjdk.java.net/jdk/pull/3546/files ]
> Webrev: [ https://webrevs.openjdk.java.net/?repo=jdk=3546=00 |
 > https://webrevs.openjdk.java.net/?repo=jdk=3546=00 ]
> Issue: [ https://bugs.openjdk.java.net/browse/JDK-8200559 |
 > https://bugs.openjdk.java.net/browse/JDK-8200559 ]
 > Stats: 185 lines in 4 files changed: 185 ins; 0 del; 0 mod
> Patch: [ https://git.openjdk.java.net/jdk/pull/3546.diff |
 > https://git.openjdk.java.net/jdk/pull/3546.diff ]
> Fetch: git fetch [ https://git.openjdk.java.net/jdk |
 > https://git.openjdk.java.net/jdk ] pull/3546/head:pull/3546

> PR: [ https://git.openjdk.java.net/jdk/pull/3546 |
 > https://git.openjdk.java.net/jdk/pull/3546 ]

ReversibleCollection proposal

[Stuart Marks]

This is a proposal to add a ReversibleCollection interface to the Collections 
Framework. I'm looking for comments on overall design before I work on detailed 
specifications and tests. Please send such comments as replies on this email thread.


Here's a link to a draft PR that contains the code diffs. It's prototype quality, 
but it should be good enough to build and try out:


https://github.com/openjdk/jdk/pull/3533

And here's a link to a class diagram showing the proposed additions:


https://cr.openjdk.java.net/~smarks/ReversibleCollection/ReversibleCollectionDiagram.pdf

Thanks,

s'marks


# Ordering and Reversibility


A long-standing concept that's been missing from collections is that of the 
positioning, sequencing, or arrangement of elements as a structural property of a 
collection. (This is sometimes called the "iteration order" of a collection.) For 
example, a HashSet is not ordered, but a List is. This concept is mostly not 
manifested in the collections API.


Iterating a collection produces elements one after another in *some* sequence. The 
concept of "ordered" determines whether this sequence is defined or whether it's a 
coincidence of implementation. What does "having an order" mean? It implies that 
there is a first element and that each element has a successor. Since collections 
have a finite number of elements, it further implies that there is a last element 
that has no successor. However, it is difficult to discern whether a collection has 
a defined order. HashSet generally iterates its elements in the same undefined 
order, and you can't actually tell that it's not a defined order.


Streams do have a notion of ordering ("encounter order") and this is preserved, 
where appropriate, through the stream pipeline. It's possible to detect this by 
testing whether its spliterator has the ORDERED characteristic. Any collection with 
a defined order will have a spliterator with this characteristic. However, this is 
quite a roundabout way to determine whether a collection has a defined order. 
Furthermore, knowing this doesn't enable any additional operations. It only provides 
constraints on the stream's implementations (keeping the elements in order) and 
provides stronger semantics for certain operations. For example, findFirst() on an 
unordered stream is the same as findAny(), but actually finds the first element if 
the stream is ordered.


The concept of ordering is thus present in the system but is surfaced only in a 
fairly indirect way. We can strengthen abstraction of ordering by making a few 
observations and considering their implications.


Given that there is a first element and a last element, the sequence of elements has 
two ends. It's reasonable to consider operations (add, get, remove) on either end. 
Indeed, the Deque interface has a full complement of operations at each end. This is 
an oft-requested feature on various other collections.


Each element except for the last has a successor, implying that each element except 
for the first has a predecessor. Thus it's reasonable to consider iterating the 
elements from first to last or from last to first (that is, in forward or reverse 
order). Indeed, the concept of iterating in reverse order appears already in bits 
and pieces in particular places around the collections:


 - List has indexOf() and lastIndexOf()
 - Deque has removeFirstOccurrence() and removeLastOccurrence()
 - List has a ListIterator with hasPrevious/previous methods
 - Deque and NavigableSet have descendingIterator methods

Given an ordered collection, though, there's no general way to iterate it in reverse 
order. Reversed iteration isn't the most common operation, but there are some 
frequent use cases, such as operating on elements in most-recently-added order. 
Questions and bug reports about this have come up repeatedly over the years.


Unfortunately, iterating in reverse order is much harder than iterating in forward 
order. There are a variety of ways to iterate in forward order. For example, given a 
List, one can do any of the following:


for (var e : list) { ... }
list.forEach(...)
list.stream()
list.toArray()

However, to iterate a list in reverse order, one must use an explicit loop over 
ListIterator:


for (var it = list.listIterator(list.size()); it.hasPrevious(); ) {
var e = it.previous();
...
}

Streaming the elements of a List in reverse order is even worse. One approach would 
be to implement a reverse-ordered Iterator that wraps a ListIterator and delegates 
hasNext/next calls to the ListIterator's hasPrevious/previous methods. Then, this 
Iterator can be turned into a Spliterator, which is then turned into a Stream. (The 
code to do this is left as an exercise for the reader.) Obtaining the elements in 
reverse via other means -- forEach() or toArray() -- is similarly difficult.


Obtaining the elements in reverse order can be accomplished by adopting a concept 
from 

Re: RFR: 8262744: Formatter '%g' conversion uses wrong format for BigDecimal rounding up to limits [v3]

[Brian Burkhalter]

On Fri, 16 Apr 2021 16:08:53 GMT, Ian Graves  wrote:

>> This fixes a bug where the formatting code for `%g` flags incorrectly tries 
>> to round `BigDecimal` after determining whether it should be a decimal 
>> numeric format or a scientific numeric format. The solution rounds before 
>> determining the correct format.
>
> Ian Graves has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Inlining some single use variables

I think this looks all right. I assume you ran all the `java.math` tests.

-

Marked as reviewed by bpb (Reviewer).

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

Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxilary classes

[Alan Bateman]

On 16/04/2021 17:40, Rafael Winterhalter wrote:

:
I will try to make my case on the mailing list. I hoped this could get resolved 
within the release of Java 17 as this would make it possible to write agents 
without use of Unsafe API to support Java 17 and later. Since agents often are 
supplementary to a broad range of Java applications, the LTS release will 
likely be an important support boundary for years and years to come.

"are supplementary to a board range of Java applications" is part of the 
concern with the proposal. If possible, it would be good if the write-up 
could separate the requirements for injection/instrumentation by 
frameworks at runtime from the requirements of tool agents. If the 
requirements cover testing time and mocking then it would useful to 
separate those too.


Just to add to Rémi's comment: For frameworks/libraries, the 
Lookup.defineClass and defineHiddenClass APIs are to define classes in 
the same run-time package as the Lookup class. There isn't any API for 
libraries/frameworks to define class in a "new run-time package". 
There's a chunky project there. Part of it is the Lookup API itself, 
part of is that there isn't an exposed way to extend the set of packages 
in a named module. Mandy has done some exploration on this topic and may 
be able to say a bit more about this.


On Java agents, then I think the discussion will eventually lead into 
putting at least some restrictions on agents loaded into a running VM. 
Agents started on the command line with -javaagent are all-powerful but 
maybe agents loaded into a target VM get a restricted Instrumentation 
object that cannot redefine modules or retransform classes in named 
modules. The narrower requirement for agents doing load time 
instrumentation to define auxiliary classes in the same package as the 
class being loaded fits with the intent of the original API and I don't 
think is controversial.


-Alan

Re: RFR: 4926314: Optimize Reader.read(CharBuffer) [v9]

[Brian Burkhalter]

On Sat, 13 Mar 2021 14:28:25 GMT, Philippe Marschall 
 wrote:

>> Implement three optimiztations for Reader.read(CharBuffer)
>> 
>> * Add a code path for heap buffers in Reader#read to use the backing array 
>> instead of allocating a new one.
>> * Change the code path for direct buffers in Reader#read to limit the 
>> intermediate allocation to `TRANSFER_BUFFER_SIZE`.
>> * Implement `InputStreamReader#read(CharBuffer)` and delegate to 
>> `StreamDecoder`.
>> * Implement `StreamDecoder#read(CharBuffer)` and avoid buffer allocation.
>
> Philippe Marschall has updated the pull request with a new target base due to 
> a merge or a rebase. The pull request now contains 15 commits:
> 
>  - Merge master
>  - Fix bug in CharArrayReader and add unit test
>  - Clean up unit tests
>  - Revert off-heap code path
>  - Replace c-style array declarations
>  - Share work buffer between #skip and #read
>  - Update copyright year
>  - Implement review comment
>  - Revert StreamDecoder changes
>  - Implement CharArrayReader#read(CharBuffer)
>  - ... and 5 more: 
> https://git.openjdk.java.net/jdk/compare/d339320e...c4c859e0

test/jdk/java/io/Reader/ReadCharBuffer.java line 83:

> 81: buffer.limit(8 + 8192 + 1);
> 82: buffer.position(8);
> 83: assertEquals(reader.read(buffer), 8192 + 1);

Lines 80 + 83 might be easier to read if replaced with

int position = 8;
limit = position + 8192 + 1;
buffer.limit(limit);
buffer.position(position);
assertEquals(reader.read(buffer), limit - position);

-

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

Re: Looking for reviewer and sponsor for 4926314

[Brian Burkhalter]

Hello Philippe,

> On Apr 16, 2021, at 8:43 AM, Philippe Marschall  wrote:
> 
> Hello
> 
> I am looking for reviewers and a sponsor for JDK-4926314: Optimize
> Reader.read(CharBuffer) [1]. The PR [2] went through quite some changes
> and is now back to few changes to reduce the impact and the risk.
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-4926314
> [2] https://github.com/openjdk/jdk/pull/1915
> 
> Cheers
> Philippe

The implementation looks fine. I added a few minor comments about the tests.

I can review and sponsor this for you.

Thanks,

Brian

Re: RFR: 4926314: Optimize Reader.read(CharBuffer) [v9]

[Brian Burkhalter]

On Sat, 13 Mar 2021 14:28:25 GMT, Philippe Marschall 
 wrote:

>> Implement three optimiztations for Reader.read(CharBuffer)
>> 
>> * Add a code path for heap buffers in Reader#read to use the backing array 
>> instead of allocating a new one.
>> * Change the code path for direct buffers in Reader#read to limit the 
>> intermediate allocation to `TRANSFER_BUFFER_SIZE`.
>> * Implement `InputStreamReader#read(CharBuffer)` and delegate to 
>> `StreamDecoder`.
>> * Implement `StreamDecoder#read(CharBuffer)` and avoid buffer allocation.
>
> Philippe Marschall has updated the pull request with a new target base due to 
> a merge or a rebase. The pull request now contains 15 commits:
> 
>  - Merge master
>  - Fix bug in CharArrayReader and add unit test
>  - Clean up unit tests
>  - Revert off-heap code path
>  - Replace c-style array declarations
>  - Share work buffer between #skip and #read
>  - Update copyright year
>  - Implement review comment
>  - Revert StreamDecoder changes
>  - Implement CharArrayReader#read(CharBuffer)
>  - ... and 5 more: 
> https://git.openjdk.java.net/jdk/compare/d339320e...c4c859e0

test/jdk/java/io/CharArrayReader/ReadCharBuffer.java line 50:

> 48: @DataProvider(name = "buffers")
> 49: public Object[][] createBuffers() {
> 50: // test both on-heap and off-heap buffers has they may use 
> different code paths

"as they may"

test/jdk/java/io/InputStreamReader/ReadCharBuffer.java line 52:

> 50: @DataProvider(name = "buffers")
> 51: public Object[][] createBuffers() {
> 52: // test both on-heap and off-heap buffers has they make use 
> different code paths

"as they may"

test/jdk/java/io/Reader/ReadCharBuffer.java line 51:

> 49: @DataProvider(name = "buffers")
> 50: public Object[][] createBuffers() {
> 51: // test both on-heap and off-heap buffers has they make use 
> different code paths

"as they may"

-

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

Re: RFR: 8263154: [macos] DMG builds have finder errors

[Andy Herrick]

On Thu, 15 Apr 2021 01:24:04 GMT, Alexander Matveev  
wrote:

> - Issue was reproducible when install-dir points to some invalid location.
>  - Fixed by defaulting DMG drag and drop location to /Applications folder and 
> --install-dir will be ignored with warning for DMG.
>  - I do not see any way to support other valid, but uncommon locations for 
> drag and drop. For example: /Users/USERNAME/Applications is not possible to 
> support since user name is not known. /usr/bin requires root privileges and 
> should contain symbolic links. Locations which does not exist also not 
> possible to support, since DMG cannot create paths. So 
> /Applications/MyCompany is not possible for DMG.

Since target directory shown by apple script is only a suggestion, and user can 
drag app to anywhere in finder, I may be able to be convinced to ignore 
install-dir on dmg builds.
I don't think we should use Log.info() to say given install-dir is ignored, it 
should just be Log.verbose(). (I started the bad precedence when I used 
Log.info for messages about foreign app-image, this should have been 
Log.verbose() too).

-

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

8252827: Caching Integer.toString just like Integer.valueOf

[Raffaello Giulietti]

Hello,

does the enhancement proposed in [1] make sense, both today and when 
wrappers will be migrated to primitive classes?

If so, it should be rather simple to add it and I could prepare a PR.
If not, the issue might perhaps be closed.


Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8252827

Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxilary classes

[Rafael Winterhalter]

This would be a problem, however. Since there is currently no official way
of defining a class, and since Java agents do not control the class loading
order, if a class was loaded for the first time, you could for example not
add a field with a type of an auxiliary class that you had planned to
inject. A class being loaded is normally the first opportunity for a Java
agent and if no witness class is available at this point, using a method
handle is no option. Since it is difficult to know if such a witness class
is available in the general case, it would also add quite a performance and
managerial toll on agent authors. I would hope that an API equally
convenient to today's unsafe options could be added.

Am Fr., 16. Apr. 2021 um 18:35 Uhr schrieb :

>
>
> --
>
> *De: *"Rafael Winterhalter" 
> *À: *"Remi Forax" 
> *Cc: *"Rafael Winterhalter" ,
> "core-libs-dev" , "serviceability-dev" <
> serviceability-...@openjdk.java.net>
> *Envoyé: *Vendredi 16 Avril 2021 18:27:46
> *Objet: *Re: RFR: 8200559: Java agents doing instrumentation need a means
> to define auxilary classes
>
> Not by my understanding. A suitable lookup requires a loaded class for the
> package. A Java agent might however not provide a handle for a class that
> is not yet loaded. Or how would you suggest to approach this ?
>
>
> yes, you need a witness class in the package you want to define a new
> class.
> Apart if you load classes in the unamed module, you can not load a class
> in a non existing package anyway (apart if you generate your own
> module-info),
> so you need at least a dummy class to define a package, so you can use it
> to get a Lookup.
>
> Rémi
>
>
>
> Am Fr., 16. Apr. 2021 um 16:21 Uhr schrieb Remi Forax :
>
>> - Mail original -
>> > De: "Rafael Winterhalter" 
>> > À: "core-libs-dev" ,
>> "serviceability-dev" 
>> > Envoyé: Vendredi 16 Avril 2021 15:52:07
>> > Objet: RFR: 8200559: Java agents doing instrumentation need a means to
>> define auxilary classes
>>
>> > To allow agents the definition of auxiliary classes, an API is needed
>> to allow
>> > this. Currently, this is often achieved by using `sun.misc.Unsafe` or
>> > `jdk.internal.misc.Unsafe` ever since the `defineClass` method was
>> removed from
>> > `sun.misc.Unsafe`.
>>
>> You can already use Lookup.defineClass() + privateLookupIn() +
>> Instrumentation.redefineModule() for that ?
>>
>> Rémi
>>
>> >
>> > -
>> >
>> > Commit messages:
>> > - 8200559: Java agents doing instrumentation need a means to define
>> auxiliary
>> > classes
>> >
>> > Changes: https://git.openjdk.java.net/jdk/pull/3546/files
>> > Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3546=00
>> >  Issue: https://bugs.openjdk.java.net/browse/JDK-8200559
>> >  Stats: 185 lines in 4 files changed: 185 ins; 0 del; 0 mod
>> >  Patch: https://git.openjdk.java.net/jdk/pull/3546.diff
>> >  Fetch: git fetch https://git.openjdk.java.net/jdk
>> pull/3546/head:pull/3546
>> >
>> > PR: https://git.openjdk.java.net/jdk/pull/3546
>>
>
>

Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxilary classes

[Rafael Winterhalter]

On Fri, 16 Apr 2021 13:44:16 GMT, Rafael Winterhalter 
 wrote:

> To allow agents the definition of auxiliary classes, an API is needed to 
> allow this. Currently, this is often achieved by using `sun.misc.Unsafe` or 
> `jdk.internal.misc.Unsafe` ever since the `defineClass` method was removed 
> from `sun.misc.Unsafe`.

The ticket was created as a reaction to a [write-up I made in January 
2018](http://mail.openjdk.java.net/pipermail/jdk-dev/2018-January/000405.html). 
I certainly did not intend to limit the scope to same-package class definitions 
for instrumented classes, and even for those I do not think a 
package-restricted API would be sufficient as I outlined in the comments to 
[JDK-8200559](https://bugs.openjdk.java.net/browse/JDK-8200559).

I will try to make my case on the mailing list. I hoped this could get resolved 
within the release of Java 17 as this would make it possible to write agents 
without use of Unsafe API to support Java 17 and later. Since agents often are 
supplementary to a broad range of Java applications, the LTS release will 
likely be an important support boundary for years and years to come.

You surely mean JEP 411 when mentioning `ProtectionDomain`? The JEP mentions

> We will not deprecate some classes in the java.security package that are 
> related to the Security Manager, for varying reasons: [...] ProtectionDomain 
> — Several significant APIs depend on ProtectionDomain, e.g., 
> ClassLoader::defineClass and Class::getProtectionDomain. ProtectionDomain 
> also has value independent of the Security Manager since it contains the 
> CodeSource of a class.

Also, since this is still a proposal, I would believe that APIs that are 
implemented before JEP 411 is realized should support `ProtectionDomain` for 
users still supporting the security manager in the transition time.

-

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

Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxilary classes

[forax]

> De: "Rafael Winterhalter" 
> À: "Remi Forax" 
> Cc: "Rafael Winterhalter" , "core-libs-dev"
> , "serviceability-dev"
> 
> Envoyé: Vendredi 16 Avril 2021 18:27:46
> Objet: Re: RFR: 8200559: Java agents doing instrumentation need a means to
> define auxilary classes

> Not by my understanding. A suitable lookup requires a loaded class for the
> package. A Java agent might however not provide a handle for a class that is
> not yet loaded. Or how would you suggest to approach this ?

yes, you need a witness class in the package you want to define a new class. 
Apart if you load classes in the unamed module, you can not load a class in a 
non existing package anyway (apart if you generate your own module-info), 
so you need at least a dummy class to define a package, so you can use it to 
get a Lookup. 

Rémi 

> Am Fr., 16. Apr. 2021 um 16:21 Uhr schrieb Remi Forax < [
> mailto:fo...@univ-mlv.fr | fo...@univ-mlv.fr ] >:

>> - Mail original -
>>> De: "Rafael Winterhalter" < [ mailto:winterhal...@openjdk.java.net |
>> > winterhal...@openjdk.java.net ] >
>>> À: "core-libs-dev" < [ mailto:core-libs-dev@openjdk.java.net |
>>> core-libs-dev@openjdk.java.net ] >, "serviceability-dev" < [
>>> mailto:serviceability-...@openjdk.java.net |
>> > serviceability-...@openjdk.java.net ] >
>> > Envoyé: Vendredi 16 Avril 2021 15:52:07
>>> Objet: RFR: 8200559: Java agents doing instrumentation need a means to 
>>> define
>> > auxilary classes

>> > To allow agents the definition of auxiliary classes, an API is needed to 
>> > allow
>> > this. Currently, this is often achieved by using `sun.misc.Unsafe` or
>> > `jdk.internal.misc.Unsafe` ever since the `defineClass` method was removed 
>> > from
>> > `sun.misc.Unsafe`.

>> You can already use Lookup.defineClass() + privateLookupIn() +
>> Instrumentation.redefineModule() for that ?

>> Rémi


>> > -

>> > Commit messages:
>> > - 8200559: Java agents doing instrumentation need a means to define 
>> > auxiliary
>> > classes

>>> Changes: [ https://git.openjdk.java.net/jdk/pull/3546/files |
>> > https://git.openjdk.java.net/jdk/pull/3546/files ]
>>> Webrev: [ https://webrevs.openjdk.java.net/?repo=jdk=3546=00 |
>> > https://webrevs.openjdk.java.net/?repo=jdk=3546=00 ]
>>> Issue: [ https://bugs.openjdk.java.net/browse/JDK-8200559 |
>> > https://bugs.openjdk.java.net/browse/JDK-8200559 ]
>> > Stats: 185 lines in 4 files changed: 185 ins; 0 del; 0 mod
>>> Patch: [ https://git.openjdk.java.net/jdk/pull/3546.diff |
>> > https://git.openjdk.java.net/jdk/pull/3546.diff ]
>>> Fetch: git fetch [ https://git.openjdk.java.net/jdk |
>> > https://git.openjdk.java.net/jdk ] pull/3546/head:pull/3546

>>> PR: [ https://git.openjdk.java.net/jdk/pull/3546 |
>> > https://git.openjdk.java.net/jdk/pull/3546 ]

Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxilary classes

[Rafael Winterhalter]

Not by my understanding. A suitable lookup requires a loaded class for the
package. A Java agent might however not provide a handle for a class that
is not yet loaded. Or how would you suggest to approach this?

Am Fr., 16. Apr. 2021 um 16:21 Uhr schrieb Remi Forax :

> - Mail original -
> > De: "Rafael Winterhalter" 
> > À: "core-libs-dev" ,
> "serviceability-dev" 
> > Envoyé: Vendredi 16 Avril 2021 15:52:07
> > Objet: RFR: 8200559: Java agents doing instrumentation need a means to
> define auxilary classes
>
> > To allow agents the definition of auxiliary classes, an API is needed to
> allow
> > this. Currently, this is often achieved by using `sun.misc.Unsafe` or
> > `jdk.internal.misc.Unsafe` ever since the `defineClass` method was
> removed from
> > `sun.misc.Unsafe`.
>
> You can already use Lookup.defineClass() + privateLookupIn() +
> Instrumentation.redefineModule() for that ?
>
> Rémi
>
> >
> > -
> >
> > Commit messages:
> > - 8200559: Java agents doing instrumentation need a means to define
> auxiliary
> > classes
> >
> > Changes: https://git.openjdk.java.net/jdk/pull/3546/files
> > Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3546=00
> >  Issue: https://bugs.openjdk.java.net/browse/JDK-8200559
> >  Stats: 185 lines in 4 files changed: 185 ins; 0 del; 0 mod
> >  Patch: https://git.openjdk.java.net/jdk/pull/3546.diff
> >  Fetch: git fetch https://git.openjdk.java.net/jdk
> pull/3546/head:pull/3546
> >
> > PR: https://git.openjdk.java.net/jdk/pull/3546
>

Re: RFR: 8262744: Formatter '%g' conversion uses wrong format for BigDecimal rounding up to limits [v2]

[Ian Graves]

On Fri, 16 Apr 2021 14:26:58 GMT, Roger Riggs  wrote:

>> Ian Graves has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Adding BigDecimal equivalents to tests
>
> src/java.base/share/classes/java/util/Formatter.java line 3826:
> 
>> 3824: BigDecimal tenToTheNegFour = BigDecimal.valueOf(1, 4);
>> 3825: BigDecimal tenToThePrec = BigDecimal.valueOf(1, -prec);
>> 3826: value = value.round(new MathContext(prec));
> 
> While you are in the area, how about inlining the creation of the 
> tenToTheNegFour and tenToThePrec values.
> They are used only once and may not be used at all. They don't appear to be 
> needed except for the comparisons.

Makes sense to me.

-

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

Re: RFR: 8262744: Formatter '%g' conversion uses wrong format for BigDecimal rounding up to limits [v3]

[Ian Graves]

> This fixes a bug where the formatting code for `%g` flags incorrectly tries 
> to round `BigDecimal` after determining whether it should be a decimal 
> numeric format or a scientific numeric format. The solution rounds before 
> determining the correct format.

Ian Graves has updated the pull request incrementally with one additional 
commit since the last revision:

  Inlining some single use variables

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3363/files
  - new: https://git.openjdk.java.net/jdk/pull/3363/files/cf40421e..45522605

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

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

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

Looking for reviewer and sponsor for 4926314

[Philippe Marschall]

Hello

I am looking for reviewers and a sponsor for JDK-4926314: Optimize
Reader.read(CharBuffer) [1]. The PR [2] went through quite some changes
and is now back to few changes to reduce the impact and the risk.

 [1] https://bugs.openjdk.java.net/browse/JDK-4926314
 [2] https://github.com/openjdk/jdk/pull/1915

Cheers
Philippe

8246334: Casting ‘double to int’ and ‘long to int’ produce different results

[Raffaello Giulietti]

Hello,

the reporter of [1] seems to understand that the current behavior is 
well specified (since ever) and correctly implemented. Also, it's 
unclear how s/he would like to enhance it.


Shouldn't [1] be closed?


Greetings
Raffaello



[1] https://bugs.openjdk.java.net/browse/JDK-8246334

Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxilary classes

[Alan Bateman]

On Fri, 16 Apr 2021 13:44:16 GMT, Rafael Winterhalter 
 wrote:

> To allow agents the definition of auxiliary classes, an API is needed to 
> allow this. Currently, this is often achieved by using `sun.misc.Unsafe` or 
> `jdk.internal.misc.Unsafe` ever since the `defineClass` method was removed 
> from `sun.misc.Unsafe`.

JDK-8200559 is about defining auxiliary classes in the same runtime package at 
load-time whereas I think the proposal in this PR is adding an unrestricted 
defineClass to the Instrumentation class. I think this will require a lot of 
discussion as there are significant issues and concerns here. An unrestricted 
defineClass might be okay for tool/java agents when started from the command 
line with -javaagent but only if the Instrumentation object is never ever 
leaked to library or application code. It could potentially be part of a large 
effort to reduce the capabilities of agents loaded via the attach mechanism. 
More generally I think we need clearer separation of the requirements of tool 
agents from the requirement of framework/libraries that want to inject proxy 
and other classes at runtime.

Separately, the proposal in JEP 410 is to terminally deprecate ProtectionDomain.

-

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

Re: RFR: 8037397: RegEx pattern matching loses character class after intersection (&&) operator [v4]

[Roger Riggs]

On Fri, 2 Apr 2021 19:15:39 GMT, Ian Graves  wrote:

>> Bug fix with the intersection `&&` operator in regex patterns. In 
>> JDK-8037397, some character classes on the right hand side of the operator 
>> are dropped in cases where nested `[..]` classes are used with non "nested" 
>> ones.
>
> Ian Graves has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Bumping copyright date.

Marked as reviewed by rriggs (Reviewer).

-

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

Re: RFR: 8262744: Formatter '%g' conversion uses wrong format for BigDecimal rounding up to limits [v2]

[Roger Riggs]

On Wed, 7 Apr 2021 02:48:00 GMT, Ian Graves  wrote:

>> This fixes a bug where the formatting code for `%g` flags incorrectly tries 
>> to round `BigDecimal` after determining whether it should be a decimal 
>> numeric format or a scientific numeric format. The solution rounds before 
>> determining the correct format.
>
> Ian Graves has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Adding BigDecimal equivalents to tests

Marked as reviewed by rriggs (Reviewer).

src/java.base/share/classes/java/util/Formatter.java line 3826:

> 3824: BigDecimal tenToTheNegFour = BigDecimal.valueOf(1, 4);
> 3825: BigDecimal tenToThePrec = BigDecimal.valueOf(1, -prec);
> 3826: value = value.round(new MathContext(prec));

While you are in the area, how about inlining the creation of the 
tenToTheNegFour and tenToThePrec values.
They are used only once and may not be used at all. They don't appear to be 
needed except for the comparisons.

-

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

Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxilary classes

[Remi Forax]

- Mail original -
> De: "Rafael Winterhalter" 
> À: "core-libs-dev" , "serviceability-dev" 
> 
> Envoyé: Vendredi 16 Avril 2021 15:52:07
> Objet: RFR: 8200559: Java agents doing instrumentation need a means to define 
> auxilary classes

> To allow agents the definition of auxiliary classes, an API is needed to allow
> this. Currently, this is often achieved by using `sun.misc.Unsafe` or
> `jdk.internal.misc.Unsafe` ever since the `defineClass` method was removed 
> from
> `sun.misc.Unsafe`.

You can already use Lookup.defineClass() + privateLookupIn() + 
Instrumentation.redefineModule() for that ?

Rémi

> 
> -
> 
> Commit messages:
> - 8200559: Java agents doing instrumentation need a means to define auxiliary
> classes
> 
> Changes: https://git.openjdk.java.net/jdk/pull/3546/files
> Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3546=00
>  Issue: https://bugs.openjdk.java.net/browse/JDK-8200559
>  Stats: 185 lines in 4 files changed: 185 ins; 0 del; 0 mod
>  Patch: https://git.openjdk.java.net/jdk/pull/3546.diff
>  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3546/head:pull/3546
> 
> PR: https://git.openjdk.java.net/jdk/pull/3546

RFR: 8200559: Java agents doing instrumentation need a means to define auxilary classes

[Rafael Winterhalter]

To allow agents the definition of auxiliary classes, an API is needed to allow 
this. Currently, this is often achieved by using `sun.misc.Unsafe` or 
`jdk.internal.misc.Unsafe` ever since the `defineClass` method was removed from 
`sun.misc.Unsafe`.

-

Commit messages:
 - 8200559: Java agents doing instrumentation need a means to define auxiliary 
classes

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

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

Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v2]

[Raffaello Giulietti]

> Hello,
> 
> here's a PR for a patch submitted on March 2020 
> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a 
> thing.
> 
> The patch has been edited to adhere to OpenJDK code conventions about 
> multi-line (block) comments. Nothing in the code proper has changed, except 
> for the addition of redundant but clarifying parentheses in some expressions.
> 
> 
> Greetings
> Raffaello

Raffaello Giulietti has updated the pull request incrementally with one 
additional commit since the last revision:

  4511638: Double.toString(double) sometimes produces incorrect results

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3402/files
  - new: https://git.openjdk.java.net/jdk/pull/3402/files/cc10a92d..22092f0c

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

  Stats: 26 lines in 1 file changed: 10 ins; 7 del; 9 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3402.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3402/head:pull/3402

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

Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

On Thu, 8 Apr 2021 21:12:21 GMT, Raffaello Giulietti 
 wrote:

> Hello,
> 
> here's a PR for a patch submitted on March 2020 
> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a 
> thing.
> 
> The patch has been edited to adhere to OpenJDK code conventions about 
> multi-line (block) comments. Nothing in the code proper has changed, except 
> for the addition of redundant but clarifying parentheses in some expressions.
> 
> 
> Greetings
> Raffaello

Modified
test/langtools/tools/javac/sym/ElementStructureTest.java
as suggested by @lahodaj

-

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

Re: RFR: 8265137: java.util.Random suddenly has new public methods nowhere documented [v5]

[Jim Laskey]

On Thu, 15 Apr 2021 12:37:52 GMT, Jim Laskey  wrote:

>> Move makeXXXSpilterator from public (@hidden) to protected. No API ch
>
> Jim Laskey has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove extraneous references to makeXXXSpliterator

I put the CSR back to draft until I was happy with the changeset.

-

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

Re: RFR: 8265137: java.util.Random suddenly has new public methods nowhere documented [v5]

[Uwe Schindler]

On Thu, 15 Apr 2021 12:37:52 GMT, Jim Laskey  wrote:

>> Move makeXXXSpilterator from public (@hidden) to protected. No API ch
>
> Jim Laskey has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove extraneous references to makeXXXSpliterator

Due to the changes now: I think the CSR got obsolete?

-

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

Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

On Fri, 16 Apr 2021 08:22:41 GMT, Jan Lahoda  wrote:

>> Hello,
>> 
>> here's a PR for a patch submitted on March 2020 
>> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was 
>> a thing.
>> 
>> The patch has been edited to adhere to OpenJDK code conventions about 
>> multi-line (block) comments. Nothing in the code proper has changed, except 
>> for the addition of redundant but clarifying parentheses in some expressions.
>> 
>> 
>> Greetings
>> Raffaello
>
> @ rgiulietti, I thought you'd simply add the test change to your patch. But I 
> can start a PR for it, if you prefer.

@lahodaj Oh, my understanding was that yours is a permanent change worth of 
integrating anyway. But yes, I can add your change to my changeset.

-

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

Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results

[Jan Lahoda]

On Thu, 8 Apr 2021 21:12:21 GMT, Raffaello Giulietti 
 wrote:

> Hello,
> 
> here's a PR for a patch submitted on March 2020 
> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a 
> thing.
> 
> The patch has been edited to adhere to OpenJDK code conventions about 
> multi-line (block) comments. Nothing in the code proper has changed, except 
> for the addition of redundant but clarifying parentheses in some expressions.
> 
> 
> Greetings
> Raffaello

@ rgiulietti, I thought you'd simply add the test change to your patch. But I 
can start a PR for it, if you prefer.

-

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

Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

On Thu, 8 Apr 2021 21:12:21 GMT, Raffaello Giulietti 
 wrote:

> Hello,
> 
> here's a PR for a patch submitted on March 2020 
> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a 
> thing.
> 
> The patch has been edited to adhere to OpenJDK code conventions about 
> multi-line (block) comments. Nothing in the code proper has changed, except 
> for the addition of redundant but clarifying parentheses in some expressions.
> 
> 
> Greetings
> Raffaello

Fine, thanks!

Will your change be integrated soon on master?
What am I supposed to do then, rebasing my branch with the updated master?

(BTW, you could make use of instanceof pattern matching if you don't need 
backward compat ;-) )

-

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

Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results

[Jan Lahoda]

On Thu, 8 Apr 2021 21:12:21 GMT, Raffaello Giulietti 
 wrote:

> Hello,
> 
> here's a PR for a patch submitted on March 2020 
> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a 
> thing.
> 
> The patch has been edited to adhere to OpenJDK code conventions about 
> multi-line (block) comments. Nothing in the code proper has changed, except 
> for the addition of redundant but clarifying parentheses in some expressions.
> 
> 
> Greetings
> Raffaello

Sure, here you are:

diff --git a/test/langtools/tools/javac/sym/ElementStructureTest.java 
b/test/langtools/tools/javac/sym/ElementStructureTest.java
index 29776ce28c2..428ba03361f 100644
--- a/test/langtools/tools/javac/sym/ElementStructureTest.java
+++ b/test/langtools/tools/javac/sym/ElementStructureTest.java
@@ -121,29 +121,22 @@ import toolbox.ToolBox;
  */
 public class ElementStructureTest {
 
-static final byte[] hash6 = new byte[] {
-(byte) 0x99, (byte) 0x34, (byte) 0x82, (byte) 0xCF,
-(byte) 0xE0, (byte) 0x53, (byte) 0xF3, (byte) 0x13,
-(byte) 0x4E, (byte) 0xCF, (byte) 0x49, (byte) 0x32,
-(byte) 0xB7, (byte) 0x52, (byte) 0x0F, (byte) 0x68
-};
 static final byte[] hash7 = new byte[] {
-(byte) 0x3C, (byte) 0x03, (byte) 0xEA, (byte) 0x4A,
-(byte) 0x62, (byte) 0xD2, (byte) 0x18, (byte) 0xE5,
-(byte) 0xA5, (byte) 0xC2, (byte) 0xB7, (byte) 0x85,
-(byte) 0x90, (byte) 0xFA, (byte) 0x98, (byte) 0xCD
+(byte) 0x65, (byte) 0x41, (byte) 0xC8, (byte) 0x17,
+(byte) 0xF0, (byte) 0xB1, (byte) 0x62, (byte) 0x9A,
+(byte) 0xD8, (byte) 0x19, (byte) 0xBA, (byte) 0xB0,
+(byte) 0xC1, (byte) 0x70, (byte) 0x5E, (byte) 0x3E
 };
 static final byte[] hash8 = new byte[] {
-(byte) 0x24, (byte) 0x38, (byte) 0x52, (byte) 0x1C,
-(byte) 0x5E, (byte) 0x83, (byte) 0x82, (byte) 0xE6,
-(byte) 0x41, (byte) 0xC2, (byte) 0xDD, (byte) 0x2A,
-(byte) 0xFD, (byte) 0xFF, (byte) 0x5E, (byte) 0x2F
+(byte) 0x83, (byte) 0x62, (byte) 0x2F, (byte) 0x1C,
+(byte) 0x95, (byte) 0x6D, (byte) 0x31, (byte) 0x18,
+(byte) 0xF5, (byte) 0xB2, (byte) 0x8C, (byte) 0x39,
+(byte) 0x81, (byte) 0x2E, (byte) 0x2C, (byte) 0x34
 };
 
 final static Map version2Hash = new HashMap<>();
 
 static {
-version2Hash.put("6", hash6);
 version2Hash.put("7", hash7);
 version2Hash.put("8", hash8);
 }
@@ -484,7 +477,7 @@ public class ElementStructureTest {
 return null;
 try {
 analyzeElement(e);
-out.write(String.valueOf(e.getConstantValue()));
+writeConstant(e.getConstantValue());
 out.write("\n");
 } catch (IOException ex) {
 ex.printStackTrace();
@@ -514,6 +507,16 @@ public class ElementStructureTest {
 throw new IllegalStateException("Should not get here.");
 }
 
+private void writeConstant(Object value) throws IOException {
+if (value instanceof Double) {
+out.write(Double.toHexString((Double) value));
+} else if (value instanceof Float) {
+out.write(Float.toHexString((Float) value));
+} else {
+out.write(String.valueOf(value));
+}
+}
+
 }
 
 final class TestFileManager implements JavaFileManager {

-

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

Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

On Thu, 8 Apr 2021 21:12:21 GMT, Raffaello Giulietti 
 wrote:

> Hello,
> 
> here's a PR for a patch submitted on March 2020 
> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a 
> thing.
> 
> The patch has been edited to adhere to OpenJDK code conventions about 
> multi-line (block) comments. Nothing in the code proper has changed, except 
> for the addition of redundant but clarifying parentheses in some expressions.
> 
> 
> Greetings
> Raffaello

Hi Jan,

I had to change a string in test
test/jdk/java/lang/String/concat/ImplicitStringConcatBoundaries.java
because it failed with the current string but passes with the new one. Indeed, 
the new implementation of Float.toString(float) produces the new string, which, 
like the current one, is correct in the sense that, upon reading, it recovers 
Float.MIN_NORMAL.

However, I didn't change the definition of MIN_NORMAL in java.lang.Float 
because there it is already expressed in hex notation.

As suggested before and by Joe, using the hex representation instead of the 
decimal would be more robust because the conversions from/to hex are almost 
trivial, hence much less subject to slight errors. So, rather than printing the 
raw bits as you suggest, you could use the hex string rendering instead.


Thanks
Raffaello

-

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

Re: RFR: 8262108: SimpleDateFormat formatting broken for sq_MK Locale [v5]

[Jaikiran Pai]

On Fri, 16 Apr 2021 04:06:54 GMT, Jaikiran Pai  wrote:

>> Can I please get a review for this proposed fix for 
>> https://bugs.openjdk.java.net/browse/JDK-8262108?
>> 
>> As noted in a comment in that issue, the bug relates to the return value of 
>> `Calendar.getDisplayNames` for the `Calendar.AM_PM` field. The 
>> implementation has started returning invalid values for the `AM_PM` field 
>> after the "day period" support was added recently in the JDK as part of 
>> https://bugs.openjdk.java.net/browse/JDK-8262108.
>> 
>> The commit here adds a check in the internal implementation of the display 
>> name handling logic, to special case the `AM_PM` field and properly convert 
>> the display name array indexes (which is an internal detail) to valid values 
>> that represent the `AM_PM` calendar field.
>> 
>> The commit also has a new jtreg test case `CalendarDisplayNamesTest` 
>> reproduces this issue and verifies the fix.
>> 
>> After this fix was introduced, I ran the test in 
>> `test/jdk/java/util/Calendar/` and that showed up a failure in an existing 
>> test case `NarrowNamesTest`. Looking at that test case, IMO, the current 
>> testing in that `NarrowNamesTest` is incorrect and is probably what hid this 
>> issue in the first place? To fix this, I have added an additional commit 
>> which updates this test case to properly test the `AM_PM` field values.
>
> Jaikiran Pai has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update existing test based on review comments

Thank you Naoto for your reviews and guidance in fixing this issue.

-

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

Integrated: 8262108: SimpleDateFormat formatting broken for sq_MK Locale

[Jaikiran Pai]

On Tue, 13 Apr 2021 11:42:41 GMT, Jaikiran Pai  wrote:

> Can I please get a review for this proposed fix for 
> https://bugs.openjdk.java.net/browse/JDK-8262108?
> 
> As noted in a comment in that issue, the bug relates to the return value of 
> `Calendar.getDisplayNames` for the `Calendar.AM_PM` field. The implementation 
> has started returning invalid values for the `AM_PM` field after the "day 
> period" support was added recently in the JDK as part of 
> https://bugs.openjdk.java.net/browse/JDK-8262108.
> 
> The commit here adds a check in the internal implementation of the display 
> name handling logic, to special case the `AM_PM` field and properly convert 
> the display name array indexes (which is an internal detail) to valid values 
> that represent the `AM_PM` calendar field.
> 
> The commit also has a new jtreg test case `CalendarDisplayNamesTest` 
> reproduces this issue and verifies the fix.
> 
> After this fix was introduced, I ran the test in 
> `test/jdk/java/util/Calendar/` and that showed up a failure in an existing 
> test case `NarrowNamesTest`. Looking at that test case, IMO, the current 
> testing in that `NarrowNamesTest` is incorrect and is probably what hid this 
> issue in the first place? To fix this, I have added an additional commit 
> which updates this test case to properly test the `AM_PM` field values.

This pull request has now been integrated.

Changeset: 64e21307
Author:Jaikiran Pai 
URL:   https://git.openjdk.java.net/jdk/commit/64e21307
Stats: 99 lines in 3 files changed: 73 ins; 18 del; 8 mod

8262108: SimpleDateFormat formatting broken for sq_MK Locale

Reviewed-by: naoto

-

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