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

Gerald Thornbrugh gerald.thornbrugh at oracle.com
Wed Dec 6 14:29:15 UTC 2017


Hi Dan,

Your changes look good.

Jerry
> On Dec 5, 2017, at 2:26 PM, Daniel D. Daugherty <daniel.daugherty at oracle.com> 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