RFR(S) 8021335: Missing synchronization when reading counters for live threads and peak thread count

[Mandy Chung]

On 10/23/18 3:30 PM, dean.long at oracle.com wrote:
> On 10/23/18 2:51 PM, David Holmes wrote:
>>
>>>> OK, but even if the regular benchmarks don't show a difference, I'd 
>>>> feel better if microbenchmarks were not affected.  What if I go 
>>>> back to the original approach and add locking:
>>>>
>>>>    static jlong get_live_thread_count()        { MutexLocker 
>>>> mu(Threads_lock); return _live_threads_count->get_value() - 
>>>> _exiting_threads_count; }
>>>>    static jlong get_daemon_thread_count()      { MutexLocker 
>>>> mu(Threads_lock); return _daemon_threads_count->get_value() - 
>>>> _exiting_daemon_threads_count; }
>>>>
>>>> along with the other cleanups around is_daemon and is_hidden_thread?
>>>>
>>>
>>> Some micro-benchmarks like SecureRandomBench show a regression with 
>>> webrev.7.  I could go back to webrev.5 and then we shouldn't need 
>>> any locking in the get_*() functions.
>>
>> I don't see version 5 discussed but I took a look and it seems okay. 
>
> Mandy had questions about the asserts in .5, and it seemed like we 
> could just set the perf counters to the same value as the atomic 
> counters, which resulted in .6.  I think the only problem with .6 is 
> that I set the perf counters in decrement_thread_counts without the 
> lock.  If I "sync" the perf counters to the atomic counters only in 
> add_thread and remove_thread, with the lock, then it's about the same 
> as .5, but without the asserts and parallel inc/dec.  If anyone likes 
> the sound of that, I can send out a new webrev. 

ThreadService::current_thread_exiting will decrement the atomic counters.
add_thread will increase atomic counters and sync the perf counters with 
the atomic counters.
remove_thread will decrement the atomic counters if thread is not 
exiting and
update the perf counters to the atomic counters.

I'm good with that.
> Or we can go with webrev.5.
>
>> My only query with that version is that it appears the actual 
>> perfCounters no longer get read by anything - is that the case?
>>
>
> There does seem to be code that references them, through their name 
> string.  That's a difference interface that I'm not familiar with, so 
> I didn't want to break it.

The perf counters are exposed via jstat tool.  I don't know who relies 
on jstat to monitor thread counts.   I suggest to check with the 
performance team if they depend on these counters.   With JFR, I think 
we should re-examine whether these perf counters should stay.

Mandy
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20181023/703135a5/attachment.html>