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

[Jesper Wilhelmsson]

Bengt,

I'm fine with CamelCase and I agree that the word thread is unnecessary.

There may be some confusion between GCParTask and ParGCTask, otherwise I think 
your suggestions are OK.
/Jesper


Bengt Rutisson skrev den 4/3/15 16:24:
>
> 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
>>
>