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

[David Lindholm]

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"

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