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

[Per Liden]

Hi,

On 2015-03-05 11:26, 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"

You don't need the #xxx on string dedup, there's only one such thread.

/Per

> "ParGC Thread#xx"
> "GC Thread#xxx"
>
> 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
>>>
>>
>