On Thu, 6 Jul 2023 16:49:47 GMT, Leo Korinth <lkori...@openjdk.org> wrote:
>> Thomas Stuefe has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> last cleanups and shade feedback
>
> src/hotspot/share/runtime/trimNative.cpp line 42:
>
>> 40: class NativeTrimmerThread : public NamedThread {
>> 41:
>> 42: Monitor* const _lock;
>
> Maybe inline this instead of having it a pointer? Even if the thread and lock
> is not destroyed until VM shutdown, I always feel the need to match new in
> constructors with delete in destructors.
I rather avoid that. There is a timing factor here: JVM issues
ThreadNative::cleanup, which stops the thread, but the thread is not guaranteed
to stop before VM ends (may be in a middle of a trim operation). I don't want
to have to think about this.
Ultimately, I don't see the point of the cleanup. I prefer a fast shutdown.
Which is in line with a lot of code in the hotspot (e.g. we don't clean global
mutexes either).
But I'll add an assert to make sure only one NativeTrimmerThread is ever
created. So we know its a singleton.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14781#discussion_r1255337316
