RFR: 8073545 - Use shorter and more descriptive names for GC worker threads

Erik Helin erik.helin at oracle.com
Thu Mar 5 15:14:51 UTC 2015


On 2015-03-05, David Lindholm wrote:
> Hi,
> 
> Thanks. New webrev available at:
> http://cr.openjdk.java.net/~mgerdin/david/8073545/webrev.01/

Looks good to me, Reviewed.

Thanks,
Erik

> Regards,
> David
> 
> On 2015-03-05 14:41, Erik Helin wrote:
> >On 2015-03-05, David Lindholm wrote:
> >>Hi,
> >>
> >>Thanks for your input. To me, threads and tasks are different things. A task
> >>is something a thread can do for a while, but when it is finished with that
> >>task it can pick something else to do (another task). This view seems to be
> >>reflected in gcTaskThread.cpp (the thread for running tasks in
> >>parallelScavenge). After creation, it runs different tasks managed by
> >>gcTaskManager. These tasks are called names ending with "task".
> >>
> >>But I agree that in most cases the word Thread is unnecessary (this was why
> >>I called the Marker Thread "Marker"). But I don't have anything against
> >>using up all the 15 chars (we should not shorten the name unnecessarily).
> >>
> >>My new suggestion:
> >>
> >>"CMS Thread#xxx"
> >>"CMS Main Thread"
> >>"G1 Refine#xxx"
> >>"G1 Marker#xxx"
> >>"G1 Main Marker"
> >>"G1 StrDedup#xxx"
> >>"ParGC Thread#xx"
> >>"GC Thread#xxx"
> >These names looks good to me. I also had a look at the patch and
> >noticed:
> >-  set_name("GC task thread#%d (ParallelGC)", which);
> >+//  set_name("GC task thread#%d (ParallelGC)", which);
> >
> >Could you please remove the line you commented out?
> >
> >Thanks,
> >Erik
> >
> >>This way our threads naming scheme will be aligned with the rest of the JVM.
> >>One could argue if '#' is necessary. We could leave it out or print '-'
> >>instead. The CompilerThreads leave them out.
> >>
> >>
> >>Thanks,
> >>David
> >>
> >>On 2015-03-04 16:24, Bengt Rutisson wrote:
> >>>Hi David and everyone,
> >>>
> >>>Naming is the most difficult part of computer science so I doubt that
> >>>there is an easy answer here. Given that the names are for debugging I
> >>>guess it is not too important exactly what the names are. More important
> >>>is that they are unique. I also think that there is really no need to have
> >>>"thread" in the name since it is pretty obvious that it is a thread.
> >>>
> >>>So, my suggestion would be to skip "thread" and to use CamelCase rather
> >>>than using spaces to save some characters.
> >>>
> >>>I don't have strong opinions on this, but here's what I'd suggest:
> >>>
> >>>-      _conc_workers = new YieldingFlexibleWorkGang("Parallel CMS
> >>>Threads",
> >>>+      _conc_workers = new YieldingFlexibleWorkGang("CMS Threads",
> >>>
> >>>"CMSTask"
> >>>
> >>>
> >>>
> >>>-  set_name("Concurrent Mark-Sweep GC Thread");
> >>>+  set_name("CMS Main Thread");
> >>>
> >>>"CMSMain"
> >>>
> >>>
> >>>
> >>>-  set_name("G1 Concurrent Refinement Thread#%d", worker_id);
> >>>+  set_name("G1 ConRefine#%d", worker_id);
> >>>
> >>>"G1ConcRefine#%d"
> >>>
> >>>
> >>>
> >>>-  _parallel_workers = new FlexibleWorkGang("G1 Parallel Marking Threads",
> >>>+  _parallel_workers = new FlexibleWorkGang("G1 Markers"
> >>>
> >>>"G1MarkTask"
> >>>
> >>>
> >>>
> >>>-  set_name("G1 Main Concurrent Mark GC Thread");
> >>>+  set_name("G1 Main Marker");
> >>>
> >>>"G1ConcMark"
> >>>
> >>>
> >>>
> >>>-  set_name("String Deduplication Thread");
> >>>+  set_name("StrDedup Thread");
> >>>
> >>>"StringDedup"
> >>>
> >>>
> >>>
> >>>-  set_name("GC task thread#%d (ParallelGC)", which);
> >>>+  set_name("ParGC Thread#%d", which);
> >>>
> >>>"ParGCTask"
> >>>
> >>>
> >>>
> >>>-    _workers = new FlexibleWorkGang("Parallel GC Threads",
> >>>ParallelGCThreads,
> >>>+    _workers = new FlexibleWorkGang("GC Threads", ParallelGCThreads,
> >>>
> >>>"GCParTask"
> >>>
> >>>
> >>>Just my thoughts. And regarding having 3 characters left for the number in
> >>>the thread name I don't know if that is too important. With the above
> >>>suggestions this will be possible in most cases but not all. Personally I
> >>>think that is good enough.
> >>>
> >>>Bengt
> >>>
> >>>
> >>>On 2015-03-04 15:11, David Lindholm wrote:
> >>>>Hi Jesper,
> >>>>
> >>>>On 2015-03-04 14:52, Jesper Wilhelmsson wrote:
> >>>>>Hi David,
> >>>>>
> >>>>>Thanks for fixing this!
> >>>>>
> >>>>>A couple of questions:
> >>>>>
> >>>>>-  set_name("G1 Concurrent Refinement Thread#%d", worker_id);
> >>>>>+  set_name("G1 ConRefine#%d", worker_id);
> >>>>>
> >>>>>Is there any refinement threads that are not concurrent? If not, could
> >>>>>we just call this "G1 Refine#%d" to simplify it slightly and remove an
> >>>>>implementation detail that doesn't need to be exposed? This would also
> >>>>>leave room for three digit numbers in case we have lots of these
> >>>>>threads on some systems.
> >>>>I discussed this with Bengt, and his input was that Concurrent
> >>>>Refinement is a well known concept in G1. I have no real opinion here,
> >>>>I'm fine with both suggestions.
> >>>>
> >>>>>-  _parallel_workers = new FlexibleWorkGang("G1 Parallel Marking
> >>>>>Threads",
> >>>>>+  _parallel_workers = new FlexibleWorkGang("G1 Markers",
> >>>>>
> >>>>>Markers is cute, but could be misunderstood. Can we call it "G1 Mark
> >>>>>Threads" instead?
> >>>>No, it is too long, the three last character with thread number won't
> >>>>fit (#xx).
> >>>>
> >>>>>-  set_name("G1 Main Concurrent Mark GC Thread");
> >>>>>+  set_name("G1 Main Marker");
> >>>>>
> >>>>>Again, "Marker" could be misunderstood. I don't have a good
> >>>>>replacement though.
> >>>>I'm open for suggestions, but I think "G1 Main Marker" works.
> >>>>
> >>>>>-  set_name("GC task thread#%d (ParallelGC)", which);
> >>>>>+  set_name("ParGC Thread#%d", which);
> >>>>>
> >>>>>I don't have a good suggestion for how to make this one character
> >>>>>shorter, but currently there is only room for two digit numbers. Maybe
> >>>>>just "GC Thread#%d". I don't think these threads will exist at the
> >>>>>same time as any other GC threads anyway.
> >>>>With your suggestion these threads would be called the same thing as the
> >>>>threads in sharedHeap. I think it is nice to quickly be able to see that
> >>>>these threads indeed belongs to the ParallelGC.
> >>>>
> >>>>>-  set_name("Gang worker#%d (%s)", id, gang->name());
> >>>>>+  set_name("%s#%d", gang->name(), id);
> >>>>>
> >>>>>Is there any limitation on the length of the name()? If it's too long
> >>>>>the number won't show. Can we add an assert to make sure it isn't too
> >>>>>long?
> >>>>I have gone through our current GangWorkers, and they fit. If you want I
> >>>>can add an assert for <= 12 characters. OTOH it is not the end of the
> >>>>world if we don't see the whole number in the debugger.
> >>>>
> >>>>
> >>>>Thanks,
> >>>>David
> >>>>
> >>>>>Thanks,
> >>>>>/Jesper
> >>>>>
> >>>>>
> >>>>>David Lindholm skrev den 4/3/15 13:48:
> >>>>>>Hi,
> >>>>>>
> >>>>>>Please review this small fix which changes the names of the GC
> >>>>>>threads to be
> >>>>>>shorter and more descriptive. There is a limit on 16 characters
> >>>>>>including the
> >>>>>>terminating null byte for this name, since pthread_set_name_np() is
> >>>>>>used. This
> >>>>>>change will make it easier to debug, as these names shows up in the
> >>>>>>debugger.
> >>>>>>
> >>>>>>Webrev: http://cr.openjdk.java.net/~brutisso/8073545/webrev.00/
> >>>>>>Bug: https://bugs.openjdk.java.net/browse/JDK-8073545
> >>>>>>
> >>>>>>Testing: Passed JPRT.
> >>>>>>
> >>>>>>
> >>>>>>Regards,
> >>>>>>David
> 



More information about the hotspot-gc-dev mailing list