On Tue, 9 May 2023 14:21:05 GMT, Volker Simonis <simo...@openjdk.org> wrote:
>> @dcubed-ojdk This is the current thread acting on itself
>
> This method (i.e. `ThreadService::remove_thread()`) is called from
> `Threads::remove()` *after* the thread was removed from the thread list:
>
> void Threads::remove(JavaThread* p, bool is_daemon) {
> ...
> // Maintain fast thread list
> ThreadsSMRSupport::remove_thread(p);
> ...
> ThreadService::remove_thread(p, is_daemon);
>
>
> But if we reach here from
> `JavaThread::cleanup_failed_attach_current_thread()` as the comment implies
> (`JavaThread::cleanup_failed_attach_current_thread()` calls
> `Threads::remove()` in the case of an attach failure), calling
> `ThreadService::incr_exited_allocated_bytes()` is probably irrelevant,
> because a thread which failed to attach can't allocate anyway.
>
> So instead of doing the call to
> `ThreadService::incr_exited_allocated_bytes()` here you can do it
> unconditionally and move it up before the check for `is_hidden_thread()`.
> This would then also account for regular thread exits where you arrive here
> from `JavaThread::exit()` and make the call to
> `ThreadService::incr_exited_allocated_bytes()` in
> `ThreadService::current_thread_exiting()` obsolete (also see my comment
> there).
Done.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13814#discussion_r1192495985
