Bug 186836

Summary: [GTK][WPE][Nicosia] Add name for Nicosia Painting Threads
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: cgarcia, clopez, mcatanzaro, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch cgarcia: review+

Description Yusuke Suzuki 2018-06-20 00:56:48 PDT
[GTK][WPE][Nicosia] Add name for Nicosia Painting Threads
Comment 1 Yusuke Suzuki 2018-06-20 00:57:50 PDT
Created attachment 343138 [details]
Patch
Comment 2 Yusuke Suzuki 2018-06-20 00:59:51 PDT
Created attachment 343139 [details]
Patch
Comment 3 Carlos Garcia Campos 2018-06-20 01:22:31 PDT
Comment on attachment 343139 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=343139&action=review

> Source/WTF/wtf/WorkerPool.h:66
> +    const char* m_name;

This is a problem if the passed name is not a static string. I think it would be safer to use a CString even if we have to duplicate it, no?
Comment 4 Yusuke Suzuki 2018-06-20 01:27:20 PDT
Comment on attachment 343139 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=343139&action=review

>> Source/WTF/wtf/WorkerPool.h:66
>> +    const char* m_name;
> 
> This is a problem if the passed name is not a static string. I think it would be safer to use a CString even if we have to duplicate it, no?

Is there the use case of the non-static string for WorkerPool name? I think ASCIILiteral would be nice, but I'm not sure CString is necessary.
Comment 5 Carlos Garcia Campos 2018-06-20 01:31:05 PDT
(In reply to Yusuke Suzuki from comment #4)
> Comment on attachment 343139 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=343139&action=review
> 
> >> Source/WTF/wtf/WorkerPool.h:66
> >> +    const char* m_name;
> > 
> > This is a problem if the passed name is not a static string. I think it would be safer to use a CString even if we have to duplicate it, no?
> 
> Is there the use case of the non-static string for WorkerPool name? I think
> ASCIILiteral would be nice, but I'm not sure CString is necessary.

I don't know, if you are sure a static string is always going to be used, I'm fine.
Comment 6 Yusuke Suzuki 2018-06-20 01:36:08 PDT
Comment on attachment 343139 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=343139&action=review

>>>> Source/WTF/wtf/WorkerPool.h:66
>>>> +    const char* m_name;
>>> 
>>> This is a problem if the passed name is not a static string. I think it would be safer to use a CString even if we have to duplicate it, no?
>> 
>> Is there the use case of the non-static string for WorkerPool name? I think ASCIILiteral would be nice, but I'm not sure CString is necessary.
> 
> I don't know, if you are sure a static string is always going to be used, I'm fine.

I would like to keep thread names as static if we do not have strong use cases, because static thread names are easy to grep, and the code shows clear intention (this thread is for "XXX") :)
Comment 7 Carlos Garcia Campos 2018-06-20 01:48:43 PDT
(In reply to Yusuke Suzuki from comment #6)
> Comment on attachment 343139 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=343139&action=review
> 
> >>>> Source/WTF/wtf/WorkerPool.h:66
> >>>> +    const char* m_name;
> >>> 
> >>> This is a problem if the passed name is not a static string. I think it would be safer to use a CString even if we have to duplicate it, no?
> >> 
> >> Is there the use case of the non-static string for WorkerPool name? I think ASCIILiteral would be nice, but I'm not sure CString is necessary.
> > 
> > I don't know, if you are sure a static string is always going to be used, I'm fine.
> 
> I would like to keep thread names as static if we do not have strong use
> cases, because static thread names are easy to grep, and the code shows
> clear intention (this thread is for "XXX") :)

Ok, is there a way to enforce it?
Comment 8 Carlos Garcia Campos 2018-06-20 01:48:51 PDT
Comment on attachment 343139 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=343139&action=review

>>>>>> Source/WTF/wtf/WorkerPool.h:66
>>>>>> +    const char* m_name;
>>>>> 
>>>>> This is a problem if the passed name is not a static string. I think it would be safer to use a CString even if we have to duplicate it, no?
>>>> 
>>>> Is there the use case of the non-static string for WorkerPool name? I think ASCIILiteral would be nice, but I'm not sure CString is necessary.
>>> 
>>> I don't know, if you are sure a static string is always going to be used, I'm fine.
>> 
>> I would like to keep thread names as static if we do not have strong use cases, because static thread names are easy to grep, and the code shows clear intention (this thread is for "XXX") :)
> 
> Ok, is there a way to enforce it?

This is a problem if the passed name is not a static string. I think it would be safer to use a CString even if we have to duplicate it, no?
Comment 9 Yusuke Suzuki 2018-06-20 01:53:24 PDT
Comment on attachment 343139 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=343139&action=review

>>>>>>> Source/WTF/wtf/WorkerPool.h:66
>>>>>>> +    const char* m_name;
>>>>>> 
>>>>>> This is a problem if the passed name is not a static string. I think it would be safer to use a CString even if we have to duplicate it, no?
>>>>> 
>>>>> Is there the use case of the non-static string for WorkerPool name? I think ASCIILiteral would be nice, but I'm not sure CString is necessary.
>>>> 
>>>> I don't know, if you are sure a static string is always going to be used, I'm fine.
>>> 
>>> I would like to keep thread names as static if we do not have strong use cases, because static thread names are easy to grep, and the code shows clear intention (this thread is for "XXX") :)
>> 
>> Ok, is there a way to enforce it?
> 
> This is a problem if the passed name is not a static string. I think it would be safer to use a CString even if we have to duplicate it, no?

Using ASCIILiteral can express that we require literal (ASCIILiteral("thread name")) since ASCIILiteral does not have implicit constructor from const char*.
And I'm planning to add user-defined literal for ASCIILiteral and remove ASCIILiteral constructor. Then, we can enforce this!
https://bugs.webkit.org/show_bug.cgi?id=186839
Comment 10 Yusuke Suzuki 2018-06-20 02:00:32 PDT
Committed r233006: <https://trac.webkit.org/changeset/233006>
Comment 11 Radar WebKit Bug Importer 2018-06-20 02:01:25 PDT
<rdar://problem/41281416>