8249528: Remove obsolete comment in G1RootProcessor::process_java_roots

Reviewed-by: tschatzl, sangheki

Author: albertnetymk

src/hotspot/share/code/nmethod.hpp

@@ -79,7 +79,7 @@ class nmethod : public CompiledMethod {
   // STW two-phase nmethod root processing helpers.
   //
   // When determining liveness of a given nmethod to do code cache unloading,
-  // some collectors need to to different things depending on whether the nmethods
+  // some collectors need to do different things depending on whether the nmethods
   // need to absolutely be kept alive during root processing; "strong"ly reachable
   // nmethods are known to be kept alive at root processing, but the liveness of
   // "weak"ly reachable ones is to be determined later.

src/hotspot/share/gc/g1/g1CodeBlobClosure.hpp

@@ -71,10 +71,6 @@ class G1CodeBlobClosure : public CodeBlobClosure {
   MarkingOopClosure _marking_oc;
 
   bool _strong;
-
-  void do_code_blob_weak(CodeBlob* cb);
-  void do_code_blob_strong(CodeBlob* cb);
-
 public:
   G1CodeBlobClosure(uint worker_id, OopClosure* oc, bool strong) :
     _oc(oc), _marking_oc(worker_id), _strong(strong) { }

src/hotspot/share/gc/g1/g1OopClosures.hpp

@@ -141,7 +141,6 @@ class G1ParCopyHelper : public OopClosure {
   inline void mark_object(oop obj);
 
   G1ParCopyHelper(G1CollectedHeap* g1h,  G1ParScanThreadState* par_scan_state);
-  ~G1ParCopyHelper() { }
 
  public:
   void set_scanned_cld(ClassLoaderData* cld) { _scanned_cld = cld; }

src/hotspot/share/gc/g1/g1RootClosures.hpp

@@ -33,9 +33,7 @@ class G1ParScanThreadState;
 
 class G1RootClosures : public CHeapObj<mtGC> {
 public:
-  virtual ~G1RootClosures() {}
-
-// Closures to process raw oops in the root set.
+  // Closures to process raw oops in the root set.
   virtual OopClosure* weak_oops() = 0;
   virtual OopClosure* strong_oops() = 0;
 

src/hotspot/share/gc/g1/g1RootProcessor.cpp

@@ -149,22 +149,35 @@ void G1RootProcessor::process_all_roots(OopClosure* oops,
 void G1RootProcessor::process_java_roots(G1RootClosures* closures,
                                          G1GCPhaseTimes* phase_times,
                                          uint worker_id) {
-  // We need to make make sure that the "strong" nmethods are processed first
-  // using the strong closure. Only after that we process the weakly reachable
-  // nmethods.
-  // We need to strictly separate the strong and weak nmethod processing because
-  // any processing claims that nmethod, i.e. will not be iterated again.
-  // Which means if an nmethod is processed first and claimed, the strong processing
-  // will not happen, and the oops reachable by that nmethod will not be marked
-  // properly.
+  // In the concurrent start pause, when class unloading is enabled, G1
+  // processes nmethods in two ways, as "strong" and "weak" nmethods.
   //
-  // That is why we process strong nmethods first, synchronize all threads via a
-  // barrier, and only then allow weak processing. To minimize the wait time at
-  // that barrier we do the strong nmethod processing first, and immediately after-
-  // wards indicate that that thread is done. Hopefully other root processing after
-  // nmethod processing is enough so there is no need to wait.
+  // 1) Strong nmethods are reachable from the thread stack frames. G1 applies
+  // the G1RootClosures::strong_codeblobs() closure on them. The closure
+  // iterates over all oops embedded inside each nmethod, and performs 3
+  // operations:
+  //   a) evacuates; relocate objects outside of collection set
+  //   b) fixes up; remap oops to reflect new addresses
+  //   c) mark; mark object alive
+  // This keeps these oops alive wrt. to the upcoming marking phase, and their
+  // classes will not be unloaded.
   //
-  // This is only required in the concurrent start pause with class unloading enabled.
+  // 2) Weak nmethods are reachable only from the code root remembered set (see
+  // G1CodeRootSet). G1 applies the G1RootClosures::weak_codeblobs() closure on
+  // them. The closure iterates over all oops embedded inside each nmethod, and
+  // performs 2 operations: a) and b).
+  // Since these oops are *not* marked, their classes can potentially be
+  // unloaded.
+  //
+  // G1 doesn't segregate strong/weak nmethods processing (finish processing
+  // all strong nmethods before starting with any weak nmethods, or vice
+  // versa), as that could lead to poor CPU utilization (a single slow thread
+  // prevents all other thread from crossing the synchronization barrier).
+  // Instead, G1 interleaves strong and weak nmethods processing via
+  // per-nmethod synchronization. A nmethod is either *strongly* or *weakly*
+  // claimed before processing. A weakly claimed nmethod could be strongly
+  // claimed again for performing marking (the c) operation above); see
+  // oops_do_process_weak and oops_do_process_strong in nmethod.hpp
   {
     G1GCParPhaseTimesTracker x(phase_times, G1GCPhaseTimes::ThreadRoots, worker_id);
     bool is_par = n_workers() > 1;