Shenandoah's IU mode allows referents to be cleared even when accessed during concurrent marking. The test TestReferenceRefersTo.java needs to be adjusted to allow for that.
Test: - [x] TestReferenceRefersTo.java + Shenandoah/IU - [x] TestReferenceRefersTo.java + Shenandoah/SATB - [x] TestReferenceRefersTo.java + G1 ------------- Commit messages: - 8262049: [TESTBUG] Shenandoah: Adjustments in TestReferenceRefersTo.java for IU mode Changes: https://git.openjdk.java.net/jdk/pull/2653/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2653&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8262049 Stats: 18 lines in 1 file changed: 12 ins; 0 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/2653.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2653/head:pull/2653 PR: https://git.openjdk.java.net/jdk/pull/2653 |
On Fri, 19 Feb 2021 19:48:51 GMT, Roman Kennke <[hidden email]> wrote:
> Shenandoah's IU mode allows referents to be cleared even when accessed during concurrent marking. The test TestReferenceRefersTo.java needs to be adjusted to allow for that. > > Test: > - [x] TestReferenceRefersTo.java + Shenandoah/IU > - [x] TestReferenceRefersTo.java + Shenandoah/SATB > - [x] TestReferenceRefersTo.java + G1 Changes requested by kbarrett (Reviewer). test/hotspot/jtreg/gc/TestReferenceRefersTo.java line 166: > 164: > 165: private static boolean isShenandoahIUMode() { > 166: return WB.getBooleanVMFlag("UseShenandoahGC") && "iu".equals(WB.getStringVMFlag("ShenandoahGCMode")); This should be using sun.hotspot.gc.GC.Shenandoah.isSelected() test/hotspot/jtreg/gc/TestReferenceRefersTo.java line 211: > 209: } else { > 210: expectNotCleared(testWeak4, "testWeak4"); > 211: } I think I would prefer to keep this test program "generic", rather than having this Shenandoah IU mode intrusion. So remove the old check of testWeak4 state here, and remove the check of obj4 below. Instead, change the later check of testWeak4 being notified, where the new test is that either testWeak4 and obj4 are both null (IU and the like) or both are non-null (SATB and others). Then add a couple of tests programs for the specific clearing or not clearing expected behaviors, with appropriate `@requires` restrictions. ------------- PR: https://git.openjdk.java.net/jdk/pull/2653 |
On Sat, 20 Feb 2021 14:01:51 GMT, Kim Barrett <[hidden email]> wrote:
>> Shenandoah's IU mode allows referents to be cleared even when accessed during concurrent marking. The test TestReferenceRefersTo.java needs to be adjusted to allow for that. >> >> Test: >> - [x] TestReferenceRefersTo.java + Shenandoah/IU >> - [x] TestReferenceRefersTo.java + Shenandoah/SATB >> - [x] TestReferenceRefersTo.java + G1 > > Changes requested by kbarrett (Reviewer). Because this is a shared test, I suggest renaming the bug to something like "[TESTBUG] Fix TestReferenceRefersTo.java for Shenandoah IU mode", and remove the gc-shenandoah label. ------------- PR: https://git.openjdk.java.net/jdk/pull/2653 |
In reply to this post by Roman Kennke
> Shenandoah's IU mode allows referents to be cleared even when accessed during concurrent marking. The test TestReferenceRefersTo.java needs to be adjusted to allow for that.
> > Test: > - [x] TestReferenceRefersTo.java + Shenandoah/IU > - [x] TestReferenceRefersTo.java + Shenandoah/SATB > - [x] TestReferenceRefersTo.java + G1 Roman Kennke has updated the pull request incrementally with one additional commit since the last revision: Split TestReferenceRefersTo test in generic and non-Shenandoah parts ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2653/files - new: https://git.openjdk.java.net/jdk/pull/2653/files/b1fffb02..9746bc85 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2653&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2653&range=00-01 Stats: 230 lines in 2 files changed: 206 ins; 20 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/2653.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2653/head:pull/2653 PR: https://git.openjdk.java.net/jdk/pull/2653 |
In reply to this post by Kim Barrett-2
On Sat, 20 Feb 2021 13:55:33 GMT, Kim Barrett <[hidden email]> wrote:
> This should be using sun.hotspot.gc.GC.Shenandoah.isSelected() Yes, but which int constant should be used there? Doesn't matter much, I'm not using this at all anymore, following your other suggestions. > test/hotspot/jtreg/gc/TestReferenceRefersTo.java line 211: > >> 209: } else { >> 210: expectNotCleared(testWeak4, "testWeak4"); >> 211: } > > I think I would prefer to keep this test program "generic", rather than having this Shenandoah IU mode intrusion. So remove the old check of testWeak4 state here, and remove the check of obj4 below. Instead, change the later check of testWeak4 being notified, where the new test is that either testWeak4 and obj4 are both null (IU and the like) or both are non-null (SATB and others). Then add a couple of tests programs for the specific clearing or not clearing expected behaviors, with appropriate `@requires` restrictions. Right, that is even better. I made the base test generic, extracted the offending parts into its own test, and will push the Shenandoah specific test under a different PR. Thank you! ------------- PR: https://git.openjdk.java.net/jdk/pull/2653 |
In reply to this post by Roman Kennke
> Shenandoah's IU mode allows referents to be cleared even when accessed during concurrent marking. The test TestReferenceRefersTo.java needs to be adjusted to allow for that.
> > Test: > - [x] TestReferenceRefersTo.java + Shenandoah/IU > - [x] TestReferenceRefersTo.java + Shenandoah/SATB > - [x] TestReferenceRefersTo.java + G1 Roman Kennke has updated the pull request incrementally with one additional commit since the last revision: Fix compilation failures after renames ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2653/files - new: https://git.openjdk.java.net/jdk/pull/2653/files/9746bc85..54a027c9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2653&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2653&range=01-02 Stats: 8 lines in 1 file changed: 0 ins; 0 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/2653.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2653/head:pull/2653 PR: https://git.openjdk.java.net/jdk/pull/2653 |
On Mon, 22 Feb 2021 15:12:03 GMT, Roman Kennke <[hidden email]> wrote:
>> Shenandoah's IU mode allows referents to be cleared even when accessed during concurrent marking. The test TestReferenceRefersTo.java needs to be adjusted to allow for that. >> >> Test: >> - [x] TestReferenceRefersTo.java + Shenandoah/IU >> - [x] TestReferenceRefersTo.java + Shenandoah/SATB >> - [x] TestReferenceRefersTo.java + G1 > > Roman Kennke has updated the pull request incrementally with one additional commit since the last revision: > > Fix compilation failures after renames Changes requested by kbarrett (Reviewer). test/hotspot/jtreg/gc/TestReferenceRefersTo.java line 241: > 239: } > 240: if ((testWeak4 == null) != (obj4 == null)) { > 241: fail("either referent is cleared and we got notified, or neither of this happened"); It might be helpful if the failure reported which one was non-null. Not that it should ever fail... test/hotspot/jtreg/gc/TestReferenceRefersToDuringConcMark.java line 96: > 94: } > 95: > 96: private static void expectCleared(Reference<TestObject> ref, Unused. test/hotspot/jtreg/gc/TestReferenceRefersToDuringConcMark.java line 27: > 25: > 26: /* @test > 27: * @requires vm.gc != "Epsilon" I think this test "works" for Epsilon just as well as it does for Serial or Parallel or any other GC that doesn't support concurrent breakpoints, and either all should be excluded or none. test/hotspot/jtreg/gc/TestReferenceRefersToDuringConcMark.java line 28: > 26: /* @test > 27: * @requires vm.gc != "Epsilon" > 28: * @requires vm.gc != "Shenandoah" I think this test works for Shenandoah so long as it's not in IU mode. Is that possible to exclude with another `@requires` constraint? test/hotspot/jtreg/gc/TestReferenceRefersToDuringConcMark.java line 39: > 37: */ > 38: > 39: import java.lang.ref.PhantomReference; PhantomReference is unused. test/hotspot/jtreg/gc/TestReferenceRefersToDuringConcMark.java line 145: > 143: > 144: progress("acquire control of concurrent cycles"); > 145: WB.concurrentGCAcquireControl(); I think this test could be made a lot smaller and more obvious if it was explicitly just testing the keep-alive behavior of Reference.get for most concurrent collectors, rather than being a trimmed down copy of the earlier test. ------------- PR: https://git.openjdk.java.net/jdk/pull/2653 |
In reply to this post by Roman Kennke
> Shenandoah's IU mode allows referents to be cleared even when accessed during concurrent marking. The test TestReferenceRefersTo.java needs to be adjusted to allow for that.
> > Test: > - [x] TestReferenceRefersTo.java + Shenandoah/IU > - [x] TestReferenceRefersTo.java + Shenandoah/SATB > - [x] TestReferenceRefersTo.java + G1 Roman Kennke has updated the pull request incrementally with one additional commit since the last revision: Some more trimming ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2653/files - new: https://git.openjdk.java.net/jdk/pull/2653/files/54a027c9..51f2d695 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2653&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2653&range=02-03 Stats: 90 lines in 2 files changed: 1 ins; 79 del; 10 mod Patch: https://git.openjdk.java.net/jdk/pull/2653.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2653/head:pull/2653 PR: https://git.openjdk.java.net/jdk/pull/2653 |
In reply to this post by Kim Barrett-2
On Mon, 22 Feb 2021 15:25:46 GMT, Kim Barrett <[hidden email]> wrote:
>> Roman Kennke has updated the pull request incrementally with one additional commit since the last revision: >> >> Fix compilation failures after renames > > test/hotspot/jtreg/gc/TestReferenceRefersToDuringConcMark.java line 27: > >> 25: >> 26: /* @test >> 27: * @requires vm.gc != "Epsilon" > > I think this test "works" for Epsilon just as well as it does for Serial or Parallel or any other GC that doesn't support concurrent breakpoints, and either all should be excluded or none. Right. I excluded none. > test/hotspot/jtreg/gc/TestReferenceRefersToDuringConcMark.java line 28: > >> 26: /* @test >> 27: * @requires vm.gc != "Epsilon" >> 28: * @requires vm.gc != "Shenandoah" > > I think this test works for Shenandoah so long as it's not in IU mode. Is that possible to exclude with another `@requires` constraint? How would I do that? IU mode can only be distinguished by VM flag. > test/hotspot/jtreg/gc/TestReferenceRefersToDuringConcMark.java line 145: > >> 143: >> 144: progress("acquire control of concurrent cycles"); >> 145: WB.concurrentGCAcquireControl(); > > I think this test could be made a lot smaller and more obvious if it was explicitly just testing the keep-alive behavior of Reference.get for most concurrent collectors, rather than being a trimmed down copy of the earlier test. Right. I trimmed it some more. I think it's cleaner now. ------------- PR: https://git.openjdk.java.net/jdk/pull/2653 |
In reply to this post by Roman Kennke
> Shenandoah's IU mode allows referents to be cleared even when accessed during concurrent marking. The test TestReferenceRefersTo.java needs to be adjusted to allow for that.
> > Test: > - [x] TestReferenceRefersTo.java + Shenandoah/IU > - [x] TestReferenceRefersTo.java + Shenandoah/SATB > - [x] TestReferenceRefersTo.java + G1 Roman Kennke has updated the pull request incrementally with one additional commit since the last revision: Fix another compilation failure ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2653/files - new: https://git.openjdk.java.net/jdk/pull/2653/files/51f2d695..44f99b86 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2653&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2653&range=03-04 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2653.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2653/head:pull/2653 PR: https://git.openjdk.java.net/jdk/pull/2653 |
In reply to this post by Roman Kennke
On Mon, 22 Feb 2021 16:22:36 GMT, Roman Kennke <[hidden email]> wrote:
>> test/hotspot/jtreg/gc/TestReferenceRefersToDuringConcMark.java line 28: >> >>> 26: /* @test >>> 27: * @requires vm.gc != "Epsilon" >>> 28: * @requires vm.gc != "Shenandoah" >> >> I think this test works for Shenandoah so long as it's not in IU mode. Is that possible to exclude with another `@requires` constraint? > > How would I do that? IU mode can only be distinguished by VM flag. I've not tried it, but this might work. `@requires vm.gc != "Shenandoah" | vm.opt.ShenandoahGCMode != "iu"` ------------- PR: https://git.openjdk.java.net/jdk/pull/2653 |
In reply to this post by Roman Kennke
> Shenandoah's IU mode allows referents to be cleared even when accessed during concurrent marking. The test TestReferenceRefersTo.java needs to be adjusted to allow for that.
> > Test: > - [x] TestReferenceRefersTo.java + Shenandoah/IU > - [x] TestReferenceRefersTo.java + Shenandoah/SATB > - [x] TestReferenceRefersTo.java + G1 Roman Kennke has updated the pull request incrementally with one additional commit since the last revision: Allow Shenandoah for TestReferenceRefersToDuringConcMark test, except IU mode ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2653/files - new: https://git.openjdk.java.net/jdk/pull/2653/files/44f99b86..8f4d8606 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2653&range=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2653&range=04-05 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2653.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2653/head:pull/2653 PR: https://git.openjdk.java.net/jdk/pull/2653 |
In reply to this post by Kim Barrett-2
On Tue, 23 Feb 2021 08:24:51 GMT, Kim Barrett <[hidden email]> wrote:
>> How would I do that? IU mode can only be distinguished by VM flag. > > I've not tried it, but this might work. > `@requires vm.gc != "Shenandoah" | vm.opt.ShenandoahGCMode != "iu"` Indeed, it does. I changed the test requires accordingly. ------------- PR: https://git.openjdk.java.net/jdk/pull/2653 |
In reply to this post by Roman Kennke
On Tue, 23 Feb 2021 08:44:58 GMT, Roman Kennke <[hidden email]> wrote:
>> Shenandoah's IU mode allows referents to be cleared even when accessed during concurrent marking. The test TestReferenceRefersTo.java needs to be adjusted to allow for that. >> >> Test: >> - [x] TestReferenceRefersTo.java + Shenandoah/IU >> - [x] TestReferenceRefersTo.java + Shenandoah/SATB >> - [x] TestReferenceRefersTo.java + G1 > > Roman Kennke has updated the pull request incrementally with one additional commit since the last revision: > > Allow Shenandoah for TestReferenceRefersToDuringConcMark test, except IU mode Marked as reviewed by kbarrett (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/2653 |
In reply to this post by Roman Kennke
On Tue, 23 Feb 2021 08:44:58 GMT, Roman Kennke <[hidden email]> wrote:
>> Shenandoah's IU mode allows referents to be cleared even when accessed during concurrent marking. The test TestReferenceRefersTo.java needs to be adjusted to allow for that. >> >> Test: >> - [x] TestReferenceRefersTo.java + Shenandoah/IU >> - [x] TestReferenceRefersTo.java + Shenandoah/SATB >> - [x] TestReferenceRefersTo.java + G1 > > Roman Kennke has updated the pull request incrementally with one additional commit since the last revision: > > Allow Shenandoah for TestReferenceRefersToDuringConcMark test, except IU mode Good to me. ------------- Marked as reviewed by zgu (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2653 |
In reply to this post by Roman Kennke
On Fri, 19 Feb 2021 19:48:51 GMT, Roman Kennke <[hidden email]> wrote:
> Shenandoah's IU mode allows referents to be cleared even when accessed during concurrent marking. The test TestReferenceRefersTo.java needs to be adjusted to allow for that. > > Test: > - [x] TestReferenceRefersTo.java + Shenandoah/IU > - [x] TestReferenceRefersTo.java + Shenandoah/SATB > - [x] TestReferenceRefersTo.java + G1 This pull request has now been integrated. Changeset: c6eae061 Author: Roman Kennke <[hidden email]> URL: https://git.openjdk.java.net/jdk/commit/c6eae061 Stats: 139 lines in 2 files changed: 127 ins; 7 del; 5 mod 8262049: [TESTBUG] Fix TestReferenceRefersTo.java for Shenandoah IU mode Reviewed-by: kbarrett, zgu ------------- PR: https://git.openjdk.java.net/jdk/pull/2653 |
Free forum by Nabble | Edit this page |