RFR: 8188055: (ref) Add Reference::refersTo predicate [v2]
Kim Barrett
kbarrett at openjdk.java.net
Mon Oct 12 16:00:15 UTC 2020
On Mon, 12 Oct 2020 07:43:19 GMT, Per Liden <pliden at openjdk.org> wrote:
>> Kim Barrett has updated the pull request incrementally with five additional commits since the last revision:
>>
>> - improve refersTo0 descriptions
>> - basic functional test
>> - change referent access
>> - expand test
>> - remove CMS comment
>
> test/jdk/java/lang/ref/ReferenceRefersTo.java line 58:
>
>> 56: fail(kind + " doesn't refer to expected value");
>> 57: } else if (ref.refersTo(unexpectedValue)) {
>> 58: fail(kind + " refers to unexpected value");
>
> This part, and the use of obj3, below seems odd and unnecessary. If the Reference doesn't have the expected value
> there's no special reason to think it has obj3 as value (as opposed to something else). I don't think this check will
> help debugging if this test fails.
A possible failure mode is to implement the predicate incorrectly, perhaps flipping the sense of the predicate. This
may help notice such. It's distinct from the null test, since null is a special value that might be handled differently.
> test/jdk/java/lang/ref/ReferenceRefersTo.java line 42:
>
>> 40: this.value = value;
>> 41: }
>> 42: }
>
> Introducing this class seems unnecessary, since its `value` is never used. So all instances of `TestObject` could just
> be `Object`.
I'd intended to use the value field when printing failure messages, but that became a little messy and probably not
worth the trouble in the face of possible null values. So yeah, just using Object is fine; I'll make that change.
> test/hotspot/jtreg/gc/TestReferenceRefersTo.java line 186:
>
>> 184:
>> 185: expectNotValue(testWeak2, testObject3, "testWeak2");
>> 186: expectValue(testWeak3, testObject3, "testWeak3");
>
> It looks to me like the `expectNotCleared()` and `expectNotValue()` methods can be removed. All expect-tests can be
> done with just `expectCleared()` and `expectValue()`.
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.
-------------
PR: https://git.openjdk.java.net/jdk/pull/498
More information about the core-libs-dev
mailing list