Bug 186836 – [GTK][WPE][Nicosia] Add name for Nicosia Painting Threads

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

[GTK][WPE][Nicosia] Add name for Nicosia Painting Threads

[Yusuke Suzuki]

Created attachment 343138 [details]
Patch

[Yusuke Suzuki]

Created attachment 343139 [details]
Patch

[Carlos Garcia Campos]

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?

[Yusuke Suzuki]

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.

[Carlos Garcia Campos]

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

[Yusuke Suzuki]

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") :)

[Carlos Garcia Campos]

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

[Carlos Garcia Campos]

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?

[Yusuke Suzuki]

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

[Yusuke Suzuki]

Committed r233006: <https://trac.webkit.org/changeset/233006>

[Radar WebKit Bug Importer]

<rdar://problem/41281416>