Re: RFR: 8332826: Make hashCode methods in ArraysSupport friendlier [v2]
> Please review this PR, which supersedes a now withdrawn > https://github.com/openjdk/jdk/pull/14831. > > This PR replaces `ArraysSupport.vectorizedHashCode` with a set of more > user-friendly methods. Here's a summary: > > - Made the operand constants (i.e. `T_BOOLEAN` and friends) and the > `vectorizedHashCode` method private > > - Made the `vectorizedHashCode` method private, but didn't rename it. > Renaming would dramatically increase this PR review cost, because that > method's name is used by a lot of VM code. On a bright side, since the method > is now private, it's no longer callable by clients of `ArraysSupport`, thus a > problem of an inaccurate name is less severe. > > - Made the `ArraysSupport.utf16HashCode` method private > > - Moved tiny cases (i.e. 0, 1, 2) to `ArraysSupport` Pavel Rappo has updated the pull request incrementally with one additional commit since the last revision: Fix incorrect utf16 hashCode adaptation - Changes: - all: https://git.openjdk.org/jdk/pull/19414/files - new: https://git.openjdk.org/jdk/pull/19414/files/4ed451d6..adc7557d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19414=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19414=00-01 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19414.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19414/head:pull/19414 PR: https://git.openjdk.org/jdk/pull/19414
Re: RFR: 8328821: Map.of().entrySet() mutators should throw UnsupportedOperationException [v2]
> This change overrides mutator methods in the implementation returned by > `Map.of().entrySet()` to throw `UnsupportedOperationException`. Liam Miller-Cushon 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 five additional commits since the last revision: - Check m.entrySet().hashCode() in MOAT - Merge remote-tracking branch 'origin/master' into JDK-8328821-make-clear-consistent - Use AbstractImmutableSet - Throw UOE for all Map.of().entrySet() mutator methods - 8328821: Make the ImmutableCollections clear() call consistent Without overriding clear(), a call to it in an empty map would just return, as iterator.hasNext() would be false. However if calling Map.of().clear() throws an exception. To make the behavior of Map.of().entrySet().clear() consistent, we need to have an implementation of clear() for the entry set that throws as well. - Changes: - all: https://git.openjdk.org/jdk/pull/18522/files - new: https://git.openjdk.org/jdk/pull/18522/files/42d67d16..17851d80 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18522=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18522=00-01 Stats: 211051 lines in 4323 files changed: 117813 ins; 68929 del; 24309 mod Patch: https://git.openjdk.org/jdk/pull/18522.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18522/head:pull/18522 PR: https://git.openjdk.org/jdk/pull/18522
Re: RFR: 8328821: Map.of().entrySet() mutators should throw UnsupportedOperationException [v2]
On Mon, 27 May 2024 12:18:23 GMT, Chen Liang wrote:
>> src/java.base/share/classes/java/util/ImmutableCollections.java line 1323:
>>
>>> 1321: @Override
>>> 1322: public int hashCode() {
>>> 1323: return MapN.this.hashCode();
>>
>> The hash code for a `Set` is defined as the sum of the elements in the `Set`
>> (hash(`null`) == 0). The `Map. Entry` hash code is defined the same way
>> `MapN.this.hashCode` operates so this seems right. It would be nice with a
>> couple of tests that assert this invariant.
>>
>> Perhaps the existing tests already cover this?
>
> Good point, this is currently missing from MOAT in Collections.
>
> Though it can be as simple as one line here:
> https://github.com/openjdk/jdk/blob/891d5aedf12e837c9a9c7cb800fb3affa7430f00/test/jdk/java/util/Collection/MOAT.java#L1329
>
> check(m.hashCode() == m.entrySet().hashCode());
Thanks, I added the suggested assertion.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18522#discussion_r1616313870
RFR: 8332826: Make hashCode methods in ArraysSupport friendlier
Please review this PR, which supersedes a now withdrawn https://github.com/openjdk/jdk/pull/14831. This PR replaces `ArraysSupport.vectorizedHashCode` with a set of more user-friendly methods. Here's a summary: - Made the operand constants (i.e. `T_BOOLEAN` and friends) and the `vectorizedHashCode` method private - Made the `vectorizedHashCode` method private, but didn't rename it. Renaming would dramatically increase this PR review cost, because that method's name is used by a lot of VM code. On a bright side, since the method is now private, it's no longer callable by clients of `ArraysSupport`, thus a problem of an inaccurate name is less severe. - Made the `ArraysSupport.utf16HashCode` method private - Moved tiny cases (i.e. 0, 1, 2) to `ArraysSupport` - Commit messages: - Initial commit Changes: https://git.openjdk.org/jdk/pull/19414/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19414=00 Issue: https://bugs.openjdk.org/browse/JDK-8332826 Stats: 258 lines in 13 files changed: 186 ins; 32 del; 40 mod Patch: https://git.openjdk.org/jdk/pull/19414.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19414/head:pull/19414 PR: https://git.openjdk.org/jdk/pull/19414
Withdrawn: 8311864: Add ArraysSupport.hashCode(int[] a, fromIndex, length, initialValue)
On Tue, 11 Jul 2023 16:35:51 GMT, Pavel Rappo wrote: > This PR adds an internal method to calculate hash code from the provided > integer array, offset and length into that array, and the initial hash code > value. > > Current options for calculating a hash code for int[] are inflexible. It's > either ArraysSupport.vectorizedHashCode with an offset, length and initial > value, or Arrays.hashCode with just an array. > > For an arbitrary int[], unconditional vectorization might be unwarranted or > puzzling. Unfortunately, it's the only hash code method that operates on an > array subrange or accepts the initial hash code value. > > A more convenient method could be added and then used, for example, here: > > * > https://github.com/openjdk/jdk/blob/0ef03f122866f010ebf50683097e9b92e41cdaad/src/java.base/share/classes/java/util/concurrent/CopyOnWriteArrayList.java#L1076-L1083 > > * > https://github.com/openjdk/jdk/blob/0ef03f122866f010ebf50683097e9b92e41cdaad/src/java.base/share/classes/java/util/ArrayList.java#L669-L680 > > * > https://github.com/openjdk/jdk/blob/0ef03f122866f010ebf50683097e9b92e41cdaad/src/java.base/share/classes/sun/security/pkcs10/PKCS10.java#L356-L362 > > This PR adds such a method and provides tests for it. Additionally, this PR > adds tests for `null` passed to `java.util.Arrays.hashCode` overloads, > behaviour which was specified but untested. This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/14831
Re: RFR: 8331670: Deprecate the Memory-Access Methods in sun.misc.Unsafe for Removal [v4]
> This is the implementation changes for JEP 471. > > The methods in sun.misc.Unsafe for on-heap and off-heap access are deprecated > for removal. This means a removal warning at compile time. No methods have > been removed. A deprecated message is added to each of the methods but > unlikely to be seen as the JDK does not generate or publish the API docs for > this class. > > A new command line option --sun-misc-unsafe-memory-access=$value is > introduced to allow or deny access to these methods. The default proposed for > JDK 23 is "allow" so no change in behavior compared to JDK 22 or previous > releases. > > A new test is added to test the command line option settings. The existing > micros for FFM that use Unsafe are updated to suppress the removal warning at > compile time. A new micro is introduced with a small sample of methods to > ensure the changes doesn't cause any perf regressions ([sample > results](https://cr.openjdk.org/~alanb/8331670-results.txt)). > > For now, the changes include the update to the man page for the "java" > command. It might be that this has to be separated out so that it goes with > other updates in the release. Alan Bateman has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 13 commits: - Merge - Fix typo in comment - Merge - Merge - Add removal to DISABLED_WARNINGS Fail at startup if bad value specified to option - Merge - Update man page - Update man page - Fix comment - Merge - ... and 3 more: https://git.openjdk.org/jdk/compare/985b9ce7...78fc22d7 - Changes: https://git.openjdk.org/jdk/pull/19174/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19174=03 Stats: 1234 lines in 8 files changed: 1150 ins; 5 del; 79 mod Patch: https://git.openjdk.org/jdk/pull/19174.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19174/head:pull/19174 PR: https://git.openjdk.org/jdk/pull/19174
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v3]
On Mon, 27 May 2024 16:08:04 GMT, Adam Sotona wrote: >> src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java line 822: >> >>> 820:.iconst_0() // false >>> 821:.aload(0)// classLoader >>> 822:.invokestatic(CD_Class, "forName", >>> MTD_Class_String_boolean_ClassLoader); >> >> We can probably replace this `forName(name, false, thisClassLoader)` with >> loading a class constant to reduce load on symbols. > > I'm wondering why all the `Class.forName(... .getClassLoader())` > is necessary? > Simple `ldc ` seems to work well. Indeed, I initially added it because a JBS ticket recommended so. ldc instruction has the same behavior as such as forName call. - PR Review Comment: https://git.openjdk.org/jdk/pull/19410#discussion_r1616241702
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v3]
On Mon, 27 May 2024 12:24:31 GMT, Chen Liang wrote: >> Adam Sotona has updated the pull request incrementally with one additional >> commit since the last revision: >> >> performance improvements > > src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java line 822: > >> 820:.iconst_0() // false >> 821:.aload(0)// classLoader >> 822:.invokestatic(CD_Class, "forName", >> MTD_Class_String_boolean_ClassLoader); > > We can probably replace this `forName(name, false, thisClassLoader)` with > loading a class constant to reduce load on symbols. I'm wondering why all the `Class.forName(... .getClassLoader())` is necessary? Simple `ldc ` seems to work well. - PR Review Comment: https://git.openjdk.org/jdk/pull/19410#discussion_r1616232095
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v5]
> [JDK-8294961](https://bugs.openjdk.org/browse/JDK-8294961) changed to use > classfile API for reflection proxy-generation. Actual implementation of > `ProxyGenerator` is focused on performance, however it causes JDK bootstrap > regressions. `ProxyGenerator.TEMPLATE` class model is statically created and > each proxy class is transformed from the template. > > This patch is intended to examine plain proxy generation impact on > performance and JDK bootstrap (vs proxy transformation from template). > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: simplification of the proxy class loading - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/eaf30201..3d975d28 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=04 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=03-04 Stats: 60 lines in 1 file changed: 0 ins; 50 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/19410.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19410/head:pull/19410 PR: https://git.openjdk.org/jdk/pull/19410
Re: RFR: 8332064: Implementation of Structured Concurrency (Third Preview) [v2]
> There aren't any API or implementations changes in third preview but the JEP > number/title needs to be bumped for the javadoc page. Alan Bateman 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 - Initial commit - Changes: - all: https://git.openjdk.org/jdk/pull/19175/files - new: https://git.openjdk.org/jdk/pull/19175/files/cff7b36b..c9ae4566 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19175=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19175=00-01 Stats: 63129 lines in 1184 files changed: 47890 ins; 9587 del; 5652 mod Patch: https://git.openjdk.org/jdk/pull/19175.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19175/head:pull/19175 PR: https://git.openjdk.org/jdk/pull/19175
Integrated: 8327964: Simplify BigInteger.implMultiplyToLen intrinsic
On Tue, 12 Mar 2024 10:44:54 GMT, Yudi Zheng wrote: > Moving array construction within BigInteger.implMultiplyToLen intrinsic > candidate to its caller simplifies the intrinsic implementation in JIT > compiler. This pull request has now been integrated. Changeset: ed81a478 Author:Yudi Zheng Committer: Martin Doerr URL: https://git.openjdk.org/jdk/commit/ed81a478e175631f1de69eb4b43f927629fefd74 Stats: 146 lines in 17 files changed: 11 ins; 82 del; 53 mod 8327964: Simplify BigInteger.implMultiplyToLen intrinsic Reviewed-by: mdoerr, amitkumar, kvn, fyang - PR: https://git.openjdk.org/jdk/pull/18226
Re: RFR: 8327964: Simplify BigInteger.implMultiplyToLen intrinsic [v7]
On Fri, 24 May 2024 15:12:28 GMT, Yudi Zheng wrote: >> Moving array construction within BigInteger.implMultiplyToLen intrinsic >> candidate to its caller simplifies the intrinsic implementation in JIT >> compiler. > > Yudi Zheng 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 eight additional commits since > the last revision: > > - Merge remote-tracking branch 'upstream/master' into JDK-8327964 > - address comments. > - address comments. > - address comment. > - address comment. > - address comment. > - address comment. > - Simplify BigInteger.implMultiplyToLen intrinsic Thanks for the reviews! Mach5 testing looks good except for a couple known timeouts unrelated to this PR. GHA test failure is due to [JDK-8332923](https://bugs.openjdk.org/browse/JDK-8332923). - PR Comment: https://git.openjdk.org/jdk/pull/18226#issuecomment-2133444527
Re: RFR: 8292955: Collections.checkedMap Map.merge does not properly check key and value [v3]
On Thu, 7 Mar 2024 05:33:16 GMT, Lei Zhu wrote: >> When the specified key did not associated with a value, should check the >> `key` and `value` type. > > Lei Zhu has updated the pull request incrementally with one additional commit > since the last revision: > > Use testNG builtin functionalities and modify the test function name @minborg Is it possible for you to review this collection bugfix? - PR Comment: https://git.openjdk.org/jdk/pull/18141#issuecomment-2133400541
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v3]
On Mon, 27 May 2024 11:47:15 GMT, Adam Sotona wrote: >> [JDK-8294961](https://bugs.openjdk.org/browse/JDK-8294961) changed to use >> classfile API for reflection proxy-generation. Actual implementation of >> `ProxyGenerator` is focused on performance, however it causes JDK bootstrap >> regressions. `ProxyGenerator.TEMPLATE` class model is statically created and >> each proxy class is transformed from the template. >> >> This patch is intended to examine plain proxy generation impact on >> performance and JDK bootstrap (vs proxy transformation from template). >> >> Please review. >> >> Thank you, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > performance improvements src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java line 76: > 74: > 75: private static final MethodTypeDesc > 76: MTD_boolean = MethodTypeDescImpl.ofValidated(CD_boolean, > ConstantUtils.EMPTY_CLASSDESC), Maybe we can change the `MethodTypeDescImpl.ofValidated` to varargs so we don't need explicit array initializations. src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java line 822: > 820:.iconst_0() // false > 821:.aload(0)// classLoader > 822:.invokestatic(CD_Class, "forName", > MTD_Class_String_boolean_ClassLoader); We can probably replace this `forName(name, false, thisClassLoader)` with loading a class constant to reduce load on symbols. - PR Review Comment: https://git.openjdk.org/jdk/pull/19410#discussion_r1615978269 PR Review Comment: https://git.openjdk.org/jdk/pull/19410#discussion_r1615982718
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v4]
> [JDK-8294961](https://bugs.openjdk.org/browse/JDK-8294961) changed to use > classfile API for reflection proxy-generation. Actual implementation of > `ProxyGenerator` is focused on performance, however it causes JDK bootstrap > regressions. `ProxyGenerator.TEMPLATE` class model is statically created and > each proxy class is transformed from the template. > > This patch is intended to examine plain proxy generation impact on > performance and JDK bootstrap (vs proxy transformation from template). > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: more improvements - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/29ca6a4e..eaf30201 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=03 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=02-03 Stats: 14 lines in 1 file changed: 3 ins; 0 del; 11 mod Patch: https://git.openjdk.org/jdk/pull/19410.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19410/head:pull/19410 PR: https://git.openjdk.org/jdk/pull/19410
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v23]
On Thu, 23 May 2024 18:52:53 GMT, Alan Bateman wrote: > Yes, I want to help you get this one over the line. Thanks! Appreciate that. - PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2133375454
Re: RFR: 8328821: Map.of().entrySet() mutators should throw UnsupportedOperationException
On Mon, 27 May 2024 07:24:47 GMT, Per Minborg wrote:
>> This change overrides mutator methods in the implementation returned by
>> `Map.of().entrySet()` to throw `UnsupportedOperationException`.
>
> src/java.base/share/classes/java/util/ImmutableCollections.java line 1323:
>
>> 1321: @Override
>> 1322: public int hashCode() {
>> 1323: return MapN.this.hashCode();
>
> The hash code for a `Set` is defined as the sum of the elements in the `Set`
> (hash(`null`) == 0). The `Map. Entry` hash code is defined the same way
> `MapN.this.hashCode` operates so this seems right. It would be nice with a
> couple of tests that assert this invariant.
>
> Perhaps the existing tests already cover this?
Good point, this is currently missing from MOAT in Collections.
Though it can be as simple as one line here:
https://github.com/openjdk/jdk/blob/891d5aedf12e837c9a9c7cb800fb3affa7430f00/test/jdk/java/util/Collection/MOAT.java#L1329
check(m.hashCode() == m.entrySet().hashCode());
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18522#discussion_r1615976036
Re: RFR: 8331051: Add an `@since` checker test for `java.base` module [v8]
> This checker checks the values of the `@since` tag found in the documentation > comment for an element against the release in which the element first > appeared. > > Real since value of an API element is computed as the oldest release in which > the given API element was introduced. That is: > - for modules, classes and interfaces, the release in which the element with > the given qualified name was introduced > - for constructors, the release in which the constructor with the given VM > descriptor was introduced > - for methods and fields, the release in which the given method or field with > the given VM descriptor became a member of its enclosing class or interface, > whether direct or inherited > > Effective since value of an API element is computed as follows: > - if the given element has a `@since` tag in its javadoc, it is used > - in all other cases, return the effective since value of the enclosing > element > > The since checker verifies that for every API element, the real since value > and the effective since value are the same, and reports an error if they are > not. > > Preview method are handled as per JEP 12, if `@PreviewFeature` is used > consistently going forward then the checker doesn't need to be updated with > every release. The checker has explicit knowledge of preview elements that > came before `JDK 14` because they weren't marked in a machine understandable > way and preview elements that came before `JDK 17` that didn't have > `@PreviewFeature`. > > Important note : We only check code written since `JDK 9` as the releases > used to determine the expected value of `@since` tags are taken from the > historical data built into `javac` which only goes back that far > > The intial comment at the beginning of `SinceChecker.java` holds more > information into the program. > > I already have filed issues and fixed some wrong tags like in #18640, #18032, > #18030, #18055, #18373, #18954, #18972. Nizar Benalla has updated the pull request incrementally with one additional commit since the last revision: - removed unused parameter `i` - make rest of methods private - ".toString" instead of "toString" Signed-off-by: Nizar Benalla - Changes: - all: https://git.openjdk.org/jdk/pull/18934/files - new: https://git.openjdk.org/jdk/pull/18934/files/fc10107a..85b2d1a5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18934=07 - incr: https://webrevs.openjdk.org/?repo=jdk=18934=06-07 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/18934.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18934/head:pull/18934 PR: https://git.openjdk.org/jdk/pull/18934
Re: RFR: 8332922: Test java/io/IO/IO.java fails when /usr/bin/expect not exist [v2]
On Sun, 26 May 2024 07:24:16 GMT, SendaoYan wrote: >> Hi all, >> When there is no `/usr/bin/expect` in system, `throw new SkippedException` >> will not make the jvm exit in `@BeforeAll` junit stage, thus this will cause >> this testcase run failed. So I make change from `throw new SkippedException` >> to `Assumptions.abort` to avoid this issue. >> Only change the testcase, no risk. >> >> Thanks. > > SendaoYan has updated the pull request incrementally with two additional > commits since the last revision: > > - delete "static final int JCK_STATUS_BASE = 95;" > >Signed-off-by: sendaoYan > - 8332922: Test java/io/IO/IO.java fails when /usr/bin/expect not exist > >Signed-off-by: sendaoYan Check the other methods on the Assumptions class, they are older than this. - PR Comment: https://git.openjdk.org/jdk/pull/19403#issuecomment-2133314171
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v3]
> [JDK-8294961](https://bugs.openjdk.org/browse/JDK-8294961) changed to use > classfile API for reflection proxy-generation. Actual implementation of > `ProxyGenerator` is focused on performance, however it causes JDK bootstrap > regressions. `ProxyGenerator.TEMPLATE` class model is statically created and > each proxy class is transformed from the template. > > This patch is intended to examine plain proxy generation impact on > performance and JDK bootstrap (vs proxy transformation from template). > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: performance improvements - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/1d4edea5..29ca6a4e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=01-02 Stats: 77 lines in 2 files changed: 39 ins; 9 del; 29 mod Patch: https://git.openjdk.org/jdk/pull/19410.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19410/head:pull/19410 PR: https://git.openjdk.org/jdk/pull/19410
Re: RFR: 8332922: Test java/io/IO/IO.java fails when /usr/bin/expect not exist [v2]
On Sun, 26 May 2024 07:24:16 GMT, SendaoYan wrote: >> Hi all, >> When there is no `/usr/bin/expect` in system, `throw new SkippedException` >> will not make the jvm exit in `@BeforeAll` junit stage, thus this will cause >> this testcase run failed. So I make change from `throw new SkippedException` >> to `Assumptions.abort` to avoid this issue. >> Only change the testcase, no risk. >> >> Thanks. > > SendaoYan has updated the pull request incrementally with two additional > commits since the last revision: > > - delete "static final int JCK_STATUS_BASE = 95;" > >Signed-off-by: sendaoYan > - 8332922: Test java/io/IO/IO.java fails when /usr/bin/expect not exist > >Signed-off-by: sendaoYan Thanks for fixing that. I can see that `Assumptions.abort` is a rather new addition: since 5.9. What was the correct way to abort a test in these circumstances prior to that method? - PR Comment: https://git.openjdk.org/jdk/pull/19403#issuecomment-2133269802
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v2]
> [JDK-8294961](https://bugs.openjdk.org/browse/JDK-8294961) changed to use > classfile API for reflection proxy-generation. Actual implementation of > `ProxyGenerator` is focused on performance, however it causes JDK bootstrap > regressions. `ProxyGenerator.TEMPLATE` class model is statically created and > each proxy class is transformed from the template. > > This patch is intended to examine plain proxy generation impact on > performance and JDK bootstrap (vs proxy transformation from template). > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: manual stack maps - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/246efa11..1d4edea5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=00-01 Stats: 39 lines in 1 file changed: 18 ins; 0 del; 21 mod Patch: https://git.openjdk.org/jdk/pull/19410.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19410/head:pull/19410 PR: https://git.openjdk.org/jdk/pull/19410
Integrated: 8332885: Clarify failure_handler self-tests
On Fri, 24 May 2024 12:16:25 GMT, Ludvig Janiuk wrote: > Adding commetns to clarify that the failure_handler selftests are intended to > be run manually. Ideally this would include a more thorough description of > the exepcted outputs, but this is what I have time to add right now. This pull request has now been integrated. Changeset: 08891553 Author:Ludvig Janiuk URL: https://git.openjdk.org/jdk/commit/08891553bbd3d71337d8a94c75051db74e15903f Stats: 16 lines in 2 files changed: 16 ins; 0 del; 0 mod 8332885: Clarify failure_handler self-tests Reviewed-by: lmesnik - PR: https://git.openjdk.org/jdk/pull/19393
Re: RFR: 8332885: Clarify failure_handler self-tests [v2]
> Adding commetns to clarify that the failure_handler selftests are intended to > be run manually. Ideally this would include a more thorough description of > the exepcted outputs, but this is what I have time to add right now. Ludvig Janiuk has updated the pull request incrementally with three additional commits since the last revision: - styling - typo - reflow - Changes: - all: https://git.openjdk.org/jdk/pull/19393/files - new: https://git.openjdk.org/jdk/pull/19393/files/26a7be5c..a9d2cab2 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19393=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19393=00-01 Stats: 7 lines in 1 file changed: 1 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/19393.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19393/head:pull/19393 PR: https://git.openjdk.org/jdk/pull/19393
Integrated: 8332898: failure_handler: log directory of commands
On Fri, 24 May 2024 14:34:39 GMT, Ludvig Janiuk wrote: > Also log the directory in which the command used by failure_handler was > executed. While often the same, it isn't always, and so it this should > simplify troubleshooting for someone looking at this at a glance without > being a failure_handler expert. > > Example output after this change: > > > [2024-05-24 14:26:46] [/usr/bin/pmap, -p, 2233017] timeout=2 in > //JTwork/scratch > > > before: > > > [2024-05-24 14:26:46] [/usr/bin/pmap, -p, 2233017] timeout=2 > This pull request has now been integrated. Changeset: 7f0ad513 Author:Ludvig Janiuk URL: https://git.openjdk.org/jdk/commit/7f0ad513c30359816ac840f821ca0a22d723a642 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8332898: failure_handler: log directory of commands Reviewed-by: lmesnik - PR: https://git.openjdk.org/jdk/pull/19396
Re: RFR: 8332885: Clarify failure_handler self-tests
On Fri, 24 May 2024 19:26:29 GMT, Erik Joelsson wrote: >> Adding commetns to clarify that the failure_handler selftests are intended >> to be run manually. Ideally this would include a more thorough description >> of the exepcted outputs, but this is what I have time to add right now. > > test/failure_handler/README line 115: > >> 113: generated artifacts and cannot be run as part of a CI. They are tests >> which timeout, crash, >> 114: fail in various ways and you can check the failure_handler output >> yourself. They might also >> 115: leave processes running on your machine so be extra careful about >> cleaning up. > > These lines are longer than the text blocks in the rest of this file. Could > you adjust formatting to match? Sure, thanks for noticing. - PR Review Comment: https://git.openjdk.org/jdk/pull/19393#discussion_r1615787886
RFR: 8332457: Examine startup overheads from JDK-8294961
[JDK-8294961](https://bugs.openjdk.org/browse/JDK-8294961) changed to use classfile API for reflection proxy-generation. Actual implementation of `ProxyGenerator` is focused on performance, however it causes JDK bootstrap regressions. `ProxyGenerator.TEMPLATE` class model is statically created and each proxy class is transformed from the template. This patch is intended to examine plain proxy generation impact on performance and JDK bootstrap (vs proxy transformation from template). Please review. Thank you, Adam - Commit messages: - 8332457: Examine startup overheads from JDK-8294961 Changes: https://git.openjdk.org/jdk/pull/19410/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19410=00 Issue: https://bugs.openjdk.org/browse/JDK-8332457 Stats: 233 lines in 1 file changed: 27 ins; 113 del; 93 mod Patch: https://git.openjdk.org/jdk/pull/19410.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19410/head:pull/19410 PR: https://git.openjdk.org/jdk/pull/19410
Re: RFR: 8328821: Map.of().entrySet() mutators should throw UnsupportedOperationException
On Wed, 27 Mar 2024 17:36:28 GMT, Liam Miller-Cushon wrote:
> This change overrides mutator methods in the implementation returned by
> `Map.of().entrySet()` to throw `UnsupportedOperationException`.
src/java.base/share/classes/java/util/ImmutableCollections.java line 1323:
> 1321: @Override
> 1322: public int hashCode() {
> 1323: return MapN.this.hashCode();
The hash code for a `Set` is defined as the sum of the elements in the `Set`
(hash(`null`) == 0). The `Map. Entry` hash code is defined the same way
`MapN.this.hashCode` operates so this seems right. It would be nice with a
couple of tests that assert this invariant.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18522#discussion_r1615591889
Re: RFR: 8332922: Test java/io/IO/IO.java fails when /usr/bin/expect not exist [v2]
On Sun, 26 May 2024 07:24:16 GMT, SendaoYan wrote: >> Hi all, >> When there is no `/usr/bin/expect` in system, `throw new SkippedException` >> will not make the jvm exit in `@BeforeAll` junit stage, thus this will cause >> this testcase run failed. So I make change from `throw new SkippedException` >> to `Assumptions.abort` to avoid this issue. >> Only change the testcase, no risk. >> >> Thanks. > > SendaoYan has updated the pull request incrementally with two additional > commits since the last revision: > > - delete "static final int JCK_STATUS_BASE = 95;" > >Signed-off-by: sendaoYan > - 8332922: Test java/io/IO/IO.java fails when /usr/bin/expect not exist > >Signed-off-by: sendaoYan > /sponsor Thanks. - PR Comment: https://git.openjdk.org/jdk/pull/19403#issuecomment-2132757338
Integrated: 8332589: ubsan: unix/native/libjava/ProcessImpl_md.c:562:5: runtime error: null pointer passed as argument 2, which is declared to never be null
On Tue, 21 May 2024 14:28:38 GMT, Matthias Baesken wrote: > When building with ubsan enabled (--enable-uban) on Linux x86_64 and doing > jtreg tests afterwards I run into this error : > > /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:562:5: runtime error: > null pointer passed as argument 2, which is declared to never be null > #0 0x7fd95bec78d8 in spawnChild > /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:562 > #1 0x7fd95bec78d8 in startChild > /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:612 > #2 0x7fd95bec78d8 in Java_java_lang_ProcessImpl_forkAndExec > /jdk/src/java.base/unix/native/libjava/ProcessImpl_md.c:712 > #3 0x7fd93797a06d () > > this is the memcpy call getting an unexpected null pointer : > memcpy(buf+offset, c->pdir, sp.dirlen); gets a second parameter null. > Something similar was discussed and fixed here > https://bugs.python.org/issue27570 for Python . > > Similar issue in OpenJDK _ > https://bugs.openjdk.org/browse/JDK-8332473 > 8332473: ubsan: growableArray.hpp:290:10: runtime error: null pointer passed > as argument 1, which is declared to never be null This pull request has now been integrated. Changeset: 16dba04e Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/16dba04e8dfa871f8056480a42a9baeb24a2fb24 Stats: 12 lines in 1 file changed: 9 ins; 0 del; 3 mod 8332589: ubsan: unix/native/libjava/ProcessImpl_md.c:562:5: runtime error: null pointer passed as argument 2, which is declared to never be null Reviewed-by: rriggs, mdoerr - PR: https://git.openjdk.org/jdk/pull/19329
Integrated: 8332922: Test java/io/IO/IO.java fails when /usr/bin/expect not exist
On Sun, 26 May 2024 02:58:02 GMT, SendaoYan wrote: > Hi all, > When there is no `/usr/bin/expect` in system, `throw new SkippedException` > will not make the jvm exit in `@BeforeAll` junit stage, thus this will cause > this testcase run failed. So I make change from `throw new SkippedException` > to `Assumptions.abort` to avoid this issue. > Only change the testcase, no risk. > > Thanks. This pull request has now been integrated. Changeset: 4e8deb39 Author:SendaoYan Committer: Daniel Jeliński URL: https://git.openjdk.org/jdk/commit/4e8deb396e38c69de22b6348dca637d814d73aef Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod 8332922: Test java/io/IO/IO.java fails when /usr/bin/expect not exist Reviewed-by: djelinski - PR: https://git.openjdk.org/jdk/pull/19403
