Re: [webkit-dev] 'final' class specifier and 'final' method specifier

[Fujii Hironori]

On Fri, Dec 21, 2018 at 5:02 AM Konstantin Tokarev 
wrote:

> >
> > On Thu, Dec 20, 2018 at 7:42 AM  wrote:
> >> In that case, I'll point out that C++ Core Guidelines has a rule
> "Virtual functions should specify exactly one of virtual, override, or
> final".
> >> (
> http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rh-override)
> >>
> >> Their tl;dr:
> >> "
> >> • virtual means exactly and only “this is a new virtual
> function.”
> >> • override means exactly and only “this is a non-final
> overrider.”
> >> • final means exactly and only “this is a final overrider.”
> >> "
> >>
> >> FWIW, they also have a rule "Use final sparingly" with the note that
> "Claims of performance improvements from final should be substantiated."
> >> (http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rh-final)
> >
> > C.128 is a same rule with the current WebKit coding style guidelines.
> > But, I think C.128 makes sense with C.139.
> > C.139 is against to Bug 192844.
> > After Bug 192844 update, we will have a lot of 'final' classes, not
> sparignly.
>
> Do you have an idea how to automate this? Otherwise we'll never reach the
> state where all leaf classes are final, because doing it manually will
> take lots of
> time, and I see no way to enforce the rule in new code
>
>
 I don't have such intelligent plan. I don't like the divergence
between WebKit Code Style Guidelines and WebKit source code. I've
often got review feedbacks that I should modernize the code
around my change. I'm going to change them manually, and all
patches will be checked by reviewer's eyes. There is a previous
such effort.

Bug 159802 – Add final keyword to WebCore/svg classes
https://bugs.webkit.org/show_bug.cgi?id=159802


It seems that I shouldn't change the current 'final' method specifier rule
of WebKit.
Then, I think the sentences should be revised such like C.128.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Reducing globals

[Alex Christensen]

It’s been three weeks.  Is anyone maintaining the soup or curl loading code?

> On Dec 3, 2018, at 1:03 PM, Alex Christensen  wrote:
> 
> 
> 
>> On Dec 3, 2018, at 12:55 PM, basuke.suz...@sony.com wrote:
>> 
>> Alex,
>> 
>> Got it. Curl port will catch up this move soon.
> Great!  Thanks!
>> 
>> I just want to confirm my understanding about Network Session. It is pretty 
>> similar concept with Cocoa's URLSession, isn't it?
> It tries.
>> 
>> Curl Port uses default NetworkSession at everywhere so that it is almost 
>> same with global NetworkProcess. We need to move forward to support 
>> NetworkSession correctly.
>> 
>> -
>> Basuke Suzuki
>> SONY PlayStation
>> 
>> 
>>> -Original Message-
>>> From: webkit-dev  On Behalf Of Alex
>>> Christensen
>>> Sent: Thursday, November 29, 2018 6:15 PM
>>> To: Webkit Development List 
>>> Subject: [webkit-dev] Reducing globals
>>> 
>>> I am embarking on a journey to reduce the number of global variables and
>>> singletons we use instead member variables on the proper objects.  Feel 
>>> free to
>>> join!
>>> 
>>> Specifically, I’m looking into reducing the number of members in the
>>> NetworkProcessCreationParameters structure.  Many of them need to go to
>>> NetworkSessionCreationParameters instead.  Could those maintaining the
>>> libsoup and libcurl networking implementations please lend a hand and move 
>>> the
>>> members enclosed in USE(SOUP) or USE(CURL)?  I have done similar moves in
>>> https://trac.webkit.org/changeset/238654/webkit and
>>> https://trac.webkit.org/changeset/238630/webkit if you would like a pattern 
>>> to
>>> follow.
>>> ___
>>> webkit-dev mailing list
>>> webkit-dev@lists.webkit.org
>>> https://lists.webkit.org/mailman/listinfo/webkit-dev
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] the name "AtomicString"

[Maciej Stachowiak]

> On Dec 20, 2018, at 11:24 AM, Geoffrey Garen  wrote:
> 
 So hard to pronounce though! Why not UniqueString? It’s not quite as 
 explicit but close enough. 
