Bug 186836 - [GTK][WPE][Nicosia] Add name for Nicosia Painting Threads
Summary: [GTK][WPE][Nicosia] Add name for Nicosia Painting Threads
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-06-20 00:56 PDT by Yusuke Suzuki
Modified: 2018-06-20 02:01 PDT (History)
4 users (show)

See Also:


Attachments
Patch (4.78 KB, patch)
2018-06-20 00:57 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (6.00 KB, patch)
2018-06-20 00:59 PDT, Yusuke Suzuki
cgarcia: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>