RFR(XS): 8191787 - move private inline functions from thread.inline.hpp -> thread.cpp

[coleen.phillimore at oracle.com]

Yes, definitely the "kitchen sink collection".    I meant instead of :

    // Safe Memory Reclamation (SMR) support:
+ // The coordination between Threads::release_stable_list() and
+ // Threads::smr_delete() uses the smr_delete_lock in order to
+ // reduce the traffic on the Threads_lock.
    static Monitor*              _smr_delete_lock;
+ static Monitor* smr_delete_lock() { return _smr_delete_lock; }
    // The '_cnt', '_max' and '_times" fields are enabled via
    // -XX:+EnableThreadSMRStatistics (see thread.cpp for a
    // description about each field):
    static uint                  _smr_delete_lock_wait_cnt;
    static uint                  _smr_delete_lock_wait_max;
+ // The smr_delete_notify flag is used for proper double-check
+ // locking in order to reduce the traffic on the smr_delete_lock.
    static volatile uint         _smr_delete_notify;
+ static bool smr_delete_notify();
+ static void set_smr_delete_notify();
+ static void clear_smr_delete_notify();
    static volatile uint         _smr_deleted_thread_cnt;
+ static void inc_smr_deleted_thread_cnt();
    static volatile uint         _smr_deleted_thread_time_max;
+ static void update_smr_deleted_thread_time_max(uint new_value);
    static volatile uint         _smr_deleted_thread_times;
+ static void add_smr_deleted_thread_times(uint add_value);
    static ThreadsList* volatile _smr_java_thread_list;
    static ThreadsList*          get_smr_java_thread_list();
    static ThreadsList*          xchg_smr_java_thread_list(ThreadsList* new_list);
    static uint64_t              _smr_java_thread_list_alloc_cnt;
    static uint64_t              _smr_java_thread_list_free_cnt;

To have the below.  They're all together but fields and functions have 
been separated.

    // Safe Memory Reclamation (SMR) support:
+ // The coordination between Threads::release_stable_list() and
+ // Threads::smr_delete() uses the smr_delete_lock in order to
+ // reduce the traffic on the Threads_lock.
    static Monitor*              _smr_delete_lock;
    // The '_cnt', '_max' and '_times" fields are enabled via
    // -XX:+EnableThreadSMRStatistics (see thread.cpp for a
    // description about each field):
    static uint                  _smr_delete_lock_wait_cnt;
    static uint                  _smr_delete_lock_wait_max;
+ // The smr_delete_notify flag is used for proper double-check
+ // locking in order to reduce the traffic on the smr_delete_lock.
    static volatile uint         _smr_delete_notify;
    static volatile uint         _smr_deleted_thread_cnt;
    static volatile uint         _smr_deleted_thread_time_max;
    static volatile uint         _smr_deleted_thread_times;
    static ThreadsList* volatile _smr_java_thread_list;
    static uint64_t              _smr_java_thread_list_alloc_cnt;
    static uint64_t              _smr_java_thread_list_free_cnt;

+ static Monitor* smr_delete_lock() { return _smr_delete_lock; } + 
static bool smr_delete_notify();
+ static void set_smr_delete_notify();
+ static void clear_smr_delete_notify();
+ static void inc_smr_deleted_thread_cnt();
+ static void update_smr_deleted_thread_time_max(uint new_value);
+ static void add_smr_deleted_thread_times(uint add_value);
    static ThreadsList*          get_smr_java_thread_list();
    static ThreadsList*          xchg_smr_java_thread_list(ThreadsList* new_list);

Thanks,
Coleen

On 11/29/17 5:11 PM, Daniel D. Daugherty wrote:
> On 11/29/17 5:04 PM, coleen.phillimore at oracle.com wrote:
>>
>> http://cr.openjdk.java.net/~dcubed/8191787-webrev/jdk10-0/src/hotspot/share/runtime/thread.hpp.udiff.html 
>>
>>
>> Can you put a section with the static data member declarations, then 
>> a blank line and then have a section with the functions? We don't 
>> usually mix them like that (maybe in thread.hpp because it's the 
>> kitchen sink but not everywhere else).  It makes it hard to read.
>
> We're trying to keep all the "stuff" associated with a field together.
> It's definitely not a style typically in use in HotSpot, but we're
> experimenting with it because thread.hpp is currently a kitchen sink
> collection. Some folks would say it is a mess... :-)
>
> I'm going to take a look this cleanup bug also:
>
> JDK-8191789 migrate more Thread-SMR stuff from thread.[ch]pp -> 
> threadSMR.[ch]pp
> https://bugs.openjdk.java.net/browse/JDK-8191789
>
> since it will primarily be "code motion" fix also. Keeping "stuff"
> together will make that cleanup bug easier...
>
> Would you be okay with this fix (8191787) as it is?
>
>
>> Otherwise, looks fine.  Thank you for moving these!
>
> Thanks!
>
> Dan
>
>
>>
>> Coleen
>>
>> On 11/29/17 4:16 PM, Daniel D. Daugherty wrote:
>>> Greetings,
>>>
>>> Coleen, this is one of your Thread-SMR follow-up suggestions so I need
>>> to hear from you on this thread. Thanks!
>>>
>>> I have a simple cleanup fix for Thread-SMR. The bug is:
>>>
>>>     JDK-8191787 move private inline functions from thread.inline.hpp 
>>> -> thread.cpp
>>>     https://bugs.openjdk.java.net/browse/JDK-8191787
>>>
>>> This fix is pure code motion:
>>>
>>> - moving inline functions from thread.inline.hpp -> thread.cpp
>>> - making a few functions in thread.hpp private instead of public
>>>
>>> Here is the webrev URL:
>>>
>>> http://cr.openjdk.java.net/~dcubed/8191787-webrev/jdk10-0
>>>
>>> This fix was (over) tested with a Mach5 tier[1-5] run. There were no
>>> unexpected test failures.
>>>
>>> Thanks, in advance, for any comments, questions or suggestions.
>>>
>>> Dan
>>
>