>>> 
>>> Wouldn’t it be confusing to use UniqueString type for a string that is 
>>> *common* in order to save memory?
>> 
>> I would interpret it as UniqueString(foo) means “give me the unique copy of 
>> string foo”. You use a unique copy so you can use the same string in many 
>> places without wasting memory, or excess time on string compares. It’s used 
>> in many places, but there is only one. (Maybe we should call it 
>> HighlanderString? OK, not serious.)
> 
> By definition, any string that has been uniqued is unique.
> 
> So, maybe we like “unique” or maybe we don’t. But if we like “unique”, it’s 
> strictly better than “uniqued”.
> 
>>> Personally, I like the AtomString proposal as it is close to the naming we 
>>> are used to and addresses the issue raised (atomic has a different meaning 
>>> with threading).
>>> Also, I had never heard of interned strings before.
> 
> AtomicString has two features:
> 
> (1) You do comparison by pointer / object identity;
> 
> (2) You never allocate two objects for the same sequence of characters.
> 
> JavaScript symbols offer (1) but avoid (2):
> 
>   let a = Symbol(“The string of the past!”);
>   let b = Symbol(“The string of the past!”);
>   a == b; // false
>   a === b; // false
> 
> Today we call (1) “UniquedStringImpl” and (1) + (2) “AtomicStringImpl”.
> 
> If we rename (1) + (2) to “UniqueString” or “UniquedString”, we need a new 
> name for (1) alone.

It seems like (1) alone is not actually “uniqued". In the case of symbols at 
least, it's the opposite: the string deliberately kept distinct from all other 
strings of equal value, so pointer equality is used as a check for equality by 
pointer identity, rather than a shortcut for equality by value. 

From what I can tell, UniquedStringImpl exists mainly to be a base class for 
SymbolImpl and AtomicStringImpl. So you could imagine giving it a more verbose 
and explicit name, like PointerEqualityStringImpl. However, JSC seems to use 
UniquedStringImpl directly in a bunch of cases, I think to make it convenient 
for a property name to be either an AtomicString or a Symbol. Maybe a longer 
name for that case is ok?

Regards,
Maciej


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] 'final' class specifier and 'final' method specifier

[Konstantin Tokarev]

20.12.2018, 09:17, "Fujii Hironori" :
> Thank you very much for the feedbacks.
>
> On Thu, Dec 20, 2018 at 4:52 AM Konstantin Tokarev  wrote:
>> 19.12.2018, 12:53, "Fujii Hironori" :
>>> I'd like to change this because 'final' doesn't necessarily imply
>>> 'override'. See the following stackoverflow:
>>> https://stackoverflow.com/questions/29412412/does-final-imply-override
>>
>> It does imply override, unless it is used in a declaration of new virtual 
>> method
>> (which has no practical meaning fwiw)
>
> You are right. C++ allows using 'final' with 'virtual' without overriding.
> Even though I don't know practical use-cases for it, C++ allows it because 
> 'final' doesn't mean overriding.
> And, this is the only reason 'final' doesn't necessarily imply override.
> This is a kind of chicken egg problem. I don't know which is true:
>
> 1. C++ allows it because 'final' doesn't mean overriding.
> 2. 'final' doesn't necessarily imply override because C++ allows it

I see no problem here, because our code style checker forbids using 'final' and 
'virtual' in the
same declaration, so it's only allowed in overriding context.

>
> On Thu, Dec 20, 2018 at 6:28 AM Konstantin Tokarev  wrote:
>> 19.12.2018, 23:27, "Michael Catanzaro" :
>>> On Wed, Dec 19, 2018 at 1:58 PM, Konstantin Tokarev 
>>> wrote:
  Adding override to method which already has final specifier doesn't
  affect anything,
  because both final and override may ony be used on virtual methods
>>>
>>> FWIW I prefer override because it's much more clear what that keyword
>>> is used for.
>>
>> If class itself has "final" specifier, "override" on methods works in the 
>> same way
>> as "final", and I agree that it conveys intention more clear.
>
> I think so, especially after I will update the code style guidelines in Bug 
> 192844.
>
>> Bug 192844 – Update code style guidelines for using 'final' specifier for 
>> all classes which has no derived classes
>> https://bugs.webkit.org/show_bug.cgi?id=192844
>
>> However, Darin in [1]
>> suggested that we use "final" aggressively to avoid accidentally losing 
>> compiler
>> optimization (i.e. devirtualization of call)
>>
>> [1] https://lists.webkit.org/pipermail/webkit-dev/2016-March/028022.html
>
> I think this won't be a problem after all classes which has no derived 
> classes are capped with 'final'.

However, it might be a problem when leaf class with 'final' becomes non-final.

