Re: RFR: 8178355: IdentityHashMap uses identity-based comparison for values everywhere except remove(K,V) and replace(K,V,V) [v4]
On Fri, 6 May 2022 22:05:35 GMT, liach wrote: >> Explicitly implement `remove` and `replace` in `IdentityHashMap` to compare >> values by identity. Updated API documentation of these two methods >> ([Preview](https://cr.openjdk.java.net/~liach/8178355/IdentityHashMap.html#remove(java.lang.Object,java.lang.Object))) >> to mention such behavior. > > liach 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: > > - Move tests > - Merge branch 'master' into fix/identityhashmap-default > - Fix assertions > - Revamp test and changes. Let ci run the tests > - Fix indent > - 8178355: IdentityHashMap uses identity-based comparison for values > everywhere except remove(K,V) and replace(K,V,V) Let's wait - PR: https://git.openjdk.org/jdk/pull/8259
Integrated: 8293020: jmod should not be treated as "small" tool for large modules
On Mon, 29 Aug 2022 10:13:29 GMT, Aleksey Shipilev wrote: > This is similar to > [JDK-8245168](https://bugs.openjdk.org/browse/JDK-8245168), but the blanket > change to allow all modules be compiled with default options is a net loss. > Instead, we can hand-pick the major offenders (large modules) where running > jmod with normal tool options improves performance. > > I instrumented the jmod to tell me the times it needs to create individual > modules, and hand-picked three top modules that take multiple seconds to run. > > Motivational `make clean-images images` times: > > > # x86_64 Server, release > > # Baseline > real 0m12.040s > user 1m4.872s > sys 0m10.805s > > # Patched > real 0m10.785s ; <--- 1.2s faster > user 1m7.031s > sys 0m10.985s > > # x86_64 Server, fastdebug > > # Baseline > real 0m19.263s > user 2m42.317s > sys 0m18.537s > > # Patched > real 0m17.911s ; <--- 1.1s faster > user 2m52.810s > sys 0m19.092s > > > # x86_64 Server, slowdebug > > # Baseline > real 0m44.799s > user 10m7.106s > sys 0m17.578s > > # Patched > real 0m46.975s ; <--- 2.5 sec slower > user 11m1.155s > sys 0m17.060s > > > I think we can accept the `slowdebug` regression in favor of improvements on > `release` and `fastdebug` that most people seem to be building every day. This pull request has now been integrated. Changeset: d2eed079 Author:Aleksey Shipilev URL: https://git.openjdk.org/jdk/commit/d2eed079c35022b61be28f7111612ecdcb3c2b30 Stats: 8 lines in 2 files changed: 6 ins; 0 del; 2 mod 8293020: jmod should not be treated as "small" tool for large modules Reviewed-by: erikj, ihse, alanb - PR: https://git.openjdk.org/jdk/pull/10062
Re: RFR: JDK-8173605: Remove support for source and target 1.7 option in javac
On Tue, 30 Aug 2022 06:26:25 GMT, Alan Bateman wrote: >> Update to remove support for -source/-target/--release 7 from javac. >> >> As seen in the PR, many test fails are affected. Further refactorings of >> javac's implementation that can be made from dropping 7 support are left as >> future work. > > test/jdk/java/lang/reflect/OldenCompilingWithDefaults.java line 28: > >> 26: * @bug 8009267 >> 27: * @summary Verify uses of isAnnotationPresent compile under older >> source versions >> 28: * @compile OldenCompilingWithDefaults.java > > I skimmed through the description of JDK-8009267 and I'm wondering if this > test is of any value now. Certainly less useful than it once was! I can remove it in the next push. - PR: https://git.openjdk.org/jdk/pull/10074
Re: RFR: 8065554: MatchResult should provide values of named-capturing groups [v4]
On Tue, 30 Aug 2022 20:17:04 GMT, Raffaello Giulietti wrote: >> test/jdk/java/util/regex/NamedGroupsTests.java line 45: >> >>> 43: >>> 44: testMatchResultStartEndGroup(); >>> 45: } >> >> I haven't gone through all the tests in great detail (yet), but it occurs to >> me that we potentially have THREE implementations of some of the logic, and >> strictly speaking we should test all the code paths. The three >> implementations are in: >> >> 1. Matcher >> 2. Matcher$ImmutableMatchResult >> 3. MatchResult's default methods >> >> I took a quick look and it looks like Matcher and >> Matcher$ImmutableMatchResult override the default methods, so the default >> methods themselves need to be tested. This is essentially testing the >> `@implSpec`. The typical way to do that is to have the test create its own >> MatchResult implementation(s). There might need to be implementations that >> do and do not implement `namedGroups`, in order to test UOE. They might also >> need some state to cover various cases of no-match, has-match with and >> without group names, etc. > > An implementation that overrides `namedGroups` without overriding the other > methods accepting group names is `Matcher$ImmutableMatchResult`, which is > already exercised in the tests. OK, as long as all the code paths are covered. >> test/jdk/java/util/regex/NamedGroupsTests.java line 51: >> >>> 49: testMatchResultStartEndGroup2(); >>> 50: testMatchResultStartEndGroup3(); >>> 51: } >> >> The three numbered tests here are a little hard to follow. Looks like these >> tests are >> >> 1. test existing group names, with no match >> 2. test existing group names, with a successful match >> 3. test nonexistent group names, with a successful match >> >> On test names, sometimes people provide extremely verbose test names such as >> `testThatExistingGroupNameWithMatchReturnsNegativeOrNull` which I think is >> overkill, but having a name that's somewhat descriptive would be helpful. >> >> It looks like a case is missing, which is a test for a nonexistent group >> name on a MatchResult that doesn't have a successful match. I'm not sure >> which is checked first; I think the implementation would throw IAE, because >> of the nonexistent name, regardless of whether or not the MatchResult has a >> match. >> >> However, I don't think we've specified this, and in fact I don't think we >> want to. In general though, if multiple error conditions can arise in the >> same operation, the general style is not to constrain implementations to >> check for things in a particular order. Thus either IAE or ISE would be >> acceptable. Perhaps a test should be added for that. (Hm, might want to take >> another look at the specs regarding this issue.) > >> (Hm, might want to take another look at the specs regarding this issue.) > > Not sure who wants to take another look. If that it's you, then I'll wait > with the CSR. > I'll change the method names to something a bit more speaking. Sorry I should have been more specific. "Somebody" should take another look. :-) Well, anyway, I did, and the specification as written does not indicate which error condition is checked first. I think this is OK, so I don't think any changes are necessary. You might mention this in the text of the CSR; I know that Joe and I have discussed this issue previously, and he might have a recommendation. - PR: https://git.openjdk.org/jdk/pull/1
Re: RFR: 8065554: MatchResult should provide values of named-capturing groups [v4]
On Tue, 30 Aug 2022 19:27:26 GMT, Stuart Marks wrote: >> Raffaello Giulietti has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8065554: MatchResult should provide values of named-capturing groups > > test/jdk/java/util/regex/NamedGroupsTests.java line 45: > >> 43: >> 44: testMatchResultStartEndGroup(); >> 45: } > > I haven't gone through all the tests in great detail (yet), but it occurs to > me that we potentially have THREE implementations of some of the logic, and > strictly speaking we should test all the code paths. The three > implementations are in: > > 1. Matcher > 2. Matcher$ImmutableMatchResult > 3. MatchResult's default methods > > I took a quick look and it looks like Matcher and > Matcher$ImmutableMatchResult override the default methods, so the default > methods themselves need to be tested. This is essentially testing the > `@implSpec`. The typical way to do that is to have the test create its own > MatchResult implementation(s). There might need to be implementations that do > and do not implement `namedGroups`, in order to test UOE. They might also > need some state to cover various cases of no-match, has-match with and > without group names, etc. An implementation that overrides `namedGroups` without overriding the other methods accepting group names is `Matcher$ImmutableMatchResult`, which is already exercised in the tests. > (Hm, might want to take another look at the specs regarding this issue.) Not sure who wants to take another look. If that it's you, then I'll wait with the CSR. I'll change the method names to something a bit more speaking. - PR: https://git.openjdk.org/jdk/pull/1
Re: RFR: 8065554: MatchResult should provide values of named-capturing groups [v4]
On Mon, 29 Aug 2022 13:49:52 GMT, Raffaello Giulietti wrote: >> Add support for named groups to java.util.regex.MatchResult > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 8065554: MatchResult should provide values of named-capturing groups test/jdk/java/util/regex/NamedGroupsTests.java line 45: > 43: > 44: testMatchResultStartEndGroup(); > 45: } I haven't gone through all the tests in great detail (yet), but it occurs to me that we potentially have THREE implementations of some of the logic, and strictly speaking we should test all the code paths. The three implementations are in: 1. Matcher 2. Matcher$ImmutableMatchResult 3. MatchResult's default methods I took a quick look and it looks like Matcher and Matcher$ImmutableMatchResult override the default methods, so the default methods themselves need to be tested. This is essentially testing the `@implSpec`. The typical way to do that is to have the test create its own MatchResult implementation(s). There might need to be implementations that do and do not implement `namedGroups`, in order to test UOE. They might also need some state to cover various cases of no-match, has-match with and without group names, etc. - PR: https://git.openjdk.org/jdk/pull/1
Re: RFR: 8065554: MatchResult should provide values of named-capturing groups [v4]
On Mon, 29 Aug 2022 13:49:52 GMT, Raffaello Giulietti wrote: >> Add support for named groups to java.util.regex.MatchResult > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 8065554: MatchResult should provide values of named-capturing groups test/jdk/java/util/regex/NamedGroupsTests.java line 51: > 49: testMatchResultStartEndGroup2(); > 50: testMatchResultStartEndGroup3(); > 51: } The three numbered tests here are a little hard to follow. Looks like these tests are 1. test existing group names, with no match 2. test existing group names, with a successful match 3. test nonexistent group names, with a successful match On test names, sometimes people provide extremely verbose test names such as `testThatExistingGroupNameWithMatchReturnsNegativeOrNull` which I think is overkill, but having a name that's somewhat descriptive would be helpful. It looks like a case is missing, which is a test for a nonexistent group name on a MatchResult that doesn't have a successful match. I'm not sure which is checked first; I think the implementation would throw IAE, because of the nonexistent name, regardless of whether or not the MatchResult has a match. However, I don't think we've specified this, and in fact I don't think we want to. In general though, if multiple error conditions can arise in the same operation, the general style is not to constrain implementations to check for things in a particular order. Thus either IAE or ISE would be acceptable. Perhaps a test should be added for that. (Hm, might want to take another look at the specs regarding this issue.) - PR: https://git.openjdk.org/jdk/pull/1
Re: RFR: 8293020: jmod should not be treated as "small" tool for large modules [v3]
On Mon, 29 Aug 2022 18:01:36 GMT, Aleksey Shipilev wrote: >> This is similar to >> [JDK-8245168](https://bugs.openjdk.org/browse/JDK-8245168), but the blanket >> change to allow all modules be compiled with default options is a net loss. >> Instead, we can hand-pick the major offenders (large modules) where running >> jmod with normal tool options improves performance. >> >> I instrumented the jmod to tell me the times it needs to create individual >> modules, and hand-picked three top modules that take multiple seconds to run. >> >> Motivational `make clean-images images` times: >> >> >> # x86_64 Server, release >> >> # Baseline >> real 0m12.040s >> user 1m4.872s >> sys 0m10.805s >> >> # Patched >> real 0m10.785s ; <--- 1.2s faster >> user 1m7.031s >> sys 0m10.985s >> >> # x86_64 Server, fastdebug >> >> # Baseline >> real 0m19.263s >> user 2m42.317s >> sys 0m18.537s >> >> # Patched >> real 0m17.911s ; <--- 1.1s faster >> user 2m52.810s >> sys 0m19.092s >> >> >> # x86_64 Server, slowdebug >> >> # Baseline >> real 0m44.799s >> user 10m7.106s >> sys 0m17.578s >> >> # Patched >> real 0m46.975s ; <--- 2.5 sec slower >> user 11m1.155s >> sys 0m17.060s >> >> >> I think we can accept the `slowdebug` regression in favor of improvements on >> `release` and `fastdebug` that most people seem to be building every day. > > Aleksey Shipilev has updated the pull request incrementally with one > additional commit since the last revision: > > Use := Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.org/jdk/pull/10062
Re: RFR: 8065554: MatchResult should provide values of named-capturing groups [v4]
On Mon, 29 Aug 2022 13:49:52 GMT, Raffaello Giulietti wrote: >> Add support for named groups to java.util.regex.MatchResult > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 8065554: MatchResult should provide values of named-capturing groups Spec looks good. Let me know when you're done updating the CSR so I can mark it reviewed. - PR: https://git.openjdk.org/jdk/pull/1
Re: RFR: 8293020: jmod should not be treated as "small" tool for large modules [v3]
On Mon, 29 Aug 2022 18:01:36 GMT, Aleksey Shipilev wrote: >> This is similar to >> [JDK-8245168](https://bugs.openjdk.org/browse/JDK-8245168), but the blanket >> change to allow all modules be compiled with default options is a net loss. >> Instead, we can hand-pick the major offenders (large modules) where running >> jmod with normal tool options improves performance. >> >> I instrumented the jmod to tell me the times it needs to create individual >> modules, and hand-picked three top modules that take multiple seconds to run. >> >> Motivational `make clean-images images` times: >> >> >> # x86_64 Server, release >> >> # Baseline >> real 0m12.040s >> user 1m4.872s >> sys 0m10.805s >> >> # Patched >> real 0m10.785s ; <--- 1.2s faster >> user 1m7.031s >> sys 0m10.985s >> >> # x86_64 Server, fastdebug >> >> # Baseline >> real 0m19.263s >> user 2m42.317s >> sys 0m18.537s >> >> # Patched >> real 0m17.911s ; <--- 1.1s faster >> user 2m52.810s >> sys 0m19.092s >> >> >> # x86_64 Server, slowdebug >> >> # Baseline >> real 0m44.799s >> user 10m7.106s >> sys 0m17.578s >> >> # Patched >> real 0m46.975s ; <--- 2.5 sec slower >> user 11m1.155s >> sys 0m17.060s >> >> >> I think we can accept the `slowdebug` regression in favor of improvements on >> `release` and `fastdebug` that most people seem to be building every day. > > Aleksey Shipilev has updated the pull request incrementally with one > additional commit since the last revision: > > Use := If anyone from core-libs crowd wants to chime in, welcome. Otherwise, I'll integrate soon. - PR: https://git.openjdk.org/jdk/pull/10062
Re: RFR: 8275535: Retrying a failed authentication on multiple LDAP servers can lead to users blocked
On Wed, 20 Oct 2021 13:35:22 GMT, Martin Balao wrote: > I'd like to propose a fix for JDK-8275535. This fix reverts the behavior to > the state previous to JDK-8160768, where an authentication failure stops from > trying other LDAP servers with the same credentials [1]. After JDK-8160768 we > have 2 possible loops to stop: the one that iterates over different URLs and > the one that iterates over different endpoints (after a DNS query that > returns multiple values). > > No test regressions observed in jdk/com/sun/jndi/ldap. > > -- > [1] - https://hg.openjdk.java.net/jdk/jdk/rev/a609d549992a#l2.137 Will this fix be back-ported to JDK 8/11/17? - PR: https://git.openjdk.org/jdk/pull/6043
Re: RFR: 8291651: CleanerTest.java fails with "Cleanable was cleaned" [v2]
> CleanerTest is failing intermittently on Aarch64, with -Xcomp, and using a > VirtualThread for the Cleaner in the test. > The extensively relies on references processing and invokes GC on a very > short cycle. > The existing 10ms delay between GC requests is too short for the changes to > reference states to be reliably visible. > > The delay between gc requests is extended to 200ms and the tests pass > reliably. > There are small improvements in diagnostic messages and cleanup of unused > imports. Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Cleanup of messages that describe the cleaning function to be run or not. Switched to using jtreg @enablePreview. - Changes: - all: https://git.openjdk.org/jdk/pull/10048/files - new: https://git.openjdk.org/jdk/pull/10048/files/8dee8e8f..d98a3b02 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=10048=01 - incr: https://webrevs.openjdk.org/?repo=jdk=10048=00-01 Stats: 11 lines in 1 file changed: 1 ins; 0 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/10048.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10048/head:pull/10048 PR: https://git.openjdk.org/jdk/pull/10048
Integrated: 8293009: Remove unused field 'millisPerHour' in DateFormatSymbols
On Wed, 24 Aug 2022 19:22:51 GMT, Andrey Turbanov wrote: > Field `java.text.DateFormatSymbols#millisPerHour` is unused. It was unused in > initial OpenJDK sources. This pull request has now been integrated. Changeset: d3d2e669 Author:Andrey Turbanov URL: https://git.openjdk.org/jdk/commit/d3d2e669b7bdfbd91cab7b918bc62cf1879cc95b Stats: 6 lines in 1 file changed: 0 ins; 5 del; 1 mod 8293009: Remove unused field 'millisPerHour' in DateFormatSymbols Reviewed-by: naoto, jpai - PR: https://git.openjdk.org/jdk/pull/10005
Re: RFR: 8282648: Weaken the InflaterInputStream specification in order to allow faster Zip implementations [v12]
On Mon, 29 Aug 2022 11:58:15 GMT, Volker Simonis wrote: >> Add an API note to `InflaterInputStream::read(byte[] b, int off, int len)` >> to highlight that it might write more bytes than the returned number of >> inflated bytes into the buffer `b`. >> >> The superclass `java.io.InputStream` specifies that `read(byte[] b, int off, >> int len)` will leave the content beyond the last read byte in the read >> buffer `b` unaffected. However, the overridden `read` method in >> `InflaterInputStream` passes the read buffer `b` to >> `Inflater::inflate(byte[] b, int off, int len)` which doesn't provide this >> guarantee. Depending on implementation details, `Inflater::inflate` might >> write more than the returned number of inflated bytes into the buffer `b`. >> >> ### TL;DR >> >> `java.util.zip.Inflater` is the Java wrapper class for zlib's inflater >> functionality. `Inflater::inflate(byte[] output, int off, int len)` >> currently calls zlib's native `inflate(..)` function and passes the address >> of `output[off]` and `len` to it via JNI. >> >> The specification of zlib's `inflate(..)` function (i.e. the [API >> documentation in the original zlib >> implementation](https://github.com/madler/zlib/blob/cacf7f1d4e3d44d871b605da3b647f07d718623f/zlib.h#L400)) >> doesn't give any guarantees with regard to usage of the output buffer. It >> only states that upon completion the function will return the number of >> bytes that have been written (i.e. "inflated") into the output buffer. >> >> The original zlib implementation only wrote as many bytes into the output >> buffer as it inflated. However, this is not a hard requirement and newer, >> more performant implementations of the zlib library like >> [zlib-chromium](https://chromium.googlesource.com/chromium/src/third_party/zlib/) >> or [zlib-cloudflare](https://github.com/cloudflare/zlib) can use more bytes >> of the output buffer than they actually inflate as a scratch buffer. See >> https://github.com/simonis/zlib-chromium for a more detailed description of >> their approach and its performance benefit. >> >> These new zlib versions can still be used transparently from Java (e.g. by >> putting them into the `LD_LIBRARY_PATH` or by using `LD_PRELOAD`), because >> they still fully comply to specification of `Inflater::inflate(..)`. >> However, we might run into problems when using the `Inflater` functionality >> from the `InflaterInputStream` class. `InflaterInputStream` is derived from >> from `InputStream` and as such, its `read(byte[] b, int off, int len)` >> method is quite constrained. It specifically specifies that if *k* bytes >> have been read, then "these bytes will be stored in elements `b[off]` >> through `b[off+`*k*`-1]`, leaving elements `b[off+`*k*`]` through >> `b[off+len-1]` **unaffected**". But `InflaterInputStream::read(byte[] b, int >> off, int len)` (which is constrained by `InputStream::read(..)`'s >> specification) calls `Inflater::inflate(byte[] b, int off, int len)` and >> directly passes its output buffer down to the native zlib `inflate(..)` >> method which is free to change the bytes beyond `b[off+` *k*`]` (where *k* is the number of inflated bytes). >> >> From a practical point of view, I don't see this as a big problem, because >> callers of `InflaterInputStream::read(byte[] b, int off, int len)` can never >> know how many bytes will be written into the output buffer `b` (and in fact >> its content can always be completely overwritten). It therefore makes no >> sense to depend on any data there being untouched after the call. Also, >> having used zlib-cloudflare productively for about two years, we haven't >> seen real-world issues because of this behavior yet. However, from a >> specification point of view it is easy to artificially construct a program >> which violates `InflaterInputStream::read(..)`'s postcondition if using one >> of the alterantive zlib implementations. A recently integrated JTreg test >> (test/jdk/jdk/nio/zipfs/ZipFSOutputStreamTest.java) "unintentionally" fails >> with zlib-chromium but can fixed easily to run with alternative >> implementations as well (see >> [JDK-8283756](https://bugs.openjdk.java.net/browse/JDK-8283756)). > > Volker Simonis has updated the pull request incrementally with one additional > commit since the last revision: > > Updated JavaDoc according to @mbreinhold's suggestions Volker, Thank you for your patience while we worked through all of the nuances of this The changes look good to me - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.org/jdk/pull/7986
Re: RFR: 8292407: Improve Weak CAS VarHandle/Unsafe tests resilience under spurious failures [v7]
On Tue, 30 Aug 2022 14:18:11 GMT, Aleksey Shipilev wrote: >> We have a few reports that existing Weak* VarHandle tests are still flaky, >> for example on large AArch64 machines or small RISC-V machines. >> >> The flakiness is intrinsic to the nature of Weak* operations under tests, >> that can spuriously fail. The last attempt to fix these was >> [JDK-8155739](https://bugs.openjdk.org/browse/JDK-8155739). We need to >> strengthen these a bit more. >> >> The actual values depend on the successful testing on known-failing >> platforms. I ballparked bumping the attempts 5x and introducing the delay >> would help without exploding test time in worst cases. > > Aleksey Shipilev has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 12 commits: > > - Rewind back to 100 attempts, 1ms delay > - Merge branch 'master' into JDK-8292407-varhandle-weak-resilient > - Merge branch 'master' into JDK-8292407-varhandle-weak-resilient > - Pull Handles.get out of the weak retry loop > - Drop weakDelay to 1 > - Merge branch 'master' into JDK-8292407-varhandle-weak-resilient > - Rework timeouts > - Merge branch 'master' into JDK-8292407-varhandle-weak-resilient > - Merge branch 'master' into JDK-8292407-varhandle-weak-resilient > - Update copyrights > - ... and 2 more: https://git.openjdk.org/jdk/compare/b3450e93...fd6aa17b So, I have been playing with HiFive Unmatched board, and I can see that even in the single-threaded mode there weak CASes can spuriously fail when JIT compiler threads are very active at the same time. I suspect L1-tagging LR/SC fails just because we context-switch out a lot. On HiFive Unmatched, without any delay, it is common to see the long-tailed attempt counts that exceed 200 attempts. Adding the 1ms delay drops that long tail to fit under 10 attempts; I suspect because it provides a natural backoff on over-subscribed system, as thread would return back later if no resources are currently available. Still, that does not seem to hold when the rest of the system is running other parallel tests. Running `java/lang/invoke/VarHandles` with 50 attempts and 1ms delay still ocassionally fails the weak CAS tests. Running with 100 attempts and 1ms delay seems to pass it well (tried several times) -- which is what current version does. I think this is as good as it gets for a default mode. I also ran other platform tests, and there spurious failures are either not observed at all (x86 and friends), or we have 1..2 retries very rarely (for example, on AArch64 without LSE) . This means the current 100 attempts would almost never be taken on those platforms, and thus we can avoid introducing any platform-specific test config selection. - PR: https://git.openjdk.org/jdk/pull/9889
Re: RFR: 8292407: Improve Weak CAS VarHandle/Unsafe tests resilience under spurious failures [v7]
> We have a few reports that existing Weak* VarHandle tests are still flaky, > for example on large AArch64 machines or small RISC-V machines. > > The flakiness is intrinsic to the nature of Weak* operations under tests, > that can spuriously fail. The last attempt to fix these was > [JDK-8155739](https://bugs.openjdk.org/browse/JDK-8155739). We need to > strengthen these a bit more. > > The actual values depend on the successful testing on known-failing > platforms. I ballparked bumping the attempts 5x and introducing the delay > would help without exploding test time in worst cases. Aleksey Shipilev has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 12 commits: - Rewind back to 100 attempts, 1ms delay - Merge branch 'master' into JDK-8292407-varhandle-weak-resilient - Merge branch 'master' into JDK-8292407-varhandle-weak-resilient - Pull Handles.get out of the weak retry loop - Drop weakDelay to 1 - Merge branch 'master' into JDK-8292407-varhandle-weak-resilient - Rework timeouts - Merge branch 'master' into JDK-8292407-varhandle-weak-resilient - Merge branch 'master' into JDK-8292407-varhandle-weak-resilient - Update copyrights - ... and 2 more: https://git.openjdk.org/jdk/compare/b3450e93...fd6aa17b - Changes: https://git.openjdk.org/jdk/pull/9889/files Webrev: https://webrevs.openjdk.org/?repo=jdk=9889=06 Stats: 951 lines in 45 files changed: 800 ins; 0 del; 151 mod Patch: https://git.openjdk.org/jdk/pull/9889.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9889/head:pull/9889 PR: https://git.openjdk.org/jdk/pull/9889
Re: RFR: 8065554: MatchResult should provide values of named-capturing groups [v4]
On Mon, 29 Aug 2022 13:49:52 GMT, Raffaello Giulietti wrote: >> Add support for named groups to java.util.regex.MatchResult > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 8065554: MatchResult should provide values of named-capturing groups If the most recent commit is OK in terms of Javadoc, I'll update the CSR accordingly - PR: https://git.openjdk.org/jdk/pull/1
Re: RFR: JDK-8289798: Update to use jtreg 7 [v5]
On Tue, 23 Aug 2022 11:22:55 GMT, Christian Stein wrote: >> Please review the change to update to using jtreg 7. >> >> The primary change is to the `jib-profiles.js` file, which specifies the >> version of jtreg to use, for those systems that rely on this file. In >> addition, the `requiredVersion` has been updated in the various `TEST.ROOT` >> files. > > Christian Stein 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 branch 'openjdk:master' into JDK-8289798-jtreg-7 > - Update configure to check for jtreg 7 or later > - Merge branch 'openjdk:master' into JDK-8289798-jtreg-7 > - Merge branch 'openjdk:master' into JDK-8289798-jtreg-7 > - Revert junit.java > >Commit > https://github.com/openjdk/jdk/commit/45c3e898ed538545921395372fe507e9111401e1 > deleted this file > - Merge branch 'openjdk:master' into JDK-8289798-jtreg-7 > - Update to new JUnit JAR file > >With version 7 of `jtreg` comes JUnit Platform 1.8.2 (also known as "JUnit > 5.8.2"). >This change updates the hard-coded library tag value to the new JUnit JAR > file. >Therefore, it partly addresses https://bugs.openjdk.org/browse/JDK-8292316, >in order to unblock the "Update to use jtreg 7" PR 9393: >https://github.com/openjdk/jdk/pull/9393 > - Merge branch 'openjdk:master' into JDK-8289798-jtreg-7 > - Merge branch 'openjdk:master' into JDK-8289798-jtreg-7 > - Merge branch 'openjdk:master' into JDK-8289798-jtreg-7 > - ... and 3 more: https://git.openjdk.org/jdk/compare/cf006774...bbf53448 Marked as reviewed by jpai (Reviewer). - PR: https://git.openjdk.org/jdk/pull/9393
Re: RFR: JDK-8289798: Update to use jtreg 7 [v5]
On Tue, 30 Aug 2022 09:14:12 GMT, Jaikiran Pai wrote: > Hello Christian, is it intentional that in some file the minimum required > jtreg version is represented as `7` (`lib-tests.m4`) and in some other files > as `7+1` (GitHub actions config file, then the TEST.ROOT files)? Yes. Some systems require the exact version tag `7+1` others only the short version string `7`. - PR: https://git.openjdk.org/jdk/pull/9393
Re: RFR: 8293008: Replace uses of StringBuffer with StringBuilder in MergeCollation
On Wed, 24 Aug 2022 19:31:23 GMT, Andrey Turbanov wrote:
> Couple of package-private classes in `java.text` package still use
> `StringBuffer`: `MergeCollation` and `PatternEntry`.
> StringBuffer is a legacy synchronized class. StringBuilder is a direct
> replacement to StringBuffer which generally have better performance.
src/java.base/share/classes/java/text/PatternEntry.java line 55:
> 53: * Gets the current extension, quoted
> 54: */
> 55: public void appendQuotedExtension(StringBuilder toAddTo) {
Hello Andrey, this and the other `public` method that's changed in this
(package private) class don't seem to be used anywhere else other than this
class itself. So maybe you could change them to `private` as part of this
change?
-
PR: https://git.openjdk.org/jdk/pull/10007
Re: RFR: 8291651: CleanerTest.java fails with "Cleanable was cleaned"
On Fri, 26 Aug 2022 19:09:45 GMT, Roger Riggs wrote: > CleanerTest is failing intermittently on Aarch64, with -Xcomp, and using a > VirtualThread for the Cleaner in the test. > The extensively relies on references processing and invokes GC on a very > short cycle. > The existing 10ms delay between GC requests is too short for the changes to > reference states to be reliably visible. > > The delay between gc requests is extended to 200ms and the tests pass > reliably. > There are small improvements in diagnostic messages and cleanup of unused > imports. Hello Roger, The change related to increasing the time to 200ms looks fine to me and even the typo fix. The prefix to the error message isn't too clear to me. Is the "Should (not) have been run" prefix implying that the "Cleaner should (not) have been run"? While in this test, do you think we should change it to use `@enablePreview` instead of the `--enable-preview` options to the test definition section? - PR: https://git.openjdk.org/jdk/pull/10048
Re: RFR: JDK-8173605: Remove support for source and target 1.7 option in javac
On Tue, 30 Aug 2022 00:04:03 GMT, Joe Darcy wrote: > Update to remove support for -source/-target/--release 7 from javac. > > As seen in the PR, many test fails are affected. Further refactorings of > javac's implementation that can be made from dropping 7 support are left as > future work. test/jdk/java/lang/reflect/OldenCompilingWithDefaults.java line 28: > 26: * @bug 8009267 > 27: * @summary Verify uses of isAnnotationPresent compile under older source > versions > 28: * @compile OldenCompilingWithDefaults.java I skimmed through the description of JDK-8009267 and I'm wondering if this test is of any value now. - PR: https://git.openjdk.org/jdk/pull/10074
