Re: RFR: 8267521: Post JEP 411 refactoring: maximum covering > 50K [v3]
On Fri, 21 May 2021 20:37:30 GMT, Weijun Wang wrote:
>> The code change refactors classes that have a `SuppressWarnings("removal")`
>> annotation that covers more than 50KB of code. The big annotation is often
>> quite faraway from the actual deprecated API usage it is suppressing, and
>> with the annotation covering too big a portion it's easy to call other
>> deprecated methods without being noticed.
>>
>> The code change shows some common solutions to avoid such too wide
>> annotations:
>>
>> 1. Extract calls into a method and add annotation on that method
>> 2. Assign the return value of a deprecated method call to a new local
>> variable and add annotation on the declaration, and then assign that value
>> to the original l-value if not void. The local variable will be called `tmp`
>> if later reassigned or `dummy` if it will be discarded.
>> 3. Put declaration and assignment into a single statement if possible.
>> 4. Rewrite code to achieve #3 above.
>>
>> I'll add a copyright year update commit before integration.
>
> Weijun Wang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> update FtpClient.java
The dependent pull request has now been integrated, and the target branch of
this pull request has been updated. This means that changes from the dependent
pull request can start to show up as belonging to this pull request, which may
be confusing for reviewers. To remedy this situation, simply merge the latest
changes from the new target branch into this pull request by running commands
similar to these in the local repository for your personal fork:
git checkout 8266459
git fetch https://git.openjdk.java.net/jdk master
git merge FETCH_HEAD
# if there are conflicts, follow the instructions given by git merge
git commit -m "Merge master"
git push
-
PR: https://git.openjdk.java.net/jdk/pull/4138
Re: RFR: 8268339: Upstream: 8267989: Exceptions thrown during upcalls should be handled (Pt. 2)
On Mon, 7 Jun 2021 16:38:01 GMT, Jorn Vernee wrote: > Hi, > > This is part 2 of a two part upstreaming process of the patch mentioned in > the subject line. The patch was split into 2 in order to document 2 separate > specification changes that arose from a change in the implementation, with 2 > separate CSRs. The first patch can be found here: > https://github.com/openjdk/jdk/pull/4395 > > This patch adds uniform exception handling for exceptions thrown during FLA > upcalls. Currently, these exceptions are either handled similarly to the VMs > `CATCH` macro, or ignored after which control returns to unsuspecting native > code, until control returns to Java, which will then handle the exception. > The handling depends on the invocation mode. > > Both of these are bad. The former because a stack trace is not printed and > instead the VM exits with a fatal error. The latter is bad because an upcall > completing abruptly and returning to native code which has no idea an > exception occurred is unsafe, in the sense that invariants about the state of > the program that the native code depends on might no longer hold. > > This patch adds uniform exception handling that replaces both of these cases > (see `SharedUtils::handleUncaughtException`), which prints the exception > stack trace, and then unconditionally exits the VM, which is the only safe > course of action at that point. > > Exceptions thrown by upcalls should instead be handled during the upcall > itself, for instance by translating the exception into an error code that is > then returned to the native caller, which can deal with it appropriately. > > See also the original review for panama-foreign: > https://github.com/openjdk/panama-foreign/pull/543 > > Thanks, > Jorn > > Testing: `jdk_foreign` test suite. > > Thanks, > Jorn The dependent pull request has now been integrated, and the target branch of this pull request has been updated. This means that changes from the dependent pull request can start to show up as belonging to this pull request, which may be confusing for reviewers. To remedy this situation, simply merge the latest changes from the new target branch into this pull request by running commands similar to these in the local repository for your personal fork: git checkout Upstream_Excp_Hndl git fetch https://git.openjdk.java.net/jdk master git merge FETCH_HEAD # if there are conflicts, follow the instructions given by git merge git commit -m "Merge master" git push - PR: https://git.openjdk.java.net/jdk/pull/4396
Re: RFR: 8278065: Refactor subclassAudits to use ClassValue
On Wed, 1 Dec 2021 14:45:23 GMT, Roman Kennke wrote: > As a follow-up to #6375, this change refactors > java.io.ObjectInputStream.Caches#subclassAudits and > java.io.ObjectOutputStream.Caches#subclassAudits to use ClassValue instead of > SoftReference, similar to what we did in #6375 for > java.io.ObjectStreamClass.Caches#localDescs. Then we can now also remove the > common machinery java.io.ObjectStreamClass#processQueue and > java.io.ObjectStreamClass.WeakClassKey. > > Testing: > - [x] tier1 > - [x] tier2 > - [ ] tier3 The dependent pull request has now been integrated, and the target branch of this pull request has been updated. This means that changes from the dependent pull request can start to show up as belonging to this pull request, which may be confusing for reviewers. To remedy this situation, simply merge the latest changes from the new target branch into this pull request by running commands similar to these in the local repository for your personal fork: git checkout JDK-8277072 git fetch https://git.openjdk.java.net/jdk master git merge FETCH_HEAD # if there are conflicts, follow the instructions given by git merge git commit -m "Merge master" git push - PR: https://git.openjdk.java.net/jdk/pull/6637
Re: RFR: 8284856: Add a test case for checking UnicodeScript entity numbers [v2]
On Thu, 14 Apr 2022 22:27:20 GMT, Naoto Sato wrote: >> Added the test case, and eliminated the immediate hashmap value, replaced >> with the ordinal number of `Character.UnicodeScript.UNKNOWN`. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Fixed a typo The dependent pull request has now been integrated, and the target branch of this pull request has been updated. This means that changes from the dependent pull request can start to show up as belonging to this pull request, which may be confusing for reviewers. To remedy this situation, simply merge the latest changes from the new target branch into this pull request by running commands similar to these in the local repository for your personal fork: git checkout fix_8186958 git fetch https://git.openjdk.java.net/jdk master git merge FETCH_HEAD # if there are conflicts, follow the instructions given by git merge git commit -m "Merge master" git push - PR: https://git.openjdk.java.net/jdk/pull/8253
Re: RFR: 8283689: Update the foreign linker VM implementation [v16]
On Thu, 12 May 2022 16:01:17 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.com/openjdk/jdk/pull/7959/commits/08e22e1b468c5c8f0cfd7135c72849944068aa7a >> [7]: >> https://github.com/openjdk/jdk/pull/7959/commits/451cd9edf54016c182dab21a8b26bd8b609fc062 >> [8]: >> https://github.com/openjdk/jdk/pull/7959/commits/4c851d2795afafec3a3ab17f4142ee098692068f >> [9]: >> https://github.com/openjdk/jdk/pull/7959/commits/d025377799424f31512dca2ffe95491cd5ae22f9 > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > Undo spurious changes. The dependent pull request has now been integrated, and the target branch of this pull request has been updated. This means that changes from the dependent pull request can start to show up as belonging to this pull request, which may be confusing for reviewers. To remedy this situation, simply merge the latest changes from the new target branch into this pull request by running commands similar to these in the local repository for your personal fork: git checkout JEP-19-VM-IMPL2 git fetch https://git.openjdk.java.net/jdk master git merge FETCH_HEAD # if there are conflicts, follow the instructions given by git merge git commit -m "Merge master" git push - PR: https://git.openjdk.java.net/jdk/pull/7959
Re: RFR: 8262889: Compiler implementation for Record Patterns [v5]
On Tue, 10 May 2022 09:57:48 GMT, Jan Lahoda wrote: >> 8262889: Compiler implementation for Record Patterns >> >> A first version of a patch that introduces record patterns into javac as a >> preview feature. For the specification, please see: >> http://cr.openjdk.java.net/~gbierman/jep427+405/jep427+405-20220426/specs/patterns-switch-record-patterns-jls.html >> >> There are two notable tricky parts: >> -in the parser, it was necessary to improve the `analyzePattern` method to >> handle nested/record patterns, while still keeping error recovery reasonable >> -in the `TransPatterns`, the desugaring of the record patterns is very >> straightforward - effectivelly the record patterns are desugared into >> guards/conditions. This will likely be improved in some future >> version/preview >> >> `MatchException` has been extended to cover additional cases related to >> record patterns. > > Jan Lahoda has updated the pull request incrementally with one additional > commit since the last revision: > > Fixing (non-)exhaustiveness for enum. The dependent pull request has now been integrated, and the target branch of this pull request has been updated. This means that changes from the dependent pull request can start to show up as belonging to this pull request, which may be confusing for reviewers. To remedy this situation, simply merge the latest changes from the new target branch into this pull request by running commands similar to these in the local repository for your personal fork: git checkout patterns-record-deconstruction3 git fetch https://git.openjdk.java.net/jdk master git merge FETCH_HEAD # if there are conflicts, follow the instructions given by git merge git commit -m "Merge master" git push - PR: https://git.openjdk.java.net/jdk/pull/8516
Re: RFR: 8286669: Replace MethodHandle specialization with ASM in mainline [v4]
On Tue, 17 May 2022 16:22:15 GMT, Jorn Vernee wrote: >> Hi, >> >> This PR brings over commits from the panama-foreign repo. These commits >> mostly pertain to the switch to ASM and away from MethodHandle combinators >> for the binding recipe specialization. But, there is one more commit which >> adds freeing of downcall stubs, since those changes were mostly Java as well. >> >> Thanks, >> Jorn >> >> Testing: `run-test-jdk_foreign` with `-Dgenerator.sample.factor=-1` on >> Windows and Linux. > > Jorn Vernee has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 10 commits: > > - Merge branch 'pr/7959' into JEP-19-ASM > - BootstrapMethodError -> ExceptionInInitializerError > - Use unaligned layout constants when filling in reconstituted structs (was > accidentally dropped change) > - Fix LinkUpcall benchmark > - 8286306: Upcall wrapper class sharing > >Reviewed-by: mcimadamore > - Polish > - 8281595: ASM-ify scope acquire/release for down call parameters >8281387: Some downcall shapes show unexpected allocations > >Co-authored-by: Maurizio Cimadamore >Reviewed-by: mcimadamore > - 8281228: Preview branch's CLinker.downcallHandle crashes inside asm > >Reviewed-by: sundar, jvernee > - 8278414: Replace binding recipe customization using MH combinators with > bytecode spinning > >Reviewed-by: mcimadamore > - 8276648: Downcall stubs are never freed > >Reviewed-by: mcimadamore The dependent pull request has now been integrated, and the target branch of this pull request has been updated. This means that changes from the dependent pull request can start to show up as belonging to this pull request, which may be confusing for reviewers. To remedy this situation, simply merge the latest changes from the new target branch into this pull request by running commands similar to these in the local repository for your personal fork: git checkout JEP-19-ASM git fetch https://git.openjdk.java.net/jdk master git merge FETCH_HEAD # if there are conflicts, follow the instructions given by git merge git commit -m "Merge master" git push - PR: https://git.openjdk.java.net/jdk/pull/8685
Re: RFR: 8287353: Use snippet tag instead of pre tag in Javadoc of InterruptedException
On Wed, 27 Apr 2022 13:33:29 GMT, Thiago Henrique Hüpner wrote: >> 8287353: Use snippet tag instead of pre tag in Javadoc of >> InterruptedException > > If this is a valid change, could someone please create a Jira issue to link > to it? @Thihup Please do not rebase or force-push to an active PR as it invalidates existing review comments. All changes will be squashed into a single commit automatically when integrating. See [OpenJDK Developers’ Guide](https://openjdk.java.net/guide/#working-with-pull-requests) for more information. - PR: https://git.openjdk.java.net/jdk/pull/8400
Re: RFR: 8284493: Fix rounding error in computeNextExponential [v2]
On Thu, 2 Jun 2022 03:00:32 GMT, Chris Hennick wrote: >> Chris Hennick has refreshed the contents of this pull request, and previous >> commits have been removed. The incremental views will show differences >> compared to the previous content of the PR. The pull request contains one >> new commit since the last revision: >> >> Fix rounding error in computeNextExponential; use FMA >> >> Repeatedly adding DoubleZigguratTables.exponentialX0 to extra causes a >> rounding error to accumulate at the tail of the distribution; this fixes >> that by tracking the multiple of exponentialX0 as a long. >> >> Add computeWinsorizedNextExponential for testability > > In addition to the changes discussed heretofore, I've also changed line 1382 > to eliminate unneeded tail exploration; this should make `nextGaussian` > faster at high percentiles (probably measurable at 99%ile; should definitely > be measurable at at 99.99%ile). @Pr0methean Please do not rebase or force-push to an active PR as it invalidates existing review comments. All changes will be squashed into a single commit automatically when integrating. See [OpenJDK Developers’ Guide](https://openjdk.java.net/guide/#working-with-pull-requests) for more information. - PR: https://git.openjdk.java.net/jdk/pull/8131
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v3]
On Tue, 7 Jun 2022 13:30:50 GMT, Thiago Henrique Hüpner wrote: >> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved >> flags is used > > Thiago Henrique Hüpner has refreshed the contents of this pull request, and > previous commits have been removed. The incremental views will show > differences compared to the previous content of the PR. The pull request > contains one new commit since the last revision: > > Fix copyright year @Thihup Please do not rebase or force-push to an active PR as it invalidates existing review comments. All changes will be squashed into a single commit automatically when integrating. See [OpenJDK Developers’ Guide](https://openjdk.java.net/guide/#working-with-pull-requests) for more information. - PR: https://git.openjdk.java.net/jdk/pull/9049
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v6]
On Tue, 7 Jun 2022 14:11:57 GMT, Gaurav Chaudhari wrote: >> Gaurav Chaudhari has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - 8285838: Cleanup of trailing whitespace >> - 8285838: Corrected month comparison check for TZ DST > > Appreciate the suggestion, I fixed up my test and simplified it as you said, > and checked against a few spawned LocalDate(s) to verify the functionality is > still good. @Deigue Please do not rebase or force-push to an active PR as it invalidates existing review comments. All changes will be squashed into a single commit automatically when integrating. See [OpenJDK Developers’ Guide](https://openjdk.java.net/guide/#working-with-pull-requests) for more information. - PR: https://git.openjdk.java.net/jdk/pull/8661
