RFR: 8313224: Avoid calling JavaThread::current() in MemAllocator::Allocation constructor
Ioi Lam
iklam at openjdk.org
Fri Jul 28 04:48:49 UTC 2023
On Thu, 27 Jul 2023 21:54:55 GMT, David Holmes <dholmes at openjdk.org> wrote:
>> Summary: Avoid calling `JavaThread::current()` since we already have a `Thread*` in `MemAllocator::_thread`.
>>
>> Although the latter is declared to be a `Thead*` and not a `JavaThread*`, the calling context requires the current thread to be a `JavaThread`, and `MemAllocator::_thread` was initialized to be `Thread::current`, so I changed the code from
>>
>>
>> _thread(JavaThread::current()),
>>
>>
>> to
>>
>>
>> assert(Thread::current()->is_Java_thread(), "must be used by JavaThreads only");
>> assert(Thread::current() == allocator._thread, "do not pass MemAllocator across threads");
>> _thread = JavaThread::cast(allocator._thread);
>>
>>
>> I also clean up a few other lines to limit the explicit use of `JavaThread*`.
>>
>> This fix improved `java --version` by about 0.6% for my prototype of [JDK-8310823](https://bugs.openjdk.org/browse/JDK-8310823), which allocates ~24000 heap objects at VM start-up.
>
> src/hotspot/share/gc/shared/memAllocator.cpp line 100:
>
>> 98:
>> 99: public:
>> 100: PreserveObj(Thread* thread, oop* obj_ptr)
>
> Why did you change this? Following through the usage call chain it is only used by the `_thread` from `Allocation` which has to be a `JavaThread`.
I wanted to minimize the use of `JavaThread`. Most of the MemAllocator APIs take `Thread`, so any occurrence of `JavaThread` looks suspicious and may cause people to unwittingly cast from `Thread` to `JavaThread` in the future.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15058#discussion_r1277083833
More information about the hotspot-gc-dev
mailing list