RFR: Reimplement TsanOopMap support using WeakHandle and ResizeableResourceHashtable [v6]

Man Cao manc at openjdk.org
Fri Jul 26 23:34:47 UTC 2024


On Fri, 26 Jul 2024 23:08:52 GMT, Jiangli Zhou <jiangli at openjdk.org> wrote:

>> See the `oop` class definition [here](https://github.com/openjdk/jdk/blob/0898ab7f7496689e5de52a5dc4530ca21def1fca/src/hotspot/share/oops/oopsHierarchy.hpp#L53-L115). Note that `CHECK_UNHANDLED_OOPS` is only enabled in [fastdebug](https://github.com/openjdk/jdk/blob/0898ab7f7496689e5de52a5dc4530ca21def1fca/make/hotspot/lib/JvmFlags.gmk#L77-L81), not slowdebug.
>> As said in the other comment,  we could declare `TsanOopMapTableKey::_obj` as an `oop`, but make sure to test with fastdebug build.
>
> I just reread your earlier suggestion more closely. 
> 
>> is it better to do oopDesc* obj = _wh.peek()?
> 
> WeakHandle only defines `peek()` with `oop` as return type (see `inline oop peek() const;`). With `CHECK_UNHANDLED_OOPS`, the implicit cast from `oop` to `oopDesc*` is handled by the `oopDesc* ()` operator. I think we should keep `oop obj = _wh.peek();` for consistency with the return type declared in `peek`. 
> 
> For the issue with `obj != _obj`, changing `obj != cast_to_oop(_obj)` resolves it.

Yes, I'm aware `oopDesc* obj = _wh.peek()` will do the cast with the `oopDesc* ()` operator under `CHECK_UNHANDLED_OOPS`.

The point is to avoid unnecessary type casting and conversion if possible.  `oopDesc* obj = _wh.peek()` will just do the cast once, and avoid later casting with `obj != _obj` and `_obj = obj`.

Alternative is to declare `TsanOopMapTableKey::_obj` as an `oop`, which is probably cleaner.

-------------

PR Review Comment: https://git.openjdk.org/tsan/pull/19#discussion_r1693689617


More information about the tsan-dev mailing list