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