RFR: 8317141: Remove unused validIndex method from URLClassPath$JarLoader
Can I please get a review of this change which removes unused (internal) method from the `private` `URLClassPath$JarLoader`? The `validIndex` method which is being removed here was being used when JAR index was supported. We removed support for JAR index in https://bugs.openjdk.org/browse/JDK-8302819. This method is now a leftover and no longer used. The commit in this PR also removes a couple of other member fields in `URLClassPath$JarLoader`. These fields too were used previously when JAR index was in use and are no longer used. tier1, tier2 and tier3 tests continue to pass with this change. - Commit messages: - 8317141: Remove unused validIndex method from URLClassPath Changes: https://git.openjdk.org/jdk/pull/15976/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15976=00 Issue: https://bugs.openjdk.org/browse/JDK-8317141 Stats: 41 lines in 1 file changed: 0 ins; 38 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/15976.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15976/head:pull/15976 PR: https://git.openjdk.org/jdk/pull/15976
Re: ReservedStackAccess in ThreadLocal
Unexpected behaviour is too common phrase - got it As i sad i’ve experienced it in https://github.com/spring-projects/spring-framework/blob/main/spring-aop/src/main/java/org/springframework/aop/interceptor/ExposeInvocationInterceptor.java Error on ThreadLocal#set in finally section there lead to leaving wrong state of TL (in my case it was non-null value into TL, that Spring’s AOP treats like “part of recursion calls” and AOP will return null insteadof expected value (for all execution in such “poisoned” thread) I believe that the same situation is possible with ThreadLocal#remove too > 29 сент. 2023 г., в 05:37, David Holmes написал(а): > > On 29/09/2023 6:33 am, Егор Зиборов wrote: >> Hello, Alan >> I’ve experienced SO in ThreadLocal#set on my production instance >> (more than that it‘s happened in Spring’s ExposeInvocationInterceptor >> that uses TL as context storage)As a result of SO in ThreadLocal >> we’ve experienced unexpected errors with Spring AOP, but I’m sure >> that it could affects lots of frameworks > SOE will nearly always lead to unexpected errors, so that in itself is not > sufficient justification for trying to apply ReservedStackAccess. We added > RSA to address an insidious situation where SOE would leave ReentrantLocks in > a broken state - and we wanted ReentrantLock to be as reliable as built-in > monitor locks (or as near as feasible). So you need a very strong case to > apply RSA elsewhere. You also need to be sure that the stack needed from the > annotated method is small enough for the RSA mechanism to protect. Also note > that RSA doesn't prevent SOE it simply defers it out of the critical code > protected by SOE, so the problem is simply pushed to the caller. > > Just my 2c. > > David > >>> 28 сент. 2023 г., в 19:55, Alan Bateman написал(а): >>> >>> On 28/09/2023 17:21, Егор Зиборов wrote: Hello, everyone I'm new there and writing this letter during advice of Dalibor Topic I faced an issue with SOE in ThreadLocal and want to add this java9 annotation on ThreadLocal#set. Does anyone have any concerns about it? >>> This annotation is for very core/critical areas that are particularly >>> sensitive to SO. Did the SO that you observe just happen to be in >>> ThreadLocal.set or is there more to this story? Asking because there are >>> dozens places in java.base where the argument could be made but we have to >>> be very careful to not sprinkle it about. >>> >>> -Alab >>>
Re: ReservedStackAccess in ThreadLocal
On 29/09/2023 6:33 am, Егор Зиборов wrote: Hello, Alan I’ve experienced SO in ThreadLocal#set on my production instance (more than that it‘s happened in Spring’s ExposeInvocationInterceptor that uses TL as context storage)As a result of SO in ThreadLocal we’ve experienced unexpected errors with Spring AOP, but I’m sure that it could affects lots of frameworks SOE will nearly always lead to unexpected errors, so that in itself is not sufficient justification for trying to apply ReservedStackAccess. We added RSA to address an insidious situation where SOE would leave ReentrantLocks in a broken state - and we wanted ReentrantLock to be as reliable as built-in monitor locks (or as near as feasible). So you need a very strong case to apply RSA elsewhere. You also need to be sure that the stack needed from the annotated method is small enough for the RSA mechanism to protect. Also note that RSA doesn't prevent SOE it simply defers it out of the critical code protected by SOE, so the problem is simply pushed to the caller. Just my 2c. David 28 сент. 2023 г., в 19:55, Alan Bateman написал(а): On 28/09/2023 17:21, Егор Зиборов wrote: Hello, everyone I'm new there and writing this letter during advice of Dalibor Topic I faced an issue with SOE in ThreadLocal and want to add this java9 annotation on ThreadLocal#set. Does anyone have any concerns about it? This annotation is for very core/critical areas that are particularly sensitive to SO. Did the SO that you observe just happen to be in ThreadLocal.set or is there more to this story? Asking because there are dozens places in java.base where the argument could be made but we have to be very careful to not sprinkle it about. -Alab
Re: RFR: 8316998: Remove redundant type arguments in the java.util.stream package [v3]
On Fri, 29 Sep 2023 00:21:52 GMT, Mourad Abbay wrote: > Are you referring to the expand of lambdas and the extra whitespaces ? Yes, sometimes the IDE just does it automatically! - PR Comment: https://git.openjdk.org/jdk/pull/15936#issuecomment-1740160022
Re: RFR: 8316998: Remove redundant type arguments in the java.util.stream package [v3]
On Fri, 29 Sep 2023 00:35:08 GMT, Paul Sandoz wrote: > > Are you referring to the expand of lambdas and the extra whitespaces ? > > Yes, sometimes the IDE just does it automatically! Is there a way I can make the IDE respect the code style? - PR Comment: https://git.openjdk.org/jdk/pull/15936#issuecomment-1740161157
Re: RFR: 8316998: Remove redundant type arguments in the java.util.stream package [v3]
On Fri, 29 Sep 2023 00:37:16 GMT, Mourad Abbay wrote: > > > Are you referring to the expand of lambdas and the extra whitespaces ? > > > > > > Yes, sometimes the IDE just does it automatically! > > Is there a way I can make the IDE respect the code style? I doubt it, since 1) there is no agreed on code style in OpenJDK (although there have been attempts to propose one); and 2) each source file might have its own style (and there may even be inconsistencies within the same file). - PR Comment: https://git.openjdk.org/jdk/pull/15936#issuecomment-1740165050
Re: RFR: 8317283: jpackage tests run osx-specific checks on windows and linux
On Thu, 28 Sep 2023 23:38:51 GMT, Alexey Semenyuk wrote: > - Don't run osx specific checks on Linux and Windows > - Rework checks output from > > [17:31:52.845] TRACE: assertTrue(): Unexptected value in app image file for > > [17:31:52.860] TRACE: assertTrue(): Unexptected value in app image file for > > > to > > [22:07:46.519] TRACE: assertEquals(false): Check for unexptected value in app > image file for > [22:07:46.519] TRACE: assertEquals(false): Check for unexptected value in app > image file for Looks good. - Marked as reviewed by almatvee (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15975#pullrequestreview-1649891012
Re: RFR: 8316998: Remove redundant type arguments in the java.util.stream package [v3]
On Thu, 28 Sep 2023 22:05:15 GMT, Paul Sandoz wrote: > Looks good. Note a general rule to mostly follow is to try and stick to the > prevailing style in source being modified and not make additional and > unrelated changes (no matter how tempting it might be). In this case i don't > have strong opinions on those changes and they did not detract from reviewing > the code. Are you referring to the expand of lambdas and the extra whitespaces ? - PR Comment: https://git.openjdk.org/jdk/pull/15936#issuecomment-1740152170
Integrated: JDK-8316559: Refactor some util/Calendar tests to JUnit
On Wed, 20 Sep 2023 23:20:43 GMT, Justin Lu wrote: > Please review this PR which converts some tests under _Calendar_ to use > JUnit. These tests either previously used the internal _IntlTest_, or used no > framework at all. > > Any files named BugXXX.java will be renamed after review. This pull request has now been integrated. Changeset: 355811a9 Author:Justin Lu URL: https://git.openjdk.org/jdk/commit/355811a996544c54cde9ff232450f5e5c8e1e632 Stats: 813 lines in 10 files changed: 339 ins; 259 del; 215 mod 8316559: Refactor some util/Calendar tests to JUnit Reviewed-by: naoto, lancea - PR: https://git.openjdk.org/jdk/pull/15853
RFR: 8317283: jpackage tests run osx-specific checks on windows and linux
- Don't run osx specific checks on Linux and Windows - Rework checks output from [17:31:52.845] TRACE: assertTrue(): Unexptected value in app image file for [17:31:52.860] TRACE: assertTrue(): Unexptected value in app image file for to [22:07:46.519] TRACE: assertEquals(false): Check for unexptected value in app image file for [22:07:46.519] TRACE: assertEquals(false): Check for unexptected value in app image file for - Commit messages: - 8317283: jpackage tests run osx-specific checks on windows and linux Changes: https://git.openjdk.org/jdk/pull/15975/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15975=00 Issue: https://bugs.openjdk.org/browse/JDK-8317283 Stats: 11 lines in 1 file changed: 4 ins; 0 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/15975.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15975/head:pull/15975 PR: https://git.openjdk.org/jdk/pull/15975
Integrated: 8317264: Pattern.Bound has `static` fields that should be `static final`.
On Thu, 28 Sep 2023 17:20:47 GMT, Eamonn McManus wrote: > It looks to have been an oversight that `final` was omitted. The fields are > never assigned after initialization. `final` leads to shorter bytecode. This pull request has now been integrated. Changeset: ecb5e8a0 Author:Eamonn McManus URL: https://git.openjdk.org/jdk/commit/ecb5e8a03f67c92d7956201de1fa7d07cc6af9cb Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod 8317264: Pattern.Bound has `static` fields that should be `static final`. Reviewed-by: psandoz - PR: https://git.openjdk.org/jdk/pull/15967
Re: RFR: 8288899: java/util/concurrent/ExecutorService/CloseTest.java failed with "InterruptedException: sleep interrupted" [v43]
> 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. Doug Lea has updated the pull request incrementally with one additional commit since the last revision: Undo stray edit - Changes: - all: https://git.openjdk.org/jdk/pull/14301/files - new: https://git.openjdk.org/jdk/pull/14301/files/85c26d2c..9b714224 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14301=42 - incr: https://webrevs.openjdk.org/?repo=jdk=14301=41-42 Stats: 2 lines in 1 file changed: 1 ins; 1 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/14301.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14301/head:pull/14301 PR: https://git.openjdk.org/jdk/pull/14301
Integrated: 8317119: Remove unused imports in the java.util.stream package
On Wed, 27 Sep 2023 17:45:58 GMT, Mourad Abbay wrote: > Remove unused imports in the java.util.stream package. This pull request has now been integrated. Changeset: f2c221de Author:Mourad Abbay Committer: Paul Sandoz URL: https://git.openjdk.org/jdk/commit/f2c221def1071e3200e502d0c40ace73a1d1967a Stats: 11 lines in 3 files changed: 0 ins; 9 del; 2 mod 8317119: Remove unused imports in the java.util.stream package Reviewed-by: naoto, iris, psandoz - PR: https://git.openjdk.org/jdk/pull/15949
Re: RFR: 8317119: Remove unused imports in the java.util.stream package [v2]
On Wed, 27 Sep 2023 22:44:52 GMT, Mourad Abbay wrote: >> Remove unused imports in the java.util.stream package. > > Mourad Abbay has updated the pull request incrementally with one additional > commit since the last revision: > > Update copyright year. Marked as reviewed by psandoz (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/15949#pullrequestreview-1649773090
Re: RFR: 8317034: Remove redundant type cast in the java.util.stream package [v3]
On Wed, 27 Sep 2023 22:47:43 GMT, Mourad Abbay wrote: >> Remove redundant type cast in the java.util.stream package. > > Mourad Abbay has updated the pull request incrementally with one additional > commit since the last revision: > > Update copyright year. Marked as reviewed by psandoz (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/15942#pullrequestreview-1649773625
Re: RFR: 8316998: Remove redundant type arguments in the java.util.stream package [v3]
On Wed, 27 Sep 2023 22:44:07 GMT, Mourad Abbay wrote: >> Remove cases of redundant type arguments in the java.util.stream package. > > Mourad Abbay has updated the pull request incrementally with one additional > commit since the last revision: > > Update copyright year. Looks good. Note a general rule to mostly follow is to try and stick to the prevailing style in source being modified and not make additional and unrelated changes (no matter how tempting it might be). In this case i don't have strong opinions on those changes and they did not detract from reviewing the code. - Marked as reviewed by psandoz (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15936#pullrequestreview-1649772340
Re: RFR: 8316150: Refactor get chars and string size [v21]
On Sun, 24 Sep 2023 02:46:38 GMT, 温绍锦 wrote: >> 1. Reduce duplicate stringSize code >> 2. Move java.lang.StringLatin1.getChars to >> jdk.internal.util.DecimalDigits::getCharLatin1，not only java.lang, other >> packages also need to use this method > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > restore StringUTF16.getChars(int,int,int,byte[]) and > StringUTF16.getChars(long,int,int,byte[]) There are too many changes that need to be made to reduce the duplication of code in the digits method and the getCharsLatin1. Can we temporarily allow duplicate code for digits and getCharLatin1? Revert changes related to FormatItem. After this PR is completed, submit a new PR to change FormatItem and reduce duplicate code. - PR Comment: https://git.openjdk.org/jdk/pull/15699#issuecomment-1740061373
Re: RFR: 8317264: Pattern.Bound has `static` fields that should be `static final`. [v2]
On Thu, 28 Sep 2023 20:39:13 GMT, Eamonn McManus wrote: >> It looks to have been an oversight that `final` was omitted. The fields are >> never assigned after initialization. `final` leads to shorter bytecode. > > Eamonn McManus 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 two additional > commits since the last revision: > > - Merge branch 'openjdk:master' into staticfinal > - In `Pattern.Bound`, make some constants `static final`. > >It looks to have been an oversight that `final` was omitted. The fields are >never assigned after initialization. `final` leads to shorter bytecode. Marked as reviewed by psandoz (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/15967#pullrequestreview-1649761096
Re: RFR: 8316704: Regex-free parsing of Formatter and FormatProcessor specifiers [v15]
> @cl4es made performance optimizations for the simple specifiers of > String.format in PR https://github.com/openjdk/jdk/pull/2830. Based on the > same idea, I continued to make improvements. I made patterns like %2d %02d > also be optimized. > > The following are the test results based on MacBookPro M1 Pro: > > > -Benchmark Mode Cnt Score Error Units > -StringFormat.complexFormat avgt 15 1862.233 ? 217.479 ns/op > -StringFormat.int02Format avgt 15 312.491 ? 26.021 ns/op > -StringFormat.intFormat avgt 1584.432 ? 4.145 ns/op > -StringFormat.longFormatavgt 1587.330 ? 6.111 ns/op > -StringFormat.stringFormat avgt 1563.985 ? 11.366 ns/op > -StringFormat.stringIntFormat avgt 1587.422 ? 0.147 ns/op > -StringFormat.widthStringFormat avgt 15 250.740 ? 32.639 ns/op > -StringFormat.widthStringIntFormat avgt 15 312.474 ? 16.309 ns/op > > +Benchmark Mode CntScoreError Units > +StringFormat.complexFormat avgt 15 740.626 ? 66.671 ns/op > (+151.45) > +StringFormat.int02Format avgt 15 131.049 ? 0.432 ns/op > (+138.46) > +StringFormat.intFormat avgt 15 67.229 ? 4.155 ns/op > (+25.59) > +StringFormat.longFormatavgt 15 66.444 ? 0.614 ns/op > (+31.44) > +StringFormat.stringFormat avgt 15 62.619 ? 4.652 ns/op (+2.19) > +StringFormat.stringIntFormat avgt 15 89.606 ? 13.966 ns/op (-2.44) > +StringFormat.widthStringFormat avgt 15 52.462 ? 15.649 ns/op > (+377.95) > +StringFormat.widthStringIntFormat avgt 15 101.814 ? 3.147 ns/op > (+206.91) 温绍锦 has updated the pull request incrementally with one additional commit since the last revision: Improve the readability of parseArgument, suggestion from @rgiulietti - Changes: - all: https://git.openjdk.org/jdk/pull/15776/files - new: https://git.openjdk.org/jdk/pull/15776/files/b19dc51f..ad7f3bd1 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15776=14 - incr: https://webrevs.openjdk.org/?repo=jdk=15776=13-14 Stats: 15 lines in 1 file changed: 3 ins; 5 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/15776.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15776/head:pull/15776 PR: https://git.openjdk.org/jdk/pull/15776
Re: RFR: 8316704: Regex-free parsing of Formatter and FormatProcessor specifiers [v14]
On Thu, 28 Sep 2023 21:12:37 GMT, 温绍锦 wrote: >> @cl4es made performance optimizations for the simple specifiers of >> String.format in PR https://github.com/openjdk/jdk/pull/2830. Based on the >> same idea, I continued to make improvements. I made patterns like %2d %02d >> also be optimized. >> >> The following are the test results based on MacBookPro M1 Pro: >> >> >> -Benchmark Mode Cnt Score Error Units >> -StringFormat.complexFormat avgt 15 1862.233 ? 217.479 ns/op >> -StringFormat.int02Format avgt 15 312.491 ? 26.021 ns/op >> -StringFormat.intFormat avgt 1584.432 ? 4.145 ns/op >> -StringFormat.longFormatavgt 1587.330 ? 6.111 ns/op >> -StringFormat.stringFormat avgt 1563.985 ? 11.366 ns/op >> -StringFormat.stringIntFormat avgt 1587.422 ? 0.147 ns/op >> -StringFormat.widthStringFormat avgt 15 250.740 ? 32.639 ns/op >> -StringFormat.widthStringIntFormat avgt 15 312.474 ? 16.309 ns/op >> >> +Benchmark Mode CntScoreError Units >> +StringFormat.complexFormat avgt 15 740.626 ? 66.671 ns/op >> (+151.45) >> +StringFormat.int02Format avgt 15 131.049 ? 0.432 ns/op >> (+138.46) >> +StringFormat.intFormat avgt 15 67.229 ? 4.155 ns/op >> (+25.59) >> +StringFormat.longFormatavgt 15 66.444 ? 0.614 ns/op >> (+31.44) >> +StringFormat.stringFormat avgt 15 62.619 ? 4.652 ns/op >> (+2.19) >> +StringFormat.stringIntFormat avgt 15 89.606 ? 13.966 ns/op >> (-2.44) >> +StringFormat.widthStringFormat avgt 15 52.462 ? 15.649 ns/op >> (+377.95) >> +StringFormat.widthStringIntFormat avgt 15 101.814 ? 3.147 ns/op >> (+206.91) > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > Fix from @rgiulietti review I know, so I'm asking for your opinion, and if you don't agree, I won't submit big changes. There have been a lot of changes now, and I will continue to complete this PR based on the current version. - PR Comment: https://git.openjdk.org/jdk/pull/15776#issuecomment-1740015776
Re: RFR: 8316704: Regex-free parsing of Formatter and FormatProcessor specifiers [v14]
> @cl4es made performance optimizations for the simple specifiers of > String.format in PR https://github.com/openjdk/jdk/pull/2830. Based on the > same idea, I continued to make improvements. I made patterns like %2d %02d > also be optimized. > > The following are the test results based on MacBookPro M1 Pro: > > > -Benchmark Mode Cnt Score Error Units > -StringFormat.complexFormat avgt 15 1862.233 ? 217.479 ns/op > -StringFormat.int02Format avgt 15 312.491 ? 26.021 ns/op > -StringFormat.intFormat avgt 1584.432 ? 4.145 ns/op > -StringFormat.longFormatavgt 1587.330 ? 6.111 ns/op > -StringFormat.stringFormat avgt 1563.985 ? 11.366 ns/op > -StringFormat.stringIntFormat avgt 1587.422 ? 0.147 ns/op > -StringFormat.widthStringFormat avgt 15 250.740 ? 32.639 ns/op > -StringFormat.widthStringIntFormat avgt 15 312.474 ? 16.309 ns/op > > +Benchmark Mode CntScoreError Units > +StringFormat.complexFormat avgt 15 740.626 ? 66.671 ns/op > (+151.45) > +StringFormat.int02Format avgt 15 131.049 ? 0.432 ns/op > (+138.46) > +StringFormat.intFormat avgt 15 67.229 ? 4.155 ns/op > (+25.59) > +StringFormat.longFormatavgt 15 66.444 ? 0.614 ns/op > (+31.44) > +StringFormat.stringFormat avgt 15 62.619 ? 4.652 ns/op (+2.19) > +StringFormat.stringIntFormat avgt 15 89.606 ? 13.966 ns/op (-2.44) > +StringFormat.widthStringFormat avgt 15 52.462 ? 15.649 ns/op > (+377.95) > +StringFormat.widthStringIntFormat avgt 15 101.814 ? 3.147 ns/op > (+206.91) 温绍锦 has updated the pull request incrementally with one additional commit since the last revision: Fix from @rgiulietti review - Changes: - all: https://git.openjdk.org/jdk/pull/15776/files - new: https://git.openjdk.org/jdk/pull/15776/files/3ff5121a..b19dc51f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15776=13 - incr: https://webrevs.openjdk.org/?repo=jdk=15776=12-13 Stats: 12 lines in 1 file changed: 1 ins; 4 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/15776.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15776/head:pull/15776 PR: https://git.openjdk.org/jdk/pull/15776
Re: RFR: 8303959: tools/jpackage/share/RuntimePackageTest.java fails with java.lang.AssertionError missing files
On Thu, 28 Sep 2023 20:58:30 GMT, Alexey Semenyuk wrote: > Don't use JDK image from `$JAVA_HOME` as a value of `--runtime-image` > jpackage cli option. Use jlink to create a runtime in test work dir if the > default runtime is not specified and pass the location of jlink output. Looks good. - Marked as reviewed by almatvee (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15971#pullrequestreview-1649720770
RFR: 8303959: tools/jpackage/share/RuntimePackageTest.java fails with java.lang.AssertionError missing files
Don't use JDK image from `$JAVA_HOME` as a value of `--runtime-image` jpackage cli option. Use jlink to create a runtime in test work dir if the default runtime is not specified and pass the location of jlink output. - Commit messages: - 8303959: tools/jpackage/share/RuntimePackageTest.java fails with java.lang.AssertionError missing files Changes: https://git.openjdk.org/jdk/pull/15971/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15971=00 Issue: https://bugs.openjdk.org/browse/JDK-8303959 Stats: 22 lines in 1 file changed: 18 ins; 1 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/15971.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15971/head:pull/15971 PR: https://git.openjdk.org/jdk/pull/15971
Re: RFR: 8283689: Update the foreign linker VM implementation [v23]
On Tue, 17 May 2022 15:53:05 GMT, Jorn Vernee wrote:
>> Hi,
>>
>> This PR updates the VM implementation of the foreign linker, by bringing
>> over commits from the panama-foreign repo.
>>
>> This is split off from the main JEP integration for 19, since we have
>> limited resources to handle this. As such, this PR might fall over to 20,
>> but it would be nice if we could get it into 19.
>>
>> I've written up an overview of the Linker architecture here:
>> http://cr.openjdk.java.net/~jvernee/docs/FL_Overview.html it might be useful
>> to read that first.
>>
>> This patch moves from the "legacy" implementation, to what is currently
>> implemented in the panama-foreign repo, except for replacing the use of
>> method handle combinators with ASM. That will come in a later path. To
>> recap. This PR contains the following changes:
>>
>> 1. VM stubs for downcalls are now generated up front, instead of lazily by
>> C2 [1].
>> 2. the VM support for upcalls/downcalls now support all possible call
>> shapes. And VM stubs and Java code implementing the buffered invocation
>> strategy has been removed [2], [3], [4], [5].
>> 3. The existing C2 intrinsification support for the `linkToNative` method
>> handle linker was no longer needed and has been removed [6] (support might
>> be re-added in another form later).
>> 4. Some other cleanups, such as: OptimizedEntryBlob (for upcalls) now
>> implements RuntimeBlob directly. Binding to java classes has been rewritten
>> to use javaClasses.h/cpp (this wasn't previously possible due to these java
>> classes being in an incubator module) [7], [8], [9].
>>
>> While the patch mostly consists of VM changes, there are also some Java
>> changes to support (2).
>>
>> The original commit structure has been mostly retained, so it might be
>> useful to look at a specific commit, or the corresponding patch in the
>> [panama-foreign](https://github.com/openjdk/panama-foreign/pulls?q=is%3Apr)
>> repo as well. I've also left some inline comments to explain some of the
>> changes, which will hopefully make reviewing easier.
>>
>> Testing: Tier1-4
>>
>> Thanks,
>> Jorn
>>
>> [1]:
>> https://github.com/openjdk/jdk/pull/7959/commits/048b88156814579dca1f70742061ad24942fd358
>> [2]:
>> https://github.com/openjdk/jdk/pull/7959/commits/2fbbef472b4c2b4fee5ede2f18cd81ab61e88f49
>> [3]:
>> https://github.com/openjdk/jdk/pull/7959/commits/8a957a4ed9cc8d1f708ea8777212eb51ab403dc3
>> [4]:
>> https://github.com/openjdk/jdk/pull/7959/commits/35ba1d964f1de4a77345dc58debe0565db4b0ff3
>> [5]:
>> https://github.com/openjdk/jdk/pull/7959/commits/4e72aae22920300c5ffa16fed805b62ed9092120
>> [6]: https://github
>
> Jorn Vernee has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains 105 commits:
>
> - Merge branch 'master' into JEP-19-VM-IMPL2
> - ifdef NOT_PRODUCT -> ifndef PRODUCT
> - Missing ASSERT -> NOT_PRODUCT
> - Cleanup UL usage
> - Fix failure with SPEC disabled (accidentally dropped change)
> - indentation
> - fix space
> - Merge branch 'master' into JEP-19-VM-IMPL2
> - Undo spurious changes.
> - Merge branch 'JEP-19-VM-IMPL2' of https://github.com/JornVernee/jdk into
> JEP-19-VM-IMPL2
> - ... and 95 more: https://git.openjdk.org/jdk/compare/af07919e...c3c1421b
src/hotspot/cpu/aarch64/universalNativeInvoker_aarch64.cpp line 105:
> 103:
> 104: RuntimeStub* stub =
> 105: RuntimeStub::new_runtime_stub("nep_invoker_blob",
Is it acceptable for a VM fatal error to occur when the `RuntimeStub` cannot be
allocated due to a (temporarily?) full code cache? If not, then you may want to
do something like I'm doing in https://github.com/openjdk/jdk/pull/15970.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/7959#discussion_r1340644955
Re: RFR: 5066247: Refine the spec of equals() and hashCode() for j.text.Format classes [v7]
> Please review this PR and [CSR](https://bugs.openjdk.org/browse/JDK-8315720) > which refines the spec of `equals()` and `hashCode()` in `java.text.Format` > related classes. > > The current spec for most of these methods is either "_Overrides > _" or are incomplete/wrong (i.e. see `ChoiceFormat`). > > This fix adjusts the spec to provide a consistent definition for the > overridden methods and specify what is being compared/used to generate a hash > code value. > > For implementations that use at most a few fields, the values are stated, > otherwise a more general term is used as a substitution (i.e. see > `DecimalFormat`). Justin Lu has updated the pull request incrementally with one additional commit since the last revision: Implement CSR review and WS fix in CompactNumberFormat - Changes: - all: https://git.openjdk.org/jdk/pull/15459/files - new: https://git.openjdk.org/jdk/pull/15459/files/b4e7ddd1..d64e3f6c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15459=06 - incr: https://webrevs.openjdk.org/?repo=jdk=15459=05-06 Stats: 57 lines in 9 files changed: 9 ins; 10 del; 38 mod Patch: https://git.openjdk.org/jdk/pull/15459.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15459/head:pull/15459 PR: https://git.openjdk.org/jdk/pull/15459
Re: RFR: 8317264: Pattern.Bound has `static` fields that should be `static final`. [v2]
> It looks to have been an oversight that `final` was omitted. The fields are > never assigned after initialization. `final` leads to shorter bytecode. Eamonn McManus 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 two additional commits since the last revision: - Merge branch 'openjdk:master' into staticfinal - In `Pattern.Bound`, make some constants `static final`. It looks to have been an oversight that `final` was omitted. The fields are never assigned after initialization. `final` leads to shorter bytecode. - Changes: - all: https://git.openjdk.org/jdk/pull/15967/files - new: https://git.openjdk.org/jdk/pull/15967/files/0a63d3f6..a10cf868 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15967=01 - incr: https://webrevs.openjdk.org/?repo=jdk=15967=00-01 Stats: 255 lines in 11 files changed: 209 ins; 2 del; 44 mod Patch: https://git.openjdk.org/jdk/pull/15967.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15967/head:pull/15967 PR: https://git.openjdk.org/jdk/pull/15967
Re: ReservedStackAccess in ThreadLocal
Hello, Alan I’ve experienced SO in ThreadLocal#set on my production instance (more than that it‘s happened in Spring’s ExposeInvocationInterceptor that uses TL as context storage)As a result of SO in ThreadLocal we’ve experienced unexpected errors with Spring AOP, but I’m sure that it could affects lots of frameworks > > 28 сент. 2023 г., в 19:55, Alan Bateman написал(а): > > On 28/09/2023 17:21, Егор Зиборов wrote: >> Hello, everyone >> >> I'm new there and writing this letter during advice of Dalibor Topic >> >> I faced an issue with SOE in ThreadLocal and want to add this java9 >> annotation on ThreadLocal#set. Does anyone have any concerns about it? >> > This annotation is for very core/critical areas that are particularly > sensitive to SO. Did the SO that you observe just happen to be in > ThreadLocal.set or is there more to this story? Asking because there are > dozens places in java.base where the argument could be made but we have to be > very careful to not sprinkle it about. > > -Alab >
Re: RFR: 8288899: java/util/concurrent/ExecutorService/CloseTest.java failed with "InterruptedException: sleep interrupted" [v42]
> 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. Doug Lea has updated the pull request incrementally with one additional commit since the last revision: Streamline push; add redundant interrupts - Changes: - all: https://git.openjdk.org/jdk/pull/14301/files - new: https://git.openjdk.org/jdk/pull/14301/files/8b12..85c26d2c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14301=41 - incr: https://webrevs.openjdk.org/?repo=jdk=14301=40-41 Stats: 101 lines in 2 files changed: 19 ins; 37 del; 45 mod Patch: https://git.openjdk.org/jdk/pull/14301.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14301/head:pull/14301 PR: https://git.openjdk.org/jdk/pull/14301
Re: RFR: 8288899: java/util/concurrent/ExecutorService/CloseTest.java failed with "InterruptedException: sleep interrupted" [v41]
> 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. Doug Lea 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 88 additional commits since the last revision: - Merge branch 'openjdk:master' into JDK-8288899 - Possibly re-interrupt when stopping - more conservative resize checks - Merge branch 'openjdk:master' into JDK-8288899 - Ensure publishabliity on resize - Fix and improve windowing - Accommodate parallelism 0 - Merge branch 'openjdk:master' into JDK-8288899 - Fix header; don't shadow park state - Merge branch 'openjdk:master' into JDK-8288899 - ... and 78 more: https://git.openjdk.org/jdk/compare/1e78ac82...8b12 - Changes: - all: https://git.openjdk.org/jdk/pull/14301/files - new: https://git.openjdk.org/jdk/pull/14301/files/ed3b7493..8b12 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14301=40 - incr: https://webrevs.openjdk.org/?repo=jdk=14301=39-40 Stats: 6225 lines in 223 files changed: 5095 ins; 552 del; 578 mod Patch: https://git.openjdk.org/jdk/pull/14301.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14301/head:pull/14301 PR: https://git.openjdk.org/jdk/pull/14301
Re: RFR: 8317126: Redundant entries in Windows `tzmappings` file
On Thu, 28 Sep 2023 17:37:00 GMT, Naoto Sato wrote: > Removing redundant entries in `lib/tzmappings` file on Windows. The file maps > Windows time zones to Java time zones according to the region. Since `001` > means world, no region-specific entries are needed if those time zones are > the same. The diff of the generated tzmappings files, before and after the > fix, is attached. > [tzmappings.diff.txt](https://github.com/openjdk/jdk/files/12752377/tzmappings.diff.txt) Marked as reviewed by joehw (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/15968#pullrequestreview-1649478174
Re: RFR: 8317126: Redundant entries in Windows `tzmappings` file
On Thu, 28 Sep 2023 17:37:00 GMT, Naoto Sato wrote: > Removing redundant entries in `lib/tzmappings` file on Windows. The file maps > Windows time zones to Java time zones according to the region. Since `001` > means world, no region-specific entries are needed if those time zones are > the same. The diff of the generated tzmappings files, before and after the > fix, is attached. > [tzmappings.diff.txt](https://github.com/openjdk/jdk/files/12752377/tzmappings.diff.txt) Marked as reviewed by iris (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/15968#pullrequestreview-1649468588
Re: RFR: JDK-8316559: Refactor some util/Calendar tests to JUnit [v6]
> Please review this PR which converts some tests under _Calendar_ to use > JUnit. These tests either previously used the internal _IntlTest_, or used no > framework at all. > > Any files named BugXXX.java will be renamed after review. Justin Lu has updated the pull request incrementally with one additional commit since the last revision: Revert "rename files from bugXXX to something more descriptive" Renaming the files messes up the version history. This reverts commit a65b30987ba030c7698351b3afbc328b4eeb797b. - Changes: - all: https://git.openjdk.org/jdk/pull/15853/files - new: https://git.openjdk.org/jdk/pull/15853/files/a65b3098..4e60ce55 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15853=05 - incr: https://webrevs.openjdk.org/?repo=jdk=15853=04-05 Stats: 18 lines in 9 files changed: 0 ins; 0 del; 18 mod Patch: https://git.openjdk.org/jdk/pull/15853.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15853/head:pull/15853 PR: https://git.openjdk.org/jdk/pull/15853
Integrated: 8316879: RegionMatches1Tests fails if CompactStrings are disabled after JDK-8302163
On Mon, 25 Sep 2023 15:52:12 GMT, Aleksei Voitylov wrote: > test java.lang.String.RegionMatches1Tests fails on all platforms with > -XX:-CompactStrings option and on ARM32 where Compact Strings is disabled by > default. The fix is to return true immediately if len is negative, since for > negative length this condition will never be satisfied. > > Testing: JCK, JTREG passed with the fix with -XX:-CompactStrings on x86_64 > and on ARM32. This pull request has now been integrated. Changeset: cfcbfc6c Author:Aleksei Voitylov Committer: Roger Riggs URL: https://git.openjdk.org/jdk/commit/cfcbfc6cae7d8fc276c5a54917e97adea7cf5621 Stats: 29 lines in 2 files changed: 21 ins; 1 del; 7 mod 8316879: RegionMatches1Tests fails if CompactStrings are disabled after JDK-8302163 Reviewed-by: simonis, rgiulietti, rriggs - PR: https://git.openjdk.org/jdk/pull/15906
Re: RFR: JDK-8316696: Remove the testing base classes: IntlTest and CollatorTest [v3]
> Please review this PR which removes the i18n related testing base classes > `IntlTest` and `CollatorTest` and converts all the tests that use them, > > IntlTest and CollatorTest are testing classes which are extended by tests in > `text/`, `util/Locale`, `util/TimeZone`, and `util/Calendar`. The abstract > testing classes are quite dated and have caused issues such as: variation > between OS, hiding stack trace, and causing tests to spuriously pass. > > This change mainly automates a low level conversion of all the tests (75) > using the frameworks; all tests were converted to use JUnit instead. (With > the exception of `DateFormatRoundTripTest` due to the nature of the test). > > The main changes can be viewed in the following commits > > scripted changes - [c0ece01 > ](https://github.com/openjdk/jdk/commit/c0ece01e91479a020d5c6dce937dc827472b763b) > - Converts the IntlTest methods logln, log, err, and errln > - Adds the JUnit Test annotation to tests that should be ran > - Adjusts the Jtreg tags accordingly > - Remove the main method > - Insert initAll() methods for tests that previously adjusted the JVM default > Locale/TimeZone in the main method > > manual changes - > [9a54910](https://github.com/openjdk/jdk/commit/9a5491065a94a4dc7a05194f3b8330efba8077b7) > - Some tests that had extensive logic in the main method or did not follow > the general IntlTest format had to be manually adjusted > - Also clarified some tests that had optional argument setup in the main > method (now removed) > > removal of IntlTest and CollatorTest - > [8ee9f9c](https://github.com/openjdk/jdk/commit/8ee9f9c79f79210ee6244186970d87a1cac05556) > - Removes both the test classes. > - Replaces CollatorTest with CollatorTestUtils > > Tiers 1-3 clean Justin Lu has updated the pull request incrementally with one additional commit since the last revision: cleanup util classes in text/testlib - Changes: - all: https://git.openjdk.org/jdk/pull/15954/files - new: https://git.openjdk.org/jdk/pull/15954/files/f90266f4..3c676794 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15954=02 - incr: https://webrevs.openjdk.org/?repo=jdk=15954=01-02 Stats: 40 lines in 3 files changed: 20 ins; 2 del; 18 mod Patch: https://git.openjdk.org/jdk/pull/15954.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15954/head:pull/15954 PR: https://git.openjdk.org/jdk/pull/15954
Re: RFR: 8317126: Redundant entries in Windows `tzmappings` file
On Thu, 28 Sep 2023 17:37:00 GMT, Naoto Sato wrote: > Removing redundant entries in `lib/tzmappings` file on Windows. The file maps > Windows time zones to Java time zones according to the region. Since `001` > means world, no region-specific entries are needed if those time zones are > the same. The diff of the generated tzmappings files, before and after the > fix, is attached. > [tzmappings.diff.txt](https://github.com/openjdk/jdk/files/12752377/tzmappings.diff.txt) Marked as reviewed by lancea (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/15968#pullrequestreview-1649440371
RFR: 8317126: Redundant entries in Windows `tzmappings` file
Removing redundant entries in `lib/tzmappings` file on Windows. The file maps Windows time zones to Java time zones according to the region. Since `001` means world, no region-specific entries are needed if those time zones are the same. The diff of the generated tzmappings files, before and after the fix, is attached. [tzmappings.diff.txt](https://github.com/openjdk/jdk/files/12752377/tzmappings.diff.txt) - Commit messages: - initial commit Changes: https://git.openjdk.org/jdk/pull/15968/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15968=00 Issue: https://bugs.openjdk.org/browse/JDK-8317126 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/15968.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15968/head:pull/15968 PR: https://git.openjdk.org/jdk/pull/15968
RFR: 8317264: In `Pattern.Bound`, make some constants `static final`
It looks to have been an oversight that `final` was omitted. The fields are never assigned after initialization. `final` leads to shorter bytecode. - Commit messages: - In `Pattern.Bound`, make some constants `static final`. Changes: https://git.openjdk.org/jdk/pull/15967/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15967=00 Issue: https://bugs.openjdk.org/browse/JDK-8317264 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/15967.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15967/head:pull/15967 PR: https://git.openjdk.org/jdk/pull/15967
Re: RFR: 8303374: Compiler Implementation for Primitive types in patterns, instanceof, and switch (Preview) [v2]
On Thu, 28 Sep 2023 16:46:11 GMT, Aggelos Biboudis wrote: >> This is the first draft of a patch for Primitive types in patterns, >> instanceof, and switch (Preview). >> >> Draft spec here: >> https://cr.openjdk.org/~abimpoudis/instanceof/instanceof-20230913/specs/instanceof-jls.html > > Aggelos Biboudis has updated the pull request incrementally with one > additional commit since the last revision: > > Apply suggestions from code review > > Co-authored-by: Raffaello Giulietti At the moment the translation is uniform, (simply) computing the method name from the static types of the arguments and generating calls to those methods. This happens from two places, one from the `instanceof`: > https://github.com/openjdk/jdk/pull/15638/files#diff-bc0df6dce7f74078bfca1e90bec75d7bb3d8b338933be725da78f0a8a7a0c78dR2999 and one from the `SwitchBootstraps` for `switch` (open the SwitchBootstraps file to see the diff, Github doesn't open it automatically): > https://github.com/openjdk/jdk/pull/15638/files#diff-d71d82d68cc87a9c7179421f4a57eacdba822eb3e6a2b413ddb35f0b891a3764R246 I will try to find a good way to share code between the two places, by avoiding the further duplication of the code (@mcimadamore, @vicente-romero-oracle, @lahodaj any ideas?). In any case (with code duplication or not) maybe I can introduce a small table to "string intern" those names and avoid this computation altogether (including the map that you wrote above). - PR Comment: https://git.openjdk.org/jdk/pull/15638#issuecomment-1739759884
Re: RFR: 8316879: RegionMatches1Tests fails if CompactStrings are disabled after JDK-8302163 [v6]
On Wed, 27 Sep 2023 14:13:05 GMT, Aleksei Voitylov wrote: >> test java.lang.String.RegionMatches1Tests fails on all platforms with >> -XX:-CompactStrings option and on ARM32 where Compact Strings is disabled by >> default. The fix is to return true immediately if len is negative, since for >> negative length this condition will never be satisfied. >> >> Testing: JCK, JTREG passed with the fix with -XX:-CompactStrings on x86_64 >> and on ARM32. > > Aleksei Voitylov has updated the pull request incrementally with one > additional commit since the last revision: > > address review comments Thanks everyone for prompt reviews! Could someone sponsor this? I also intend to backport to 21u soon. - PR Comment: https://git.openjdk.org/jdk/pull/15906#issuecomment-1739739777
Re: RFR: 8316971: Add Lint warning for restricted method calls [v2]
On Thu, 28 Sep 2023 15:36:30 GMT, Maurizio Cimadamore
wrote:
>> This patch adds a new lint warning category, namely `-Xlint:restricted` to
>> enable warnings on restricted method calls.
>>
>> The patch is relatively straightforward: javac marks methods that are marked
>> with the `@Restricted` annotation with a corresponding internal flag. This
>> is done both in `Annotate` when compiling JDK from source, and in
>> `ClassReader` when JDK classfiles are read. When calls to methods marked
>> with the special flag are found, a new warning is issued.
>>
>> While there are some similarities between this new warning and the preview
>> API warnings, the compiler does *not* emit a mandatory note when a
>> compilation unit is found to have one or more restricted method calls. In
>> other words, this is just a plain lint warning.
>>
>> The output from javac looks as follows:
>>
>>
>> Foo.java:6: warning: [restricted] MemorySegment.reinterpret(long) is a
>> restricted method.
>> Arena.ofAuto().allocate(10).reinterpret(100);
>> ^
>> (Restricted methods are unsafe, and, if used incorrectly, they might crash
>> the JVM or result in memory corruption)
>
> Maurizio Cimadamore has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Update warning message
test/langtools/tools/javac/RestrictedMethods.java line 34:
> 32: }
> 33: }
> 34: }
shouldn't this test include a case using method references? For example:
void m(MemorySegment m) {
foo(m::reinterpret);
}
void foo(LongFunction f) {}
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15964#discussion_r1340470730
Re: RFR: JDK-8316696: Remove the testing base classes: IntlTest and CollatorTest [v2]
> Please review this PR which removes the i18n related testing base classes > `IntlTest` and `CollatorTest` and converts all the tests that use them, > > IntlTest and CollatorTest are testing classes which are extended by tests in > `text/`, `util/Locale`, `util/TimeZone`, and `util/Calendar`. The abstract > testing classes are quite dated and have caused issues such as: variation > between OS, hiding stack trace, and causing tests to spuriously pass. > > This change mainly automates a low level conversion of all the tests (75) > using the frameworks; all tests were converted to use JUnit instead. (With > the exception of `DateFormatRoundTripTest` due to the nature of the test). > > The main changes can be viewed in the following commits > > scripted changes - [c0ece01 > ](https://github.com/openjdk/jdk/commit/c0ece01e91479a020d5c6dce937dc827472b763b) > - Converts the IntlTest methods logln, log, err, and errln > - Adds the JUnit Test annotation to tests that should be ran > - Adjusts the Jtreg tags accordingly > - Remove the main method > - Insert initAll() methods for tests that previously adjusted the JVM default > Locale/TimeZone in the main method > > manual changes - > [9a54910](https://github.com/openjdk/jdk/commit/9a5491065a94a4dc7a05194f3b8330efba8077b7) > - Some tests that had extensive logic in the main method or did not follow > the general IntlTest format had to be manually adjusted > - Also clarified some tests that had optional argument setup in the main > method (now removed) > > removal of IntlTest and CollatorTest - > [8ee9f9c](https://github.com/openjdk/jdk/commit/8ee9f9c79f79210ee6244186970d87a1cac05556) > - Removes both the test classes. > - Replaces CollatorTest with CollatorUtils > > Tiers 1-3 clean Justin Lu has updated the pull request incrementally with one additional commit since the last revision: CollatorUtils -> CollatorTestUtils - Changes: - all: https://git.openjdk.org/jdk/pull/15954/files - new: https://git.openjdk.org/jdk/pull/15954/files/8ee9f9c7..f90266f4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15954=01 - incr: https://webrevs.openjdk.org/?repo=jdk=15954=00-01 Stats: 50 lines in 16 files changed: 3 ins; 0 del; 47 mod Patch: https://git.openjdk.org/jdk/pull/15954.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15954/head:pull/15954 PR: https://git.openjdk.org/jdk/pull/15954
Re: RFR: 8303374: Compiler Implementation for Primitive types in patterns, instanceof, and switch (Preview) [v2]
On Thu, 28 Sep 2023 16:46:11 GMT, Aggelos Biboudis wrote: >> This is the first draft of a patch for Primitive types in patterns, >> instanceof, and switch (Preview). >> >> Draft spec here: >> https://cr.openjdk.org/~abimpoudis/instanceof/instanceof-20230913/specs/instanceof-jls.html > > Aggelos Biboudis has updated the pull request incrementally with one > additional commit since the last revision: > > Apply suggestions from code review > > Co-authored-by: Raffaello Giulietti There might be good reasons to have: - `byte_char()` and `short_char()` in addition to `int_char()` - `short_byte()` and `char_byte()` in addition to `int_byte()` - `char_short()` in addition to `int_short()` but I cannot see the rationale. Maybe due to the language rules? Or are they needed for the implementation of patterns, instanceof and switch? - PR Comment: https://git.openjdk.org/jdk/pull/15638#issuecomment-1739727897
Re: ReservedStackAccess in ThreadLocal
On 28/09/2023 17:21, Егор Зиборов wrote: Hello, everyone I'm new there and writing this letter during advice of Dalibor Topic I faced an issue with SOE in ThreadLocal and want to add this java9 annotation on ThreadLocal#set. Does anyone have any concerns about it? This annotation is for very core/critical areas that are particularly sensitive to SO. Did the SO that you observe just happen to be in ThreadLocal.set or is there more to this story? Asking because there are dozens places in java.base where the argument could be made but we have to be very careful to not sprinkle it about. -Alab
Re: RFR: 8303374: Compiler Implementation for Primitive types in patterns, instanceof, and switch (Preview) [v2]
> This is the first draft of a patch for Primitive types in patterns, > instanceof, and switch (Preview). > > Draft spec here: > https://cr.openjdk.org/~abimpoudis/instanceof/instanceof-20230913/specs/instanceof-jls.html Aggelos Biboudis has updated the pull request incrementally with one additional commit since the last revision: Apply suggestions from code review Co-authored-by: Raffaello Giulietti - Changes: - all: https://git.openjdk.org/jdk/pull/15638/files - new: https://git.openjdk.org/jdk/pull/15638/files/d354f7eb..1ae06ac8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15638=01 - incr: https://webrevs.openjdk.org/?repo=jdk=15638=00-01 Stats: 7 lines in 1 file changed: 0 ins; 0 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/15638.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15638/head:pull/15638 PR: https://git.openjdk.org/jdk/pull/15638
ReservedStackAccess in ThreadLocal
Hello, everyone I'm new there and writing this letter during advice of Dalibor Topic I faced an issue with SOE in ThreadLocal and want to add this java9 annotation on ThreadLocal#set. Does anyone have any concerns about it? Thank you in advance
Integrated: 8316974: ListFormat creation is unsuccessful for some of the supported Locales
On Tue, 26 Sep 2023 21:49:11 GMT, Naoto Sato wrote: > Some CLDR locales have partial list patterns, such as only the "end" pattern, > and expect "start" and "middle" patterns to be inherited from parent locales. > Made the code capable of the inheritance. This pull request has now been integrated. Changeset: 3481a485 Author:Naoto Sato URL: https://git.openjdk.org/jdk/commit/3481a485716a1949706a4dcb94181b07e88e804d Stats: 53 lines in 3 files changed: 47 ins; 0 del; 6 mod 8316974: ListFormat creation is unsuccessful for some of the supported Locales Reviewed-by: joehw, rriggs - PR: https://git.openjdk.org/jdk/pull/15935
Re: RFR: 8308753: Class-File API transition to Preview [v2]
On Tue, 26 Sep 2023 12:32:37 GMT, Adam Sotona wrote: >> Classfile API is an internal library under package `jdk.internal.classfile` >> in JDK 21. >> This pull request turns the Classfile API into a preview feature and moves >> it into `java.lang.classfile`. >> It repackages all uses across JDK and tests and adds lots of missing Javadoc. >> >> This PR goes in sync with >> [JDK-8308754](https://bugs.openjdk.org/browse/JDK-8308754): Class-File API >> (Preview) (CSR) >> and [JDK-8280389](https://bugs.openjdk.org/browse/JDK-8280389): Class-File >> API (Preview) (JEP). >> >> Online javadoc is available at: >> https://cr.openjdk.org/~asotona/JDK-8308753-preview/api/java.base/java/lang/classfile/package-summary.html >> >> In addition to the primary transition to preview, this pull request also >> includes: >> - All Classfile* classes ranamed to ClassFile* (based on JEP discussion). >> - A new preview feature, `CLASSFILE_API`, has been added. >> - Buildsystem tool required a little patch to support annotated modules. >> - All JDK modules using the Classfile API are newly participating in the >> preview. >> - All tests that use the Classfile API now have preview enabled. >> - A few Javac tests not allowing preview have been partially reverted; their >> conversion can be re-applied when the Classfile API leaves preview mode. >> >> Despite the number of affected files, this pull request is relatively >> straight-forward. The preview version of the Classfile API is based on the >> internal version of the library from the JDK master branch, and there are no >> API features added. >> >> Please review this pull request to help the Classfile API turn into a >> preview. >> >> Any comments are welcome. >> >> Thanks, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > PreviewFeature annotated with JEP 457 I don't see anything specific to `javadoc` - PR Review: https://git.openjdk.org/jdk/pull/15706#pullrequestreview-1649221047
Re: RFR: 8316971: Add Lint warning for restricted method calls [v2]
On Thu, 28 Sep 2023 15:36:30 GMT, Maurizio Cimadamore wrote: >> This patch adds a new lint warning category, namely `-Xlint:restricted` to >> enable warnings on restricted method calls. >> >> The patch is relatively straightforward: javac marks methods that are marked >> with the `@Restricted` annotation with a corresponding internal flag. This >> is done both in `Annotate` when compiling JDK from source, and in >> `ClassReader` when JDK classfiles are read. When calls to methods marked >> with the special flag are found, a new warning is issued. >> >> While there are some similarities between this new warning and the preview >> API warnings, the compiler does *not* emit a mandatory note when a >> compilation unit is found to have one or more restricted method calls. In >> other words, this is just a plain lint warning. >> >> The output from javac looks as follows: >> >> >> Foo.java:6: warning: [restricted] MemorySegment.reinterpret(long) is a >> restricted method. >> Arena.ofAuto().allocate(10).reinterpret(100); >> ^ >> (Restricted methods are unsafe, and, if used incorrectly, they might crash >> the JVM or result in memory corruption) > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Update warning message question shouldn't the new Restricted annotation be annotated with the @PreviewFeature annotation? it depends on a preview feature src/jdk.compiler/share/classes/com/sun/tools/javac/code/Flags.java line 393: > 391: > 392: /** > 393: * Flag to indicate sealed class/interface declaration. this javadoc needs to be adjusted to restricted methods - PR Comment: https://git.openjdk.org/jdk/pull/15964#issuecomment-1739605513 PR Review Comment: https://git.openjdk.org/jdk/pull/15964#discussion_r1340351919
Re: RFR: 8316971: Add Lint warning for restricted method calls [v2]
> This patch adds a new lint warning category, namely `-Xlint:restricted` to > enable warnings on restricted method calls. > > The patch is relatively straightforward: javac marks methods that are marked > with the `@Restricted` annotation with a corresponding internal flag. This is > done both in `Annotate` when compiling JDK from source, and in `ClassReader` > when JDK classfiles are read. When calls to methods marked with the special > flag are found, a new warning is issued. > > While there are some similarities between this new warning and the preview > API warnings, the compiler does *not* emit a mandatory note when a > compilation unit is found to have one or more restricted method calls. In > other words, this is just a plain lint warning. > > The output from javac looks as follows: > > > Foo.java:6: warning: [restricted] MemorySegment.reinterpret(long) is a > restricted method. > Arena.ofAuto().allocate(10).reinterpret(100); > ^ > (Restricted methods are unsafe, and, if used incorrectly, they might crash > the JVM or result in memory corruption) Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Update warning message - Changes: - all: https://git.openjdk.org/jdk/pull/15964/files - new: https://git.openjdk.org/jdk/pull/15964/files/8370e79d..21b7d860 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15964=01 - incr: https://webrevs.openjdk.org/?repo=jdk=15964=00-01 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/15964.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15964/head:pull/15964 PR: https://git.openjdk.org/jdk/pull/15964
Re: RFR: 8316971: Add Lint warning for restricted method calls
On Thu, 28 Sep 2023 13:13:31 GMT, Maurizio Cimadamore wrote: > This patch adds a new lint warning category, namely `-Xlint:restricted` to > enable warnings on restricted method calls. > > The patch is relatively straightforward: javac marks methods that are marked > with the `@Restricted` annotation with a corresponding internal flag. This is > done both in `Annotate` when compiling JDK from source, and in `ClassReader` > when JDK classfiles are read. When calls to methods marked with the special > flag are found, a new warning is issued. > > While there are some similarities between this new warning and the preview > API warnings, the compiler does *not* emit a mandatory note when a > compilation unit is found to have one or more restricted method calls. In > other words, this is just a plain lint warning. > > The output from javac looks as follows: > > > Foo.java:6: warning: [restricted] MemorySegment.reinterpret(long) is a > restricted method. > Arena.ofAuto().allocate(10).reinterpret(100); > ^ > (Restricted methods are unsafe, and, if used incorrectly, they might crash > the JVM or result in memory corruption) Build changes look trivially fine. - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15964#pullrequestreview-1649137432
Re: RFR: 8316704: Regex-free parsing of Formatter and FormatProcessor specifiers [v13]
On Wed, 27 Sep 2023 21:26:46 GMT, 温绍锦 wrote: >> @cl4es made performance optimizations for the simple specifiers of >> String.format in PR https://github.com/openjdk/jdk/pull/2830. Based on the >> same idea, I continued to make improvements. I made patterns like %2d %02d >> also be optimized. >> >> The following are the test results based on MacBookPro M1 Pro: >> >> >> -Benchmark Mode Cnt Score Error Units >> -StringFormat.complexFormat avgt 15 1862.233 ? 217.479 ns/op >> -StringFormat.int02Format avgt 15 312.491 ? 26.021 ns/op >> -StringFormat.intFormat avgt 1584.432 ? 4.145 ns/op >> -StringFormat.longFormatavgt 1587.330 ? 6.111 ns/op >> -StringFormat.stringFormat avgt 1563.985 ? 11.366 ns/op >> -StringFormat.stringIntFormat avgt 1587.422 ? 0.147 ns/op >> -StringFormat.widthStringFormat avgt 15 250.740 ? 32.639 ns/op >> -StringFormat.widthStringIntFormat avgt 15 312.474 ? 16.309 ns/op >> >> +Benchmark Mode CntScoreError Units >> +StringFormat.complexFormat avgt 15 740.626 ? 66.671 ns/op >> (+151.45) >> +StringFormat.int02Format avgt 15 131.049 ? 0.432 ns/op >> (+138.46) >> +StringFormat.intFormat avgt 15 67.229 ? 4.155 ns/op >> (+25.59) >> +StringFormat.longFormatavgt 15 66.444 ? 0.614 ns/op >> (+31.44) >> +StringFormat.stringFormat avgt 15 62.619 ? 4.652 ns/op >> (+2.19) >> +StringFormat.stringIntFormat avgt 15 89.606 ? 13.966 ns/op >> (-2.44) >> +StringFormat.widthStringFormat avgt 15 52.462 ? 15.649 ns/op >> (+377.95) >> +StringFormat.widthStringIntFormat avgt 15 101.814 ? 3.147 ns/op >> (+206.91) > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > Refactor according to rgiulietti's suggestion and add testcases Meaningful external reviews take a _lot_ of engineering time on such a popular platform as the JDK. They make sense only on somehow stable code, otherwise they are a waste of human resources. So sure, take your time to stabilize your code, make your tests, but please refrain from keep on changing too much once you publish a PR. If reviewers have to face continuous changes, they might become uninterested in reviewing. - PR Comment: https://git.openjdk.org/jdk/pull/15776#issuecomment-1739462969
Re: RFR: 8316704: Regex-free parsing of Formatter and FormatProcessor specifiers [v12]
On Wed, 27 Sep 2023 19:51:25 GMT, Raffaello Giulietti
wrote:
>> 温绍锦 has updated the pull request incrementally with one additional commit
>> since the last revision:
>>
>> fix : the exception thrown when the input does not include conversion is
>> different from baselne.
>
> You might consider this alternative, which IMO is simpler and more readable.
>
>
> int parse() {
> // %[argument_index$][flags][width][.precision][t]conversion
> // %(\d+$)?([-#+ 0,(<]*)?(\d+)?(.\d+)?([tT])?([a-zA-Z%])
> parseArgument();
> parseFlag();
> parseWidth();
> int precisionSize = parsePrecision();
> if (precisionSize < 0) {
> return 0;
> }
>
> // ([tT])?([a-zA-Z%])
> char t = '\0', conversion = '\0';
> if ((c == 't' || c == 'T') && off + 1 < max) {
> char c1 = s.charAt(off + 1);
> if (isConversion(c1)) {
> t = c;
> conversion = c1;
> off += 2;
> }
> }
> if (conversion == '\0' && isConversion(c)) {
> conversion = c;
> ++off;
> }
>
> if (argSize + flagSize + widthSize + precisionSize + t +
> conversion != 0) {
> if (al != null) {
> FormatSpecifier formatSpecifier
> = new FormatSpecifier(s, start, argSize,
> flagSize, widthSize, precisionSize, t, conversion);
> al.add(formatSpecifier);
> }
> return off - start;
> }
> return 0;
> }
>
> private void parseArgument() {
> // (\d+$)?
> int i = off;
> for (; i < max && isDigit(c = s.charAt(i)); ++i); // empty body
> if (i == max || c != '$') {
> c = first;
> return;
> }
> ++i; // skip '$'
> if (i < max) {
> c = s.charAt(i);
> }
> argSize = i - off;
> off = i;
> }
>
> private void parseFlag() {
> // ([-#+ 0,(<]*)?
> int i = off;
> for (; i < max && Flags.isFlag(c = s.charAt(i)); ++i); // empty
> body
> flagSize = i - off;
> off = i;
> }
>
> private void parseWidth() {
> // (\d+)?
> int i = off;
> for (; i < max && isDigit(c = s.charAt(i)); ++i); // empty body
> widthSize = i - off;
> off = i;
> }
>
> private int parsePrecision() {
> // (.\d+)?
> if (c != '.') {
> return 0;
> }
> int i = off + 1;
> for (; i < max && isDigit(c = s.charAt(i)); ++i); // empty bod...
@rgiulietti @cl4es Sorry, I plan to make a change, Now there is part of the
parser code in the constructor of FormatSpecifier. This is not clear. I plan to
move the parser part of the constructor of FormatSpecifier into
FormatSpecifierParser. This will be clearer and the performance will be faster.
* now
FormatSpecifier(
String s,
int i,
int argSize,
int flagSize,
int widthSize,
int precisionSize,
char t,
char conversion
) {
int argEnd = i + argSize;
int flagEnd = argEnd + flagSize;
int widthEnd = flagEnd + widthSize;
int precesionEnd = widthEnd + precisionSize;
if (argSize > 0) {
index(s, i, argEnd);
}
if (flagSize > 0) {
flags(s, argEnd, flagEnd);
}
if (widthSize > 0) {
width(s, flagEnd, widthEnd);
}
if (precisionSize > 0) {
precision(s, widthEnd, precesionEnd);
}
if (t != '\0') {
dt = true;
if (t == 'T') {
flags = Flags.add(flags, Flags.UPPERCASE);
}
}
conversion(conversion);
check();
}
* plan to:
FormatSpecifier(int index, int flags, int width, int precision, char t,
char conversion) {
if (index > 0) {
this.index = index;
}
if (flags > 0) {
this.flags = flags;
if (Flags.contains(flags, Flags.PREVIOUS))
this.index = -1;
}
if (width > 0) {
this.width = width;
}
this.precision = precision;
if (t != '\0') {
dt = true;
if (t == 'T') {
this.flags = Flags.add(flags, Flags.UPPERCASE);
Re: RFR: 8316971: Add Lint warning for restricted method calls
On Thu, 28 Sep 2023 13:13:31 GMT, Maurizio Cimadamore
wrote:
> This patch adds a new lint warning category, namely `-Xlint:restricted` to
> enable warnings on restricted method calls.
>
> The patch is relatively straightforward: javac marks methods that are marked
> with the `@Restricted` annotation with a corresponding internal flag. This is
> done both in `Annotate` when compiling JDK from source, and in `ClassReader`
> when JDK classfiles are read. When calls to methods marked with the special
> flag are found, a new warning is issued.
>
> While there are some similarities between this new warning and the preview
> API warnings, the compiler does *not* emit a mandatory note when a
> compilation unit is found to have one or more restricted method calls. In
> other words, this is just a plain lint warning.
>
> The output from javac looks as follows:
>
>
> Foo.java:6: warning: [restricted] MemorySegment.reinterpret(long) is a
> restricted method.
> Arena.ofAuto().allocate(10).reinterpret(100);
> ^
> (Restricted methods are unsafe, and, if used incorrectly, they might crash
> the JVM or result in memory corruption)
test/langtools/tools/javac/RestrictedMethods.java line 5:
> 3: * @bug 8316971
> 4: * @summary Smoke test for restricted method call warnings
> 5: * @compile/fail/ref=RestrictedMethods.out -Xlint:restricted -Werror
> -XDrawDiagnostics --enable-preview --source ${jdk.version}
> RestrictedMethods.java
The `--enable-preview` related options will need to be removed when FFM is
finalized
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15964#discussion_r1340165979
Re: RFR: 8312522: Implementation of Foreign Function & Memory API [v32]
> This patch contains the implementation of the foreign linker & memory API JEP > for Java 22. The initial patch is composed of commits brought over directly > from the [panama-foreign repo](https://github.com/openjdk/panama-foreign). > The main changes found in this patch come from the following PRs: > > 1. https://github.com/openjdk/panama-foreign/pull/836 Where previous > iterations supported converting Java strings to and from native strings in > the UTF-8 encoding, we've extended the supported encoding to all the > encodings found in the `java.nio.charset.StandardCharsets` class. > 2. https://github.com/openjdk/panama-foreign/pull/838 We dropped the > `MemoryLayout::sequenceLayout` factory method which inferred the size of the > sequence to be `Long.MAX_VALUE`, as this led to confusion among clients. A > client is now required to explicitly specify the sequence size. > 3. https://github.com/openjdk/panama-foreign/pull/839 A new API was added: > `Linker::canonicalLayouts`, which exposes a map containing the > platform-specific mappings of common C type names to memory layouts. > 4. https://github.com/openjdk/panama-foreign/pull/840 Memory access > varhandles, as well as byte offset and slice handles derived from memory > layouts, now feature an additional 'base offset' coordinate that is added to > the offset computed by the handle. This allows composing these handles with > other offset computation strategies that may not be based on the same memory > layout. This addresses use-cases where clients are working with 'dynamic' > layouts, whose size might not be known statically, such as variable length > arrays, or variable size matrices. > 5. https://github.com/openjdk/panama-foreign/pull/841 Remove this now > redundant API. Clients can simply use the difference between the base address > of two memory segments. > 6. https://github.com/openjdk/panama-foreign/pull/845 Disambiguate uses of > `SegmentAllocator::allocateArray`, by renaming methods that both allocate + > initialize memory segments to `allocateFrom`. (see the original PR for the > problematic case) > 7. https://github.com/openjdk/panama-foreign/pull/846 Improve the > documentation for variadic functions. > 8. https://github.com/openjdk/panama-foreign/pull/849 Several test fixes to > make sure the `jdk_foreign` tests can pass on 32-bit machines, taking > linux-x86 as a test bed. > 9. https://github.com/openjdk/panama-foreign/pull/850 Make the linker API > required. The `Linker::nativeLinker` method is not longer allowed to throw an > `UnsupportedOperationException` on ... Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: review @enablePreview from java/foreign/TestRestricted test - Changes: - all: https://git.openjdk.org/jdk/pull/15103/files - new: https://git.openjdk.org/jdk/pull/15103/files/72650c44..17dacbbd Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15103=31 - incr: https://webrevs.openjdk.org/?repo=jdk=15103=30-31 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/15103.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15103/head:pull/15103 PR: https://git.openjdk.org/jdk/pull/15103
Re: RFR: 8316971: Add Lint warning for restricted method calls
On Thu, 28 Sep 2023 13:13:31 GMT, Maurizio Cimadamore wrote: > This patch adds a new lint warning category, namely `-Xlint:restricted` to > enable warnings on restricted method calls. > > The patch is relatively straightforward: javac marks methods that are marked > with the `@Restricted` annotation with a corresponding internal flag. This is > done both in `Annotate` when compiling JDK from source, and in `ClassReader` > when JDK classfiles are read. When calls to methods marked with the special > flag are found, a new warning is issued. > > While there are some similarities between this new warning and the preview > API warnings, the compiler does *not* emit a mandatory note when a > compilation unit is found to have one or more restricted method calls. In > other words, this is just a plain lint warning. > > The output from javac looks as follows: > > > Foo.java:6: warning: [restricted] MemorySegment.reinterpret(long) is a > restricted method. > Arena.ofAuto().allocate(10).reinterpret(100); > ^ > (Restricted methods are unsafe, and, if used incorrectly, they might crash > the JVM or result in memory corruption) make/modules/java.base/Java.gmk line 26: > 24: # > 25: > 26: DISABLED_WARNINGS_java += this-escape restricted I've disabled restricted warnings in java.base as there's a lot of restricted calls in the Linker API implementation :-) make/test/BuildMicrobenchmark.gmk line 94: > 92: SMALL_JAVA := false, \ > 93: CLASSPATH := $(MICROBENCHMARK_CLASSPATH), \ > 94: DISABLED_WARNINGS := restricted this-escape processing rawtypes cast > serial preview, \ This is needed so that we can compile FFM API benchmarks. test/langtools/tools/javac/diags/examples.not-yet.txt line 219: > 217: compiler.err.annotation.unrecognized.attribute.name > 218: > 219: # this one is transitional (waiting for FFM API to exit preview) Note: in principle I could have added an example for this diagnostic. In practice, the fact that FFM is still a preview API makes this a bit difficult - because we need the sample to also have enable preview flags, and also to catch a bunch of preview diagnostics (some of which I can't add directly as they are also excluded on this file). Since this PR already adds a test, I opted to exclude the diagnostic sample for now, and I will come back later (after FFM is no longer preview) to add one (so as to minimize disruption). - PR Review Comment: https://git.openjdk.org/jdk/pull/15964#discussion_r1340137452 PR Review Comment: https://git.openjdk.org/jdk/pull/15964#discussion_r1340137937 PR Review Comment: https://git.openjdk.org/jdk/pull/15964#discussion_r1340142294
RFR: 8316971: Add Lint warning for restricted method calls
This patch adds a new lint warning category, namely `-Xlint:restricted` to enable warnings on restricted method calls. The patch is relatively straightforward: javac marks methods that are marked with the `@Restricted` annotation with a corresponding internal flag. This is done both in `Annotate` when compiling JDK from source, and in `ClassReader` when JDK classfiles are read. When calls to methods marked with the special flag are found, a new warning is issued. While there are some similarities between this new warning and the preview API warnings, the compiler does *not* emit a mandatory note when a compilation unit is found to have one or more restricted method calls. In other words, this is just a plain lint warning. The output from javac looks as follows: Foo.java:6: warning: [restricted] MemorySegment.reinterpret(long) is a restricted method. Arena.ofAuto().allocate(10).reinterpret(100); ^ (Restricted methods are unsafe, and, if used incorrectly, they might crash the JVM or result in memory corruption) - Commit messages: - Add tests - Initial push Changes: https://git.openjdk.org/jdk/pull/15964/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15964=00 Issue: https://bugs.openjdk.org/browse/JDK-8316971 Stats: 95 lines in 14 files changed: 92 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/15964.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15964/head:pull/15964 PR: https://git.openjdk.org/jdk/pull/15964
Re: RFR: 8312522: Implementation of Foreign Function & Memory API [v31]
> This patch contains the implementation of the foreign linker & memory API JEP > for Java 22. The initial patch is composed of commits brought over directly > from the [panama-foreign repo](https://github.com/openjdk/panama-foreign). > The main changes found in this patch come from the following PRs: > > 1. https://github.com/openjdk/panama-foreign/pull/836 Where previous > iterations supported converting Java strings to and from native strings in > the UTF-8 encoding, we've extended the supported encoding to all the > encodings found in the `java.nio.charset.StandardCharsets` class. > 2. https://github.com/openjdk/panama-foreign/pull/838 We dropped the > `MemoryLayout::sequenceLayout` factory method which inferred the size of the > sequence to be `Long.MAX_VALUE`, as this led to confusion among clients. A > client is now required to explicitly specify the sequence size. > 3. https://github.com/openjdk/panama-foreign/pull/839 A new API was added: > `Linker::canonicalLayouts`, which exposes a map containing the > platform-specific mappings of common C type names to memory layouts. > 4. https://github.com/openjdk/panama-foreign/pull/840 Memory access > varhandles, as well as byte offset and slice handles derived from memory > layouts, now feature an additional 'base offset' coordinate that is added to > the offset computed by the handle. This allows composing these handles with > other offset computation strategies that may not be based on the same memory > layout. This addresses use-cases where clients are working with 'dynamic' > layouts, whose size might not be known statically, such as variable length > arrays, or variable size matrices. > 5. https://github.com/openjdk/panama-foreign/pull/841 Remove this now > redundant API. Clients can simply use the difference between the base address > of two memory segments. > 6. https://github.com/openjdk/panama-foreign/pull/845 Disambiguate uses of > `SegmentAllocator::allocateArray`, by renaming methods that both allocate + > initialize memory segments to `allocateFrom`. (see the original PR for the > problematic case) > 7. https://github.com/openjdk/panama-foreign/pull/846 Improve the > documentation for variadic functions. > 8. https://github.com/openjdk/panama-foreign/pull/849 Several test fixes to > make sure the `jdk_foreign` tests can pass on 32-bit machines, taking > linux-x86 as a test bed. > 9. https://github.com/openjdk/panama-foreign/pull/850 Make the linker API > required. The `Linker::nativeLinker` method is not longer allowed to throw an > `UnsupportedOperationException` on ... Jorn Vernee has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 59 commits: - Merge branch 'master' into JEP22 - drop unneeded @compile tags from jtreg tests - Use IAE instead of UOE for unsupported char sets - Use abort instead of IEA when encountering wrong value for ENA attrib. - Fix visibility issues Reviewed-by: mcimadamore - Review comments - fix typos - Tweak support for restricted methods Reviewed-by: jvernee, pminborg - Split note about byte order/alignment out of header - review comments - ... and 49 more: https://git.openjdk.org/jdk/compare/3481ecb2...72650c44 - Changes: https://git.openjdk.org/jdk/pull/15103/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15103=30 Stats: 4352 lines in 258 files changed: 2211 ins; 1190 del; 951 mod Patch: https://git.openjdk.org/jdk/pull/15103.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15103/head:pull/15103 PR: https://git.openjdk.org/jdk/pull/15103
Re: RFR: 8316704: Regex-free parsing of Formatter and FormatProcessor specifiers [v12]
On Wed, 27 Sep 2023 19:13:02 GMT, 温绍锦 wrote: >> @cl4es made performance optimizations for the simple specifiers of >> String.format in PR https://github.com/openjdk/jdk/pull/2830. Based on the >> same idea, I continued to make improvements. I made patterns like %2d %02d >> also be optimized. >> >> The following are the test results based on MacBookPro M1 Pro: >> >> >> -Benchmark Mode Cnt Score Error Units >> -StringFormat.complexFormat avgt 15 1862.233 ? 217.479 ns/op >> -StringFormat.int02Format avgt 15 312.491 ? 26.021 ns/op >> -StringFormat.intFormat avgt 1584.432 ? 4.145 ns/op >> -StringFormat.longFormatavgt 1587.330 ? 6.111 ns/op >> -StringFormat.stringFormat avgt 1563.985 ? 11.366 ns/op >> -StringFormat.stringIntFormat avgt 1587.422 ? 0.147 ns/op >> -StringFormat.widthStringFormat avgt 15 250.740 ? 32.639 ns/op >> -StringFormat.widthStringIntFormat avgt 15 312.474 ? 16.309 ns/op >> >> +Benchmark Mode CntScoreError Units >> +StringFormat.complexFormat avgt 15 740.626 ? 66.671 ns/op >> (+151.45) >> +StringFormat.int02Format avgt 15 131.049 ? 0.432 ns/op >> (+138.46) >> +StringFormat.intFormat avgt 15 67.229 ? 4.155 ns/op >> (+25.59) >> +StringFormat.longFormatavgt 15 66.444 ? 0.614 ns/op >> (+31.44) >> +StringFormat.stringFormat avgt 15 62.619 ? 4.652 ns/op >> (+2.19) >> +StringFormat.stringIntFormat avgt 15 89.606 ? 13.966 ns/op >> (-2.44) >> +StringFormat.widthStringFormat avgt 15 52.462 ? 15.649 ns/op >> (+377.95) >> +StringFormat.widthStringIntFormat avgt 15 101.814 ? 3.147 ns/op >> (+206.91) > > 温绍锦 has updated the pull request incrementally with one additional commit > since the last revision: > > fix : the exception thrown when the input does not include conversion is > different from baselne. src/java.base/share/classes/java/util/Formatter.java line 3136: > 3134: int flagEnd = argEnd + flagSize; > 3135: int widthEnd = flagEnd + widthSize; > 3136: int precesionEnd = widthEnd + precisionSize; Suggestion: int precisionEnd = widthEnd + precisionSize; - PR Review Comment: https://git.openjdk.org/jdk/pull/15776#discussion_r1339135086
Re: RFR: 8316704: Regex-free parsing of Formatter and FormatProcessor specifiers [v13]
On Wed, 27 Sep 2023 21:26:46 GMT, 温绍锦 wrote:
>> @cl4es made performance optimizations for the simple specifiers of
>> String.format in PR https://github.com/openjdk/jdk/pull/2830. Based on the
>> same idea, I continued to make improvements. I made patterns like %2d %02d
>> also be optimized.
>>
>> The following are the test results based on MacBookPro M1 Pro:
>>
>>
>> -Benchmark Mode Cnt Score Error Units
>> -StringFormat.complexFormat avgt 15 1862.233 ? 217.479 ns/op
>> -StringFormat.int02Format avgt 15 312.491 ? 26.021 ns/op
>> -StringFormat.intFormat avgt 1584.432 ? 4.145 ns/op
>> -StringFormat.longFormatavgt 1587.330 ? 6.111 ns/op
>> -StringFormat.stringFormat avgt 1563.985 ? 11.366 ns/op
>> -StringFormat.stringIntFormat avgt 1587.422 ? 0.147 ns/op
>> -StringFormat.widthStringFormat avgt 15 250.740 ? 32.639 ns/op
>> -StringFormat.widthStringIntFormat avgt 15 312.474 ? 16.309 ns/op
>>
>> +Benchmark Mode CntScoreError Units
>> +StringFormat.complexFormat avgt 15 740.626 ? 66.671 ns/op
>> (+151.45)
>> +StringFormat.int02Format avgt 15 131.049 ? 0.432 ns/op
>> (+138.46)
>> +StringFormat.intFormat avgt 15 67.229 ? 4.155 ns/op
>> (+25.59)
>> +StringFormat.longFormatavgt 15 66.444 ? 0.614 ns/op
>> (+31.44)
>> +StringFormat.stringFormat avgt 15 62.619 ? 4.652 ns/op
>> (+2.19)
>> +StringFormat.stringIntFormat avgt 15 89.606 ? 13.966 ns/op
>> (-2.44)
>> +StringFormat.widthStringFormat avgt 15 52.462 ? 15.649 ns/op
>> (+377.95)
>> +StringFormat.widthStringIntFormat avgt 15 101.814 ? 3.147 ns/op
>> (+206.91)
>
> 温绍锦 has updated the pull request incrementally with one additional commit
> since the last revision:
>
> Refactor according to rgiulietti's suggestion and add testcases
For maintainability purposes, we should strive for simplicity and readability.
src/java.base/share/classes/java/util/Formatter.java line 2899:
> 2897: if (c == '.') {
> 2898: // (\.\d+)?
> 2899: precisionSize = parsePrecision();
`precisionSize == 0` has two different meanings: (1) there's a '.' not followed
by digits and (2) there's no precision field at all.
In the proposed
[alternative](https://github.com/openjdk/jdk/pull/15776#issuecomment-1737986458),
case (1) is indicated by `-1`, while case (2) is indicated by `0`. Besides,
the decision is taken in the `parsePrecision()` method, not partly here and
partly in the method.
src/java.base/share/classes/java/util/Formatter.java line 2934:
> 2932: // (\d+\$)?
> 2933: int i = off;
> 2934: for (; i < max && isDigit(c); c = next(++i)); // empty body
No need for `next()` if done as in the proposed alternative.
src/java.base/share/classes/java/util/Formatter.java line 2947:
> 2945: c = first;
> 2946: }
> 2947: }
I think the logic in the proposed alternative is more readable.
src/java.base/share/classes/java/util/Formatter.java line 2953:
> 2951: // ([-#+ 0,(\<]*)?
> 2952: int i = off;
> 2953: for (; i < max && Flags.isFlag(c); c = next(++i)); //
> empty body
No need for `next()` if done as in the proposed alternative.
src/java.base/share/classes/java/util/Formatter.java line 2961:
> 2959: // (\d+)?
> 2960: int i = off;
> 2961: for (; i < max && isDigit(c); c = next(++i)); // empty body
You are using `next()` here, but the simpler solution in `parsePrecision()`.
I think consistency simplifies understanding.
src/java.base/share/classes/java/util/Formatter.java line 2978:
> 2976: }
> 2977:
> 2978: private char next(int off) {
There's no real need for this more expensive method, compared with a simple
`s.charAt(i)`, in the alternative proposed
[before](https://github.com/openjdk/jdk/pull/15776#issuecomment-1737986458).
-
PR Review: https://git.openjdk.org/jdk/pull/15776#pullrequestreview-1647449239
PR Review Comment: https://git.openjdk.org/jdk/pull/15776#discussion_r1339852931
PR Review Comment: https://git.openjdk.org/jdk/pull/15776#discussion_r1339856693
PR Review Comment: https://git.openjdk.org/jdk/pull/15776#discussion_r1339856725
PR Review Comment: https://git.openjdk.org/jdk/pull/15776#discussion_r1339857403
PR Review Comment: https://git.openjdk.org/jdk/pull/15776#discussion_r1339844252
PR Review Comment: https://git.openjdk.org/jdk/pull/15776#discussion_r1339842414
Integrated: 8316970: Add internal annotation to mark restricted methods
On Wed, 27 Sep 2023 16:09:01 GMT, Maurizio Cimadamore wrote: > This patch adds a new internal annotation that is used to mark all restricted > me > thods in the FFM API. The new annotation is similar to the one we used for > preview API methods. > > We plan to use the new annotation for adding new javac warnings when calling > restricted methods, as well as to add better javadoc support for restricted > methods. > > I have added a test which, similar to the test for `@CallerSensitive` checks > all methods in `java.base` that are annotated with `@Restricted` and makes > sure they conform to the list of restricted methods. This pull request has now been integrated. Changeset: 79812515 Author:Maurizio Cimadamore URL: https://git.openjdk.org/jdk/commit/798125152ba40ff2d093711629f275b5d74f0bcb Stats: 221 lines in 7 files changed: 221 ins; 0 del; 0 mod 8316970: Add internal annotation to mark restricted methods Reviewed-by: jvernee, iris, alanb - PR: https://git.openjdk.org/jdk/pull/15947
Re: RFR: 8315532: Compiler Implementation for Unnamed Variables and Patterns [v3]
On Thu, 28 Sep 2023 08:38:27 GMT, Aggelos Biboudis wrote: >> I agree. Also, this is not helped by the fact that in this particular >> message, the `_` does not appear in a variable declaration. Maybe we should >> use two distinct messages for when `_` is found as an "expression" (e.g. an >> identifier) and when it is used as a declaration _name_. > > @mcimadamore @lahodaj I attempted to do this split for non variables that do > not allow underscore. I print the simple version of the error message. Is > that what you meant Maurizio? > > I also fixed the `UnderscoreAsIdent` adding the missing `--release 9`. It was > the same omission as with other tests that @lahodaj pointed. This looks much better! - PR Review Comment: https://git.openjdk.org/jdk/pull/15649#discussion_r1339840526
Re: RFR: 8315532: Compiler Implementation for Unnamed Variables and Patterns [v3]
On Thu, 28 Sep 2023 08:40:20 GMT, Aggelos Biboudis wrote: >> This PR finalizes the feature of unnamed variables and patterns. >> >> - >> ### Progress >> - [ ] Change must be properly reviewed (1 review required, with at least 1 >> [Reviewer](https://openjdk.org/bylaws#reviewer)) >> - [x] Change must not contain extraneous whitespace >> - [x] Commit message must refer to an issue >> - [ ] Change requires CSR request >> [JDK-8315851](https://bugs.openjdk.org/browse/JDK-8315851) to be approved >> >> >> >> ### Reviewing >> Using git >> >> Checkout this PR locally: \ >> `$ git fetch https://git.openjdk.org/jdk.git pull/15649/head:pull/15649` \ >> `$ git checkout pull/15649` >> >> Update a local copy of the PR: \ >> `$ git checkout pull/15649` \ >> `$ git pull https://git.openjdk.org/jdk.git pull/15649/head` >> >> >> Using Skara CLI tools >> >> Checkout this PR locally: \ >> `$ git pr checkout 15649` >> >> View PR using the GUI difftool: \ >> `$ git pr show -t 15649` >> >> >> Using diff file >> >> Download this PR as a diff file: \ >> > href="https://git.openjdk.org/jdk/pull/15649.diff;>https://git.openjdk.org/jdk/pull/15649.diff >> >> >> >> >> ### Webrev >> [Link to Webrev >> Comment](https://git.openjdk.org/jdk/pull/15649#issuecomment-1733906010) > > Aggelos Biboudis has updated the pull request incrementally with one > additional commit since the last revision: > > Address review by @mcimadamore Marked as reviewed by mcimadamore (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/15649#pullrequestreview-1648347760
Re: RFR: 8315532: Compiler Implementation for Unnamed Variables and Patterns [v3]
On Tue, 26 Sep 2023 10:21:06 GMT, Maurizio Cimadamore
wrote:
>> test/langtools/tools/javac/lambda/IdentifierTest9.out line 33:
>>
>>> 31: IdentifierTest.java:152:17: compiler.err.use.of.underscore.not.allowed
>>> 32: IdentifierTest.java:158:16:
>>> compiler.err.use.of.underscore.not.allowed.with.brackets
>>> 33: IdentifierTest.java:160:25: compiler.err.use.of.underscore.not.allowed
>>
>> Note the errors are like:
>>
>> /home/jlahoda/src/jdk/jdk2/test/langtools/tools/javac/lambda/IdentifierTest.java:160:
>> error: as of release 21, the underscore keyword '_' is only allowed to
>> declare
>> for(String _s : _ ) {
>> ^
>> unnamed patterns, local variables, exception parameters or lambda
>> parameters
>>
>>
>> At least the release version should be updated, but to not let the message
>> to be broken into two lines like this. Maybe produce a top-level diagnostic
>> along the line of "underscore not allowed here" with sub-diagnostic with the
>> details (which then can (maybe?) span multiple lines).
>>
>> Also, the wording sounds to me like if there was a restriction in JDK 22,
>> while underscore is actually permitted on more places than before.
>>
>> So, maybe something like:
>>
>>
>> underscore not allowed here
>> as of release 9, ''_'' is a keyword, and may not be used as an identifier
>> as of release 21, ''_'' can be used as a name in the declaration of unnamed
>> patterns, local variables, exception parameters or lambda parameters
>>
>> ?
>> Just an idea.
>
> I agree. Also, this is not helped by the fact that in this particular
> message, the `_` does not appear in a variable declaration. Maybe we should
> use two distinct messages for when `_` is found as an "expression" (e.g. an
> identifier) and when it is used as a declaration _name_.
@mcimadamore @lahodaj I attempted to do this split for non variables that do
not allow underscore. I print the simple version of the error message. Is that
what you meant Maurizio?
I also fixed the `UnderscoreAsIdent` adding the missing `--release 9`. It was
the same omission as with other tests that @lahodaj pointed.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15649#discussion_r1339746518
Re: RFR: 8315532: Compiler Implementation for Unnamed Variables and Patterns [v3]
> This PR finalizes the feature of unnamed variables and patterns. > > - > ### Progress > - [ ] Change must be properly reviewed (1 review required, with at least 1 > [Reviewer](https://openjdk.org/bylaws#reviewer)) > - [x] Change must not contain extraneous whitespace > - [x] Commit message must refer to an issue > - [ ] Change requires CSR request > [JDK-8315851](https://bugs.openjdk.org/browse/JDK-8315851) to be approved > > > > ### Reviewing > Using git > > Checkout this PR locally: \ > `$ git fetch https://git.openjdk.org/jdk.git pull/15649/head:pull/15649` \ > `$ git checkout pull/15649` > > Update a local copy of the PR: \ > `$ git checkout pull/15649` \ > `$ git pull https://git.openjdk.org/jdk.git pull/15649/head` > > > Using Skara CLI tools > > Checkout this PR locally: \ > `$ git pr checkout 15649` > > View PR using the GUI difftool: \ > `$ git pr show -t 15649` > > > Using diff file > > Download this PR as a diff file: \ > href="https://git.openjdk.org/jdk/pull/15649.diff;>https://git.openjdk.org/jdk/pull/15649.diff > > > > > ### Webrev > [Link to Webrev > Comment](https://git.openjdk.org/jdk/pull/15649#issuecomment-1733906010) Aggelos Biboudis has updated the pull request incrementally with one additional commit since the last revision: Address review by @mcimadamore - Changes: - all: https://git.openjdk.org/jdk/pull/15649/files - new: https://git.openjdk.org/jdk/pull/15649/files/3becc945..630b128e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15649=02 - incr: https://webrevs.openjdk.org/?repo=jdk=15649=01-02 Stats: 128 lines in 11 files changed: 41 ins; 3 del; 84 mod Patch: https://git.openjdk.org/jdk/pull/15649.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15649/head:pull/15649 PR: https://git.openjdk.org/jdk/pull/15649
Re: RFR: 8312522: Implementation of Foreign Function & Memory API [v30]
On Wed, 27 Sep 2023 16:50:33 GMT, Jorn Vernee wrote: >> This patch contains the implementation of the foreign linker & memory API >> JEP for Java 22. The initial patch is composed of commits brought over >> directly from the [panama-foreign >> repo](https://github.com/openjdk/panama-foreign). The main changes found in >> this patch come from the following PRs: >> >> 1. https://github.com/openjdk/panama-foreign/pull/836 Where previous >> iterations supported converting Java strings to and from native strings in >> the UTF-8 encoding, we've extended the supported encoding to all the >> encodings found in the `java.nio.charset.StandardCharsets` class. >> 2. https://github.com/openjdk/panama-foreign/pull/838 We dropped the >> `MemoryLayout::sequenceLayout` factory method which inferred the size of the >> sequence to be `Long.MAX_VALUE`, as this led to confusion among clients. A >> client is now required to explicitly specify the sequence size. >> 3. https://github.com/openjdk/panama-foreign/pull/839 A new API was added: >> `Linker::canonicalLayouts`, which exposes a map containing the >> platform-specific mappings of common C type names to memory layouts. >> 4. https://github.com/openjdk/panama-foreign/pull/840 Memory access >> varhandles, as well as byte offset and slice handles derived from memory >> layouts, now feature an additional 'base offset' coordinate that is added to >> the offset computed by the handle. This allows composing these handles with >> other offset computation strategies that may not be based on the same memory >> layout. This addresses use-cases where clients are working with 'dynamic' >> layouts, whose size might not be known statically, such as variable length >> arrays, or variable size matrices. >> 5. https://github.com/openjdk/panama-foreign/pull/841 Remove this now >> redundant API. Clients can simply use the difference between the base >> address of two memory segments. >> 6. https://github.com/openjdk/panama-foreign/pull/845 Disambiguate uses of >> `SegmentAllocator::allocateArray`, by renaming methods that both allocate + >> initialize memory segments to `allocateFrom`. (see the original PR for the >> problematic case) >> 7. https://github.com/openjdk/panama-foreign/pull/846 Improve the >> documentation for variadic functions. >> 8. https://github.com/openjdk/panama-foreign/pull/849 Several test fixes to >> make sure the `jdk_foreign` tests can pass on 32-bit machines, taking >> linux-x86 as a test bed. >> 9. https://github.com/openjdk/panama-foreign/pull/850 Make the linker API >> required. The `Linker::nativeLinker` method is not longer allowed to throw >> an `UnsupportedO... > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > drop unneeded @compile tags from jtreg tests Marked as reviewed by alanb (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/15103#pullrequestreview-1648059254
