Re: RFR: 8233760: Result of BigDecimal.toString throws overflow exception on new BigDecimal(str) [v2]
On Fri, 27 May 2022 22:59:47 GMT, Raffaello Giulietti wrote: >> BigDecimal(String) currently fails to accept some strings produced by >> BigDecimal.toString(). This PR removes this limitation. > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 8233760: Result of BigDecimal.toString throws overflow exception on new > BigDecimal(str) Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8905
Re: RFR: JDK-8281001 Class::forName(String) defaults to system class loader if the caller is null [v5]
On Wed, 8 Jun 2022 02:40:15 GMT, Tim Prinzing wrote: >> The Class::forName behavior change to match JNI FindClass is a compatible >> change and seems pretty attractive as it would be expected that >> Class::forName would give the same behavior as FindClass which uses the >> system classloader. The test for 8281006 was enhanced to test for this >> change. Merged master to pick up fixes to unrelated test failures to reduce >> noise. > > Tim Prinzing has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains eight commits: > > - formatting improvement > - Merge branch 'master' into JDK-8281001 > - Fixed the build of the native c++ test NullCallerTest to specify >the c++ std library as part of the build. Changed the test to >use iostream instead of printf. Enabled the test for Class::forName >which is now located in test/jdk/jni/nullCaller (as part of the >merge of JDK-8287171). > - Merge branch 'master' into JDK-8281001 > - make javadoc consistent with other caller sensitve methods > - Added javadoc comment > - Merge branch 'master' into JDK-8281001 > - JDK-8281001 Examine the behavior of Class::forName if the caller is null Looks good. Thanks for refactoring the tests, making the addition of a new test case much cleaner. test/jdk/jni/nullCaller/NullCallerTest.java line 27: > 25: /** > 26: * @test > 27: * @bug 8280902 8281000 8281001 8281003 8281006 nit: append the bug rather than keeping the list in an increasing order. - Marked as reviewed by mchung (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8711
Integrated: 8287353: Use snippet tag instead of pre tag in Javadoc of InterruptedException
On Tue, 26 Apr 2022 15:04:15 GMT, Thiago Henrique Hüpner wrote: > 8287353: Use snippet tag instead of pre tag in Javadoc of InterruptedException This pull request has now been integrated. Changeset: 7df48f97 Author:Thiago Henrique Hüpner Committer: Jaikiran Pai URL: https://git.openjdk.java.net/jdk/commit/7df48f97d23fdeba032ddec51b6a6e6ad02d14cd Stats: 5 lines in 1 file changed: 0 ins; 0 del; 5 mod 8287353: Use snippet tag instead of pre tag in Javadoc of InterruptedException Reviewed-by: jpai - PR: https://git.openjdk.java.net/jdk/pull/8400
Re: RFR: JDK-8281001 Class::forName(String) defaults to system class loader if the caller is null [v5]
> The Class::forName behavior change to match JNI FindClass is a compatible > change and seems pretty attractive as it would be expected that > Class::forName would give the same behavior as FindClass which uses the > system classloader. The test for 8281006 was enhanced to test for this > change. Merged master to pick up fixes to unrelated test failures to reduce > noise. Tim Prinzing has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains eight commits: - formatting improvement - Merge branch 'master' into JDK-8281001 - Fixed the build of the native c++ test NullCallerTest to specify the c++ std library as part of the build. Changed the test to use iostream instead of printf. Enabled the test for Class::forName which is now located in test/jdk/jni/nullCaller (as part of the merge of JDK-8287171). - Merge branch 'master' into JDK-8281001 - make javadoc consistent with other caller sensitve methods - Added javadoc comment - Merge branch 'master' into JDK-8281001 - JDK-8281001 Examine the behavior of Class::forName if the caller is null - Changes: https://git.openjdk.java.net/jdk/pull/8711/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8711=04 Stats: 15 lines in 4 files changed: 8 ins; 3 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/8711.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8711/head:pull/8711 PR: https://git.openjdk.java.net/jdk/pull/8711
Re: [External] : Re: RFR: 8286850: [macos] Add support for signing user provided app image [v2]
Hi Michael, Yes, this is correct. It is a three step process as you outlined it below. Thanks, Alexander On Jun 7, 2022, at 12:00 AM, Michael Hall mailto:mik3h...@gmail.com>> wrote: Alexander, I had an existing local GitHub repo for the jdk I updated that appeared to accept the parameters you indicated. It generated a jdk 19. If you are saying I’m not getting the main branch or the update for some reason has dependencies I’m not getting I would have to determine how to correctly get these, or, I guess wait for a release that has all the necessary. Determining if this worked as expected before a release seemed like it would be a good idea. Yes, the point of my original suggestion was to allow generating the application unsigned, then do post-processing - like modify the default Info.plist, and finally separately sign. I thought your change provided the means to do this by first generating an unsigned image using —type app-image, then on a separate invocation indicate the app-image and sign and package it. ./build/*/images/jdk/bin/jpackage --type app-image --app-image If I follow you now it isn’t a two step but a three step process. 1) Generate unsigned application and do post-processing. 2) Sign modified app-image 3) DMG or PKG the modified and signed app-image Is this correct? Thanks, Mike
Re: RFR: 8202449: overflow handling in Random.doubles [v3]
On Tue, 24 May 2022 12:58:45 GMT, Raffaello Giulietti wrote: >> Extend the range of Random.doubles(double, double) and similar methods. > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 8202449: overflow handling in Random.doubles Marked as reviewed by darcy (Reviewer). src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java line 733: > 731: /* avoids overflow at the cost of 3 more multiplications > */ > 732: float halfOrigin = 0.5f * origin; > 733: r = (r * (0.5f * bound - halfOrigin) + halfOrigin) * > 2.0f; This could be done in double arithmetic, but I think it is better to keep it in float arithmetic for similarity with the code in the double version of the method. - PR: https://git.openjdk.java.net/jdk/pull/8791
Re: CFV: new Core Libraries Group member: Roger Riggs
Vote: yes -Joe
Re: CFV: new Core Libraries Group member: Roger Riggs
Vote: yes On 6/7/22 10:43 AM, Stuart Marks wrote: I hereby nominate Roger Riggs [1] to membership in the Core Libraries Group [2]. Roger has been a member of the Oracle Core Libraries team since 2014. He has made significant contributions in the areas of java.time, Legacy Serialization and Serialization Filtering, Processes, and many others. He has integrated hundreds of changes into the JDK project [3], and he is also a Reviewer on the JDK project. Votes are due by June 21, 2022, 23:59 PDT. Only current members of the Core Libraries Group [4] are eligible to vote on this nomination. Votes must be cast in the open by replying to this mailing list, as described here [5]. Results are determined by lazy consensus [6]. s'marks [1] https://openjdk.java.net/census#rriggs [2] https://openjdk.java.net/groups/core-libs/ [3] https://github.com/search?q=author-name%3A%22Roger%20Riggs%22+repo%3Aopenjdk%2Fjdk+merge%3Afalse=Commits [4] https://openjdk.java.net/census#core-libs [5] https://openjdk.java.net/groups/#member-vote [6] https://openjdk.java.net/bylaws#lazy-consensus
Re: CVF: new Core Libraries Group member: Naoto Sato
Vote: yes On 6/6/22 5:52 PM, Stuart Marks wrote: I hereby nominate Naoto Sato [1] to membership in the Core Libraries Group [2]. Naoto Sato has been on the Core Libraries team for over a decade and has contributed hundreds of changes to the JDK project [3]. His most significant recent contribution is JEP 400: UTF-8 by Default [4]. Naoto is also lead of the Internationalization Group [5] and a Reviewer on the JDK project. Votes are due by June 20, 2022, 23:59 PDT. Only current members of the Core Libraries Group [6] are eligible to vote on this nomination. Votes must be cast in the open by replying to this mailing list, as described here [7]. Results are determined by lazy consensus [8]. s'marks [1] https://openjdk.java.net/census#naoto [2] https://openjdk.java.net/groups/core-libs/ [3] https://github.com/openjdk/jdk/search?q=author-name%3Anaoto=commits [4] https://openjdk.java.net/jeps/400 [5] https://openjdk.java.net/groups/i18n/ [6] https://openjdk.java.net/census#core-libs [7] https://openjdk.java.net/groups/#member-vote [8] https://openjdk.java.net/bylaws#lazy-consensus
Re: RFR: 8283660: Convert com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java finalizer to Cleaner [v12]
On Tue, 7 Jun 2022 21:19:02 GMT, Brent Christian wrote: > > The commented out printf/println's should be removed before committing. > > Do you mean the pre-existing `println`s in LdapSearchEnumeration.java? Usually, I would mean any that were added for this issue. The changes in indentation (as presented by Github) mislead me. Keep them if they were there before. - PR: https://git.openjdk.java.net/jdk/pull/8311
Re: RFR: 8283660: Convert com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java finalizer to Cleaner [v13]
> Please review this change to replace the finalizer in > `AbstractLdapNamingEnumeration` with a Cleaner. > > The pieces of state required for cleanup (`LdapCtx homeCtx`, `LdapResult > res`, and `LdapClient enumClnt`) are moved to a static inner class . From > there, the change is fairly mechanical. > > Details of note: > 1. Some operations need to change the state values (the update() method is > probably the most interesting). > 2. Subclasses need to access `homeCtx`; I added a `homeCtx()` method to read > `homeCtx` from the superclass's `state`. > > The test case is based on a copy of > `com/sun/jndi/ldap/blits/AddTests/AddNewEntry.java`. A more minimal test case > might be possible, but this was done for expediency. > > The test only confirms that the new Cleaner use does not keep the object > reachable. It only tests `LdapSearchEnumeration` (not `LdapNamingEnumeration` > or `LdapBindingEnumeration`, though all are subclasses of > `AbstractLdapNamingEnumeration`). > > Thanks. Brent Christian has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 23 commits: - Merge branch 'master' into remove-finalizers - reference emunCtx and homeCtx consistently in constructor - Restore EnumCtx to being an inner class, to keep all the state together in the code - Restore comments in ldap capture file - Update test files - add copyright, etc - added getters to EnumCtx, and moved it to top level ; use getters + local variables to reduce code changes - test comment udpate - Update .ldap capture file - Check that cleanup was performed correctly - Merge branch 'master' into remove-finalizers - ... and 13 more: https://git.openjdk.java.net/jdk/compare/b7a34f72...afd1c9b5 - Changes: https://git.openjdk.java.net/jdk/pull/8311/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8311=12 Stats: 623 lines in 6 files changed: 352 ins; 103 del; 168 mod Patch: https://git.openjdk.java.net/jdk/pull/8311.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8311/head:pull/8311 PR: https://git.openjdk.java.net/jdk/pull/8311
Re: RFR: 8283660: Convert com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java finalizer to Cleaner [v12]
On Tue, 7 Jun 2022 18:45:17 GMT, Roger Riggs wrote: > The commented out printf/println's should be removed before committing. Do you mean the pre-existing `println`s in LdapSearchEnumeration.java? - PR: https://git.openjdk.java.net/jdk/pull/8311
Integrated: 8285081: Improve XPath operators count accuracy
On Fri, 3 Jun 2022 18:17:55 GMT, Joe Wang wrote: > Adjust how XPath operators are counted to improve accuracy. This change does > not affect how XPath works. > > Test: > Tier2 passed; > JCK XML tests passed. This pull request has now been integrated. Changeset: 8e078391 Author:Joe Wang URL: https://git.openjdk.java.net/jdk/commit/8e0783917975075aae5d586f0076d5093afb0b62 Stats: 63 lines in 3 files changed: 40 ins; 3 del; 20 mod 8285081: Improve XPath operators count accuracy Reviewed-by: naoto, lancea - PR: https://git.openjdk.java.net/jdk/pull/9022
Re: CFV: new Core Libraries Group member: Roger Riggs
Vote: yes Mandy
Re: RFR: 8285081: Improve XPath operators count accuracy [v3]
On Fri, 3 Jun 2022 23:56:31 GMT, Joe Wang wrote: >> Adjust how XPath operators are counted to improve accuracy. This change does >> not affect how XPath works. >> >> Test: >> Tier2 passed; >> JCK XML tests passed. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > remove unused parameter Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/9022
Re: CFV: new Core Libraries Group member: Roger Riggs
Vote: yes Iris
Re: CFV: new Core Libraries Group member: Roger Riggs
Vote: yes
Re: RFR: 8283660: Convert com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java finalizer to Cleaner [v12]
On Mon, 6 Jun 2022 21:59:56 GMT, Brent Christian wrote: >> Please review this change to replace the finalizer in >> `AbstractLdapNamingEnumeration` with a Cleaner. >> >> The pieces of state required for cleanup (`LdapCtx homeCtx`, `LdapResult >> res`, and `LdapClient enumClnt`) are moved to a static inner class . From >> there, the change is fairly mechanical. >> >> Details of note: >> 1. Some operations need to change the state values (the update() method is >> probably the most interesting). >> 2. Subclasses need to access `homeCtx`; I added a `homeCtx()` method to read >> `homeCtx` from the superclass's `state`. >> >> The test case is based on a copy of >> `com/sun/jndi/ldap/blits/AddTests/AddNewEntry.java`. A more minimal test >> case might be possible, but this was done for expediency. >> >> The test only confirms that the new Cleaner use does not keep the object >> reachable. It only tests `LdapSearchEnumeration` (not >> `LdapNamingEnumeration` or `LdapBindingEnumeration`, though all are >> subclasses of `AbstractLdapNamingEnumeration`). >> >> Thanks. > > Brent Christian has updated the pull request incrementally with one > additional commit since the last revision: > > Restore EnumCtx to being an inner class, to keep all the state together in > the code trivial comments. The commented out printf/println's should be removed before committing. src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java line 152: > 150: // Ensures that context won't get closed from underneath us > 151: this.enumCtx.homeCtx.incEnumCount(); > 152: this.cleanable = LDAP_CLEANER.register(this, enumCtx); Use `enumCtx.getHomeCts()` above. Use `this.enumCtx` in call to `register`. For consistency. - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8311
Re: RFR: 8287596: Reorg jdk.test.lib.util.ForceGC [v7]
> This is a follow up update per comments in [JDK-8287384 > PR](https://github.com/openjdk/jdk/pull/8907). The tier1 and tier2 test in > open part looks good to me. Please help to run Mach5 just case the closed > test cases are impacted. Xue-Lei Andrew Fan has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains ten commits: - Merge - Merge master - Merge - add timeout parameter - rollback is not in this branch - rollback JDK-8287384 - back to wait 1 second - Remove trailing white space - 8287596: Reorg jdk.test.lib.util.ForceGC - Changes: https://git.openjdk.java.net/jdk/pull/8979/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8979=06 Stats: 86 lines in 10 files changed: 12 ins; 29 del; 45 mod Patch: https://git.openjdk.java.net/jdk/pull/8979.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8979/head:pull/8979 PR: https://git.openjdk.java.net/jdk/pull/8979
Re: RFR: 8283726: x86_64 intrinsics for compareUnsigned method in Integer and Long
On Tue, 7 Jun 2022 17:14:18 GMT, Quan Anh Mai wrote:
> Hi,
>
> This patch implements intrinsics for `Integer/Long::compareUnsigned` using
> the same approach as the JVM does for long and floating-point comparisons.
> This allows efficient and reliable usage of unsigned comparison in Java,
> which is a basic operation and is important for range checks such as
> discussed in #8620 .
>
> Thank you very much.
Please add microbenchmark and show its results.
src/hotspot/share/opto/subnode.hpp line 217:
> 215:
> //--CmpU3Node--
> 216: // Compare 2 unsigned values, returning integer value (-1, 0 or 1).
> 217: class CmpU3Node : public CmpUNode {
Place it after `CmpUNode` class.
-
PR: https://git.openjdk.java.net/jdk/pull/9068
CFV: new Core Libraries Group member: Roger Riggs
I hereby nominate Roger Riggs [1] to membership in the Core Libraries Group [2]. Roger has been a member of the Oracle Core Libraries team since 2014. He has made significant contributions in the areas of java.time, Legacy Serialization and Serialization Filtering, Processes, and many others. He has integrated hundreds of changes into the JDK project [3], and he is also a Reviewer on the JDK project. Votes are due by June 21, 2022, 23:59 PDT. Only current members of the Core Libraries Group [4] are eligible to vote on this nomination. Votes must be cast in the open by replying to this mailing list, as described here [5]. Results are determined by lazy consensus [6]. s'marks [1] https://openjdk.java.net/census#rriggs [2] https://openjdk.java.net/groups/core-libs/ [3] https://github.com/search?q=author-name%3A%22Roger%20Riggs%22+repo%3Aopenjdk%2Fjdk+merge%3Afalse=Commits [4] https://openjdk.java.net/census#core-libs [5] https://openjdk.java.net/groups/#member-vote [6] https://openjdk.java.net/bylaws#lazy-consensus
RFR: 8283726: x86_64 intrinsics for compare method in Integer and Long
Hi, This patch implements intrinsics for `Integer/Long::compareUnsigned` using the same approach as the JVM does for long and floating-point comparisons. This allows efficient and reliable usage of unsigned comparison in Java, which is a basic operation and is important for range checks such as discussed in #8620 . Thank you very much. - Commit messages: - format - compare unsigned Changes: https://git.openjdk.java.net/jdk/pull/9068/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9068=00 Issue: https://bugs.openjdk.org/browse/JDK-8283726 Stats: 235 lines in 13 files changed: 224 ins; 0 del; 11 mod Patch: https://git.openjdk.java.net/jdk/pull/9068.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9068/head:pull/9068 PR: https://git.openjdk.java.net/jdk/pull/9068
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v11]
Hi, the aim of this PR was firstly to have a correct implementation of the spec. Speed improvement was a secondary goal, which was intentionally pursued with conventional Java arithmetic and just a few calls to simple methods in the standard library. Whether the Vector API (which is still in incubation phase) might help in this specific case remains to be explored. As a first impression, there seem to be no good spots to parallelize using SIMD instructions. But engineers with more SIMD experience might find creative ways to make good use of them. HTH Raffaello On 2022-06-07 18:35, LifeIsStrange wrote: On Mon, 9 May 2022 12:50:31 GMT, Raffaello Giulietti wrote: Marked as reviewed by limck...@github.com (no known OpenJDK username). @limck599 While we at OpenJDK appreciate constructive reviews from GitHub users not registered in the [census](https://openjdk.java.net/census), only officially nominated reviewers have the authority to approve this PR. @rgiulietti friendly ping, noob question: Do you believe some parts of the algorithm could exploit explicit SIMD instructions (via the high level [Vector API](https://openjdk.java.net/jeps/417)) for even better performance (such as https://github.com/wrandelshofer/FastDoubleParser ) ? - PR: https://git.openjdk.java.net/jdk/pull/3402
Re: CVF: new Core Libraries Group member: Naoto Sato
Vote: yes Mandy
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v11]
On Mon, 9 May 2022 12:50:31 GMT, Raffaello Giulietti wrote: >> Marked as reviewed by limck...@github.com (no known OpenJDK username). > > @limck599 > While we at OpenJDK appreciate constructive reviews from GitHub users not > registered in the [census](https://openjdk.java.net/census), only officially > nominated reviewers have the authority to approve this PR. @rgiulietti friendly ping, noob question: Do you believe some parts of the algorithm could exploit explicit SIMD instructions (via the high level [Vector API](https://openjdk.java.net/jeps/417)) for even better performance (such as https://github.com/wrandelshofer/FastDoubleParser ) ? - PR: https://git.openjdk.java.net/jdk/pull/3402
Integrated: 8273346: Expand library mappings to IEEE 754 operations
On Wed, 25 May 2022 00:19:41 GMT, Joe Darcy wrote: > Generally add apiNote's to map from Java library methods to particular IEEE > 754 operations. For now, I only added such notes to java.lang.Math and not > java.lang.StrictMath. This pull request has now been integrated. Changeset: 5d4ea9b9 Author:Joe Darcy URL: https://git.openjdk.java.net/jdk/commit/5d4ea9b9549b762b7c207e5c2ee65bc51591433b Stats: 156 lines in 4 files changed: 143 ins; 0 del; 13 mod 8273346: Expand library mappings to IEEE 754 operations Reviewed-by: bpb - PR: https://git.openjdk.java.net/jdk/pull/8876
Re: RFR: 8273346: Expand library mappings to IEEE 754 operations [v5]
> Generally add apiNote's to map from Java library methods to particular IEEE > 754 operations. For now, I only added such notes to java.lang.Math and not > java.lang.StrictMath. Joe Darcy has updated the pull request incrementally with one additional commit since the last revision: Correct typo found in review and reflow paragraph. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8876/files - new: https://git.openjdk.java.net/jdk/pull/8876/files/ba3e9a75..b82f2668 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8876=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8876=03-04 Stats: 8 lines in 1 file changed: 0 ins; 0 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/8876.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8876/head:pull/8876 PR: https://git.openjdk.java.net/jdk/pull/8876
Re: RFR: 8287541: Files.writeString fails to throw IOException for charset "windows-1252" [v2]
On Mon, 6 Jun 2022 15:50:39 GMT, Naoto Sato wrote: >> The code path calls `String.getBytesNoRepl()`, but it blindly replaces >> unmappable characters with replacements if the encoder is an `ArrayEncoder`. >> Changed only to do so if `doReplace` is `true` in >> `String.encodeWithEncoder()`. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Changed per suggestion Marked as reviewed by aturbanov (Committer). - PR: https://git.openjdk.java.net/jdk/pull/9019
Re: CVF: new Core Libraries Group member: Naoto Sato
Vote: Yes On 6/6/22 8:52 PM, Stuart Marks wrote: I hereby nominate Naoto Sato [1] to membership in the Core Libraries Group [2].
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
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v6]
On Wed, 1 Jun 2022 13:32:36 GMT, Gaurav Chaudhari wrote: >> This fix ensures that when a lookup for a custom TZ code fails, and an >> attempt is made to find the GMT offset in order to get the current time, >> Daylight savings rules are applied correctly. > > 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. - PR: https://git.openjdk.java.net/jdk/pull/8661
Re: RFR: 8285838: DST not applying properly with zone id offset set with TZ env variable [v7]
> This fix ensures that when a lookup for a custom TZ code fails, and an > attempt is made to find the GMT offset in order to get the current time, > Daylight savings rules are applied correctly. Gaurav Chaudhari has updated the pull request incrementally with one additional commit since the last revision: 8285838: Simplified Daylight period checks for test. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8661/files - new: https://git.openjdk.java.net/jdk/pull/8661/files/87832663..f855493a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8661=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8661=05-06 Stats: 12 lines in 1 file changed: 0 ins; 6 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/8661.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8661/head:pull/8661 PR: https://git.openjdk.java.net/jdk/pull/8661
RFR: 8287908: Use non-cloning reflection methods where acceptable
Instead of `Executable.getParameterTypes()` we could use `Executable.getSharedParameterTypes()` in trusted code. Same is applicable for `Executable.getExceptionTypes()`. - Commit messages: - 8287908: Use non-cloning reflection methods where acceptable Changes: https://git.openjdk.java.net/jdk/pull/9064/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9064=00 Issue: https://bugs.openjdk.org/browse/JDK-8287908 Stats: 9 lines in 4 files changed: 0 ins; 0 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/9064.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9064/head:pull/9064 PR: https://git.openjdk.java.net/jdk/pull/9064
Re: RFR: 8287903: Reduce runtime of java.math microbenchmarks
On Tue, 7 Jun 2022 12:34:25 GMT, Claes Redestad wrote: > - Reduce runtime by running fewer forks, fewer iterations, less warmup. All > micros tested in this group appear to stabilize very quickly. > - Refactor BigIntegers to avoid re-running some (most) micros over and over > with parameter values that don't affect them. > > Expected runtime down from 14 hours to 15 minutes. Looks good. Will make these so much more practical to use. - Marked as reviewed by ecaspole (Committer). PR: https://git.openjdk.java.net/jdk/pull/9062
Integrated: 8286571: java source launcher from a minimal jdk image containing jdk.compiler fails with --enable-preview option
On Tue, 17 May 2022 12:47:25 GMT, Adam Sotona wrote: > ### Problem description > Minimal jdk image created by jlink with the only jdk.compiler module and its > dependencies > fails to run java source launcher correctly (for example when --source N is > specified). > Failing source launcher is only one the expressions of internal jdk.compiler > malfunction, another example is failure to run javac with --release option. > > ### Root cause > Module jdk.compiler requires zip filesystem (jdk.zipfs module) to parse > ct.sym file and so to provide full functionality. > Module jdk.zipfs is not included in the minimal JDK image, because module > jdk.compiler does not declare it as "requires" in its module info. > > ### Alternative patch > The problem can be alternatively resolved by complex code checks in > jdk.compiler to provide user with valid error message, however that solution > would be just a workaround for jdk.compiler dual functionality (with or > without presence of jdk.zipfs module). Compiler functionality without access > to ct.sym through jdk.zipfs is very limited. > > ### Proposed fix > This patch fixes the problem by explicit declaration of jdk.compiler > dependency on jdk.zipfs. > Plus added specific test case. > > Please review the patch or raise objections against declaration of > jdk.compiler dependent on jdk.zipfs. > > Thanks, > Adam This pull request has now been integrated. Changeset: 905bcbe3 Author:Adam Sotona URL: https://git.openjdk.java.net/jdk/commit/905bcbe34eb9750f6f7f12a577733c71a31d7972 Stats: 76 lines in 3 files changed: 29 ins; 37 del; 10 mod 8286571: java source launcher from a minimal jdk image containing jdk.compiler fails with --enable-preview option Reviewed-by: jlahoda - PR: https://git.openjdk.java.net/jdk/pull/8747
Re: RFR: 8286571: java source launcher from a minimal jdk image containing jdk.compiler fails with --enable-preview option [v3]
On Wed, 18 May 2022 06:30:34 GMT, Adam Sotona wrote: >> ### Problem description >> Minimal jdk image created by jlink with the only jdk.compiler module and its >> dependencies >> fails to run java source launcher correctly (for example when --source N is >> specified). >> Failing source launcher is only one the expressions of internal jdk.compiler >> malfunction, another example is failure to run javac with --release option. >> >> ### Root cause >> Module jdk.compiler requires zip filesystem (jdk.zipfs module) to parse >> ct.sym file and so to provide full functionality. >> Module jdk.zipfs is not included in the minimal JDK image, because module >> jdk.compiler does not declare it as "requires" in its module info. >> >> ### Alternative patch >> The problem can be alternatively resolved by complex code checks in >> jdk.compiler to provide user with valid error message, however that solution >> would be just a workaround for jdk.compiler dual functionality (with or >> without presence of jdk.zipfs module). Compiler functionality without access >> to ct.sym through jdk.zipfs is very limited. >> >> ### Proposed fix >> This patch fixes the problem by explicit declaration of jdk.compiler >> dependency on jdk.zipfs. >> Plus added specific test case. >> >> Please review the patch or raise objections against declaration of >> jdk.compiler dependent on jdk.zipfs. >> >> Thanks, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > corrected LimitedImage test description Looks like a reasonable compromise to me. - Marked as reviewed by jlahoda (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8747
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: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v3]
> 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 - Changes: - all: https://git.openjdk.java.net/jdk/pull/9049/files - new: https://git.openjdk.java.net/jdk/pull/9049/files/544ee0f7..2acbedb1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9049=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9049=01-02 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/9049.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9049/head:pull/9049 PR: https://git.openjdk.java.net/jdk/pull/9049
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v2]
> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved > flags is used Thiago Henrique Hüpner has updated the pull request incrementally with one additional commit since the last revision: Fix copyright year - Changes: - all: https://git.openjdk.java.net/jdk/pull/9049/files - new: https://git.openjdk.java.net/jdk/pull/9049/files/cd25d8ad..544ee0f7 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9049=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9049=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/9049.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9049/head:pull/9049 PR: https://git.openjdk.java.net/jdk/pull/9049
Re: RFR: 8287809: Revisit implementation of memory session [v2]
> This is a cleanup of the memory session implementation. The main new concept > is that `MemorySessionImpl` is split into two parts: there is an > implementation of memory session and then there is a state abstraction > (`MemorySessionImpl.State`). This allows to share the state across multiple > session views, in a more clean way. The big consequence of this change is > that the routines on `ScopedMemoryAccess` now have to be defined in terms of > the state abstraction (but the changes are mostly mechanical). > > I have consolidated the implementation quite a bit, by removing all the > duplicated logic for issuing similar-looking exceptions. I have also > addressed an issue with `checkValidState` throwing a _new_ > `WrongThreadException` instead of using a singleton (which is what the logic > for closing down shared session requires, to avoid stack walks that are too > deep). > > `MemorySession.State::checkValidState` is now fully monomorphic; when looking > at benchmarks, this seems to be the best solution in order to make things > fast. Specializing implmentations to remove few plain checks does not buy > enough, and always has the risk of adding profile pollution. Maurizio Cimadamore has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 14 commits: - Merge branch 'master' into cleanup_memory_session_impl_state - Add null check on Buffer::checkState - Add docs Simplify liveness check in Buffer Drop redundant import in DirectBuffer - Simplify checkValidState - Add fastpath for implicit session state - Merge branch 'master' into cleanup_memory_session_impl_keep_list - Fix asNonCloseable to return self Improve direct buffer code with isImplicit predicate - Drop MemorySession interface type from AbstractMemorySessionImpl - Simplify code by removing intermediate getUnsafeBase/getUnsafeOffset methods - Simplify readOnly check - ... and 4 more: https://git.openjdk.java.net/jdk/compare/8d28734e...5b8f7246 - Changes: https://git.openjdk.java.net/jdk/pull/9017/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9017=01 Stats: 1752 lines in 39 files changed: 407 ins; 525 del; 820 mod Patch: https://git.openjdk.java.net/jdk/pull/9017.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9017/head:pull/9017 PR: https://git.openjdk.java.net/jdk/pull/9017
RFR: 8287903: Reduce runtime of java.math microbenchmarks
- Reduce runtime by running fewer forks, fewer iterations, less warmup. All micros tested in this group appear to stabilize very quickly. - Refactor BigIntegers to avoid re-running some (most) micros over and over with parameter values that don't affect them. Expected runtime down from 14 hours to 15 minutes. - Commit messages: - Cleanup SmallShifts, reduce param space - Annotations not cascading to static nested class - Warmup/Measurement not cascading to static nested class - Reduce runtime of java.math micros - Reduce runtime of java.math micros Changes: https://git.openjdk.java.net/jdk/pull/9062/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9062=00 Issue: https://bugs.openjdk.org/browse/JDK-8287903 Stats: 104 lines in 4 files changed: 66 ins; 33 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/9062.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9062/head:pull/9062 PR: https://git.openjdk.java.net/jdk/pull/9062
Re: RFR: 8287007: [cgroups] Consistently use stringStream throughout parsing code [v2]
> Please review this cleanup change in the cgroup subsystem which used to use > hard-coded stack allocated > buffers for concatenating strings in memory. We can use `stringStream` > instead which doesn't have the issue > of hard-coding maximum lengths (and related checks) and makes the code, thus, > easier to read. > > While at it, I've written basic `gtests` verifying current behaviour (and the > same for the JDK code). From > a functionality point of view this is a no-op (modulo the one bug in the > substring case which seems to be > there since day one). I'd prefer if we could defer any change in > functionality to a separate bug as this is > meant to only use stringStream throughout the cgroups code. > > Testing: > - [x] Container tests on Linux x86_64 cgroups v1 and cgroups v2 > - [x] Added tests, which I've verified also pass before the stringStream > change > > Thoughts? Severin Gehwolf 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 six additional commits since the last revision: - Merge branch 'master' into jdk-8287007-string-stream - Add cgroups v2 java test - use stringStream in cgroups v2 - Add gtest for cgroups v2 code path Also fixes the bug when cgroup path is '/'. - 8287007: [cgroups] Consistently use stringStream throughout parsing code - 8287007: [cgroups] Consistently use stringStream throughout parsing code - Changes: - all: https://git.openjdk.java.net/jdk/pull/8969/files - new: https://git.openjdk.java.net/jdk/pull/8969/files/a7b156a4..8047fe37 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8969=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8969=00-01 Stats: 60055 lines in 839 files changed: 34126 ins; 19620 del; 6309 mod Patch: https://git.openjdk.java.net/jdk/pull/8969.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8969/head:pull/8969 PR: https://git.openjdk.java.net/jdk/pull/8969
RFR: 8287902: UnreadableRB case in MissingResourceCauseTest is not working reliably on Windows
The test `test/jdk/java/util/ResourceBundle/Control/MissingResourceCauseTest.java` verifies different failure modes of resource bundles. One of the failures is that the runner class, `MissingResourceCauseTestRun.java`, creates a file `UnreadableRB`, and runs `chmod 000` on it, to make it unreadable by the test. Then MissingResourceCauseTest is called, and the `UnreadableRB` file is removed. This does not work reliably on Windows. On msys2, `chmod` is essentially a no-op, so the file is not made unreadable, and hence the test fails. In my personal cygwin test environment, the chmod command does have some effect, but it is still not enough to make the file unreadable, and so the test fails. The test was originally a shell script test that got converted to Java in [JDK-4354216](https://bugs.openjdk.org/browse/JDK-8213127). The original shell script code explicitly excluded Windows from testing. This was changed in the rewrite, for reasons I cannot determine. What suprises me, though, is the "how can this ever has worked???" factor. Apparently the test passes on the current Cygwin setup on GHA. I have failed to reproduce the working conditions that makes a file actually unreadable for the owner on Windows, neither on my GHA test repo, nor locally. I've searched the web to figure out how to properly set file permissions on Windows to make the file unreadable, using Windows native tools, to no avail. I've even asked a [Stack Overflow question](https://stackoverflow.com/questions/72528318/what-file-permissions-make-a-file-unreadable-by-owner-in-windows); which as of yet is still unanswered. Since I feel I've spent far more time than reasonable trying to get this to work properly, I suggest we instead skip the unreadable test on Windows. It is clearly unstable and highly depending on the Windows environment, the test was never originally supported or intended for Windows, and at the of the day, testing file unreadability is not an important regression test for [JDK-4354216](https://bugs.openjdk.org/browse/JDK-4354216). - Commit messages: - 8287902: UnreadableRB case in MissingResourceCauseTest is not working reliably on Windows Changes: https://git.openjdk.java.net/jdk/pull/9061/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9061=00 Issue: https://bugs.openjdk.org/browse/JDK-8287902 Stats: 3 lines in 1 file changed: 3 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/9061.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9061/head:pull/9061 PR: https://git.openjdk.java.net/jdk/pull/9061
Integrated: 8287896: PropertiesTest.sh fail on msys2
On Tue, 7 Jun 2022 10:14:47 GMT, Magnus Ihse Bursie wrote: > The test `test/jdk/java/util/Currency/PropertiesTest.sh` fails on msys2, > since it does not properly detect this environment. This pull request has now been integrated. Changeset: f1dd559e Author:Magnus Ihse Bursie URL: https://git.openjdk.java.net/jdk/commit/f1dd559e20342b892d0c1ed0314e5bba451bc5d3 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod 8287896: PropertiesTest.sh fail on msys2 Reviewed-by: naoto - PR: https://git.openjdk.java.net/jdk/pull/9057
Integrated: 8287860: Revise usage of volatile in j.u.Locale
On Mon, 6 Jun 2022 12:58:39 GMT, Сергей Цыпанов wrote: > - cached hash code of `Locale` and `Locale$LanguageRange` shouldn't be > volatile, even in case of race in the worst case it is recalculated at most > once per thread > - `defaultLocale` field is read multiple times in `initDefault()` > - `isoLanguages` is accessed multiple times in `getISOLanguages()` > - `languageTag` is read twice in `toLanguageTag()` This pull request has now been integrated. Changeset: 4fe0ca9e Author:Sergey Tsypanov Committer: Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/4fe0ca9ec3b995eb113ac214219cae22f8c9 Stats: 24 lines in 2 files changed: 5 ins; 0 del; 19 mod 8287860: Revise usage of volatile in j.u.Locale Reviewed-by: naoto - PR: https://git.openjdk.java.net/jdk/pull/9041
Re: RFR: 8287896: PropertiesTest.sh fail on msys2
On Tue, 7 Jun 2022 10:14:47 GMT, Magnus Ihse Bursie wrote: > The test `test/jdk/java/util/Currency/PropertiesTest.sh` fails on msys2, > since it does not properly detect this environment. Looks good. Thanks for the change. - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/9057
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v20]
On Wed, 25 May 2022 00:35:24 GMT, Joe Darcy wrote: >> This is an early review of changes to better model JVM access flags, that is >> "modifiers" like public, protected, etc. but explicitly at a VM level. >> >> Language level modifiers and JVM level access flags are closely related, but >> distinct. There are concepts that overlap in the two domains (public, >> private, etc.), others that only have a language-level modifier (sealed), >> and still others that only have an access flag (synthetic). >> >> The existing java.lang.reflect.Modifier class is inadequate to model these >> subtleties. For example, the bit positions used by access flags on different >> kinds of elements overlap (such as "volatile" for fields and "bridge" for >> methods. Just having a raw integer does not provide sufficient context to >> decode the corresponding language-level string. Methods like >> Modifier.methodModifiers() were introduced to cope with this situation. >> >> With additional modifiers and flags on the horizon with projects like >> Valhalla, addressing the existent modeling deficiency now ahead of time is >> reasonable before further strain is introduced. >> >> This PR in its current form is meant to give the overall shape of the API. >> It is missing implementations to map from, say, method modifiers to access >> flags, taking into account overlaps in bit positions. >> >> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in >> once the API is further along. > > Joe Darcy 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 32 additional commits since > the last revision: > > - Target JDK 20 rather than 19. > - Merge branch 'master' into JDK-8266670 > - Add mask values to constants' javadoc. > - Implement review feedback from mlchung. > - Fix type in @throws tag. > - Merge branch 'master' into JDK-8266670 > - Respond to review feedback. > - Merge branch 'master' into JDK-8266670 > - Make workding changes suggested in review feedback. > - Merge branch 'master' into JDK-8266670 > - ... and 22 more: > https://git.openjdk.java.net/jdk/compare/187b34b0...05cf2d8b Marked as reviewed by asotona (Committer). - PR: https://git.openjdk.java.net/jdk/pull/7445
RFR: 8287896: PropertiesTest.sh fail on msys2
The test `test/jdk/java/util/Currency/PropertiesTest.sh` fails on msys2, since it does not properly detect this environment. - Commit messages: - 8287896: PropertiesTest.sh fail on msys2 Changes: https://git.openjdk.java.net/jdk/pull/9057/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9057=00 Issue: https://bugs.openjdk.org/browse/JDK-8287896 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/9057.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9057/head:pull/9057 PR: https://git.openjdk.java.net/jdk/pull/9057
Re: RFR: 8202449: overflow handling in Random.doubles [v3]
On Tue, 24 May 2022 12:58:45 GMT, Raffaello Giulietti wrote: >> Extend the range of Random.doubles(double, double) and similar methods. > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 8202449: overflow handling in Random.doubles Hi, the major code changes are in `RandomSupport` in methods checkRange(float,float) checkRange(double,double) where the checks are more tolerant, and in boundedNextDouble(RandomGenerator,double,double) boundedNextFloat(RandomGenerator,float,float) that extend the accepted range. In case the range size overflows, the methods half the range, generate a value inside the reduced range and finally double the outcome. - PR: https://git.openjdk.java.net/jdk/pull/8791
Re: RFR: 8287007: [cgroups] Consistently use stringStream throughout parsing code
On Wed, 1 Jun 2022 09:18:38 GMT, Severin Gehwolf wrote: > Please review this cleanup change in the cgroup subsystem which used to use > hard-coded stack allocated > buffers for concatenating strings in memory. We can use `stringStream` > instead which doesn't have the issue > of hard-coding maximum lengths (and related checks) and makes the code, thus, > easier to read. > > While at it, I've written basic `gtests` verifying current behaviour (and the > same for the JDK code). From > a functionality point of view this is a no-op (modulo the one bug in the > substring case which seems to be > there since day one). I'd prefer if we could defer any change in > functionality to a separate bug as this is > meant to only use stringStream throughout the cgroups code. > > Testing: > - [x] Container tests on Linux x86_64 cgroups v1 and cgroups v2 > - [x] Added tests, which I've verified also pass before the stringStream > change > > Thoughts? @iklam Would you have some cycles to review this, please? - PR: https://git.openjdk.java.net/jdk/pull/8969
Integrated: 8287663 Add a regression test for JDK-8287073
On Thu, 2 Jun 2022 14:32:28 GMT, Severin Gehwolf wrote: > This adds a regression test for a recent fix (JDK-8287073). I've restructured > the linux specific JDK code to call a separate static function to enable this > test. It'll help future tests too. > > Testing: > - [x] Container tests continue to pass + GHA > - [x] New regression test fails prior the code fix of JDK-8287073 and passes > with it This pull request has now been integrated. Changeset: 2d8c6490 Author:Severin Gehwolf URL: https://git.openjdk.java.net/jdk/commit/2d8c6490540e3ccea23b81129b2e4073915071e0 Stats: 36 lines in 2 files changed: 35 ins; 0 del; 1 mod 8287663: Add a regression test for JDK-8287073 Reviewed-by: iklam - PR: https://git.openjdk.java.net/jdk/pull/8993
Re: RFR: 8287663 Add a regression test for JDK-8287073
On Tue, 7 Jun 2022 04:17:44 GMT, Ioi Lam wrote: > > We should try to consolidate these test cases to improve maintainability. > > I filed [JDK-8287185](https://bugs.openjdk.org/browse/JDK-8287185) Agreed. Thanks for the review @iklam - PR: https://git.openjdk.java.net/jdk/pull/8993
Re: RFR: 8233760: Result of BigDecimal.toString throws overflow exception on new BigDecimal(str) [v2]
On Fri, 27 May 2022 22:59:47 GMT, Raffaello Giulietti wrote: >> BigDecimal(String) currently fails to accept some strings produced by >> BigDecimal.toString(). This PR removes this limitation. > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 8233760: Result of BigDecimal.toString throws overflow exception on new > BigDecimal(str) Hello, here's an explanation of the changes in `BigDecimal`. Rather than keeping track of a local `int scl` (scale) and a local `long exp` (exponent), the change only keeps track of a `long scl`, adapting it accordingly. It checks the range of `scl` before initializing the fields. This makes for a simpler code. In addition, I couldn't resist changing C-style arrays to Java style over the whole class. - PR: https://git.openjdk.java.net/jdk/pull/8905
Re: RFR: 8287522: StringConcatFactory: Add in prependers and mixers in batches [v8]
> When generating `MethodHandle`-based concatenation expressions in > `StringConcatFactory` we can reduce the number of classes generated at > runtime by creating small batches of prependers and mixers before binding > them into the root expression tree. > > Improvements on one-off tests are modest, while the improvement on > bootstrapping stress tests can be substantial > ([MixedStringCombinations.java](https://gist.github.com/cl4es/08fb581dece3a73e89bfa52337bc4248)): > > | Build |# classes| Runtime | > | --- | - | --- | > | Baseline | 31119 | 2.942s | > | Patch | 16208 | 1.958s | > > An earlier version of this patch saw a regression in the > `StringConcatFactoryBootstraps` microbenchmark. After some refactoring along > with the optimizations in #8881 and #8923 that is no longer the case, and > allocation pressure is down slightly compared to the baseline on such a > repeat-the-same-bootstrap stress test: > > Baseline: > > Benchmark Mode Cnt > Score Error Units > SCFB.makeConcatWithConstants avgt5 > 2170.039 ? 117.441 ns/op > SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt5 > 3538.020 ?4.558B/op > > This patch: > > Benchmark Mode Cnt > Score Error Units > SCFB.makeConcatWithConstants avgt5 > 2144.807 ? 162.685 ns/op > SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt5 > 3184.943 ?4.495B/op Claes Redestad 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 17 additional commits since the last revision: - Merge branch 'master' into scf_chunked - We now don't need big Species classes for shorter concats, so on some tests the improvements meant more Species class generation. Adjusting HelloClasslist - Clarify that consts are always reduced to null, even if calling with constants. Also clarify that the number of constants is paramCount + 1 by refactoring to an array - Mandy review comment #1: Cleanup LF.basicType a bit more. - Cleanup too generic String.valueOf lookups - Address review comments from @ExE-Boss - Improve bootstrap microbenchmark to include more shapes, reduce runtime - Minor stylistic cleanup - Revert change to HelloClasslist (doesn't affect generation) - Reduce allocation adding in mixers by extracting constant arg lists and caching double arg mixers - ... and 7 more: https://git.openjdk.java.net/jdk/compare/98404245...afe61394 - Changes: - all: https://git.openjdk.java.net/jdk/pull/8855/files - new: https://git.openjdk.java.net/jdk/pull/8855/files/aaa442d5..afe61394 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8855=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8855=06-07 Stats: 59296 lines in 765 files changed: 33676 ins; 19450 del; 6170 mod Patch: https://git.openjdk.java.net/jdk/pull/8855.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8855/head:pull/8855 PR: https://git.openjdk.java.net/jdk/pull/8855
Re: RFR: 8273346: Expand library mappings to IEEE 754 operations [v4]
On Mon, 6 Jun 2022 22:24:03 GMT, Joe Darcy wrote: >> Generally add apiNote's to map from Java library methods to particular IEEE >> 754 operations. For now, I only added such notes to java.lang.Math and not >> java.lang.StrictMath. > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Respond to review feedback. LGTM - PR: https://git.openjdk.java.net/jdk/pull/8876
Integrated: 8283667: [vectorapi] Vectorization for masked load with IOOBE with predicate feature
On Wed, 30 Mar 2022 10:31:59 GMT, Xiaohong Gong wrote:
> Currently the vector load with mask when the given index happens out of the
> array boundary is implemented with pure java scalar code to avoid the IOOBE
> (IndexOutOfBoundaryException). This is necessary for architectures that do
> not support the predicate feature. Because the masked load is implemented
> with a full vector load and a vector blend applied on it. And a full vector
> load will definitely cause the IOOBE which is not valid. However, for
> architectures that support the predicate feature like SVE/AVX-512/RVV, it can
> be vectorized with the predicated load instruction as long as the indexes of
> the masked lanes are within the bounds of the array. For these architectures,
> loading with unmasked lanes does not raise exception.
>
> This patch adds the vectorization support for the masked load with IOOBE
> part. Please see the original java implementation (FIXME: optimize):
>
>
> @ForceInline
> public static
> ByteVector fromArray(VectorSpecies species,
>byte[] a, int offset,
>VectorMask m) {
> ByteSpecies vsp = (ByteSpecies) species;
> if (offset >= 0 && offset <= (a.length - species.length())) {
> return vsp.dummyVector().fromArray0(a, offset, m);
> }
>
> // FIXME: optimize
> checkMaskFromIndexSize(offset, vsp, m, 1, a.length);
> return vsp.vOp(m, i -> a[offset + i]);
> }
>
> Since it can only be vectorized with the predicate load, the hotspot must
> check whether the current backend supports it and falls back to the java
> scalar version if not. This is different from the normal masked vector load
> that the compiler will generate a full vector load and a vector blend if the
> predicate load is not supported. So to let the compiler make the expected
> action, an additional flag (i.e. `usePred`) is added to the existing
> "loadMasked" intrinsic, with the value "true" for the IOOBE part while
> "false" for the normal load. And the compiler will fail to intrinsify if the
> flag is "true" and the predicate load is not supported by the backend, which
> means that normal java path will be executed.
>
> Also adds the same vectorization support for masked:
> - fromByteArray/fromByteBuffer
> - fromBooleanArray
> - fromCharArray
>
> The performance for the new added benchmarks improve about `1.88x ~ 30.26x`
> on the x86 AVX-512 system:
>
> Benchmark before After Units
> LoadMaskedIOOBEBenchmark.byteLoadArrayMaskIOOBE 737.542 1387.069 ops/ms
> LoadMaskedIOOBEBenchmark.doubleLoadArrayMaskIOOBE 118.366 330.776 ops/ms
> LoadMaskedIOOBEBenchmark.floatLoadArrayMaskIOOBE 233.832 6125.026 ops/ms
> LoadMaskedIOOBEBenchmark.intLoadArrayMaskIOOBE233.816 7075.923 ops/ms
> LoadMaskedIOOBEBenchmark.longLoadArrayMaskIOOBE 119.771 330.587 ops/ms
> LoadMaskedIOOBEBenchmark.shortLoadArrayMaskIOOBE 431.961 939.301 ops/ms
>
> Similar performance gain can also be observed on 512-bit SVE system.
This pull request has now been integrated.
Changeset: 39fa52b5
Author:Xiaohong Gong
URL:
https://git.openjdk.java.net/jdk/commit/39fa52b5f7504eca7399b863b0fb934bdce37f7e
Stats: 453 lines in 44 files changed: 174 ins; 21 del; 258 mod
8283667: [vectorapi] Vectorization for masked load with IOOBE with predicate
feature
Reviewed-by: sviswanathan, psandoz
-
PR: https://git.openjdk.java.net/jdk/pull/8035
Re: RFR: 8283667: [vectorapi] Vectorization for masked load with IOOBE with predicate feature [v3]
On Thu, 12 May 2022 16:07:54 GMT, Paul Sandoz wrote: >> Xiaohong Gong has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Rename "use_predicate" to "needs_predicate" > > Yes, the tests were run in debug mode. The reporting of the missing constant > occurs for the compiled method that is called from the method where the > constants are declared e.g.: > > 719 240bjdk.incubator.vector.Int256Vector::fromArray0 (15 > bytes) > ** Rejected vector op (LoadVectorMasked,int,8) because architecture does > not support it > ** missing constant: offsetInRange=Parm > @ 11 > jdk.incubator.vector.IntVector::fromArray0Template (22 bytes) force inline > by annotation > > > So it appears to be working as expected. A similar pattern occurs at a > lower-level for the passing of the mask class. `Int256Vector::fromArray0` > passes a constant class to `IntVector::fromArray0Template` (the compilation > of which bails out before checking that the `offsetInRange` is constant). Thanks for the review @PaulSandoz @sviswa7 @jatin-bhateja @merykitty ！ - PR: https://git.openjdk.java.net/jdk/pull/8035
Re: RFR: 8283667: [vectorapi] Vectorization for masked load with IOOBE with predicate feature [v6]
On Tue, 7 Jun 2022 04:29:40 GMT, Xiaohong Gong wrote:
>> Currently the vector load with mask when the given index happens out of the
>> array boundary is implemented with pure java scalar code to avoid the IOOBE
>> (IndexOutOfBoundaryException). This is necessary for architectures that do
>> not support the predicate feature. Because the masked load is implemented
>> with a full vector load and a vector blend applied on it. And a full vector
>> load will definitely cause the IOOBE which is not valid. However, for
>> architectures that support the predicate feature like SVE/AVX-512/RVV, it
>> can be vectorized with the predicated load instruction as long as the
>> indexes of the masked lanes are within the bounds of the array. For these
>> architectures, loading with unmasked lanes does not raise exception.
>>
>> This patch adds the vectorization support for the masked load with IOOBE
>> part. Please see the original java implementation (FIXME: optimize):
>>
>>
>> @ForceInline
>> public static
>> ByteVector fromArray(VectorSpecies species,
>>byte[] a, int offset,
>>VectorMask m) {
>> ByteSpecies vsp = (ByteSpecies) species;
>> if (offset >= 0 && offset <= (a.length - species.length())) {
>> return vsp.dummyVector().fromArray0(a, offset, m);
>> }
>>
>> // FIXME: optimize
>> checkMaskFromIndexSize(offset, vsp, m, 1, a.length);
>> return vsp.vOp(m, i -> a[offset + i]);
>> }
>>
>> Since it can only be vectorized with the predicate load, the hotspot must
>> check whether the current backend supports it and falls back to the java
>> scalar version if not. This is different from the normal masked vector load
>> that the compiler will generate a full vector load and a vector blend if the
>> predicate load is not supported. So to let the compiler make the expected
>> action, an additional flag (i.e. `usePred`) is added to the existing
>> "loadMasked" intrinsic, with the value "true" for the IOOBE part while
>> "false" for the normal load. And the compiler will fail to intrinsify if the
>> flag is "true" and the predicate load is not supported by the backend, which
>> means that normal java path will be executed.
>>
>> Also adds the same vectorization support for masked:
>> - fromByteArray/fromByteBuffer
>> - fromBooleanArray
>> - fromCharArray
>>
>> The performance for the new added benchmarks improve about `1.88x ~ 30.26x`
>> on the x86 AVX-512 system:
>>
>> Benchmark before After Units
>> LoadMaskedIOOBEBenchmark.byteLoadArrayMaskIOOBE 737.542 1387.069 ops/ms
>> LoadMaskedIOOBEBenchmark.doubleLoadArrayMaskIOOBE 118.366 330.776 ops/ms
>> LoadMaskedIOOBEBenchmark.floatLoadArrayMaskIOOBE 233.832 6125.026 ops/ms
>> LoadMaskedIOOBEBenchmark.intLoadArrayMaskIOOBE233.816 7075.923 ops/ms
>> LoadMaskedIOOBEBenchmark.longLoadArrayMaskIOOBE 119.771 330.587 ops/ms
>> LoadMaskedIOOBEBenchmark.shortLoadArrayMaskIOOBE 431.961 939.301 ops/ms
>>
>> Similar performance gain can also be observed on 512-bit SVE system.
>
> Xiaohong Gong has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains seven commits:
>
> - Add constant OFFSET_IN_RANGE and OFFSET_OUT_OF_RANGE
> - Merge branch 'jdk:master' into JDK-8283667
> - Merge branch 'jdk:master' into JDK-8283667
> - Use integer constant for offsetInRange all the way through
> - Rename "use_predicate" to "needs_predicate"
> - Rename the "usePred" to "offsetInRange"
> - 8283667: [vectorapi] Vectorization for masked load with IOOBE with
> predicate feature
LGTM.
-
PR: https://git.openjdk.java.net/jdk/pull/8035
Integrated: 8287785: Reduce runtime of java.lang.invoke microbenchmarks
On Fri, 3 Jun 2022 11:16:29 GMT, Claes Redestad wrote: > - Add explicit run configurations to java.lang.invoke micros, aiming to > reduce runtime while maintaining a decently high confidence that there's > enough warmup to produce good enough data. > > - Remove several trivial baseline micros, mainly those that only return a > static object: It's reasonable to have baseline microbenchmarks when the > baseline op is complex and you're mostly interested in checking the overhead > of doing the same thing via some MH API, but blackhole operations are now > shortcutting very quickly and timings doesn't differ from one type of object > to another, so we don't need a multitude of such baseline tests. > > Estimated runtime of `make test TEST=micro:java.lang.micro` (excluding build) > drops from just above 28 to just above 3 hours. This pull request has now been integrated. Changeset: 42261d75 Author:Claes Redestad URL: https://git.openjdk.java.net/jdk/commit/42261d752a140325496ffdd40d9ad62b189d1b3e Stats: 439 lines in 47 files changed: 276 ins; 153 del; 10 mod 8287785: Reduce runtime of java.lang.invoke microbenchmarks Reviewed-by: mchung - PR: https://git.openjdk.java.net/jdk/pull/9012
Integrated: 8287810: Reduce runtime of java.lang microbenchmarks
On Fri, 3 Jun 2022 15:05:33 GMT, Claes Redestad wrote: > - Reduce runtime by providing explicit configurations that try to run the > micros as fast as possible while giving enough time for warmup. > - Remove some excessive parameters > - Remove some benchmarks testing experimental features > (ObjectHashCode.mode_) or were simply dubious at large > (StringHttp) > > This reduces runtime of running `java.lang.[A-Z]` (only java.lang, not > java.lang.reflect etc..) from over 56 hours to 6:45. This pull request has now been integrated. Changeset: 778ed1a7 Author:Claes Redestad URL: https://git.openjdk.java.net/jdk/commit/778ed1a760d8f673811914b75e5d14e465881c91 Stats: 293 lines in 39 files changed: 192 ins; 87 del; 14 mod 8287810: Reduce runtime of java.lang microbenchmarks Reviewed-by: mchung - PR: https://git.openjdk.java.net/jdk/pull/9015
Integrated: 8287798: Reduce runtime of java.lang.reflect/runtime microbenchmarks
On Fri, 3 Jun 2022 12:46:31 GMT, Claes Redestad wrote: > Add explicit run configurations to java.lang.reflect and .runtime micros, > aiming to reduce runtime while maintaining a decently high confidence that > there's enough warmup to produce good enough data. > > Roughly halves runtime of running `java.lang.reflect|java.lang.runtime` from > 159 to 78 minutes. This pull request has now been integrated. Changeset: aa6c568a Author:Claes Redestad URL: https://git.openjdk.java.net/jdk/commit/aa6c568a06fa92263d4b74ff979eb521ae953bc8 Stats: 27 lines in 4 files changed: 25 ins; 2 del; 0 mod 8287798: Reduce runtime of java.lang.reflect/runtime microbenchmarks Reviewed-by: jvernee, mchung - PR: https://git.openjdk.java.net/jdk/pull/9013
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used
On Tue, 7 Jun 2022 00:45:17 GMT, Thiago Henrique Hüpner wrote: > 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved > flags is used The change looks okay but I think we should add a test. You'll have to check the test tree to see if there are existing tests for incubator modules packages as JAR files, there may not be as it's an unusual setup. - PR: https://git.openjdk.java.net/jdk/pull/9049
Re: RFR: 8286850: [macos] Add support for signing user provided app image [v2]
Alexander, I had an existing local GitHub repo for the jdk I updated that appeared to accept the parameters you indicated. It generated a jdk 19. If you are saying I’m not getting the main branch or the update for some reason has dependencies I’m not getting I would have to determine how to correctly get these, or, I guess wait for a release that has all the necessary. Determining if this worked as expected before a release seemed like it would be a good idea. Yes, the point of my original suggestion was to allow generating the application unsigned, then do post-processing - like modify the default Info.plist, and finally separately sign. I thought your change provided the means to do this by first generating an unsigned image using —type app-image, then on a separate invocation indicate the app-image and sign and package it. > ./build/*/images/jdk/bin/jpackage --type app-image --app-image If I follow you now it isn’t a two step but a three step process. 1) Generate unsigned application and do post-processing. 2) Sign modified app-image 3) DMG or PKG the modified and signed app-image Is this correct? Thanks, Mike
Re: CVF: new Core Libraries Group member: Naoto Sato
Vote: yes
Re: RFR: 8283667: [vectorapi] Vectorization for masked load with IOOBE with predicate feature [v5]
On Tue, 7 Jun 2022 02:22:53 GMT, Xiaohong Gong wrote:
>> test/micro/org/openjdk/bench/jdk/incubator/vector/LoadMaskedIOOBEBenchmark.java
>> line 97:
>>
>>> 95: public void byteLoadArrayMaskIOOBE() {
>>> 96: for (int i = 0; i < inSize; i += bspecies.length()) {
>>> 97: VectorMask mask = VectorMask.fromArray(bspecies, m,
>>> i);
>>
>> For other case "if (offset >= 0 && offset <= (a.length - species.length()))
>> )" we are anyways intrinsifying, should we limit this micro to work only for
>> newly optimized case.
>
> Yeah, thanks and it's really a good suggestion to limit this benchmark only
> for the IOOBE cases. I locally modified the tests to make sure only the IOOBE
> case happens and the results show good as well. But do you think it's better
> to keep as it is since we can also see the performance of the common cases to
> make sure no regressions happen? As the current benchmarks can also show the
> performance gain by this PR.
It was just to remove the noise from a targeted micro benchmark. But we can
keep it as it is.
-
PR: https://git.openjdk.java.net/jdk/pull/8035
Re: RFR: 8283667: [vectorapi] Vectorization for masked load with IOOBE with predicate feature [v5]
On Tue, 7 Jun 2022 06:41:36 GMT, Jatin Bhateja wrote: >> Yeah, thanks and it's really a good suggestion to limit this benchmark only >> for the IOOBE cases. I locally modified the tests to make sure only the >> IOOBE case happens and the results show good as well. But do you think it's >> better to keep as it is since we can also see the performance of the common >> cases to make sure no regressions happen? As the current benchmarks can also >> show the performance gain by this PR. > > It was just to remove the noise from a targeted micro benchmark. But we can > keep it as it is. OK, thanks! - PR: https://git.openjdk.java.net/jdk/pull/8035
Re: RFR: JDK-8287838: Update Float and Double to use snippets [v2]
On Mon, 6 Jun 2022 16:54:29 GMT, Joe Darcy wrote: >> src/java.base/share/classes/java/lang/Double.java line 683: >> >>> 681: * "[\\x00-\\x20]*");// Optional trailing "whitespace" >>> 682: * >>> 683: * if (Pattern.matches(fpRegex, myString)) // @link >>> substring="Pattern.matches" target ="java.util.regex.Pattern#matches" >> >> If you want to avoid the annoyingly long line then you can put the "// @link >> ..." on the previous line if you want. > > It can be done with a snippet region like this: > > > * // @link region substring="Pattern.matches" target > ="java.util.regex.Pattern#matches" > * if (Pattern.matches(fpRegex, myString)) > * Double.valueOf(myString); // Will not throw NumberFormatException > * // @end Yes, that works. The alternative is to terminate the comment line with a colon (:) and that applies markup tag to the line that follows. In this case, it would be: ``` // @link substring="Pattern.matches" target ="java.util.regex.Pattern#matches" : - PR: https://git.openjdk.java.net/jdk/pull/9034
