On Thu, 11 May 2023 03:41:46 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> src/hotspot/share/services/management.cpp line 2107:
>>
>>> 2105: // twice, once in the loop and agsin in exited_allocated_bytes if
>>> it's
>>> 2106: // removed from the list after it's encountered in the loop but
>>> before
>>> 2107: // adding exited_allocated_bytes.
>>
>> This is not an accurate description.
>>
>> Once we have the TLH we have a fixed set of threads and none of them can
>> update exited_allocated_bytes. If we read exited_allocated_bytes while the
>> TLH is held then the count can be short for two reasons:
>> 1. Newly started threads
>> 2. Terminating threads that already escaped the TLH but which hadn't updated
>> exited_allocated_bytes by the time we read it.
>> AFAICS with the current logic there is no possibility of double-accounting.
>
> Sorry I thought you had made the change to only update exited_allocated_bytes
> after the thread removed itself from the threads-list. It really makes things
> so much simpler.
I made that change. The comment now reads:
// A thread increments exited_allocated_bytes in
ThreadService::remove_thread
// only after it removes itself from the threads list, and once a TLH is
// created, no thread it references can remove itself from the threads
// list, so none can update exited_allocated_bytes. We therefore initialize
// result with exited_allocated_bytes after after we create the TLH so that
// the final result can only be short due to (1) threads that start after
// the TLH is created, or (2) terminating threads that escape TLH creation
// and don't update exited_allocated_bytes before we initialize result.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13814#discussion_r1192473858