>
> On Thu, Dec 20, 2018 at 7:42 AM  wrote:
>> In that case, I'll point out that C++ Core Guidelines has a rule "Virtual 
>> functions should specify exactly one of virtual, override, or final".
>> (http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rh-override)
>>
>> Their tl;dr:
>> "
>>         • virtual means exactly and only “this is a new virtual function.”
>>         • override means exactly and only “this is a non-final overrider.”
>>         • final means exactly and only “this is a final overrider.”
>> "
>>
>> FWIW, they also have a rule "Use final sparingly" with the note that "Claims 
>> of performance improvements from final should be substantiated."
>> (http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rh-final)
>
> C.128 is a same rule with the current WebKit coding style guidelines.
> But, I think C.128 makes sense with C.139.
> C.139 is against to Bug 192844.
> After Bug 192844 update, we will have a lot of 'final' classes, not sparignly.

Do you have an idea how to automate this? Otherwise we'll never reach the
state where all leaf classes are final, because doing it manually will take 
lots of
time, and I see no way to enforce the rule in new code

> ,
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev


-- 
Regards,
Konstantin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] the name "AtomicString"

[Geoffrey Garen]

>>> So hard to pronounce though! Why not UniqueString? It’s not quite as 
>>> explicit but close enough. 
>> 
>> Wouldn’t it be confusing to use UniqueString type for a string that is 
>> *common* in order to save memory?
> 
> I would interpret it as UniqueString(foo) means “give me the unique copy of 
> string foo”. You use a unique copy so you can use the same string in many 
> places without wasting memory, or excess time on string compares. It’s used 
> in many places, but there is only one. (Maybe we should call it 
> HighlanderString? OK, not serious.)

By definition, any string that has been uniqued is unique.

So, maybe we like “unique” or maybe we don’t. But if we like “unique”, it’s 
strictly better than “uniqued”.

>> Personally, I like the AtomString proposal as it is close to the naming we 
>> are used to and addresses the issue raised (atomic has a different meaning 
>> with threading).
>> Also, I had never heard of interned strings before.

AtomicString has two features:

(1) You do comparison by pointer / object identity;

(2) You never allocate two objects for the same sequence of characters.

JavaScript symbols offer (1) but avoid (2):

let a = Symbol(“The string of the past!”);
let b = Symbol(“The string of the past!”);
a == b; // false
a === b; // false

Today we call (1) “UniquedStringImpl” and (1) + (2) “AtomicStringImpl”.

If we rename (1) + (2) to “UniqueString” or “UniquedString”, we need a new name 
for (1) alone.

Geoff___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] the name "AtomicString"

[Maciej Stachowiak]

> On Dec 19, 2018, at 9:41 PM, Chris Dumez  wrote:
> 
>> 
>> On Dec 19, 2018, at 9:17 PM, Maciej Stachowiak > > wrote:
>> 
>> 
>> 
>>> On Dec 19, 2018, at 8:06 PM, Ryosuke Niwa >> > wrote:
>>> 
>>> On Wed, Dec 19, 2018 at 1:13 PM Simon Fraser >> > wrote:
>>> > On Dec 19, 2018, at 12:33 PM, Michael Catanzaro >> > > wrote:
>>> > 
>>> > On Tue, Dec 18, 2018 at 9:31 PM, Darin Adler >> > > wrote:
>>> >> I’ve gotten used to the name AtomicString over the years, but I wouldn’t 
>>> >> strongly object to changing it if other programmers are often confused 
>>> >> by it’s similarity to the term “atomic operations”.
>>> > 
>>> > Well there were two other developers in the thread Ryosuke linked to who 
>>> > made the exact same mistake as me, so I do think the current name is 
>>> > problematic. A change wouldn't need to be drastic, though. I think 
>>> > suggestions from the old thread like "StringAtom" or "AtomString" would 
>>> > be unproblematic. The problem is the specific word "atomic" carries an 
>>> > expectation that the object be safe to access concurrently across threads 
>>> > without locks; I think that expectation doesn't exist if not for the "ic" 
>>> > at the end.
>>> > 
>>> > FWIW I've only ever heard the "interned string" terminology prior to now.
>>> 
>>> SingletonString?
>>> UniquedString?
>>> 
>>> I do like UniquedString. That conveys what AtomicString really is. 
>>> SingletonString isn't so great since AtomicString table is still per thread.
>> 
>> So hard to pronounce though! Why not UniqueString? It’s not quite as 
>> explicit but close enough. 
> 
> Wouldn’t it be confusing to use UniqueString type for a string that is 
> *common* in order to save memory?

I would interpret it as UniqueString(foo) means “give me the unique copy of 
string foo”. You use a unique copy so you can use the same string in many 
places without wasting memory, or excess time on string compares. It’s used in 
many places, but there is only one. (Maybe we should call it HighlanderString? 
OK, not serious.)

> 
> Personally, I like the AtomString proposal as it is close to the naming we 
> are used to and addresses the issue raised (atomic has a different meaning 
> with threading).
> Also, I had never heard of interned strings before.

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev