RFR: 8313224: Avoid calling JavaThread::current() in MemAllocator::Allocation constructor
Thomas Schatzl
tschatzl at openjdk.org
Tue Aug 1 13:44:45 UTC 2023
On Thu, 27 Jul 2023 18:34:16 GMT, Ioi Lam <iklam 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.
lgtm, with some minor comments about superfluous asserts.
src/hotspot/share/gc/shared/memAllocator.cpp line 79:
> 77: _tlab_end_reset_for_sample(false)
> 78: {
> 79: assert(Thread::current()->is_Java_thread(), "must be used by JavaThreads only");
This first assert duplicates the one done by `JavaThread::cast()` given the other assert just below.
src/hotspot/share/gc/shared/memAllocator.cpp line 81:
> 79: assert(Thread::current()->is_Java_thread(), "must be used by JavaThreads only");
> 80: assert(Thread::current() == allocator._thread, "do not pass MemAllocator across threads");
> 81: _thread = JavaThread::cast(allocator._thread);
The assignment could also be done in the initializer list as it seems simple enough.
-------------
Marked as reviewed by tschatzl (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/15058#pullrequestreview-1557033074
PR Review Comment: https://git.openjdk.org/jdk/pull/15058#discussion_r1280662683
PR Review Comment: https://git.openjdk.org/jdk/pull/15058#discussion_r1280663418
More information about the hotspot-gc-dev
mailing list