Re: RFR: 8310232: java.time.Clock$TickClock.millis() fails in runtime when tick is 1 microsecond

2023-06-27 Thread Jaikiran Pai
On Mon, 26 Jun 2023 17:31:29 GMT, Naoto Sato  wrote:

> Fixing the `/ by zero` exception with tick durations less than a millisecond.

Marked as reviewed by jpai (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/14657#pullrequestreview-1500078350


Re: RFR: 8310232: java.time.Clock$TickClock.millis() fails in runtime when tick is 1 microsecond

2023-06-27 Thread Alan Bateman
On Mon, 26 Jun 2023 17:31:29 GMT, Naoto Sato  wrote:

> Fixing the `/ by zero` exception with tick durations less than a millisecond.

test/jdk/java/time/test/java/time/TestClock_Tick.java line 77:

> 75:  * Test tick clock.
> 76:  *
> 77:  * @bug 8310232

Did you mean the put the "@bug" here? If you did then I assume you'll need to 
add the original bug ID too as TestClock_Tick wasn't created for 8310232.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14657#discussion_r1243256353


Re: RFR: 8310843: Reimplement ByteArray and ByteArrayLittleEndian with Unsafe [v8]

2023-06-27 Thread Glavo
> `ByteArray` and `ByteArrayLittleEndian` are very useful tool classes that can 
> be used in many places to performance tuning.
> 
> Currently they are implemented by `VarHandle`, so using them may have some 
> impact on startup time.
> 
> This PR reimplements them using `Unsafe`, which reduces the impact on startup 
> time.

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

 - Merge branch 'openjdk:master' into unsafe
 - Merge branch 'openjdk:master' into unsafe
 - delete incorrect comments
 - delete extraneous whitespace
 - add javadoc
 - delete extraneous whitespace
 - fix test
 - update tests
 - use Preconditions.AIOOBE_FORMATTER
 - delete extraneous whitespace
 - ... and 3 more: https://git.openjdk.org/jdk/compare/9e314a37...609170c0

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14636/files
  - new: https://git.openjdk.org/jdk/pull/14636/files/2b5a4b01..609170c0

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14636&range=07
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14636&range=06-07

  Stats: 6443 lines in 195 files changed: 2377 ins; 2165 del; 1901 mod
  Patch: https://git.openjdk.org/jdk/pull/14636.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14636/head:pull/14636

PR: https://git.openjdk.org/jdk/pull/14636


Re: RFR: 8310901: Convert String::newStringNoRepl with Latin-1 to String::newStringLatin1NoRepl [v4]

2023-06-27 Thread Glavo
> Added a new method `newStringLatin1NoRepl` to the `JavaLangAccess`.
> 
> Reasons:
> 
> * Most use cases of `newStringNoRepl` use `ISO_8859_1` as the charset, 
> creating a new shortcut can make writing shorter;
> * Since all possible values of `byte` are legal Latin-1 characters, 
> `newStringLatin1NoRepl` **will not throw `CharacterCodingException`**, so 
> users can make the compiler happy without using useless try-catch statements.

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

 - Merge branch 'openjdk:master' into latin1-no-repl
 - Merge branch 'openjdk:master' into latin1-no-repl
 - update javadoc
 - clean newStringNoRepl1
 - clean newStringNoRepl1
 - Rename jla to JLA
 - Create new method JavaLangAccess::newStringLatin1NoRepl

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14655/files
  - new: https://git.openjdk.org/jdk/pull/14655/files/9958348b..512029d7

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14655&range=03
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14655&range=02-03

  Stats: 6443 lines in 195 files changed: 2377 ins; 2165 del; 1901 mod
  Patch: https://git.openjdk.org/jdk/pull/14655.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14655/head:pull/14655

PR: https://git.openjdk.org/jdk/pull/14655


Re: RFR: 8301569: list mode of jmod and jimage cannot be used normally in turkish locale [v3]

2023-06-27 Thread Glavo
> When the default Locale is `tr`, the jmod and jimage commands have the 
> following problems:
> 
> * The jmod command does not correctly recognize the `list` mode typed in 
> lowercase;
> * The jimage command cannot obtain the help information of the `list` mode.

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

 - Merge branch 'openjdk:master' into tr-locale
 - Merge branch 'openjdk:master' into tr-locale
 - list mode of jmod and jimage cannot be used normally in turkish locale

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/12281/files
  - new: https://git.openjdk.org/jdk/pull/12281/files/ae8219b6..39102397

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=12281&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=12281&range=01-02

  Stats: 6443 lines in 195 files changed: 2377 ins; 2165 del; 1901 mod
  Patch: https://git.openjdk.org/jdk/pull/12281.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/12281/head:pull/12281

PR: https://git.openjdk.org/jdk/pull/12281


Re: RFR: 8310837: Use ByteArrayLittleEndian in java.util.zip [v3]

2023-06-27 Thread Glavo
> Using `ByteArrayLittleEndian` is simpler and faster.
> 
> `make test TEST="micro:java.util.zip.ZipFileOpen"`:
> 
> 
>   Benchmark (size)  Mode  Cnt  Score  Error  Units
> - ZipFileOpen.openCloseZipFile 512  avgt   15  39052.832 ±  107.496  ns/op
> + ZipFileOpen.openCloseZipFile 512  avgt   15  36275.539 ±  663.193  ns/op
> - ZipFileOpen.openCloseZipFile1024  avgt   15  77106.494 ± 4159.300  ns/op
> + ZipFileOpen.openCloseZipFile1024  avgt   15  71955.013 ± 2296.050  ns/op

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

 - Merge branch 'openjdk:master' into zip-utils
 - Merge branch 'openjdk:master' into zip-utils
 - use ByteArrayLittleEndian in ZipUtils

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14632/files
  - new: https://git.openjdk.org/jdk/pull/14632/files/6ba44e49..c14b9620

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14632&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14632&range=01-02

  Stats: 6443 lines in 195 files changed: 2377 ins; 2165 del; 1901 mod
  Patch: https://git.openjdk.org/jdk/pull/14632.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14632/head:pull/14632

PR: https://git.openjdk.org/jdk/pull/14632


Re: RFR: 8308452: Extend internal Architecture enum with byte order and address size [v7]

2023-06-27 Thread Martin Doerr
On Mon, 26 Jun 2023 14:57:11 GMT, Roger Riggs  wrote:

>> The internal enum jdk.internal.util.Architecture does not provide 
>> information about the big or little endianness or the address size (64 or 32 
>> bits).  The endian-ness and address size are intrinsic to the architecture.
>> 
>> The values of the enum are extended to separately identify the big endian 
>> and little-endian uses of the ISA.
>> For example, `PPC64` and `PPC64LE` for the big and little-endian versions.  
>> The enum values directly reflect the build-time artifacts and resulting 
>> executables.
>> 
>> This information about an architecture will make the enum more useful 
>> especially to identify a target platform in a cross-platform use case. A 
>> method is added to map well known aliases for the platforms to the 
>> Architecture enum.
>
> Roger Riggs has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix typo

Marked as reviewed by mdoerr (Reviewer).

Still good on PPC64.

-

PR Review: https://git.openjdk.org/jdk/pull/14063#pullrequestreview-1500166353
PR Comment: https://git.openjdk.org/jdk/pull/14063#issuecomment-1608971576


[jdk21] RFR: 8310459: [BACKOUT] 8304450: [vectorapi] Refactor VectorShuffle implementation

2023-06-27 Thread Tobias Hartmann
Backport of [JDK-8310459](https://bugs.openjdk.java.net/browse/JDK-8310459). 
Applies cleanly.

Thanks,
Tobias

-

Commit messages:
 - 8310459: [BACKOUT] 8304450: [vectorapi] Refactor VectorShuffle implementation

Changes: https://git.openjdk.org/jdk21/pull/68/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk21&pr=68&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8310459
  Stats: 3895 lines in 64 files changed: 1169 ins; 1819 del; 907 mod
  Patch: https://git.openjdk.org/jdk21/pull/68.diff
  Fetch: git fetch https://git.openjdk.org/jdk21.git pull/68/head:pull/68

PR: https://git.openjdk.org/jdk21/pull/68


Re: RFR: 8301569: list mode of jmod and jimage cannot be used normally in turkish locale [v3]

2023-06-27 Thread Alan Bateman
On Tue, 27 Jun 2023 07:45:51 GMT, Glavo  wrote:

>> When the default Locale is `tr`, the jmod and jimage commands have the 
>> following problems:
>> 
>> * The jmod command does not correctly recognize the `list` mode typed in 
>> lowercase;
>> * The jimage command cannot obtain the help information of the `list` mode.
>
> Glavo has updated the pull request with a new target base due to a merge or a 
> rebase. The incremental webrev excludes the unrelated changes brought in by 
> the merge/rebase. The pull request contains three additional commits since 
> the last revision:
> 
>  - Merge branch 'openjdk:master' into tr-locale
>  - Merge branch 'openjdk:master' into tr-locale
>  - list mode of jmod and jimage cannot be used normally in turkish locale

Did you check the use of toUpperCase(Locale.ENGLISH) in JlinkTask.java and 
JImageTask.java to see if they are correct.

-

PR Comment: https://git.openjdk.org/jdk/pull/12281#issuecomment-1608981430


Re: RFR: 8301569: list mode of jmod and jimage cannot be used normally in turkish locale

2023-06-27 Thread Glavo
On Wed, 7 Jun 2023 06:32:31 GMT, Jaikiran Pai  wrote:

> Sounds fine to me and as other have agreed `Locale.ROOT` would be more 
> appropriate. Would you be willing to update this PR to update the few other 
> occurences in some classes (`JImageTask`, `JlinkTask` and 
> `VersionPropsPlugin`) as noted by Mandy and Alan in their comments?

I think in most cases, there is no difference in the effect between 
`toUpperCase(Locale.ROOT)` and `toUpperCase(Locale.ENGLISH)`.

If you think it is appropriate to replace `Locale.ENGLISH` with `Locale.ROOT` 
in this PR, I can do so. I believe that compared to `Locale.ENGLISH`, the 
`Locale.ROOT is more suitable for use in more places.

-

PR Comment: https://git.openjdk.org/jdk/pull/12281#issuecomment-1608995613


Re: RFR: 8288899: java/util/concurrent/ExecutorService/CloseTest.java failed with "InterruptedException: sleep interrupted"

2023-06-27 Thread Andrey Turbanov
On Sat, 3 Jun 2023 14:08:02 GMT, Doug Lea  wrote:

> Addresses Jdk 8288899 : java/util/concurrent/ExecutorService/CloseTest.java 
> failed with "InterruptedException: sleep interrupted" and related issues.
> 
> This is a major ForkJoin update (and hard to review -- sorry) that finally 
> addresses incompatibilities between ExecutorService and ForkJoinPool (which 
> claims to implement it), with the goal of avoiding continuing bug reports and 
> incompatibilities. Doing this required reworking internal control to use 
> phaser/seqlock-style versioning schemes (affecting nearly every method) that 
> ensure consistent data structures and actions without requiring global 
> synchronization or locking on every task execution that would massively 
> degrade performance. The previous lack of a solution to this was the main 
> reason for these incompatibilities.

src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 2717:

> 2715: this.config = asyncMode ? FIFO : 0;
> 2716: this.keepAlive = Math.max(unit.toMillis(keepAliveTime), 
> TIMEOUT_SLOP);
> 2717: int corep = Math.clamp(corePoolSize, p, MAX_CAP);

it it expected that `int corePoolSize` is now unused?

src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 2795:

> 2793: int n = (qs == null) ? 0 : qs.length;
> 2794: for (int l = n; l > 0; --l, ++r) {
> 2795: int j; WorkQueue q; Thread o; // cancel tasks; 
> interrupt workers

`int j` is unused

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14301#discussion_r1243352906
PR Review Comment: https://git.openjdk.org/jdk/pull/14301#discussion_r1243349508


Re: RFR: 8308452: Extend internal Architecture enum with byte order and address size [v7]

2023-06-27 Thread Amit Kumar
On Mon, 26 Jun 2023 14:57:11 GMT, Roger Riggs  wrote:

>> The internal enum jdk.internal.util.Architecture does not provide 
>> information about the big or little endianness or the address size (64 or 32 
>> bits).  The endian-ness and address size are intrinsic to the architecture.
>> 
>> The values of the enum are extended to separately identify the big endian 
>> and little-endian uses of the ISA.
>> For example, `PPC64` and `PPC64LE` for the big and little-endian versions.  
>> The enum values directly reflect the build-time artifacts and resulting 
>> executables.
>> 
>> This information about an architecture will make the enum more useful 
>> especially to identify a target platform in a cross-platform use case. A 
>> method is added to map well known aliases for the platforms to the 
>> Architecture enum.
>
> Roger Riggs has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix typo

Looks good for s390.

-

Marked as reviewed by amitkumar (Committer).

PR Review: https://git.openjdk.org/jdk/pull/14063#pullrequestreview-1500362213


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v25]

2023-06-27 Thread Chen Liang
On Mon, 26 Jun 2023 09:57:31 GMT, 温绍锦  wrote:

>> By optimizing the implementation of java.lang.Long#fastUUID, the performance 
>> of the java.util.UUID#toString method can be significantly improved.
>> 
>> The following are the test results of JMH: 
>> 
>> Benchmark Mode  Cnt  Score  Error   Units
>> UUIDUtilsBenchmark.new   thrpt5  92676.550 ±  292.213  ops/ms
>> UUIDUtilsBenchmark.original  thrpt5  37040.165 ± 1023.532  ops/ms
>
> 温绍锦 has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   use ISO_8859_1.INSTANCE directly instead of StandardCharsets.ISO_8859_1

src/java.base/share/classes/java/util/UUID.java line 1:

> 1: /*

You should update the copyright year in the license header of UUID.java from 
2022 to 2023.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1243490923


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v25]

2023-06-27 Thread Chen Liang
On Mon, 26 Jun 2023 09:57:31 GMT, 温绍锦  wrote:

>> By optimizing the implementation of java.lang.Long#fastUUID, the performance 
>> of the java.util.UUID#toString method can be significantly improved.
>> 
>> The following are the test results of JMH: 
>> 
>> Benchmark Mode  Cnt  Score  Error   Units
>> UUIDUtilsBenchmark.new   thrpt5  92676.550 ±  292.213  ops/ms
>> UUIDUtilsBenchmark.original  thrpt5  37040.165 ± 1023.532  ops/ms
>
> 温绍锦 has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   use ISO_8859_1.INSTANCE directly instead of StandardCharsets.ISO_8859_1

For your info, the reviews required are approving GitHub reviews.

This patch looks great in shape; currently, I think the startup impact of 
ByteArray. If `UUID.toString` is used in early startup, this patch will 
increase the startup time for VarHandle initialization. #14636 replaces 
VarHandle with direct unsafe usage, which significantly reduces startup cost.

A search online says that `-Xlog:class+init=info:file=trace.log` can be added 
to VM options to print class initialization order. I think we might use it to 
check the startup impact by seeing if certain classes are loaded earlier now. 
Disclaimer: I personally haven't tried this; please correct me immediately if 
there is a better way to measure the startup impact.

-

PR Comment: https://git.openjdk.org/jdk/pull/14578#issuecomment-1609221851


Re: RFR: 8310182: DateTimeFormatter date formats (ISO_LOCAL_DATE) separated with hyphen, not dash [v2]

2023-06-27 Thread Lance Andersen
On Tue, 27 Jun 2023 03:39:30 GMT, Naoto Sato  wrote:

>> Replacing the ambiguous `dash` with `hyphen-minus` for more clarity. There 
>> are other locations than `ISO_LOCAL_DATE` that have the same description. 
>> Those are corrected too.
>
> Naoto Sato has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Reflecting a review comment

Marked as reviewed by lancea (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/14620#pullrequestreview-1500501347


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v25]

2023-06-27 Thread Alan Bateman
On Mon, 26 Jun 2023 09:57:31 GMT, 温绍锦  wrote:

>> By optimizing the implementation of java.lang.Long#fastUUID, the performance 
>> of the java.util.UUID#toString method can be significantly improved.
>> 
>> The following are the test results of JMH: 
>> 
>> Benchmark Mode  Cnt  Score  Error   Units
>> UUIDUtilsBenchmark.new   thrpt5  92676.550 ±  292.213  ops/ms
>> UUIDUtilsBenchmark.original  thrpt5  37040.165 ± 1023.532  ops/ms
>
> 温绍锦 has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   use ISO_8859_1.INSTANCE directly instead of StandardCharsets.ISO_8859_1

I'm looking through [6b61a715], update 24.

Moving fastUUID out of Long is good.

UUID::toString can use StandardChars.ISO_8859_1, no need to use sun.nio.cs code 
here.

The list of imports in java.util.UUID is a bit messy now, can you clean this up.

It's confusing to have java.util.HexDigits and jdk.internal.util.HexDigits. It 
also creates an inconsistency with DecimalDigits and OctalDigits as the latter 
encapsulate their arrays. I assume you've done this to allow sharing of the 256 
element array. I don't object to move the array to a supporting class but it 
needs to be renamed and the class description replaced as it is not a Digits 
implementation. Adding a private constructor would help make it clear that it 
should not be instantiated.

-

PR Comment: https://git.openjdk.org/jdk/pull/14578#issuecomment-1609272188


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v26]

2023-06-27 Thread 温绍锦
> By optimizing the implementation of java.lang.Long#fastUUID, the performance 
> of the java.util.UUID#toString method can be significantly improved.
> 
> The following are the test results of JMH: 
> 
> Benchmark Mode  Cnt  Score  Error   Units
> UUIDUtilsBenchmark.new   thrpt5  92676.550 ±  292.213  ops/ms
> UUIDUtilsBenchmark.original  thrpt5  37040.165 ± 1023.532  ops/ms

温绍锦 has updated the pull request incrementally with one additional commit since 
the last revision:

  fix UUID.java import, rename jdk.util.HexDigits to jdk.util.Hex256 and make 
private constructor.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14578/files
  - new: https://git.openjdk.org/jdk/pull/14578/files/6b61a715..9fe1d705

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14578&range=25
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14578&range=24-25

  Stats: 35 lines in 3 files changed: 4 ins; 5 del; 26 mod
  Patch: https://git.openjdk.org/jdk/pull/14578.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14578/head:pull/14578

PR: https://git.openjdk.org/jdk/pull/14578


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v25]

2023-06-27 Thread 温绍锦
On Tue, 27 Jun 2023 10:08:32 GMT, Chen Liang  wrote:

>> 温绍锦 has updated the pull request incrementally with one additional commit 
>> since the last revision:
>> 
>>   use ISO_8859_1.INSTANCE directly instead of StandardCharsets.ISO_8859_1
>
> src/java.base/share/classes/java/util/UUID.java line 1:
> 
>> 1: /*
> 
> You should update the copyright year in the license header of UUID.java from 
> 2022 to 2023.

do I need to change the copyright information? I am not sure

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1243582378


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v25]

2023-06-27 Thread 温绍锦
On Mon, 26 Jun 2023 09:57:31 GMT, 温绍锦  wrote:

>> By optimizing the implementation of java.lang.Long#fastUUID, the performance 
>> of the java.util.UUID#toString method can be significantly improved.
>> 
>> The following are the test results of JMH: 
>> 
>> Benchmark Mode  Cnt  Score  Error   Units
>> UUIDUtilsBenchmark.new   thrpt5  92676.550 ±  292.213  ops/ms
>> UUIDUtilsBenchmark.original  thrpt5  37040.165 ± 1023.532  ops/ms
>
> 温绍锦 has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   use ISO_8859_1.INSTANCE directly instead of StandardCharsets.ISO_8859_1

> I'm looking through 
> [[6b61a71](https://github.com/openjdk/jdk/commit/6b61a715caeeb586d5480a3a70f90bd1811d1335)],
>  update 24.
> 
> Moving fastUUID out of Long is good.
> 
> UUID::toString can use StandardCharsets. ISO_8859_1, no need to use 
> sun.nio.cs code here.
> 
> The list of imports in java.util.UUID is a bit messy now, can you clean this 
> up.
> 
> It's confusing to have java.util.HexDigits and jdk.internal.util.HexDigits. 
> It also creates an inconsistency with DecimalDigits and OctalDigits as the 
> latter encapsulate their arrays. I assume you've done this to allow sharing 
> of the 256 element array. I don't object to move the array to a supporting 
> class but it needs to be renamed and the class description replaced as it is 
> not a Digits implementation. Adding a private constructor would help make it 
> clear that it should not be instantiated.

very good suggestion, i have modified it according to your suggestion.

-

PR Comment: https://git.openjdk.org/jdk/pull/14578#issuecomment-1609310739


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v25]

2023-06-27 Thread Glavo
On Tue, 27 Jun 2023 11:27:18 GMT, 温绍锦  wrote:

> do I need to change the copyright information? I am not sure

Yes, you should update the copyright information of each file you have modified.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1243607127


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v25]

2023-06-27 Thread Chen Liang
On Tue, 27 Jun 2023 11:50:40 GMT, Glavo  wrote:

>> do I need to change the copyright information? I am not sure
>
>> do I need to change the copyright information? I am not sure
> 
> Yes, you should update the copyright information of each file you have 
> modified.

The years in the header are in the form of "Copyright creationYear, 
lastModifiedYear, xxx". All other files already have their last modified year 
as 2023; only this one doesn't.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1243692902


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v26]

2023-06-27 Thread Chen Liang
On Tue, 27 Jun 2023 11:27:18 GMT, 温绍锦  wrote:

>> By optimizing the implementation of java.lang.Long#fastUUID, the performance 
>> of the java.util.UUID#toString method can be significantly improved.
>> 
>> The following are the test results of JMH: 
>> 
>> Benchmark Mode  Cnt  Score  Error   Units
>> UUIDUtilsBenchmark.new   thrpt5  92676.550 ±  292.213  ops/ms
>> UUIDUtilsBenchmark.original  thrpt5  37040.165 ± 1023.532  ops/ms
>
> 温绍锦 has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   fix UUID.java import, rename jdk.util.HexDigits to jdk.util.Hex256 and make 
> private constructor.

src/java.base/share/classes/java/util/UUID.java line 479:

> 477: ((long) hex256[((int) (msb >> 56)) & 0xff] << 48)
> 478: | ((long) hex256[((int) (msb >> 48)) & 0xff] << 
> 32)
> 479: | ((long) hex256[((int) (msb >> 40)) & 0xff] << 
> 16)

The long cast seems redundant here.

src/java.base/share/classes/jdk/internal/util/Hex256.java line 34:

> 32:  */
> 33: public final class Hex256 {
> 34: private Hex256(){

Suggestion:

private Hex256() {

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1243683233
PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1243688988


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v26]

2023-06-27 Thread Daniel Fuchs
On Tue, 27 Jun 2023 11:27:18 GMT, 温绍锦  wrote:

>> By optimizing the implementation of java.lang.Long#fastUUID, the performance 
>> of the java.util.UUID#toString method can be significantly improved.
>> 
>> The following are the test results of JMH: 
>> 
>> Benchmark Mode  Cnt  Score  Error   Units
>> UUIDUtilsBenchmark.new   thrpt5  92676.550 ±  292.213  ops/ms
>> UUIDUtilsBenchmark.original  thrpt5  37040.165 ± 1023.532  ops/ms
>
> 温绍锦 has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   fix UUID.java import, rename jdk.util.HexDigits to jdk.util.Hex256 and make 
> private constructor.

src/java.base/share/classes/jdk/internal/util/Hex256.java line 38:

> 36: 
> 37: @Stable
> 38: public static final short[] DIGITS;

It's a bit smelly to have a public static field of type array.
Static analysers are likely to flag this.
It would be better to make the field private and have a public static method 
that returns DIGITS.clone() - then each class that needs it (and if I'm not 
mistaken there are only two) could encapsulate its own private copy.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1243711721


Re: RFR: 8294969: Convert jdk.jdeps javap to use the Classfile API [v5]

2023-06-27 Thread Adam Sotona
> javap uses proprietary com.sun.tools.classfile library to parse class files.
> 
> This patch converts javap to use Classfile API.
> 
> Please review.
> 
> Thanks,
> Adam

Adam Sotona has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now contains 220 commits:

 - fixed JavapTask
 - Merge branch 'master' into JDK-8294969-javap
 - Merge branch 'master' into JDK-8294969-javap
   
   # Conflicts:
   #
src/java.base/share/classes/jdk/internal/classfile/constantpool/ConstantPoolException.java
 - fixed build
 - Merge branch 'master' into JDK-8294969-javap
   
   # Conflicts:
   #src/java.base/share/classes/module-info.java
 - more consolidation on IllegalArgumentException
 - Merge branch 'master' into JDK-8294969-javap
 - fixed TestClassNameWarning
 - Merge branch 'master' into JDK-8294969-javap
   
   # Conflicts:
   #src/java.base/share/classes/module-info.java
 - consolidated safeguarding of IAE in javap
 - ... and 210 more: https://git.openjdk.org/jdk/compare/05e9c41e...d5a65c49

-

Changes: https://git.openjdk.org/jdk/pull/11411/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=11411&range=04
  Stats: 3638 lines in 22 files changed: 835 ins; 1716 del; 1087 mod
  Patch: https://git.openjdk.org/jdk/pull/11411.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/11411/head:pull/11411

PR: https://git.openjdk.org/jdk/pull/11411


Re: RFR: 8310182: DateTimeFormatter date formats (ISO_LOCAL_DATE) separated with hyphen, not dash [v2]

2023-06-27 Thread Roger Riggs
On Tue, 27 Jun 2023 03:39:30 GMT, Naoto Sato  wrote:

>> Replacing the ambiguous `dash` with `hyphen-minus` for more clarity. There 
>> are other locations than `ISO_LOCAL_DATE` that have the same description. 
>> Those are corrected too.
>
> Naoto Sato has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Reflecting a review comment

Marked as reviewed by rriggs (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/14620#pullrequestreview-1500879927


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v26]

2023-06-27 Thread Glavo
On Tue, 27 Jun 2023 13:10:40 GMT, Daniel Fuchs  wrote:

> It's a bit smelly to have a public static field of type array. Static 
> analysers are likely to flag this. It would be better to make the field 
> private and have a public static method that returns DIGITS.clone() - then 
> each class that needs it (and if I'm not mistaken there are only two) could 
> encapsulate its own private copy.

I don't think this is a problem. If users can access JDK internal packages, it 
can do many things. For example, through 
`JavaLangAccess::getEnumConstantsShared`, an array shared within JDK can be 
obtained.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1243750135


Re: RFR: 8310914: Remove 2 malformed java/foreign ProblemList entries

2023-06-27 Thread Jorn Vernee
On Tue, 27 Jun 2023 06:22:06 GMT, Jaikiran Pai  wrote:

> > The `java/foreign/callarranger/TestAarch64CallArranger.java` test was 
> > removed,
> 
> It looks like this test is still present in mainline 
> https://github.com/openjdk/jdk/blob/master/test/jdk/java/foreign/callarranger/TestLinuxAArch64CallArranger.java

Note that that's a different test. The original was split into multiple 
different files, one per platform (hence "removed").

-

PR Comment: https://git.openjdk.org/jdk/pull/14664#issuecomment-1609514584


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v26]

2023-06-27 Thread Chen Liang
On Tue, 27 Jun 2023 13:33:09 GMT, Glavo  wrote:

>> src/java.base/share/classes/jdk/internal/util/Hex256.java line 38:
>> 
>>> 36: 
>>> 37: @Stable
>>> 38: public static final short[] DIGITS;
>> 
>> It's a bit smelly to have a public static field of type array.
>> Static analysers are likely to flag this.
>> It would be better to make the field private and have a public static method 
>> that returns DIGITS.clone() - then each class that needs it (and if I'm not 
>> mistaken there are only two) could encapsulate its own private copy.
>
>> It's a bit smelly to have a public static field of type array. Static 
>> analysers are likely to flag this. It would be better to make the field 
>> private and have a public static method that returns DIGITS.clone() - then 
>> each class that needs it (and if I'm not mistaken there are only two) could 
>> encapsulate its own private copy.
> 
> I don't think this is a problem. If users can access JDK internal packages, 
> it can do many things. For example, through 
> `JavaLangAccess::getEnumConstantsShared`, an array shared within JDK can be 
> obtained.
> 
> Now that we trust the permission control of JPMS, such a requirement seems 
> somewhat rigid.

This array takes some time to prepare. We don't want each class to copy a large 
array when we can just prepare it once and use it everywhere within the JDK 
implementation. This array is, under no circumstances, leaked to users so it's 
safe.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1243758911


Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work [v4]

2023-06-27 Thread Matthias Baesken
> Currently, a number of tests fail on macOS because they miss the core file 
> (e.g. serviceability/sa/TestJmapCore.java).
> The reason is that configure detects on some setups that codesign does not 
> work ("checking if debug mode codesign is possible... no) .
> So adding the needed entitlement to generate cores is not done in the build. 
> This is currently not checked later in the tests.
> But without the entitlement, a core is not generated.

Matthias Baesken has updated the pull request incrementally with one additional 
commit since the last revision:

  small adjustments

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14562/files
  - new: https://git.openjdk.org/jdk/pull/14562/files/619b3578..09eb93c6

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14562&range=03
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14562&range=02-03

  Stats: 4 lines in 3 files changed: 0 ins; 0 del; 4 mod
  Patch: https://git.openjdk.org/jdk/pull/14562.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14562/head:pull/14562

PR: https://git.openjdk.org/jdk/pull/14562


Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work [v3]

2023-06-27 Thread Matthias Baesken
On Thu, 22 Jun 2023 11:51:15 GMT, Matthias Baesken  wrote:

>> Currently, a number of tests fail on macOS because they miss the core file 
>> (e.g. serviceability/sa/TestJmapCore.java).
>> The reason is that configure detects on some setups that codesign does not 
>> work ("checking if debug mode codesign is possible... no) .
>> So adding the needed entitlement to generate cores is not done in the build. 
>> This is currently not checked later in the tests.
>> But without the entitlement, a core is not generated.
>
> Matthias Baesken has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   move javaPath into caller

Hi Chris,  I renamed the method and adjusted the comment.

-

PR Comment: https://git.openjdk.org/jdk/pull/14562#issuecomment-1609529567


Re: RFR: 8291065: Creating a VarHandle for a static field triggers class initialization [v13]

2023-06-27 Thread Chen Liang
> Also fixed the bug with NPE in `IndirectVarHandle::isAccessModeSupported`.
> 
> A few implementation-detail methods in VarHandle are now documented with the 
> implied constraints to avoid subtle problems in the future. Changed 
> `IndirectVarHandle` to call `asDirect` lazily to accomodate the lazy 
> VarHandle changes. Also changed VarHandleBaseTest to report the whole 
> incorrect type of exception caught than swallow it and leaving only a message.
> 
> Current problems:
> - [ ] The lazy var handle is quite slow on the first invocation.
>- As seen in the benchmark, users can first call 
> `Lookup::ensureInitialized` to create an eager handle.
>- After that, the lazy handle has a performance on par with the regular 
> var handle.
> - [ ] The class-loading-based test is not in a unit test
>- The test frameworks don't seem to offer fine-grained control for 
> class-loading detection or reliable unloading
> 
> 
> BenchmarkMode  Cnt  Score   Error 
>  Units
> VarHandleLazyStaticInvocation.initializedInvocation  avgt   30  0.817 ± 0.012 
>  ns/op
> VarHandleLazyStaticInvocation.lazyInvocation avgt   30  0.805 ± 0.007 
>  ns/op
> 
> 
> BenchmarkMode  Cnt Score 
> Error  Units
> BenchmarkMode  Cnt   Score
> Error  Units
> LazyStaticColdStart.methodHandleCreateEagerss   10  36.890 ±  
> 2.891  us/op
> LazyStaticColdStart.methodHandleCreateLazy ss   10  18.340 ±  
> 1.537  us/op
> LazyStaticColdStart.methodHandleInitializeCallEagerss   10  50.000 ±  
> 5.590  us/op
> LazyStaticColdStart.methodHandleInitializeCallLazy ss   10  90.550 ± 
> 10.142  us/op
> LazyStaticColdStart.varHandleCreateEager   ss   10  36.610 ±  
> 2.685  us/op
> LazyStaticColdStart.varHandleCreateLazyss   10  18.200 ±  
> 1.811  us/op
> LazyStaticColdStart.varHandleInitializeCallEager   ss   10  71.680 ± 
> 11.097  us/op
> LazyStaticColdStart.varHandleInitializeCallLazyss   10  72.090 ±  
> 4.494  us/op

Chen Liang has updated the pull request incrementally with one additional 
commit since the last revision:

  Split the concepts of asDirect and target

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/13821/files
  - new: https://git.openjdk.org/jdk/pull/13821/files/0ac094b8..99960727

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=13821&range=12
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=13821&range=11-12

  Stats: 214 lines in 8 files changed: 25 ins; 56 del; 133 mod
  Patch: https://git.openjdk.org/jdk/pull/13821.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/13821/head:pull/13821

PR: https://git.openjdk.org/jdk/pull/13821


Re: RFR: 8291065: Creating a VarHandle for a static field triggers class initialization [v14]

2023-06-27 Thread Chen Liang
> Also fixed the bug with NPE in `IndirectVarHandle::isAccessModeSupported`.
> 
> A few implementation-detail methods in VarHandle are now documented with the 
> implied constraints to avoid subtle problems in the future. Changed 
> `IndirectVarHandle` to call `asDirect` lazily to accomodate the lazy 
> VarHandle changes. Also changed VarHandleBaseTest to report the whole 
> incorrect type of exception caught than swallow it and leaving only a message.
> 
> Current problems:
> - [ ] The lazy var handle is quite slow on the first invocation.
>- As seen in the benchmark, users can first call 
> `Lookup::ensureInitialized` to create an eager handle.
>- After that, the lazy handle has a performance on par with the regular 
> var handle.
> - [ ] The class-loading-based test is not in a unit test
>- The test frameworks don't seem to offer fine-grained control for 
> class-loading detection or reliable unloading
> 
> 
> BenchmarkMode  Cnt  Score   Error 
>  Units
> VarHandleLazyStaticInvocation.initializedInvocation  avgt   30  0.817 ± 0.012 
>  ns/op
> VarHandleLazyStaticInvocation.lazyInvocation avgt   30  0.805 ± 0.007 
>  ns/op
> 
> 
> BenchmarkMode  Cnt Score 
> Error  Units
> BenchmarkMode  Cnt   Score
> Error  Units
> LazyStaticColdStart.methodHandleCreateEagerss   10  36.890 ±  
> 2.891  us/op
> LazyStaticColdStart.methodHandleCreateLazy ss   10  18.340 ±  
> 1.537  us/op
> LazyStaticColdStart.methodHandleInitializeCallEagerss   10  50.000 ±  
> 5.590  us/op
> LazyStaticColdStart.methodHandleInitializeCallLazy ss   10  90.550 ± 
> 10.142  us/op
> LazyStaticColdStart.varHandleCreateEager   ss   10  36.610 ±  
> 2.685  us/op
> LazyStaticColdStart.varHandleCreateLazyss   10  18.200 ±  
> 1.811  us/op
> LazyStaticColdStart.varHandleInitializeCallEager   ss   10  71.680 ± 
> 11.097  us/op
> LazyStaticColdStart.varHandleInitializeCallLazyss   10  72.090 ±  
> 4.494  us/op

Chen Liang has updated the pull request incrementally with one additional 
commit since the last revision:

  Rollback VHG changes

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/13821/files
  - new: https://git.openjdk.org/jdk/pull/13821/files/99960727..8cbb2083

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=13821&range=13
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=13821&range=12-13

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

PR: https://git.openjdk.org/jdk/pull/13821


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v26]

2023-06-27 Thread Daniel Fuchs
On Tue, 27 Jun 2023 13:38:11 GMT, Chen Liang  wrote:

>>> It's a bit smelly to have a public static field of type array. Static 
>>> analysers are likely to flag this. It would be better to make the field 
>>> private and have a public static method that returns DIGITS.clone() - then 
>>> each class that needs it (and if I'm not mistaken there are only two) could 
>>> encapsulate its own private copy.
>> 
>> I don't think this is a problem. If users can access JDK internal packages, 
>> they can do many things. For example, through 
>> `JavaLangAccess::getEnumConstantsShared`, an array shared within JDK can be 
>> obtained.
>> 
>> Now that we trust the permission control of JPMS, such a requirement seems 
>> somewhat rigid.
>
> This array takes some time to prepare. We don't want each class to copy a 
> large array when we can just prepare it once and use it everywhere within the 
> JDK implementation. This array is, under no circumstances, leaked to users so 
> it's safe.

What I suggest is to prepare the array only once (in the static block as it is 
now), but have each class that use it encapsulate is own copy - obtained from 
clone(). Surely 256 shorts is not so large that we can't have two arrays?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1243804755


Re: RFR: 8291065: Creating a VarHandle for a static field triggers class initialization [v11]

2023-06-27 Thread Chen Liang
On Fri, 9 Jun 2023 18:59:41 GMT, Paul Sandoz  wrote:

>> Chen Liang has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Remove meaningless target calls and clear outdated cache as needed
>
> Something was bothering me about the current complexity with method handles 
> and the potential interactions with indirect var handles. Its hard to reason 
> about all the interactions so i needed to take a deeper dive, which i really 
> should have done earlier on. Apologies if we are circling around this many 
> times.
> 
> Since we are now leaning on `LazyInitializingVarHandle::target` to initialize 
> I think we can just do this:
> 
> private void initialize() {
> UNSAFE.ensureClassInitialized(refc);
> this.initialized = true;
> }
> 
> @Override
> public MethodHandle toMethodHandle(AccessMode accessMode) {
> if (initialized) {
> return target.toMethodHandle(accessMode);
> } else {
> if (isAccessModeSupported(accessMode)) {
> MethodHandle mh = 
> target.getMethodHandle(accessMode.ordinal());
> // Ensure target on this VH is called to initialize the class
> return mh.bindTo(this);
> }
> else {
> // Ensure an UnsupportedOperationException is thrown
> return MethodHandles.varHandleInvoker(accessMode, 
> accessModeType(accessMode)).
> bindTo(this);
> }
> }
> }
> 
> 
> However, construction of an indirect VH will result in initialization:
> 
> private IndirectVarHandle(VarHandle target, Class value, Class[] 
> coordinates,
>   BiFunction 
> handleFactory, VarForm form, boolean exact) {
> super(form, exact);
> this.handleFactory = handleFactory;
> this.target = target;
> this.directTarget = target.target();
> this.value = value;
> this.coordinates = coordinates;
> }
> 
> 
> Since this is not performance sensitive code we could check if target is an 
> instance of `LazyInitializingVarHandle` then conditionally get it's target if 
> initialized e.g.,
> 
>this.directTarget = target instanceof LazyInitializingVarHandle lazyTarget 
> ? lazyTarget.lazyTarget() : target.target();
> 
> ?

@PaulSandoz I think this patch as of now is what I want:
1. Lazy VH works both when obtained directly or delegated by an Indirect VH
2. In the most common usage scenario (direct calls on VH), the lazy VH has a 
good initial performance and a similar long-term performance compared to the 
eager VH
3. The invasive changes to existing structures are minimized.

For the performance concerns of `getMethodHandle` and `toMethodHandle` (mainly 
creation and initial invocation), I find that the VH invocation semantics 
itself has some problems: for example, it's quite pointless to pass a 
VH-specific direct VH to a VH-specific `getMethodHandle`. My focus on this 
patch will be to ensure the correctness of MH-related content, but caring about 
their performance is out of scope for me.

-

PR Comment: https://git.openjdk.org/jdk/pull/13821#issuecomment-1609577849


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v26]

2023-06-27 Thread Glavo
On Tue, 27 Jun 2023 14:02:52 GMT, Daniel Fuchs  wrote:

> What I suggest is to prepare the array only once (in the static block as it 
> is now), but have each class that use it encapsulate is own copy - obtained 
> from clone(). Surely 256 shorts is not so large that we can't have two arrays?

The point is that it doesn't make any sense, so I don't feel like we should pay 
anything for it.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1243812535


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v26]

2023-06-27 Thread Chen Liang
On Tue, 27 Jun 2023 14:11:41 GMT, Alan Bateman  wrote:

> What I suggest is to prepare the array only once (in the static block as it 
> is now), but have each class that use it encapsulate is own copy - obtained 
> from clone(). Surely 256 shorts is not so large that we can't have two arrays?

I really wish we have frozen arrays, which are safely sharable like records. 
The closest we have right now is a final array with `@Stable`, which indicates 
array elements are lazy (any non-default value read can be treated as a 
constant by JIT); since this array is explicitly `@Stable final`, we can share 
it within the JDK like any other immutable objects.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1243822811


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v26]

2023-06-27 Thread Alan Bateman
On Tue, 27 Jun 2023 14:07:43 GMT, Glavo  wrote:

> > What I suggest is to prepare the array only once (in the static block as it 
> > is now), but have each class that use it encapsulate is own copy - obtained 
> > from clone(). Surely 256 shorts is not so large that we can't have two 
> > arrays?
> 
> The point is that it doesn't make any sense, so I don't feel like we should 
> pay anything for it.

Daniel makes a good point. Sadly, jdk.internal.util is exported to other 
modules so it does need to be looked at from an integrity perspective.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1243819226


Re: RFR: 8310848: Convert ClassDesc and MethodTypeDesc to be stored in static final fields [v4]

2023-06-27 Thread Chen Liang
> This would encourage Classfile API users to use the descriptors as constants, 
> which can improve performance by avoiding repeated validation and reusing 
> cached descriptor strings for MethodTypeDesc. This patch updates usages in 
> the main codebase and benchmarks; tests are left untouched.

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

 - Merge branch 'master' into fix/cd-usage
 - Merge branch 'master' into fix/cd-usage
 - Fix build
 - 8310848: Convert ClassDesc and MethodTypeDesc to be stored in static final 
fields

-

Changes: https://git.openjdk.org/jdk/pull/14640/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14640&range=03
  Stats: 241 lines in 6 files changed: 61 ins; 94 del; 86 mod
  Patch: https://git.openjdk.org/jdk/pull/14640.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14640/head:pull/14640

PR: https://git.openjdk.org/jdk/pull/14640


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v26]

2023-06-27 Thread Roger Riggs
On Tue, 27 Jun 2023 11:27:18 GMT, 温绍锦  wrote:

>> By optimizing the implementation of java.lang.Long#fastUUID, the performance 
>> of the java.util.UUID#toString method can be significantly improved.
>> 
>> The following are the test results of JMH: 
>> 
>> Benchmark Mode  Cnt  Score  Error   Units
>> UUIDUtilsBenchmark.new   thrpt5  92676.550 ±  292.213  ops/ms
>> UUIDUtilsBenchmark.original  thrpt5  37040.165 ± 1023.532  ops/ms
>
> 温绍锦 has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   fix UUID.java import, rename jdk.util.HexDigits to jdk.util.Hex256 and make 
> private constructor.

in general, do not rush to integrate, given the number and variety of comments 
and commenters, it is good practice to wait 24 hours after the last comment to 
see that comments have stabilized. 
Given the global scope of OpenJDK, reviewers are in most/every timezone.
And formally, every PR must have the required number of approvals from official 
Reviewers.
We're not there yet.

src/java.base/share/classes/jdk/internal/util/Hex256.java line 31:

> 29: 
> 30: /**
> 31:  * Provides a hexadecimal cache array of values from 0 to 255

A bit more information about the contents and use would be useful for future 
developers and perhaps an example of the use so they don't have to go find 
where it is used and decipher the code.
Each element contains the ascii encoded hex characters for the index. It may be 
obvious which character is the low nibble for the low byte but it might be good 
to say so.

src/java.base/share/classes/jdk/internal/util/Hex256.java line 38:

> 36: 
> 37: @Stable
> 38: public static final short[] DIGITS;

If UUID shows up in startup class loading this array would be a candidate for 
loading by CDS, as mentioned in earlier review comments. Please create a 
separate issue to evaluate and add the code to load the array using CDS.

-

PR Comment: https://git.openjdk.org/jdk/pull/14578#issuecomment-1609604257
PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1243790783
PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1243817685


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v26]

2023-06-27 Thread Chen Liang
On Tue, 27 Jun 2023 14:13:48 GMT, Chen Liang  wrote:

>>> > What I suggest is to prepare the array only once (in the static block as 
>>> > it is now), but have each class that use it encapsulate is own copy - 
>>> > obtained from clone(). Surely 256 shorts is not so large that we can't 
>>> > have two arrays?
>>> 
>>> The point is that it doesn't make any sense, so I don't feel like we should 
>>> pay anything for it.
>> 
>> Daniel makes a good point. Sadly, jdk.internal.util is exported to other 
>> modules so it does need to be looked at from an integrity perspective.
>
>> What I suggest is to prepare the array only once (in the static block as it 
>> is now), but have each class that use it encapsulate is own copy - obtained 
>> from clone(). Surely 256 shorts is not so large that we can't have two 
>> arrays?
> 
> I really wish we have frozen arrays, which are safely sharable like records. 
> The closest we have right now is a final array with `@Stable`, which 
> indicates array elements are lazy (any non-default value read can be treated 
> as a constant by JIT); since this array is explicitly `@Stable final`, we can 
> share it within the JDK like any other immutable objects.

On second thought, we should be able to create a getter like

@ForceInline
static short hex256(int byteValue) {
return HEX256[byteValue];
}

and switch usages to that getter instead. This is a better approach than 
cloning the array and makes the array safe from modification by modules that 
get jdk.internal.util exports.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1243840048


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v26]

2023-06-27 Thread Glavo
On Tue, 27 Jun 2023 14:23:53 GMT, Chen Liang  wrote:

>>> What I suggest is to prepare the array only once (in the static block as it 
>>> is now), but have each class that use it encapsulate is own copy - obtained 
>>> from clone(). Surely 256 shorts is not so large that we can't have two 
>>> arrays?
>> 
>> I really wish we have frozen arrays, which are safely sharable like records. 
>> The closest we have right now is a final array with `@Stable`, which 
>> indicates array elements are lazy (any non-default value read can be treated 
>> as a constant by JIT); since this array is explicitly `@Stable final`, we 
>> can share it within the JDK like any other immutable objects.
>
> On second thought, we should be able to create a getter like
> 
> @ForceInline
> static short hex256(int byteValue) {
> return HEX256[byteValue];
> }
> 
> and switch usages to that getter instead. This is a better approach than 
> cloning the array and makes the array safe from modification by modules that 
> get jdk.internal.util exports.

> Sadly, jdk.internal.util is exported to other modules so it does need to be 
> looked at from an integrity perspective.

This is indeed a problem to consider.

Maybe we can move this array back into `HexDigits`. Then we create a new 
package `jdk.internal.digits` and move `Digits`, `DecimalDigits`, `HexDigits`, 
`OctalDigits` all into it.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1243854585


List extending Collection/SequencedCollection

2023-06-27 Thread Ryan Ernst
Hi core-libs-dev,

I know various threads have existed over the past few months on
SequencedCollection and its implications on compatibility. I wanted to
raise this semi-related issue we noticed recently after beginning
testing against Java 21.

Elasticsearch began testing against Java 21, and noticed a series of
failures in Painless (our builtin scripting language, which
dynamically compiles to bytecode). Most tests that used List failed to
compile, with several unknown methods (eg add). Painless builds a
hierarchy of classes it knows about for method resolution. This
hierarchy didn't know anything about SequencedCollection since we
currently compile against Java 17. Now, this was ultimately a bug in
Painless, because we knew there could be cases where we encounter
unknown classes in the hierarchy (eg a class which is not blessed to
be visible in the language). However, I think the reason we found that
bug (List extending from SequencedCollection) might still cause issues
for some.

While debugging the issue, something that struck me as interesting in
the SequencedCollection hierarchy is the difference between List and
SortedSet. Both of these classes were made to be compatible with
sequenced classes by adding the new classes as super interfaces,
SequencedCollection and SequencedSet, respectively. However, while
SortedSet was *added* as a super interface, List had its super
interface Collection *replaced* by SequencedCollection.

I don't know enough about the rampdown process to know if this is
still fixable in Java 21. It is probably an extreme edge case that
doesn't matter, since eg instanceof Collection will still work in
existing code. But for consistency, it would seem better to maintain
Collection as a direct super interface of List. Is there any reason
such a change doesn't fit with the collections architecture going
forward?


Re: RFR: 8309819: Clarify API note in Class::getName and MethodType::toMethodDescriptorString [v9]

2023-06-27 Thread Chen Liang
> The API specification for descriptorString not being a strict inverse of 
> Class::forName and MethodType::fromDescriptorString are not entirely correct.
> 
> 1. Class::descriptorString was never an inverse of Class::forName, which 
> takes a binary name instead. The note about different class loaders is moved 
> to getName, as ClassDesc requires an explicit lookup for resolution already.
> 2. MethodType::toMethodDescriptorString ends with a meaningless sentence: 
> "fromMethodDescriptorString, because the latter requires a suitable class 
> loader argument.", and the "Note:" section can be replaced with an `@apiNote`.
> 3. Both of these didn't mention hidden classes (or other 
> non-nominally-describable classes) as a reason that prevents the inversion 
> operation, in addition to distinct class loaders. Added valid method type 
> descriptor/binary name as a prerequisite for the distinct class loader 
> explanation.
> 
> A few user-defined anchor links are replaced with updated javadoc link tag 
> format as well. The explicit html-style links in `@see` tags are unchanged in 
> order to retain the non-code output.
> 
> The rendered specifications:
> https://cr.openjdk.org/~liach/8309819/06/java.base/java/lang/Class.html
> https://cr.openjdk.org/~liach/8309819/06/java.base/java/lang/invoke/MethodType.html

Chen Liang has updated the pull request incrementally with one additional 
commit since the last revision:

  produce identical component descriptors in descriptor strings -> cannot be 
distinguished in descriptor strings

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14411/files
  - new: https://git.openjdk.org/jdk/pull/14411/files/96b142e0..3bea7506

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14411&range=08
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14411&range=07-08

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

PR: https://git.openjdk.org/jdk/pull/14411


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v27]

2023-06-27 Thread 温绍锦
> By optimizing the implementation of java.lang.Long#fastUUID, the performance 
> of the java.util.UUID#toString method can be significantly improved.
> 
> The following are the test results of JMH: 
> 
> Benchmark Mode  Cnt  Score  Error   Units
> UUIDUtilsBenchmark.new   thrpt5  92676.550 ±  292.213  ops/ms
> UUIDUtilsBenchmark.original  thrpt5  37040.165 ± 1023.532  ops/ms

温绍锦 has updated the pull request incrementally with one additional commit since 
the last revision:

  Update src/java.base/share/classes/jdk/internal/util/Hex256.java
  
  Co-authored-by: liach <7806504+li...@users.noreply.github.com>

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14578/files
  - new: https://git.openjdk.org/jdk/pull/14578/files/9fe1d705..fe9efa6b

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14578&range=26
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14578&range=25-26

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

PR: https://git.openjdk.org/jdk/pull/14578


Re: RFR: 8281658: Add a security category to the java -XshowSettings option [v8]

2023-06-27 Thread Sean Coffey
> New functionality in the -XshowSettings menu to display relevant information 
> about JDK security configuration

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

 - Avoid sharing INDENT variables
 - Merge branch 'master' into 8281658-showsettings-security
 - Don't allow bad subcommand values for security component
 - restore more informative help message
 - Split long properties for ; also
 - Pass PrintStream to security helper
 - Refactor out security code to helper class
 - Print aliases. Order Provider type/service output.
 - Incorporate review comments from Roger and tweak some code
 - Merge branch 'master' into 8281658-showsettings-security
 - ... and 5 more: https://git.openjdk.org/jdk/compare/698e2b5c...7e6f5090

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14394/files
  - new: https://git.openjdk.org/jdk/pull/14394/files/77c9f5e2..7e6f5090

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14394&range=07
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14394&range=06-07

  Stats: 31805 lines in 1150 files changed: 12787 ins; 12829 del; 6189 mod
  Patch: https://git.openjdk.org/jdk/pull/14394.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14394/head:pull/14394

PR: https://git.openjdk.org/jdk/pull/14394


Re: RFR: 8301569: list mode of jmod and jimage cannot be used normally in turkish locale [v4]

2023-06-27 Thread Glavo
> When the default Locale is `tr`, the jmod and jimage commands have the 
> following problems:
> 
> * The jmod command does not correctly recognize the `list` mode typed in 
> lowercase;
> * The jimage command cannot obtain the help information of the `list` mode.

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

  update JImageTask, JlinkTask and VersionPropsPlugin

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/12281/files
  - new: https://git.openjdk.org/jdk/pull/12281/files/39102397..3eb4407b

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=12281&range=03
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=12281&range=02-03

  Stats: 5 lines in 3 files changed: 1 ins; 0 del; 4 mod
  Patch: https://git.openjdk.org/jdk/pull/12281.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/12281/head:pull/12281

PR: https://git.openjdk.org/jdk/pull/12281


Re: RFR: 8301569: list mode of jmod and jimage cannot be used normally in turkish locale [v3]

2023-06-27 Thread Glavo
On Tue, 27 Jun 2023 07:45:51 GMT, Glavo  wrote:

>> When the default Locale is `tr`, the jmod and jimage commands have the 
>> following problems:
>> 
>> * The jmod command does not correctly recognize the `list` mode typed in 
>> lowercase;
>> * The jimage command cannot obtain the help information of the `list` mode.
>
> Glavo has updated the pull request with a new target base due to a merge or a 
> rebase. The incremental webrev excludes the unrelated changes brought in by 
> the merge/rebase. The pull request contains three additional commits since 
> the last revision:
> 
>  - Merge branch 'openjdk:master' into tr-locale
>  - Merge branch 'openjdk:master' into tr-locale
>  - list mode of jmod and jimage cannot be used normally in turkish locale

I have updated `JImageTask`, `JlinkTask`, and `VersionPropsPlugin`.

-

PR Comment: https://git.openjdk.org/jdk/pull/12281#issuecomment-1609718114


Integrated: 8222329: Readable read(CharBuffer) does not specify that 0 is returned when there is no remaining space in buffer

2023-06-27 Thread Brian Burkhalter
On Thu, 22 Jun 2023 18:22:35 GMT, Brian Burkhalter  wrote:

> Clarify the behavior of `java.lang.Readable` when the specified 
> `java.nio.CharBuffer` parameter is empty but read-only, and when it is full.

This pull request has now been integrated.

Changeset: 58bb6555
Author:Brian Burkhalter 
URL:   
https://git.openjdk.org/jdk/commit/58bb6555e783e4627f57c3c8281183c474d581c9
Stats: 23 lines in 3 files changed: 11 ins; 0 del; 12 mod

8222329: Readable read(CharBuffer) does not specify that 0 is returned when 
there is no remaining space in buffer

Reviewed-by: rriggs, lancea, alanb

-

PR: https://git.openjdk.org/jdk/pull/14616


RFR: 8310892: ScopedValue throwing StructureViolationException should be clearer

2023-06-27 Thread Alan Bateman
Docs only update to add a missing `@throws StructureViolationException` and 
make it clearer when the exception is thrown. In the future we might re-visit 
this so that the description is in one place rather than in each method.

-

Commit messages:
 - Initial commit

Changes: https://git.openjdk.org/jdk/pull/14679/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14679&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8310892
  Stats: 60 lines in 1 file changed: 12 ins; 6 del; 42 mod
  Patch: https://git.openjdk.org/jdk/pull/14679.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14679/head:pull/14679

PR: https://git.openjdk.org/jdk/pull/14679


Integrated: 8310890: Normalize identifier names

2023-06-27 Thread Pavel Rappo
On Mon, 26 Jun 2023 14:07:03 GMT, Pavel Rappo  wrote:

> Please review this cleanup PR to normalize names of identifiers which are 
> Java variables/fields or tokens in text files. Those names either contain a 
> pronoun that is very rarely used in code, or seem like they contain such a 
> pronoun, which, in fact, they don't. Either way, the goal is to improve 
> readability and clarity.
> 
> Also, this PR fixes a few related typos.

This pull request has now been integrated.

Changeset: f6133edb
Author:Pavel Rappo 
URL:   
https://git.openjdk.org/jdk/commit/f6133edb08dd7a7d764638c5b1cdd5c3e56ed64e
Stats: 184 lines in 10 files changed: 0 ins; 0 del; 184 mod

8310890: Normalize identifier names

Reviewed-by: naoto, rriggs

-

PR: https://git.openjdk.org/jdk/pull/14653


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v28]

2023-06-27 Thread 温绍锦
> By optimizing the implementation of java.lang.Long#fastUUID, the performance 
> of the java.util.UUID#toString method can be significantly improved.
> 
> The following are the test results of JMH: 
> 
> Benchmark Mode  Cnt  Score  Error   Units
> UUIDUtilsBenchmark.new   thrpt5  92676.550 ±  292.213  ops/ms
> UUIDUtilsBenchmark.original  thrpt5  37040.165 ± 1023.532  ops/ms

温绍锦 has updated the pull request incrementally with one additional commit since 
the last revision:

  change copyright years, and rename jdk.internal.util.Hex256 to 
jdk.internal.digits.Hex256, and add comments.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14578/files
  - new: https://git.openjdk.org/jdk/pull/14578/files/fe9efa6b..b81f12d1

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14578&range=27
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14578&range=26-27

  Stats: 154 lines in 4 files changed: 98 ins; 53 del; 3 mod
  Patch: https://git.openjdk.org/jdk/pull/14578.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14578/head:pull/14578

PR: https://git.openjdk.org/jdk/pull/14578


Integrated: 8310849: Pattern matching for instanceof and arrayType cleanup in j.l.invoke and j.l.reflect

2023-06-27 Thread Chen Liang
On Sun, 25 Jun 2023 09:18:34 GMT, Chen Liang  wrote:

> This patch touches java.lang.reflect and java.lang.invoke packages. It 
> replaces instanceof + cast with pattern matching and updates  
> Array.newInstance().getClass() patterns with arrayType() for obtaining array 
> types of a class.

This pull request has now been integrated.

Changeset: 2bd4136b
Author:Chen Liang 
Committer: Mandy Chung 
URL:   
https://git.openjdk.org/jdk/commit/2bd4136bdb74599e358a22c83ffc685a2c0db4d2
Stats: 292 lines in 34 files changed: 0 ins; 81 del; 211 mod

8310849: Pattern matching for instanceof and arrayType cleanup in j.l.invoke 
and j.l.reflect

Reviewed-by: mchung, darcy

-

PR: https://git.openjdk.org/jdk/pull/14642


Re: RFR: 8310232: java.time.Clock$TickClock.millis() fails in runtime when tick is 1 microsecond

2023-06-27 Thread Naoto Sato
On Tue, 27 Jun 2023 07:23:21 GMT, Alan Bateman  wrote:

>> Fixing the `/ by zero` exception with tick durations less than a millisecond.
>
> test/jdk/java/time/test/java/time/TestClock_Tick.java line 77:
> 
>> 75:  * Test tick clock.
>> 76:  *
>> 77:  * @bug 8310232
> 
> Did you mean the put the "@bug" here? If you did then I assume you'll need to 
> add the original bug ID too as TestClock_Tick wasn't created for 8310232.

I meant to put the bug id here, but you are right that it would confuse readers 
as if it is the bug that created this regression test. Will remove it and leave 
the bug id only for the added test.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14657#discussion_r1244011818


Integrated: 8310182: DateTimeFormatter date formats (ISO_LOCAL_DATE) separated with hyphen, not dash

2023-06-27 Thread Naoto Sato
On Thu, 22 Jun 2023 20:36:48 GMT, Naoto Sato  wrote:

> Replacing the ambiguous `dash` with `hyphen-minus` for more clarity. There 
> are other locations than `ISO_LOCAL_DATE` that have the same description. 
> Those are corrected too.

This pull request has now been integrated.

Changeset: ec45bd64
Author:Naoto Sato 
URL:   
https://git.openjdk.org/jdk/commit/ec45bd64d504d579aef54c924fb8ca75a944036f
Stats: 6 lines in 1 file changed: 0 ins; 0 del; 6 mod

8310182: DateTimeFormatter date formats (ISO_LOCAL_DATE) separated with hyphen, 
not dash

Reviewed-by: rriggs, darcy, iris, lancea

-

PR: https://git.openjdk.org/jdk/pull/14620


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v26]

2023-06-27 Thread 温绍锦
On Tue, 27 Jun 2023 14:32:14 GMT, Glavo  wrote:

>> On second thought, we should be able to create a getter like
>> 
>> @ForceInline
>> static short hex256(int byteValue) {
>> return HEX256[byteValue];
>> }
>> 
>> and switch usages to that getter instead. This is a better approach than 
>> cloning the array and makes the array safe from modification by modules that 
>> get jdk.internal.util exports.
>
>> Sadly, jdk.internal.util is exported to other modules so it does need to be 
>> looked at from an integrity perspective.
> 
> This is indeed a problem to consider.
> 
> Maybe we can move this array back into `HexDigits`. Then we create a new 
> package `jdk.internal.digits` and move `Digits`, `DecimalDigits`, 
> `HexDigits`, `OctalDigits` all into it.

Hex256 has been moved to jdk.internal.digits, jdk.internal.digits is not 
exported.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1244011030


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v26]

2023-06-27 Thread 温绍锦
On Tue, 27 Jun 2023 13:54:43 GMT, Roger Riggs  wrote:

>> 温绍锦 has updated the pull request incrementally with one additional commit 
>> since the last revision:
>> 
>>   fix UUID.java import, rename jdk.util.HexDigits to jdk.util.Hex256 and 
>> make private constructor.
>
> src/java.base/share/classes/jdk/internal/util/Hex256.java line 31:
> 
>> 29: 
>> 30: /**
>> 31:  * Provides a hexadecimal cache array of values from 0 to 255
> 
> A bit more information about the contents and use would be useful for future 
> developers and perhaps an example of the use so they don't have to go find 
> where it is used and decipher the code.
> Each element contains the ascii encoded hex characters for the index. It may 
> be obvious which character is the low nibble for the low byte but it might be 
> good to say so.

comments have been added, including usage examples.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1244014708


RFR: JDK-8310975 java.util.FormatItemModifier should not be protected

2023-06-27 Thread Jim Laskey
The nested class is incorrectly marked as protected.

-

Commit messages:
 - Remove protected from FormatItemModifier

Changes: https://git.openjdk.org/jdk/pull/14681/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14681&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8310975
  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/14681.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14681/head:pull/14681

PR: https://git.openjdk.org/jdk/pull/14681


Re: [jdk21] RFR: 8310459: [BACKOUT] 8304450: [vectorapi] Refactor VectorShuffle implementation

2023-06-27 Thread Vladimir Kozlov
On Tue, 27 Jun 2023 07:46:24 GMT, Tobias Hartmann  wrote:

> Backport of [JDK-8310459](https://bugs.openjdk.java.net/browse/JDK-8310459). 
> Applies cleanly.
> 
> Thanks,
> Tobias

Good.

-

Marked as reviewed by kvn (Reviewer).

PR Review: https://git.openjdk.org/jdk21/pull/68#pullrequestreview-1501379033


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v25]

2023-06-27 Thread Alan Bateman
On Tue, 27 Jun 2023 11:25:32 GMT, 温绍锦  wrote:

>> 温绍锦 has updated the pull request incrementally with one additional commit 
>> since the last revision:
>> 
>>   use ISO_8859_1.INSTANCE directly instead of StandardCharsets.ISO_8859_1
>
>> I'm looking through 
>> [[6b61a71](https://github.com/openjdk/jdk/commit/6b61a715caeeb586d5480a3a70f90bd1811d1335)],
>>  update 24.
>> 
>> Moving fastUUID out of Long is good.
>> 
>> UUID::toString can use StandardCharsets. ISO_8859_1, no need to use 
>> sun.nio.cs code here.
>> 
>> The list of imports in java.util.UUID is a bit messy now, can you clean this 
>> up.
>> 
>> It's confusing to have java.util.HexDigits and jdk.internal.util.HexDigits. 
>> It also creates an inconsistency with DecimalDigits and OctalDigits as the 
>> latter encapsulate their arrays. I assume you've done this to allow sharing 
>> of the 256 element array. I don't object to move the array to a supporting 
>> class but it needs to be renamed and the class description replaced as it is 
>> not a Digits implementation. Adding a private constructor would help make it 
>> clear that it should not be instantiated.
> 
> very good suggestion, i have modified it according to your suggestion.

@wenshao - adding a jdk.internal.digits package for this tiny class is 
overkill. For this PR, the simplest thing is to revert the changes to HexDigit, 
remove jdk.internal.digits.Hex256 and just put a small holder class in UUID for 
use by UUID.toString. That would be my preference to avoid splitting up 
HexDigits and creating inconsistencies in code used for string templates.

-

PR Comment: https://git.openjdk.org/jdk/pull/14578#issuecomment-1609843786


Re: RFR: JDK-8310975 java.util.FormatItemModifier should not be protected

2023-06-27 Thread Alan Bateman
On Tue, 27 Jun 2023 16:11:43 GMT, Jim Laskey  wrote:

> The nested class is incorrectly marked as protected.

Marked as reviewed by alanb (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/14681#pullrequestreview-1501387697


Re: RFR: JDK-8310975 java.util.FormatItemModifier should not be protected

2023-06-27 Thread Joe Darcy
On Tue, 27 Jun 2023 16:11:43 GMT, Jim Laskey  wrote:

> The nested class is incorrectly marked as protected.

Marked as reviewed by darcy (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/14681#pullrequestreview-1501391798


Re: RFR: 6983726: Reimplement MethodHandleProxies.asInterfaceInstance [v18]

2023-06-27 Thread Chen Liang
> As John Rose has pointed out in this issue, the current j.l.r.Proxy based 
> implementation of MethodHandleProxies.asInterface has a few issues:
> 1. Exposes too much information via Proxy supertype (and WrapperInstance 
> interface)
> 2. Does not allow future expansion to support SAM[^1] abstract classes
> 3. Slow (in fact, very slow)
> 
> This patch addresses all 3 problems:
> 1. It updates the WrapperInstance methods to take an `Empty` to avoid method 
> clashes
> 2. This patch obtains already generated classes from a ClassValue by the 
> requested interface type; the ClassValue can later be updated to compute 
> implementation generation for abstract classes as well.
> 3. This patch's faster than old implementation in general.
> 
> 
> Benchmark  Mode  Cnt  
> Score  Error  Units
> MethodHandleProxiesAsIFInstance.baselineAllocCompute   avgt   15  
> 1.483 ±0.025  ns/op
> MethodHandleProxiesAsIFInstance.baselineComputeavgt   15  
> 0.264 ±0.006  ns/op
> MethodHandleProxiesAsIFInstance.testCall   avgt   15  
> 1.773 ±0.040  ns/op
> MethodHandleProxiesAsIFInstance.testCreate avgt   15  
>16.754 ±0.411  ns/op
> MethodHandleProxiesAsIFInstance.testCreateCall avgt   15  
>19.609 ±1.598  ns/op
> MethodHandleProxiesAsIFInstanceCall.callDoable avgt   15  
> 0.424 ±0.024  ns/op
> MethodHandleProxiesAsIFInstanceCall.callHandle avgt   15  
> 1.936 ±0.008  ns/op
> MethodHandleProxiesAsIFInstanceCall.callInterfaceInstance  avgt   15  
> 1.766 ±0.014  ns/op
> MethodHandleProxiesAsIFInstanceCall.callLambda avgt   15  
> 0.414 ±0.005  ns/op
> MethodHandleProxiesAsIFInstanceCall.constantDoable avgt   15  
> 0.271 ±0.006  ns/op
> MethodHandleProxiesAsIFInstanceCall.constantHandle avgt   15  
> 0.263 ±0.004  ns/op
> MethodHandleProxiesAsIFInstanceCall.constantInterfaceInstance  avgt   15  
> 0.266 ±0.005  ns/op
> MethodHandleProxiesAsIFInstanceCall.constantLambda avgt   15  
> 0.262 ±0.003  ns/op
> MethodHandleProxiesAsIFInstanceCall.direct avgt   15  
> 0.264 ±0.005  ns/op
> MethodHandleProxiesAsIFInstanceCreate.createCallInterfaceInstance  avgt   15  
>18.000 ±0.181  ns/op
> MethodHandleProxiesAsIFInstanceCreate.createCallLambda avgt   15  
> 17624.673 ± 24...

Chen Liang has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now contains 38 commits:

 - SecurityManager fixed, minimize changes
 - Merge branch 'master' into explore/mhp-iface
 - Some changes per Mandy's review. SecurityManager test fails in this patch
 - Merge branch 'master' into explore/mhp-iface
 - Merge branch 'master' into explore/mhp-iface
 - Merge branch 'master' into explore/mhp-iface
 - stage, needs to fix is mh proxy instance check
 - Merge branch 'master' into explore/mhp-iface
 - Remove assertion, no longer true with teleport definition in MHP
 - Fix tabs, and tests about modules and constructor access
 - ... and 28 more: https://git.openjdk.org/jdk/compare/05e9c41e...07cc1279

-

Changes: https://git.openjdk.org/jdk/pull/13197/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=13197&range=17
  Stats: 1315 lines in 16 files changed: 1072 ins; 150 del; 93 mod
  Patch: https://git.openjdk.org/jdk/pull/13197.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/13197/head:pull/13197

PR: https://git.openjdk.org/jdk/pull/13197


Re: RFR: JDK-8310975 java.util.FormatItemModifier should not be protected

2023-06-27 Thread Iris Clark
On Tue, 27 Jun 2023 16:11:43 GMT, Jim Laskey  wrote:

> The nested class is incorrectly marked as protected.

Marked as reviewed by iris (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/14681#pullrequestreview-1501400084


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v25]

2023-06-27 Thread Glavo
On Tue, 27 Jun 2023 11:25:32 GMT, 温绍锦  wrote:

>> 温绍锦 has updated the pull request incrementally with one additional commit 
>> since the last revision:
>> 
>>   use ISO_8859_1.INSTANCE directly instead of StandardCharsets.ISO_8859_1
>
>> I'm looking through 
>> [[6b61a71](https://github.com/openjdk/jdk/commit/6b61a715caeeb586d5480a3a70f90bd1811d1335)],
>>  update 24.
>> 
>> Moving fastUUID out of Long is good.
>> 
>> UUID::toString can use StandardCharsets. ISO_8859_1, no need to use 
>> sun.nio.cs code here.
>> 
>> The list of imports in java.util.UUID is a bit messy now, can you clean this 
>> up.
>> 
>> It's confusing to have java.util.HexDigits and jdk.internal.util.HexDigits. 
>> It also creates an inconsistency with DecimalDigits and OctalDigits as the 
>> latter encapsulate their arrays. I assume you've done this to allow sharing 
>> of the 256 element array. I don't object to move the array to a supporting 
>> class but it needs to be renamed and the class description replaced as it is 
>> not a Digits implementation. Adding a private constructor would help make it 
>> clear that it should not be instantiated.
> 
> very good suggestion, i have modified it according to your suggestion.

> @wenshao - adding a jdk.internal.digits package for this tiny class is 
> overkill. For this PR, the simplest thing is to revert the changes to 
> HexDigit, remove jdk.internal.digits.Hex256 and just put a small holder class 
> in UUID for use by UUID.toString. That would be my preference to avoid 
> splitting up HexDigits and creating inconsistencies in code used for string 
> templates.

Wenshao hopes to use this array for other optimizations in `java.lang` in the 
future.

But as for this PR, I suggest wenshao not to do so much. I suggest moving 
`DIGITS` back to `HexDigits` first to make the modifications in this PR 
smaller. Reducing modifications helps with faster review.

When you need to optimize the classes in `java.lang`, we will continue to 
discuss how to share these arrays more safely and efficiently.

-

PR Comment: https://git.openjdk.org/jdk/pull/14578#issuecomment-1609857054


Re: RFR: 8310232: java.time.Clock$TickClock.millis() fails in runtime when tick is 1 microsecond [v2]

2023-06-27 Thread Naoto Sato
> Fixing the `/ by zero` exception with tick durations less than a millisecond.

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

 - Merge branch 'master' into JDK-8310232-TickClock-ArithExp
 - Removed bugid from the test class description
 - refactor return statement
 - 8310232: java.time.Clock$TickClock.millis() fails in runtime when tick is 1 
microsecond

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14657/files
  - new: https://git.openjdk.org/jdk/pull/14657/files/050fb378..d1a984d7

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14657&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14657&range=00-01

  Stats: 9256 lines in 348 files changed: 3584 ins; 2949 del; 2723 mod
  Patch: https://git.openjdk.org/jdk/pull/14657.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14657/head:pull/14657

PR: https://git.openjdk.org/jdk/pull/14657


Re: RFR: 8310033: Clarify return value of Java Time compareTo methods [v2]

2023-06-27 Thread Joe Darcy
On Tue, 20 Jun 2023 20:54:43 GMT, Jens Lidestrom  wrote:

> > A positive number is any number that is greater than 0. **Unlike positive 
> > integers**, which include 0
> 
> math.net seems pretty alone in this. I find the notion bizarre. There seems 
> to be an overwhelming majority of sources that consider 0 to be neither 
> negative nor positive.

Yes, all the math conventions I'm aware of is that integer zero is neither 
positive nor negative. For example, the signum method returns 0 for 0, -1 for 
negative values, and 1 for positive values.

-

PR Comment: https://git.openjdk.org/jdk/pull/14479#issuecomment-1609877890


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v25]

2023-06-27 Thread Alan Bateman
On Tue, 27 Jun 2023 16:28:44 GMT, Glavo  wrote:

> Wenshao hopes to use this array for other optimizations in `java.lang` in the 
> future.
> 
> But as for this PR, I suggest wenshao not to do so much. I suggest moving 
> `DIGITS` back to `HexDigits` first to make the modifications in this PR 
> smaller. Reducing modifications helps with faster review.
> 
> When you need to optimize the classes in `java.lang`, we will continue to 
> discuss how to share these arrays more safely and efficiently.

Let's keep this PR as simple as possible. Right now, it's up to 100 comments 
and 27 iterations, it's annoying to be sending new contributor Wenshao in 
different directions. Future work that would allow safer sharing of arrays is 
future work, let's keep this PR as simple as possible. Not all applications use 
UUID.

-

PR Comment: https://git.openjdk.org/jdk/pull/14578#issuecomment-1609869958


Re: RFR: 6983726: Reimplement MethodHandleProxies.asInterfaceInstance [v18]

2023-06-27 Thread Chen Liang
On Tue, 27 Jun 2023 16:33:11 GMT, Chen Liang  wrote:

>> As John Rose has pointed out in this issue, the current j.l.r.Proxy based 
>> implementation of MethodHandleProxies.asInterface has a few issues:
>> 1. Exposes too much information via Proxy supertype (and WrapperInstance 
>> interface)
>> 2. Does not allow future expansion to support SAM[^1] abstract classes
>> 3. Slow (in fact, very slow)
>> 
>> This patch addresses all 3 problems:
>> 1. It updates the WrapperInstance methods to take an `Empty` to avoid method 
>> clashes
>> 2. This patch obtains already generated classes from a ClassValue by the 
>> requested interface type; the ClassValue can later be updated to compute 
>> implementation generation for abstract classes as well.
>> 3. This patch's faster than old implementation in general.
>> 
>> Benchmark for revision 17:
>> 
>> Benchmark  Mode  Cnt 
>>  Score   Error  Units
>> MethodHandleProxiesAsIFInstance.baselineAllocCompute   avgt   15 
>>  1.503 ± 0.021  ns/op
>> MethodHandleProxiesAsIFInstance.baselineComputeavgt   15 
>>  0.269 ± 0.005  ns/op
>> MethodHandleProxiesAsIFInstance.testCall   avgt   15 
>>  1.806 ± 0.018  ns/op
>> MethodHandleProxiesAsIFInstance.testCreate avgt   15 
>> 17.332 ± 0.210  ns/op
>> MethodHandleProxiesAsIFInstance.testCreateCall avgt   15 
>> 19.296 ± 1.371  ns/op
>> MethodHandleProxiesAsIFInstanceCall.callDoable avgt5 
>>  0.419 ± 0.004  ns/op
>> MethodHandleProxiesAsIFInstanceCall.callHandle avgt5 
>>  0.421 ± 0.004  ns/op
>> MethodHandleProxiesAsIFInstanceCall.callInterfaceInstance  avgt5 
>>  1.731 ± 0.018  ns/op
>> MethodHandleProxiesAsIFInstanceCall.callLambda avgt5 
>>  0.418 ± 0.003  ns/op
>> MethodHandleProxiesAsIFInstanceCall.constantDoable avgt5 
>>  0.263 ± 0.003  ns/op
>> MethodHandleProxiesAsIFInstanceCall.constantHandle avgt5 
>>  0.262 ± 0.002  ns/op
>> MethodHandleProxiesAsIFInstanceCall.constantInterfaceInstance  avgt5 
>>  0.262 ± 0.002  ns/op
>> MethodHandleProxiesAsIFInstanceCall.constantLambda avgt5 
>>  0.267 ± 0.019  ns/op
>> MethodHandleProxiesAsIFInstanceCall.direct avgt5 
>>  0.266 ± 0.013  ns/op
>> MethodHandleProxiesAsIFInstanceCreate.createCallInterfaceInstance  avgt5 
>> 18.057 ± 0.182 ...
>
> Chen Liang has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now contains 38 commits:
> 
>  - SecurityManager fixed, minimize changes
>  - Merge branch 'master' into explore/mhp-iface
>  - Some changes per Mandy's review. SecurityManager test fails in this patch
>  - Merge branch 'master' into explore/mhp-iface
>  - Merge branch 'master' into explore/mhp-iface
>  - Merge branch 'master' into explore/mhp-iface
>  - stage, needs to fix is mh proxy instance check
>  - Merge branch 'master' into explore/mhp-iface
>  - Remove assertion, no longer true with teleport definition in MHP
>  - Fix tabs, and tests about modules and constructor access
>  - ... and 28 more: https://git.openjdk.org/jdk/compare/05e9c41e...07cc1279

I have updated this patch again, this time updated to latest Classfile API and 
removed irrelevant changes.

1. The previous SecurityManager failure was because of 
ClassLoaders$AppClassLoader's security manager package access check. Even if we 
can patch the package access check there, we cannot rule out that another 
interface's class loader reject loading jdk.internal classes specifically 
exported to our dynamic module. Thus, I moved the check method to be part of 
the class template, and chose the ClassValue approach instead of using a marker 
interface.
2. Given this observation, exporting jdk.internal packages to modules with user 
class loaders may be problematic; we might need our custom class loaders if we 
do export jdk.internal content to a class that can see user classes.

-

PR Comment: https://git.openjdk.org/jdk/pull/13197#issuecomment-1609908127


java.util.stream.Stream: API for user-extensible intermediate operations

2023-06-27 Thread Viktor Klang
Hi core-libs-dev,

Over the past 6+ months I've been thinking about, and tinkering with, how we'd 
be able to expose a user-facing API for extensible intermediate 
java.util.stream.Stream operations—a feature envisioned all the way back when 
Streams were created.

I'm now at a point where I have a viable design and implementation, and so I'm 
turning to you for your feedback: on the direction taken; the API concepts; 
and, in particular, is there anything which I have overlooked/missed?

(If you, like myself, prefer reading pre-rendered markdown, click 
here)




# Gathering the streams

## Introduction

Java 8 introduced the `java.util.stream` API, which represents a lazily
computed, potentially unbounded sequence of values (Streams was also the first
designed-for-lambdas API in the JDK). Streams supports the ability to process
the stream either sequentially or in parallel.

A `Stream` pipeline consists of a source (collection, array, generator, etc),
zero or more intermediate operations (`Stream` -> `Stream` transforms), and an
eager terminal operation which produces a value or a side-effect.

The Streams API come with a reasonably rich, but fixed set of built-in
operations (mapping, filtering, reduction, sorting, etc), as well as an
extensible terminal operation (`Stream::collect`) that enables the stream
contents to be flexibly summarized in a variety of forms.  The resulting API is
rich enough that users have had good experience with streams, but there are
repeated requests for "please add operation X to streams".

In this document, we explore a corresponding extensible _intermediate_
operation, called `Stream::gather`, which is able to address many of the
requests we've gotten for additional operations.

## "Missing" intermediate operations

Over the years, many new operations for streams have been proposed.  Each of
them may be useful in some situation, but many of them are too narrow to be
included in the core `Stream` API.  We'd like for people to be able to plug
these operations in via an extension point, as with `Stream::collect`.

Here are some examples of proposed intermediate operations that are not easily
expressible as intermediate operations on `Stream` today.

* **Folds**.  Folding is a generalization of reduction.  With reduction, the
  result type is the same as the element type, the combiner is associative, and
  the initial value is an identity for the combiner.   For a fold, these
  conditions are not required, though we give up parallelizability.

  Example: `foldLeft("", (str,elem) -> str + " " + elem.toString())` over the 
values
  `[1,2,3]` yields `["1 2 3"]`.

  See https://bugs.openjdk.org/browse/JDK-8133680 & 
https://bugs.openjdk.org/browse/JDK-8292845

* **Unfolds**.  This takes an aggregate and decomposes it into elements.  An
  `unfold` can reverse the operation of the above fold example using `Scanner`
  or `String::split`.

  Example: `unfold(e -> new Scanner(e), Scanner::hasNextInt, Scanner::nextInt)`
  `["1 2 3"]` yields `[1,2,3]`.

  See: https://bugs.openjdk.org/browse/JDK-8151408

* **Barriers**.  Usually all operations of a stream pipeline can run
  simultaneously as data is available.  In some cases, we may wish to have a
  barrier that requires that the entirety of one operation is completed before
  providing any elements to the next operation.

  Example: `someStream.a(...).barrier(someEffect)...` would require that all
  effects from `a` are completed before executing `someEffect` or producing any
  downstream values.

  See: https://bugs.openjdk.org/browse/JDK-8294246

* **Windowing**.  Given a stream, we may wish to construct a stream which
  groups the original elements, either in overlapping or disjoint groups.

  Example: `fixedWindow(2)` over the values `[1,2,3,4,5]` yields 
`[[1,2],[3,4],[5]]`.
  Example: `slidingWindow(2)` over the values `[1,2,3]` yields 
`[[1,2],[2,3],[3,4]]`.

  See: 
https://stackoverflow.com/questions/34158634/how-to-transform-a-java-stream-into-a-sliding-window

* **Prefix scans**. Prefix scans are a stream of incremental reductions.
  Perhaps surprisingly, some prefix scans are parallelizable, and we are 
exploring adding support for that beyond `Arrays.parallelPrefix()`.

  Example: `scan((sum, next) -> sum + next)` over  values `[1,2,3,4]` yields 
`[1,3,6,10]`

  See: 
https://stackoverflow.com/questions/55265797/cumulative-sum-using-java-8-stream-api

* **Duplicated elements**.  The `distinct` operation will remove duplicates; 
sometimes we want only the elements that are duplicated.

  Example: `duplicates(Object::equals)` over values `[1,1,2,3,4]` yields `[1]`

  See: 
https://stackoverflow.com/questions/27677256/java-8-streams-to-find-the-duplicate-elements

* **Running duplicate elimination**.  Here, we want to eliminate adjacent 
repeated elements.

  Example: `collapseRuns(Object::equals)` over values `[1,1,2,3,3]` yields 
`[1,2,3]`

These are just a few of the stream oper

[jdk21] RFR: 8310182: DateTimeFormatter date formats (ISO_LOCAL_DATE) separated with hyphen, not dash

2023-06-27 Thread Naoto Sato
Hi all,

This pull request contains a backport of commit 
[ec45bd64](https://github.com/openjdk/jdk/commit/ec45bd64d504d579aef54c924fb8ca75a944036f)
 from the [openjdk/jdk](https://git.openjdk.org/jdk) repository.

The commit being backported was authored by Naoto Sato on 27 Jun 2023 and was 
reviewed by Roger Riggs, Joe Darcy, Iris Clark and Lance Andersen.

Thanks!

-

Commit messages:
 - Backport ec45bd64d504d579aef54c924fb8ca75a944036f

Changes: https://git.openjdk.org/jdk21/pull/70/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk21&pr=70&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8310182
  Stats: 6 lines in 1 file changed: 0 ins; 0 del; 6 mod
  Patch: https://git.openjdk.org/jdk21/pull/70.diff
  Fetch: git fetch https://git.openjdk.org/jdk21.git pull/70/head:pull/70

PR: https://git.openjdk.org/jdk21/pull/70


Re: RFR: 8310892: ScopedValue throwing StructureViolationException should be clearer

2023-06-27 Thread Daniel Fuchs
On Tue, 27 Jun 2023 15:34:58 GMT, Alan Bateman  wrote:

> Docs only update to add a missing `@throws StructureViolationException` and 
> make it clearer when the exception is thrown. In the future we might re-visit 
> this so that the description is in one place rather than in each method.

I like the new text, it reads well. There's a typo that has been copy pasted 
several times however.

src/java.base/share/classes/java/lang/ScopedValue.java line 403:

> 401:  * invoked directly or indirectly by the operation creates a 
> {@link StructuredTaskScope}
> 402:  * but does not {@linkplain StructuredTaskScope#close() close} 
> it, then it is detected
> 403:  * as a structure violation when the operation 
> completes (normally or ith an

Suggestion:

 * as a structure violation when the operation completes 
(normally or with an

src/java.base/share/classes/java/lang/ScopedValue.java line 433:

> 431:  * invoked directly or indirectly by the operation creates a 
> {@link StructuredTaskScope}
> 432:  * but does not {@linkplain StructuredTaskScope#close() close} 
> it, then it is detected
> 433:  * as a structure violation when the operation 
> completes (normally or ith an

Suggestion:

 * as a structure violation when the operation completes 
(normally or with an

src/java.base/share/classes/java/lang/ScopedValue.java line 497:

> 495:  * invoked directly or indirectly by the operation creates a 
> {@link StructuredTaskScope}
> 496:  * but does not {@linkplain StructuredTaskScope#close() close} 
> it, then it is detected
> 497:  * as a structure violation when the operation 
> completes (normally or ith an

Suggestion:

 * as a structure violation when the operation completes 
(normally or with an

src/java.base/share/classes/java/lang/ScopedValue.java line 565:

> 563:  * invoked directly or indirectly by the operation creates a {@link 
> StructuredTaskScope}
> 564:  * but does not {@linkplain StructuredTaskScope#close() close} it, 
> then it is detected
> 565:  * as a structure violation when the operation completes 
> (normally or ith an

Suggestion:

 * as a structure violation when the operation completes (normally 
or with an

src/java.base/share/classes/java/lang/ScopedValue.java line 601:

> 599:  * invoked directly or indirectly by the operation creates a {@link 
> StructuredTaskScope}
> 600:  * but does not {@linkplain StructuredTaskScope#close() close} it, 
> then it is detected
> 601:  * as a structure violation when the operation completes 
> (normally or ith an

Suggestion:

 * as a structure violation when the operation completes (normally 
or with an

src/java.base/share/classes/java/lang/ScopedValue.java line 636:

> 634:  * invoked directly or indirectly by the operation creates a {@link 
> StructuredTaskScope}
> 635:  * but does not {@linkplain StructuredTaskScope#close() close} it, 
> then it is detected
> 636:  * as a structure violation when the operation completes 
> (normally or ith an

Suggestion:

 * as a structure violation when the operation completes (normally 
or with an

-

PR Review: https://git.openjdk.org/jdk/pull/14679#pullrequestreview-1501483319
PR Review Comment: https://git.openjdk.org/jdk/pull/14679#discussion_r1244091134
PR Review Comment: https://git.openjdk.org/jdk/pull/14679#discussion_r1244092954
PR Review Comment: https://git.openjdk.org/jdk/pull/14679#discussion_r1244093522
PR Review Comment: https://git.openjdk.org/jdk/pull/14679#discussion_r1244094318
PR Review Comment: https://git.openjdk.org/jdk/pull/14679#discussion_r1244095201
PR Review Comment: https://git.openjdk.org/jdk/pull/14679#discussion_r1244095763


Re: RFR: JDK-8310975 java.util.FormatItemModifier should not be protected

2023-06-27 Thread Lance Andersen
On Tue, 27 Jun 2023 16:11:43 GMT, Jim Laskey  wrote:

> The nested class is incorrectly marked as protected.

Marked as reviewed by lancea (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/14681#pullrequestreview-1501494580


Re: RFR: 8310892: ScopedValue throwing StructureViolationException should be clearer

2023-06-27 Thread Alan Bateman
On Tue, 27 Jun 2023 17:19:18 GMT, Daniel Fuchs  wrote:

> I like the new text, it reads well. There's a typo that has been copy pasted 
> several times however.

Thanks, I noticed this when looking at the generated javadoc but didn't commit 
the change to fix. I'll fix it now.

-

PR Comment: https://git.openjdk.org/jdk/pull/14679#issuecomment-1609934807


Integrated: 8310828: java.sql java.sql.rowset packages have no `@since` info

2023-06-27 Thread Lance Andersen
On Mon, 26 Jun 2023 13:00:31 GMT, Lance Andersen  wrote:

> Hi all,
> 
> Please review this trivial change which adds the `@since` javadoc tag to the 
> various ` java.sql `and `java.sql.rowset` packages.
> 
> Best
> Lance

This pull request has now been integrated.

Changeset: fb283dff
Author:Lance Andersen 
URL:   
https://git.openjdk.org/jdk/commit/fb283dff04a6fe11c8a7d44498ddd2075234e4dd
Stats: 19 lines in 7 files changed: 7 ins; 4 del; 8 mod

8310828: java.sql java.sql.rowset packages have no `@since` info

Reviewed-by: naoto, iris, darcy, bpb

-

PR: https://git.openjdk.org/jdk/pull/14652


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v25]

2023-06-27 Thread 温绍锦
On Tue, 27 Jun 2023 16:35:20 GMT, Alan Bateman  wrote:

> > Wenshao hopes to use this array for other optimizations in `java.lang` in 
> > the future.
> > But as for this PR, I suggest wenshao not to do so much. I suggest moving 
> > `DIGITS` back to `HexDigits` first to make the modifications in this PR 
> > smaller. Reducing modifications helps with faster review.
> > When you need to optimize the classes in `java.lang`, we will continue to 
> > discuss how to share these arrays more safely and efficiently.
> 
> Let's keep this PR as simple as possible. Right now, it's up to 100 comments 
> and 27 iterations, it's annoying to be sending new contributor Wenshao in 
> different directions. Future work that would allow safer sharing of arrays is 
> future work, let's keep this PR as simple as possible. Not all applications 
> use UUID.

good suggestion, it has been changed.

-

PR Comment: https://git.openjdk.org/jdk/pull/14578#issuecomment-1609938471


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v29]

2023-06-27 Thread 温绍锦
> By optimizing the implementation of java.lang.Long#fastUUID, the performance 
> of the java.util.UUID#toString method can be significantly improved.
> 
> The following are the test results of JMH: 
> 
> Benchmark Mode  Cnt  Score  Error   Units
> UUIDUtilsBenchmark.new   thrpt5  92676.550 ±  292.213  ops/ms
> UUIDUtilsBenchmark.original  thrpt5  37040.165 ± 1023.532  ops/ms

温绍锦 has updated the pull request incrementally with one additional commit since 
the last revision:

  revert to HexDigits.DIGITS, keep this PR as simple as possible.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14578/files
  - new: https://git.openjdk.org/jdk/pull/14578/files/b81f12d1..4a4c64be

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14578&range=28
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14578&range=27-28

  Stats: 182 lines in 3 files changed: 63 ins; 99 del; 20 mod
  Patch: https://git.openjdk.org/jdk/pull/14578.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14578/head:pull/14578

PR: https://git.openjdk.org/jdk/pull/14578


Re: RFR: 8310892: ScopedValue throwing StructureViolationException should be clearer [v2]

2023-06-27 Thread Alan Bateman
> Docs only update to add a missing `@throws StructureViolationException` and 
> make it clearer when the exception is thrown. In the future we might re-visit 
> this so that the description is in one place rather than in each method.

Alan Bateman has updated the pull request incrementally with one additional 
commit since the last revision:

  s/ ith/with/

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14679/files
  - new: https://git.openjdk.org/jdk/pull/14679/files/ab2b91fb..08158fe8

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14679&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14679&range=00-01

  Stats: 6 lines in 1 file changed: 0 ins; 0 del; 6 mod
  Patch: https://git.openjdk.org/jdk/pull/14679.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14679/head:pull/14679

PR: https://git.openjdk.org/jdk/pull/14679


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v30]

2023-06-27 Thread 温绍锦
> By optimizing the implementation of java.lang.Long#fastUUID, the performance 
> of the java.util.UUID#toString method can be significantly improved.
> 
> The following are the test results of JMH: 
> 
> Benchmark Mode  Cnt  Score  Error   Units
> UUIDUtilsBenchmark.new   thrpt5  92676.550 ±  292.213  ops/ms
> UUIDUtilsBenchmark.original  thrpt5  37040.165 ± 1023.532  ops/ms

温绍锦 has updated the pull request incrementally with one additional commit since 
the last revision:

  remove unused import, and fix comment

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14578/files
  - new: https://git.openjdk.org/jdk/pull/14578/files/4a4c64be..26f3a6a5

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14578&range=29
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14578&range=28-29

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

PR: https://git.openjdk.org/jdk/pull/14578


Re: RFR: JDK-8310502 : Optimization for j.l.Long.fastUUID() [v29]

2023-06-27 Thread Chen Liang
On Tue, 27 Jun 2023 17:30:35 GMT, 温绍锦  wrote:

>> By optimizing the implementation of java.lang.Long#fastUUID, the performance 
>> of the java.util.UUID#toString method can be significantly improved.
>> 
>> The following are the test results of JMH: 
>> 
>> Benchmark Mode  Cnt  Score  Error   Units
>> UUIDUtilsBenchmark.new   thrpt5  92676.550 ±  292.213  ops/ms
>> UUIDUtilsBenchmark.original  thrpt5  37040.165 ± 1023.532  ops/ms
>
> 温绍锦 has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   revert to HexDigits.DIGITS, keep this PR as simple as possible.

src/java.base/share/classes/java/util/HexDigits.java line 33:

> 31: 
> 32: import static jdk.internal.digits.Hex256.hex256;
> 33: 

Suggestion:


This is now redundant and causes build failure.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14578#discussion_r1244118063


Re: [jdk21] RFR: 8310182: DateTimeFormatter date formats (ISO_LOCAL_DATE) separated with hyphen, not dash

2023-06-27 Thread Roger Riggs
On Tue, 27 Jun 2023 17:07:50 GMT, Naoto Sato  wrote:

> Hi all,
> 
> This pull request contains a backport of commit 
> [ec45bd64](https://github.com/openjdk/jdk/commit/ec45bd64d504d579aef54c924fb8ca75a944036f)
>  from the [openjdk/jdk](https://git.openjdk.org/jdk) repository.
> 
> The commit being backported was authored by Naoto Sato on 27 Jun 2023 and was 
> reviewed by Roger Riggs, Joe Darcy, Iris Clark and Lance Andersen.
> 
> Thanks!

Marked as reviewed by rriggs (Reviewer).

-

PR Review: https://git.openjdk.org/jdk21/pull/70#pullrequestreview-1501531725


Re: RFR: 8310232: java.time.Clock$TickClock.millis() fails in runtime when tick is 1 microsecond [v2]

2023-06-27 Thread Roger Riggs
On Tue, 27 Jun 2023 16:34:40 GMT, Naoto Sato  wrote:

>> Fixing the `/ by zero` exception with tick durations less than a millisecond.
>
> Naoto Sato has updated the pull request with a new target base due to a merge 
> or a rebase. The incremental webrev excludes the unrelated changes brought in 
> by the merge/rebase. The pull request contains four additional commits since 
> the last revision:
> 
>  - Merge branch 'master' into JDK-8310232-TickClock-ArithExp
>  - Removed bugid from the test class description
>  - refactor return statement
>  - 8310232: java.time.Clock$TickClock.millis() fails in runtime when tick is 
> 1 microsecond

Marked as reviewed by rriggs (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/14657#pullrequestreview-1501535673


Re: RFR: 8310232: java.time.Clock$TickClock.millis() fails in runtime when tick is 1 microsecond [v2]

2023-06-27 Thread Iris Clark
On Tue, 27 Jun 2023 16:34:40 GMT, Naoto Sato  wrote:

>> Fixing the `/ by zero` exception with tick durations less than a millisecond.
>
> Naoto Sato has updated the pull request with a new target base due to a merge 
> or a rebase. The incremental webrev excludes the unrelated changes brought in 
> by the merge/rebase. The pull request contains four additional commits since 
> the last revision:
> 
>  - Merge branch 'master' into JDK-8310232-TickClock-ArithExp
>  - Removed bugid from the test class description
>  - refactor return statement
>  - 8310232: java.time.Clock$TickClock.millis() fails in runtime when tick is 
> 1 microsecond

Marked as reviewed by iris (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/14657#pullrequestreview-1501537487


Integrated: 8308452: Extend internal Architecture enum with byte order and address size

2023-06-27 Thread Roger Riggs
On Fri, 19 May 2023 19:19:40 GMT, Roger Riggs  wrote:

> The internal enum jdk.internal.util.Architecture does not provide information 
> about the big or little endianness or the address size (64 or 32 bits).  The 
> endian-ness and address size are intrinsic to the architecture.
> 
> The values of the enum are extended to separately identify the big endian and 
> little-endian uses of the ISA.
> For example, `PPC64` and `PPC64LE` for the big and little-endian versions.  
> The enum values directly reflect the build-time artifacts and resulting 
> executables.
> 
> This information about an architecture will make the enum more useful 
> especially to identify a target platform in a cross-platform use case. A 
> method is added to map well known aliases for the platforms to the 
> Architecture enum.

This pull request has now been integrated.

Changeset: d6dd0dc3
Author:Roger Riggs 
URL:   
https://git.openjdk.org/jdk/commit/d6dd0dc3e06d42f108fe80920e1102d47a5aa583
Stats: 191 lines in 5 files changed: 121 ins; 46 del; 24 mod

8308452: Extend internal Architecture enum with byte order and address size

Reviewed-by: mdoerr, jpai, mchung, amitkumar

-

PR: https://git.openjdk.org/jdk/pull/14063


Re: RFR: 8310033: Clarify return value of Java Time compareTo methods [v3]

2023-06-27 Thread Raffaello Giulietti
On Wed, 21 Jun 2023 16:29:40 GMT, Roger Riggs  wrote:

>> In java.time packages, clarify timeline order javadoc to mention "before" 
>> and "after" in the value of the `compareTo` method return values. 
>> Add javadoc @see tags to isBefore and isAfter methods
>> 
>> Replace use of "negative" and positive with "less than zero" and "greater 
>> than zero" in javadoc @return
>> The term "positive" is ambiguous, zero is considered positive and indicates 
>> equality.
>
> Roger Riggs has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Clarify return values of date time classes

Most modern authors in math topics use positive to mean > 0, negative to mean < 
0, and 0 is neither.

In some contexts, like in the IEEE 754 standard for floating-point numbers, 
there are both distinguishable positive and a negative binary zeros. There's no 
"unsigned" or "agnostic" zero there.

The natural numbers include 0 in foundational topics like set theory (where 0 
is _defined_ as the empty set), and exclude it in others.

BTW, [math.net](https://www.math.net/positive-numbers) contradicts itself later 
on the same page. In the "Is 0 positive or negative?" section it states 
"Positive numbers are defined as any number that is greater than zero".

-

PR Comment: https://git.openjdk.org/jdk/pull/14479#issuecomment-1609970331


Re: RFR: 8310901: Convert String::newStringNoRepl with Latin-1 to String::newStringLatin1NoRepl [v4]

2023-06-27 Thread Roger Riggs
On Tue, 27 Jun 2023 07:45:34 GMT, Glavo  wrote:

>> Added a new method `newStringLatin1NoRepl` to the `JavaLangAccess`.
>> 
>> Reasons:
>> 
>> * Most use cases of `newStringNoRepl` use `ISO_8859_1` as the charset, 
>> creating a new shortcut can make writing shorter;
>> * Since all possible values of `byte` are legal Latin-1 characters, 
>> `newStringLatin1NoRepl` **will not throw `CharacterCodingException`**, so 
>> users can make the compiler happy without using useless try-catch statements.
>
> Glavo has updated the pull request with a new target base due to a merge or a 
> rebase. The incremental webrev excludes the unrelated changes brought in by 
> the merge/rebase. The pull request contains seven additional commits since 
> the last revision:
> 
>  - Merge branch 'openjdk:master' into latin1-no-repl
>  - Merge branch 'openjdk:master' into latin1-no-repl
>  - update javadoc
>  - clean newStringNoRepl1
>  - clean newStringNoRepl1
>  - Rename jla to JLA
>  - Create new method JavaLangAccess::newStringLatin1NoRepl

I don't think this is a productive change. 
It takes a local inconvenience try/catch and spreads the impact over multiple 
files and packages as well as bulking up JLA.

-

PR Comment: https://git.openjdk.org/jdk/pull/14655#issuecomment-1609977623


[jdk21] Integrated: 8310182: DateTimeFormatter date formats (ISO_LOCAL_DATE) separated with hyphen, not dash

2023-06-27 Thread Naoto Sato
On Tue, 27 Jun 2023 17:07:50 GMT, Naoto Sato  wrote:

> Hi all,
> 
> This pull request contains a backport of commit 
> [ec45bd64](https://github.com/openjdk/jdk/commit/ec45bd64d504d579aef54c924fb8ca75a944036f)
>  from the [openjdk/jdk](https://git.openjdk.org/jdk) repository.
> 
> The commit being backported was authored by Naoto Sato on 27 Jun 2023 and was 
> reviewed by Roger Riggs, Joe Darcy, Iris Clark and Lance Andersen.
> 
> Thanks!

This pull request has now been integrated.

Changeset: df26258d
Author:Naoto Sato 
URL:   
https://git.openjdk.org/jdk21/commit/df26258d80ece4a836f873660adbf1f69a3b684b
Stats: 6 lines in 1 file changed: 0 ins; 0 del; 6 mod

8310182: DateTimeFormatter date formats (ISO_LOCAL_DATE) separated with hyphen, 
not dash

Reviewed-by: rriggs
Backport-of: ec45bd64d504d579aef54c924fb8ca75a944036f

-

PR: https://git.openjdk.org/jdk21/pull/70


Re: RFR: 8310901: Convert String::newStringNoRepl with Latin-1 to String::newStringLatin1NoRepl [v4]

2023-06-27 Thread Chen Liang
On Tue, 27 Jun 2023 17:58:05 GMT, Roger Riggs  wrote:

> I don't think this is a productive change. It takes a local inconvenience 
> try/catch and spreads the impact over multiple files and packages as well as 
> bulking up JLA.

By this rationale, newStringUTF8NoRepl should be removed as well.

-

PR Comment: https://git.openjdk.org/jdk/pull/14655#issuecomment-1609997762


Re: RFR: 8310901: Convert String::newStringNoRepl with Latin-1 to String::newStringLatin1NoRepl [v4]

2023-06-27 Thread Glavo
On Tue, 27 Jun 2023 17:58:05 GMT, Roger Riggs  wrote:

> It takes a local inconvenience try/catch and spreads the impact over multiple 
> files and packages as well as bulking up JLA.

I don't quite understand what you mean.

The main goal of this PR is to get rid of those annoying try-catch, but it 
looks like you misunderstood this PR?

My English is poor, please forgive me if I misunderstood this sentence of yours.

-

PR Comment: https://git.openjdk.org/jdk/pull/14655#issuecomment-1609997263


Re: RFR: 8301569: list mode of jmod and jimage cannot be used normally in turkish locale [v4]

2023-06-27 Thread Mandy Chung
On Tue, 27 Jun 2023 15:20:33 GMT, Glavo  wrote:

>> When the default Locale is `tr`, the jmod and jimage commands have the 
>> following problems:
>> 
>> * The jmod command does not correctly recognize the `list` mode typed in 
>> lowercase;
>> * The jimage command cannot obtain the help information of the `list` mode.
>
> Glavo has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   update JImageTask, JlinkTask and VersionPropsPlugin

LGTM

-

Marked as reviewed by mchung (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/12281#pullrequestreview-1501583746


Re: RFR: 8309819: Clarify API note in Class::getName and MethodType::toMethodDescriptorString [v6]

2023-06-27 Thread Mandy Chung
On Mon, 26 Jun 2023 22:36:43 GMT, Chen Liang  wrote:

>> `MethodType::descriptorString` refers to method descriptor and 
>> `Class::descriptorString` refers to class descriptor.   I don't think it 
>> helps to say "produce identical class descriptors in method descriptors".
>
> I think we can put it simple:
> "Two distinct classes which share a common name but have different class 
> loaders cannot be distinguished in descriptor strings"
> which stands for both class and method descriptors. How does this sound?

What about "Two distinct {@code MethodType} objects can have an identical 
descriptor string as distinct classes can have the same name but different 
class loaders."?

This api note basically says that from a given descriptor string, 
`fromMethodDescriptorString` may produce a different `MethodType` with a 
different class loader.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14411#discussion_r1244169695


Re: RFR: 8301569: list mode of jmod and jimage cannot be used normally in turkish locale [v4]

2023-06-27 Thread Naoto Sato
On Tue, 27 Jun 2023 15:20:33 GMT, Glavo  wrote:

>> When the default Locale is `tr`, the jmod and jimage commands have the 
>> following problems:
>> 
>> * The jmod command does not correctly recognize the `list` mode typed in 
>> lowercase;
>> * The jimage command cannot obtain the help information of the `list` mode.
>
> Glavo has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   update JImageTask, JlinkTask and VersionPropsPlugin

Marked as reviewed by naoto (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/12281#pullrequestreview-1501594147


Re: RFR: 8308995: Update Network IO JFR events to be static mirror events

2023-06-27 Thread Tim Prinzing
On Thu, 22 Jun 2023 10:21:46 GMT, Alan Bateman  wrote:

>> The socket read/write JFR events currently use instrumentation of java.base 
>> code using templates in the jdk.jfr modules. This results in some java.base 
>> code residing in the jdk.jfr module which is undesirable.
>> 
>> JDK19 added static support for event classes. The old instrumentor classes 
>> should be replaced with mirror events using the static support.
>> 
>> In the java.base module:
>> Added two new events, jdk.internal.event.SocketReadEvent and 
>> jdk.internal.event.SocketWriteEvent.
>> java.net.Socket and sun.nio.ch.SocketChannelImpl were changed to make use of 
>> the new events.
>> 
>> In the jdk.jfr module:
>> jdk.jfr.events.SocketReadEvent and jdk.jfr.events.SocketWriteEvent were 
>> changed to be mirror events.
>> In the package jdk.jfr.internal.instrument, the classes 
>> SocketChannelImplInstrumentor, SocketInputStreamInstrumentor, and 
>> SocketOutputStreamInstrumentor were removed. The JDKEvents class was updated 
>> to reflect all of those changes.
>> 
>> The existing tests in test/jdk/jdk/jfr/event/io continue to pass with the 
>> new implementation:
>> Passed: jdk/jfr/event/io/TestSocketChannelEvents.java
>> Passed: jdk/jfr/event/io/TestSocketEvents.java
>> 
>> I added a micro benchmark which measures the overhead of handling the jfr 
>> socket events.
>> test/micro/org/openjdk/bench/java/net/SocketEventOverhead.java.
>> It needs access the jdk.internal.event package, which is done at runtime 
>> with annotations that add the extra arguments.
>> At compile time the build arguments had to be augmented in 
>> make/test/BuildMicrobenchmark.gmk
>
> src/java.base/share/classes/sun/nio/ch/SocketChannelImpl.java line 408:
> 
>> 406: @Override
>> 407: public int read(ByteBuffer buf) throws IOException {
>> 408: if (!SocketReadEvent.enabled()) {
> 
> The read/write with sun.nio.ch.SocketInputStream and SocketOutputStream does 
> not go through SC.read/write so I think SocketAdaptor read/write will need 
> attention, maybe a future PR as there are other code paths that aren't 
> covered in this PR.

I've created https://bugs.openjdk.org/browse/JDK-8310978 to drive the future PR 
to support the missing code paths

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14342#discussion_r1244182424


Integrated: JDK-8310975 java.util.FormatItemModifier should not be protected

2023-06-27 Thread Jim Laskey
On Tue, 27 Jun 2023 16:11:43 GMT, Jim Laskey  wrote:

> The nested class is incorrectly marked as protected.

This pull request has now been integrated.

Changeset: 315242b7
Author:Jim Laskey 
URL:   
https://git.openjdk.org/jdk/commit/315242b7417a4774765f139b841b385dc7c94c50
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod

8310975: java.util.FormatItemModifier should not be protected

Reviewed-by: alanb, darcy, iris, lancea

-

PR: https://git.openjdk.org/jdk/pull/14681


Re: RFR: 8308995: Update Network IO JFR events to be static mirror events

2023-06-27 Thread Tim Prinzing
On Thu, 22 Jun 2023 13:39:51 GMT, Erik Gahlin  wrote:

> In cases where the implRead/implWrite call throws an exception, shouldn't the 
> event contain that exception, or at least exception message? If it doesn't 
> should it be emitted at all, or should another event be emitted instead?

Added issue https://bugs.openjdk.org/browse/JDK-8310979 to add a field to 
SocketReadEvent and SocketWriteEvent in a separate PR

-

PR Comment: https://git.openjdk.org/jdk/pull/14342#issuecomment-1610037645


[jdk21] RFR: 8310975: java.util.FormatItemModifier should not be protected

2023-06-27 Thread Jim Laskey
The nested class is incorrectly marked as protected.

-

Commit messages:
 - Backport 315242b7417a4774765f139b841b385dc7c94c50

Changes: https://git.openjdk.org/jdk21/pull/72/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk21&pr=72&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8310975
  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk21/pull/72.diff
  Fetch: git fetch https://git.openjdk.org/jdk21.git pull/72/head:pull/72

PR: https://git.openjdk.org/jdk21/pull/72


RFR: JDK-8310830: typo in the parameter name in @throws of ClassDesc::ofDescriptor

2023-06-27 Thread Joe Darcy
Typo fix. I didn't see any other instances of similar typos in the class.

-

Commit messages:
 - JDK-8310830: typo in the parameter name in @throws of ClassDesc::ofDescriptor

Changes: https://git.openjdk.org/jdk/pull/14683/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14683&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8310830
  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/14683.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14683/head:pull/14683

PR: https://git.openjdk.org/jdk/pull/14683


[jdk21] Integrated: 8310975: java.util.FormatItemModifier should not be protected

2023-06-27 Thread Jim Laskey
On Tue, 27 Jun 2023 18:56:53 GMT, Jim Laskey  wrote:

> The nested class is incorrectly marked as protected.

This pull request has now been integrated.

Changeset: 033060b2
Author:Jim Laskey 
URL:   
https://git.openjdk.org/jdk21/commit/033060b22bfb740d25f465f0495bd9b4bea25764
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod

8310975: java.util.FormatItemModifier should not be protected

Reviewed-by: darcy
Backport-of: 315242b7417a4774765f139b841b385dc7c94c50

-

PR: https://git.openjdk.org/jdk21/pull/72


Re: [jdk21] RFR: 8310975: java.util.FormatItemModifier should not be protected

2023-06-27 Thread Joe Darcy
On Tue, 27 Jun 2023 18:56:53 GMT, Jim Laskey  wrote:

> The nested class is incorrectly marked as protected.

Marked as reviewed by darcy (Reviewer).

-

PR Review: https://git.openjdk.org/jdk21/pull/72#pullrequestreview-1501665796


Re: RFR: JDK-8310380: Handle problems in core-related tests on macOS when codesign tool does not work [v4]

2023-06-27 Thread Chris Plummer
On Tue, 27 Jun 2023 13:45:27 GMT, Matthias Baesken  wrote:

>> Currently, a number of tests fail on macOS because they miss the core file 
>> (e.g. serviceability/sa/TestJmapCore.java).
>> The reason is that configure detects on some setups that codesign does not 
>> work ("checking if debug mode codesign is possible... no) .
>> So adding the needed entitlement to generate cores is not done in the build. 
>> This is currently not checked later in the tests.
>> But without the entitlement, a core is not generated.
>
> Matthias Baesken has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   small adjustments

Marked as reviewed by cjplummer (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/14562#pullrequestreview-1501681509


Re: RFR: JDK-8310830: typo in the parameter name in @throws of ClassDesc::ofDescriptor

2023-06-27 Thread Roger Riggs
On Tue, 27 Jun 2023 19:06:26 GMT, Joe Darcy  wrote:

> Typo fix. I didn't see any other instances of similar typos in the class.

Marked as reviewed by rriggs (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/14683#pullrequestreview-1501695302


  1   2   >