RFR: Reimplement TsanOopMap support using WeakHandle and ResizeableResourceHashtable [v6]
Jiangli Zhou
jiangli at openjdk.org
Wed Jul 24 19:01:54 UTC 2024
On Wed, 24 Jul 2024 02:40:38 GMT, Man Cao <manc at openjdk.org> wrote:
>> Jiangli Zhou has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Sort moves for overlapping case only.
>
> src/hotspot/share/tsan/tsanOopMapTable.hpp line 78:
>
>> 76:
>> 77: static bool equals(const TsanOopMapTableKey& lhs, const TsanOopMapTableKey& rhs) {
>> 78: oop lhs_obj = lhs._obj != nullptr ? (oop)lhs._obj : lhs.object_no_keepalive();
>
> I think `lhs._obj` and `rhs._obj` will never be `nullptr` in practice. This is because:
> - We never add a null oop to the table.
> - Once `_wh.peek()` returns null, the associated entry is immediately removed. We never update `_obj` to null.
>
> Would it better to add an assert that `lhs._obj` and `rhs._obj` are both non-null and remove the call to `object_no_keepalive()`?
>
> (I find it a bit hard to reason why we would resort to `_wh` for equality. I know `JvmtiTagMapKey::_obj` would be null most of the time in contrast.)
I had also considered to not check if `_obj` is nullptr and decided to keep the check. The main concern is that I don't know if there could be a case where it wants to compare an entry with freed object. I feel it's safer to keep the check.
-------------
PR Review Comment: https://git.openjdk.org/tsan/pull/19#discussion_r1690293796
More information about the tsan-dev
mailing list