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