Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

8188055: (ref) Add Reference::refersTo predicate #498

Closed
wants to merge 13 commits into from
Closed

8188055: (ref) Add Reference::refersTo predicate #498

Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
4226ae7
new function
kimbarrett Sep 23, 2020
0ddc48f
remove CMS comment
kimbarrett Oct 7, 2020
2bcf123
expand test
kimbarrett Oct 7, 2020
1773052
change referent access
kimbarrett Oct 7, 2020
00b1b90
basic functional test
kimbarrett Oct 7, 2020
28f2d1b
improve refersTo0 descriptions
kimbarrett Oct 8, 2020
eb9dfa1
use Object instead of TestObject
kimbarrett Oct 12, 2020
8b7c333
cleanup nits from Mandy
kimbarrett Oct 12, 2020
efa40d7
simplify test
kimbarrett Oct 14, 2020
c1062ec
More explicit refersTo0 comment.
kimbarrett Oct 15, 2020
ab4e519
Merge branch 'master' into refersto
kimbarrett Oct 15, 2020
3a15b6a
improve wording in refersTo javadoc
kimbarrett Oct 21, 2020
79277ff
Merge branch 'master' into refersto
kimbarrett Nov 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
51 changes: 37 additions & 14 deletions test/hotspot/jtreg/gc/TestReferenceRefersTo.java
View file
Open in desktop
Expand Up @@ -136,6 +136,14 @@ private static void expectValue(Reference<TestObject> ref,
}
}

private static void expectNotValue(Reference<TestObject> ref,
TestObject value,
String which) throws Exception {
if (ref.refersTo(value)) {
fail(which + " refers to unexpected value");
}
}

private static void checkInitialStates() throws Exception {
expectValue(testPhantom1, testObject1, "testPhantom1");
expectValue(testWeak2, testObject2, "testWeak2");
Expand All @@ -146,7 +154,7 @@ private static void checkInitialStates() throws Exception {
private static void discardStrongReferences() {
testObject1 = null;
testObject2 = null;
testObject3 = null;
// testObject3 not dropped
testObject4 = null;
}

Expand Down Expand Up @@ -174,6 +182,9 @@ private static void testConcurrentCollection() throws Exception {
expectNotCleared(testWeak2, "testWeak2");
expectNotCleared(testWeak3, "testWeak3");

expectNotValue(testWeak2, testObject3, "testWeak2");
expectValue(testWeak3, testObject3, "testWeak3");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks to me like the expectNotCleared() and expectNotValue() methods can be removed. All expect-tests can be done with just expectCleared() and expectValue().

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. For example,
expectNotCleared(testWeak1, "testWeak1")
is not testing the same thing as
expectValue(testWeak1, testObject1, "testWeak1")
If the implementation is correct they will indeed always be consistent. But they could be different with an incorrect implementation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't that just depend on how you implement expectValue()/expectCleared()? You could for example implement expectValue()/expectCleared() to be supersets of expectNotCleared()/expectNotValue(), which I think would make this easier to read. Something like this:

private static void expectValue(...) {
    if (ref.refersTo(null)) {
        fail("expected " + which + " to not be cleared");
    }
 
    if (!ref.refersTo(value)) {
        fail(which + " refers to unexpected value");
    }
}

private static void expectCleared(...) {
    if (ref.refersTo(new TestObject(5))) {
        fail(which + " refers to unexpected value");
    }

    if (!ref.refersTo(null)) {
        fail("expected " + which + " to be cleared");
    }
}


// For some collectors, calling get() will keep testObject4 alive.
if (testWeak4.get() == null) {
fail("testWeak4 unexpectedly == null");
Expand All @@ -185,21 +196,30 @@ private static void testConcurrentCollection() throws Exception {
progress("verify expected clears");
expectCleared(testPhantom1, "testPhantom1");
expectCleared(testWeak2, "testWeak2");
expectCleared(testWeak3, "testWeak3");
expectNotCleared(testWeak3, "testWeak3");
expectNotCleared(testWeak4, "testWeak4");

expectNotValue(testPhantom1, testObject3, "testPhantom1");
expectValue(testWeak3, testObject3, "testWeak3");
expectNotValue(testWeak4, testObject3, "testWeak4");

progress("verify get returns expected values");
if (testWeak2.get() != null) {
fail("testWeak2.get() != null");
} else if (testWeak3.get() != null) {
fail("testWeak3.get() != null");
}

TestObject obj3 = testWeak3.get();
if (obj3 == null) {
fail("testWeak3.get() returned null");
} else if (obj3.value != 3) {
fail("testWeak3.get().value is " + obj3.value);
}

TestObject obj4 = testWeak4.get();
if (obj4 != null) {
if (obj4.value != 4) {
fail("testWeak4.get().value is " + obj4.value);
}
if (obj4 == null) {
fail("testWeak4.get() returned null");
} else if (obj4.value != 4) {
fail("testWeak4.get().value is " + obj4.value);
}

progress("verify queue entries");
Expand All @@ -224,8 +244,8 @@ private static void testConcurrentCollection() throws Exception {
fail("testPhantom1 not notified");
} else if (testWeak2 != null) {
fail("testWeak2 not notified");
} else if (testWeak3 != null) {
fail("testWeak3 not notified");
} else if (testWeak3 == null) {
fail("testWeak3 notified");
} else if (testWeak4 == null) {
if (obj4 != null) {
fail("testWeak4 notified");
Expand Down Expand Up @@ -258,15 +278,18 @@ private static void testSimpleCollection() throws Exception {
progress("verify expected clears");
expectCleared(testPhantom1, "testPhantom1");
expectCleared(testWeak2, "testWeak2");
expectCleared(testWeak3, "testWeak3");
expectNotCleared(testWeak3, "testWeak3");
expectNotCleared(testWeak4, "testWeak4");

expectNotValue(testWeak2, testObject3, "testWeak2");
expectValue(testWeak3, testObject3, "testWeak3");

progress("verify get returns expected values");
if (testWeak2.get() != null) {
fail("testWeak2.get() != null");
} else if (testWeak3.get() != null) {
fail("testWeak3.get() != null");
} else if (tw4 != testWeak4.get()) {
} else if (testWeak3.get() != testObject3) {
fail("testWeak3.get() is not expected value");
} else if (testWeak4.get() != tw4) {
fail("testWeak4.get() is not expected value");
}

Expand Down