RFR(S/M): 8191789 - migrate more Thread-SMR stuff from thread.[ch]pp -> threadSMR.[ch]pp

Daniel D. Daugherty daniel.daugherty at oracle.com
Wed Dec 6 03:31:06 UTC 2017


Coleen,

Thanks for the review!

Dan


On 12/5/17 4:37 PM, coleen.phillimore at oracle.com wrote:
> Thank you so much for moving this code.  The comment from the main 
> change with the examples look good too (just rereading it now).
>
> Thanks,
> Coleen
>
> On 12/5/17 4:26 PM, Daniel D. Daugherty wrote:
>> Greetings,
>>
>> I've updated the fix based on Stefan K's code review feedback (and my
>> own self-review)...
>>
>> Delta webrev:
>>
>> http://cr.openjdk.java.net/~dcubed/8191789-webrev/jdk10-1-delta/
>>
>> Full webrev:
>>
>> http://cr.openjdk.java.net/~dcubed/8191789-webrev/jdk10-1-full/
>>
>>
>> Summary of the changes in this round:
>>
>> - moved ThreadsList::remove_thread()
>> - moved ThreadsListSetter::~ThreadsListSetter() and 
>> ThreadsListSetter::set()
>> - remove Threads as a friend of class Thread
>> - add ThreadsSMRSupport::update_smr_tlh_stats() to allow more 
>> ThreadsSMRSupport
>>   code to be private
>>
>> - refactor Threads-SMR code in Threads::add() to call
>>   new function ThreadsSMRSupport::add_thread()
>> - refactor Threads-SMR code in Threads::remove() to call
>>   new function ThreadsSMRSupport::remove_thread()
>> - more ThreadsSMRSupport functions can be private
>> - move is_a_protected_JavaThread_with_lock() to
>>   threadSMR.inline.hpp
>>
>>
>> I have a Mach5 tier[1-5] in progress for this round...
>>
>> Thanks, in advance, for any comments, questions or suggestions.
>>
>> Dan
>>
>>
>>
>> On 12/4/17 3:22 PM, Daniel D. Daugherty wrote:
>>> Greetings,
>>>
>>> In terms of actual "new/changed" code, this is a "S" review. However,
>>> because of code motion, the changed/insert/delete counts are the size
>>> of an "M" or "L" review.
>>>
>>> Stefan K, 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 (but tedious) cleanup fix for Thread-SMR. The bug is:
>>>
>>>     JDK-8191789 migrate more Thread-SMR stuff from thread.[ch]pp -> 
>>> threadSMR.[ch]pp
>>>     https://bugs.openjdk.java.net/browse/JDK-8191789
>>>
>>> This fix is mostly code motion:
>>>
>>> - move Thread-SMR related code from thread.cpp -> threadSMR.cpp and
>>>   from thread.hpp -> threadSMR.hpp
>>>   - move Threads::_smr_* fields -> ThreadsSMRSupport class
>>>   - move Thread-SMR functions from Threads -> ThreadsSMRSupport class
>>>   - move Thread-SMR helper classes from thread.cpp -> threadsSMR.cpp
>>>   - rename a bunch of Threads::foo() calls -> ThreadsSMRSupport::foo()
>>>
>>> - collateral changes:
>>>   - DO_JAVA_THREADS macro usage by code moved to threadSMR.cpp have
>>>     to switch to JavaThreadIterator or some other function.
>>>   - Add ThreadsSMRSupport::inc_smr_java_thread_list_alloc_cnt().
>>>   - Add ThreadsSMRSupport::update_smr_java_thread_list_max().
>>>   - Cleanup "friends" for Thread class and ThreadsList class.
>>>   - Add includes of runtime/threadSMR.hpp in a few files.
>>>
>>>
>>> Here is the webrev URL:
>>>
>>> http://cr.openjdk.java.net/~dcubed/8191789-webrev/jdk10-0
>>>
>>> This fix was (over) tested with a Mach5 tier[1-5] run. There were no
>>> unexpected test failures.
>>>
>>> These changes were sanity checked a couple of ways:
>>>
>>> - Compare pre-Thread-SMR thread.[ch]pp with thread.[ch]pp after this 
>>> fix
>>>   - Make sure that remaining Thread-SMR changes in thread.cpp and
>>>     thread.hpp make sense to leave behind.
>>>   - Similar comparison for thread.inline.hpp.
>>> - Compare code removed from thread.cpp with code added to 
>>> threadSMR.cpp,
>>>   compare code removed from thread.hpp with code added to 
>>> threadSMR.hpp,
>>>   compare code removed from thread.inline.hpp with code added to
>>>   threadSMR.inline.hpp:
>>>   - Make sure the deltas make sense.
>>>
>>> Thanks, in advance, for any comments, questions or suggestions.
>>>
>>> Dan
>>>
>>
>



More information about the hotspot-runtime-dev mailing list