RFR(S) 8021335: Missing synchronization when reading counters for live threads and peak thread count
mandy.chung at oracle.com
Wed Oct 17 21:38:41 UTC 2018
On 10/17/18 2:13 PM, dean.long at oracle.com wrote:
> On 10/17/18 1:41 PM, Mandy Chung wrote:
>> On 10/16/18 7:33 PM, David Holmes wrote:
>>> Hi Dean,
>>> Thanks for tackling this.
>>> I'm still struggling to fully grasp why we need both the
>>> PerfCounters and the regular counters. I get that we have to
>>> decrement the live counts before ensure_join() has allowed
>>> Thread.join() to return, to ensure that if we then check the number
>>> of threads it has dropped by one. But I don't understand why that
>>> means we need to manage the thread count in two parts. Particularly
>>> as now you don't use the PerfCounter to return the live count, so it
>>> makes me wonder what role the PerfCounter is playing as it is
>>> temporarily inconsistent with the reported live count?
>> Perf counters were added long time back in JDK 1.4.2 for performance
>> measurement before java.lang.management API. One can use jstat tool
>> to monitor VM perf counters of a running VM. One could look into
>> the possibility of deprecating these counters and remove them over time.
>>> On 17/10/2018 9:43 AM, dean.long at oracle.com wrote:
>>> New webrev:
>> When the perf counters are updated when a thread is added/removed,
>> it's holding Threads_lock. Are the asserts in
>> ThreadService::remove_thread necessary?
> Not really. They were intended to catch the case where the atomic
> counters weren't decremented for some reason, not for the perf counters.
> Should I remove them?
Hmm... when remove_thread is called but decrement_thread_counts has not
been called. It's a bug in thread accounting. It happens to have the
perf counters that can be compared to assert. It seems not obvious.
Setting the perf counters same values as _atomic_threads_count and
_atomic_daemon_threads_count makes sense to me.
I would opt for removing the asserts but I can't think of an alternative
how to catch the issue you concern about.
>> For clarify, I think we could simply set _live_threads_count to the
>> value of _atomic_threads_count and set _daemon_threads_count to the
>> value of _atomic_daemon_threads_count.
> I think that works, even inside decrement_thread_counts() without
> holding the Threads_lock. If you agree, I'll make that change.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the serviceability-dev