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

[Stefan Karlsson]

On 2017-12-04 21:22, 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!

Seems like a good step to me.

========================================================================
http://cr.openjdk.java.net/~dcubed/8191789-webrev/jdk10-0/src/hotspot/share/runtime/threadSMR.hpp.udiff.html

Now that you have moved all these functions and variables to a class 
named ThreadsSMRSupport maybe you want to consider getting rid of the 
redundant _smr_ prefix/infix?


Preexisting: I think you missed moving this in your previous cleanup:

+  static bool is_a_protected_JavaThread_with_lock(JavaThread *thread) {
+    MutexLockerEx ml(Threads_lock->owned_by_self() ? NULL : Threads_lock);
+    return is_a_protected_JavaThread(thread);
+  }

========================================================================
http://cr.openjdk.java.net/~dcubed/8191789-webrev/jdk10-0/src/hotspot/share/runtime/thread.cpp.frames.html

Would it make sense to move the following code sections into functions 
in ThreadsSMRSupport? That way we could minimize the number of public 
functions exposed from ThreadSMRSupport.

---
4369   // Maintain fast thread list
4370   ThreadsList *new_list = 
ThreadsList::add_thread(ThreadsSMRSupport::get_smr_java_thread_list(), p);
4371   if (EnableThreadSMRStatistics) {
4372     ThreadsSMRSupport::inc_smr_java_thread_list_alloc_cnt();
4373 
ThreadsSMRSupport::update_smr_java_thread_list_max(new_list->length());
4374   }
4375   // Initial _smr_java_thread_list will not generate a 
"Threads::add" mesg.
4376   log_debug(thread, smr)("tid=" UINTX_FORMAT ": Threads::add: new 
ThreadsList=" INTPTR_FORMAT, os::current_thread_id(), p2i(new_list));
4377
4378   ThreadsList *old_list = 
ThreadsSMRSupport::xchg_smr_java_thread_list(new_list);
4379   ThreadsSMRSupport::smr_free_list(old_list);

---
4396     // Maintain fast thread list
4397     ThreadsList *new_list = 
ThreadsList::remove_thread(ThreadsSMRSupport::get_smr_java_thread_list(), 
p);
4398     if (EnableThreadSMRStatistics) {
4399       ThreadsSMRSupport::inc_smr_java_thread_list_alloc_cnt();
4400       // This list is smaller so no need to check for a "longest" 
update.
4401     }
4402
4403     // Final _smr_java_thread_list will not generate a 
"Threads::remove" mesg.
4404     log_debug(thread, smr)("tid=" UINTX_FORMAT ": Threads::remove: 
new ThreadsList=" INTPTR_FORMAT, os::current_thread_id(), p2i(new_list));
4405
4406     ThreadsList *old_list = 
ThreadsSMRSupport::xchg_smr_java_thread_list(new_list);
4407     ThreadsSMRSupport::smr_free_list(old_list);

Thanks,
StefanK

> 
> 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