Removing the WTF namespace

[Kentaro Hara]

Hi

Now that the core-modules componentization is done, we're planning to start the Blink componentization v1 [1]. The componentization is going to move a bunch of fundamental classes in platform/ to wtf/ (e.g., Timer, LifecycleObserver, Oilpan, SharedBuffer etc). In preparation for the migration, we're planning to replace the "WTF" namespace with the "blink" namespace.

Let us know if you have any concern or questions.

[1] https://docs.google.com/presentation/d/1WavDHWyjQRtmPgKYHGe-Hcb1QOoeAAnSEjXe5TlVyTI/edit#slide=id.g437b0e633_00

Note: In practice, the actual work is just to replace "WTF::" with "blink::". Patch contributions are welcome :)

--

Kentaro Hara, Tokyo, Japan

[TAMURA, Kent]

I support removing WTF namespace.

As for your componentization proposal, it's still looks unclear to me.  I'll send comments separately.

--

TAMURA Kent
Software Engineer, Google

[Kentaro Hara]

As for your componentization proposal, it's still looks unclear to me.  I'll send comments separately.

I'll make a separate announcement before moving files around. I'm not sure either if the componentization proposal covers 100% classes of the current Blink, so any feedback is welcome. (Happy to discuss offline for more details as well :-)

[Kinuko Yasuda]

Fwiw I support this namespace change too.  I might miss the FAQ about what WTF stands for a bit :)

I assume we're eventually planning to make blink (and ex-WTF) dependable on src/base, but it'll be probably announced/discussed on another thread.

Thanks for moving this forward,

[Dan Sinclair]

On Tue, May 26, 2015 at 2:17 AM Kentaro Hara <har...@chromium.org> wrote:

Now that the core-modules componentization is done, we're planning to start the Blink componentization v1 [1]. The componentization is going to move a bunch of fundamental classes in platform/ to wtf/ (e.g., Timer, LifecycleObserver, Oilpan, SharedBuffer etc). In preparation for the migration, we're planning to replace the "WTF" namespace with the "blink" namespace.

What about WTF macros like WTF_ARRAY_LENGTH and WTF classes like WTFString. Will those retain the WTF prefix for now or will they also be renamed as part of this effort?

dan

[Kentaro Hara]

What about WTF macros like WTF_ARRAY_LENGTH and WTF classes like WTFString. Will those retain the WTF prefix for now or will they also be renamed as part of this effort?

In short term, I'm planning to keep the WTF names (they wouldn't sound strange, since they are in wtf/ anyway).

In long term, I want to remove some of those WTF classes by making wtf/ depend on Chromium's base/.

[Kentaro Hara]

I might miss the FAQ about what WTF stands for a bit :)

At first we were planning to rename "wtf" to "base", but we decided not to do that because it will cause name conflicts once we make wtf/ depend on Chromium's base/. For example:

  #include "base/timer.h" // Does this "base" mean Chromium's base/ or Blink's base/?

Of course, there will be a way to avoid the name conflict, but it will be less confusing to have a different name than "base". So keep "wtf" :)

[Elliott Sprehn]

What's the advantage of removing this namespace?

I don't support this, for the same reason base and content aren't in the same namespace, all of core and wtf should not be in the same namespace.

Note that Timer and LifecycleObserver do not belong in wtf. wtf is like base, it's a reusable standard library that hopefully we'll be able to merge into base and then start using the same classes in content after the repo merge.

Please don't put all of wtf in the same namespace as core.

[Emil A Eklund]

On Tue, May 26, 2015 at 1:23 PM, Elliott Sprehn <esp...@chromium.org> wrote:
> What's the advantage of removing this namespace?
>
> I don't support this, for the same reason base and content aren't in the
> same namespace, all of core and wtf should not be in the same namespace.
>
> Note that Timer and LifecycleObserver do not belong in wtf. wtf is like
> base, it's a reusable standard library that hopefully we'll be able to merge
> into base and then start using the same classes in content after the repo
> merge.
>
> Please don't put all of wtf in the same namespace as core.

Agreed, I'm also opposed to removing the WTF namespace.

[Kentaro Hara]

What's the advantage of removing this namespace?

The advantages of the Blink componentization are explained in this document (https://docs.google.com/document/d/1vtpoNdywErT2wtbE6a64scePBnZycuRhkbuuaIpiu2I/edit). Removing the WTF namespace is the first step for the componentization.

Note that Timer and LifecycleObserver do not belong in wtf. wtf is like base,

One of the main goals of the componentization is to fix the broken dependency structure, which has resulted in a lot of unnecessary abstraction classes to hack the dependency. To fix the broken dependency, we're planning to remove platform/ and rearchitect the dependency to modules/ => core/ => wtf/ (very simple!). This means that things in platform/ that are depended on by core/ need to be moved to wtf/. This is why I want to move Timer, LifecycleObserver, Oilpan etc to wtf/. The "Oilpan and Vector" section of the above document would be helpful to understand the problem caused by mixing "things depended on by core/" in wtf/ and platform/ (Note: Oilpan is just one example, and a similar problem is taking place in other places as well).

If we really want to keep the WTF namespace, we can put Timer, LifecycleObserver etc in the WTF namespace when moving them into wtf/. However, IMHO it seems simpler to just drop the WTF namespace.

What's your concern about dropping the WTF namespace?

 it's a reusable standard library that hopefully we'll be able to merge into base and then start using the same classes in content after the repo merge.

That is our plan (i.e., make wtf/ depend on Chromium's base/ and share more code with Chromium), but I think we'll need to keep lots of code in wtf/ even after the repo merge. Since performance is the most important thing in Blink, we want to keep AtomicString, PassRefPtr, Vector with inline capacity etc in wtf/. Also Oilpan needs to hook allocation points of collections, we'll need to keep Vectors, HashMaps etc in wtf/ as well.

[Elliott Sprehn]

On Tue, May 26, 2015 at 4:45 PM, Kentaro Hara <har...@chromium.org> wrote:

What's the advantage of removing this namespace?

The advantages of the Blink componentization are explained in this document (https://docs.google.com/document/d/1vtpoNdywErT2wtbE6a64scePBnZycuRhkbuuaIpiu2I/edit). Removing the WTF namespace is the first step for the componentization.

Note that Timer and LifecycleObserver do not belong in wtf. wtf is like base,

One of the main goals of the componentization is to fix the broken dependency structure, which has resulted in a lot of unnecessary abstraction classes to hack the dependency. To fix the broken dependency, we're planning to remove platform/ and rearchitect the dependency to modules/ => core/ => wtf/ (very simple!). This means that things in platform/ that are depended on by core/ need to be moved to wtf/. This is why I want to move Timer, LifecycleObserver, Oilpan etc to wtf/. The "Oilpan and Vector" section of the above document would be helpful to understand the problem caused by mixing "things depended on by core/" in wtf/ and platform/ (Note: Oilpan is just one example, and a similar problem is taking place in other places as well).

If we really want to keep the WTF namespace, we can put Timer, LifecycleObserver etc in the WTF namespace when moving them into wtf/. However, IMHO it seems simpler to just drop the WTF namespace.

What's your concern about dropping the WTF namespace?

That code shouldn't be in the same namespace as core. You're proposing the equivalent of merging the cc and content namespaces.

wtf is our standard library, it's not part of core.

 

 it's a reusable standard library that hopefully we'll be able to merge into base and then start using the same classes in content after the repo merge.

That is our plan (i.e., make wtf/ depend on Chromium's base/ and share more code with Chromium), but I think we'll need to keep lots of code in wtf/ even after the repo merge. Since performance is the most important thing in Blink, we want to keep AtomicString, PassRefPtr, Vector with inline capacity etc in wtf/. Also Oilpan needs to hook allocation points of collections, we'll need to keep Vectors, HashMaps etc in wtf/ as well.

I think we want to be able to use Vectors with inline capacity in content, scheduler, cc, and other things too. 

- E 

[Kentaro Hara]

On Wed, May 27, 2015 at 8:51 AM, Elliott Sprehn <esp...@chromium.org> wrote:

On Tue, May 26, 2015 at 4:45 PM, Kentaro Hara <har...@chromium.org> wrote:

What's the advantage of removing this namespace?

The advantages of the Blink componentization are explained in this document (https://docs.google.com/document/d/1vtpoNdywErT2wtbE6a64scePBnZycuRhkbuuaIpiu2I/edit). Removing the WTF namespace is the first step for the componentization.

Note that Timer and LifecycleObserver do not belong in wtf. wtf is like base,

One of the main goals of the componentization is to fix the broken dependency structure, which has resulted in a lot of unnecessary abstraction classes to hack the dependency. To fix the broken dependency, we're planning to remove platform/ and rearchitect the dependency to modules/ => core/ => wtf/ (very simple!). This means that things in platform/ that are depended on by core/ need to be moved to wtf/. This is why I want to move Timer, LifecycleObserver, Oilpan etc to wtf/. The "Oilpan and Vector" section of the above document would be helpful to understand the problem caused by mixing "things depended on by core/" in wtf/ and platform/ (Note: Oilpan is just one example, and a similar problem is taking place in other places as well).

If we really want to keep the WTF namespace, we can put Timer, LifecycleObserver etc in the WTF namespace when moving them into wtf/. However, IMHO it seems simpler to just drop the WTF namespace.

What's your concern about dropping the WTF namespace?

That code shouldn't be in the same namespace as core. You're proposing the equivalent of merging the cc and content namespaces.

wtf is our standard library, it's not part of core.

I don't fully understand your point, but are you just opposing to dropping the WTF namespace? Or are you opposing to move platform/ things depended on by core/ to wtf/?

I understand that you want to keep standard things in the WTF namespace for some philosophical reasons (but the meaning of "standard" is already obscure -- we've already put a bunch of "standard" things in platform/ to hack the dependency), but I think that the advantages of fixing the broken dependency structure and simplifying the code base would be more practical and important.

 

 

 it's a reusable standard library that hopefully we'll be able to merge into base and then start using the same classes in content after the repo merge.

That is our plan (i.e., make wtf/ depend on Chromium's base/ and share more code with Chromium), but I think we'll need to keep lots of code in wtf/ even after the repo merge. Since performance is the most important thing in Blink, we want to keep AtomicString, PassRefPtr, Vector with inline capacity etc in wtf/. Also Oilpan needs to hook allocation points of collections, we'll need to keep Vectors, HashMaps etc in wtf/ as well.

I think we want to be able to use Vectors with inline capacity in content, scheduler, cc, and other things too. 

- E 

[Elliott Sprehn]

On Tue, May 26, 2015 at 5:05 PM, Kentaro Hara <har...@chromium.org> wrote:

On Wed, May 27, 2015 at 8:51 AM, Elliott Sprehn <esp...@chromium.org> wrote:

On Tue, May 26, 2015 at 4:45 PM, Kentaro Hara <har...@chromium.org> wrote:

What's the advantage of removing this namespace?

The advantages of the Blink componentization are explained in this document (https://docs.google.com/document/d/1vtpoNdywErT2wtbE6a64scePBnZycuRhkbuuaIpiu2I/edit). Removing the WTF namespace is the first step for the componentization.

Note that Timer and LifecycleObserver do not belong in wtf. wtf is like base,

One of the main goals of the componentization is to fix the broken dependency structure, which has resulted in a lot of unnecessary abstraction classes to hack the dependency. To fix the broken dependency, we're planning to remove platform/ and rearchitect the dependency to modules/ => core/ => wtf/ (very simple!). This means that things in platform/ that are depended on by core/ need to be moved to wtf/. This is why I want to move Timer, LifecycleObserver, Oilpan etc to wtf/. The "Oilpan and Vector" section of the above document would be helpful to understand the problem caused by mixing "things depended on by core/" in wtf/ and platform/ (Note: Oilpan is just one example, and a similar problem is taking place in other places as well).

If we really want to keep the WTF namespace, we can put Timer, LifecycleObserver etc in the WTF namespace when moving them into wtf/. However, IMHO it seems simpler to just drop the WTF namespace.

What's your concern about dropping the WTF namespace?

That code shouldn't be in the same namespace as core. You're proposing the equivalent of merging the cc and content namespaces.

wtf is our standard library, it's not part of core.

I don't fully understand your point, but are you just opposing to dropping the WTF namespace? Or are you opposing to move platform/ things depended on by core/ to wtf/?

Both. Platform is not WTF, and WTF is not core.

 

I understand that you want to keep standard things in the WTF namespace for some philosophical reasons (but the meaning of "standard" is already obscure -- we've already put a bunch of "standard" things in platform/ to hack the dependency), but I think that the advantages of fixing the broken dependency structure and simplifying the code base would be more practical and important.

WTF is the standard library, cc/ should be able to use WTF too.

[Kentaro Hara]

On Wed, May 27, 2015 at 9:09 AM, Elliott Sprehn <esp...@chromium.org> wrote:

On Tue, May 26, 2015 at 5:05 PM, Kentaro Hara <har...@chromium.org> wrote:

On Wed, May 27, 2015 at 8:51 AM, Elliott Sprehn <esp...@chromium.org> wrote:

On Tue, May 26, 2015 at 4:45 PM, Kentaro Hara <har...@chromium.org> wrote:

What's the advantage of removing this namespace?

The advantages of the Blink componentization are explained in this document (https://docs.google.com/document/d/1vtpoNdywErT2wtbE6a64scePBnZycuRhkbuuaIpiu2I/edit). Removing the WTF namespace is the first step for the componentization.

Note that Timer and LifecycleObserver do not belong in wtf. wtf is like base,

One of the main goals of the componentization is to fix the broken dependency structure, which has resulted in a lot of unnecessary abstraction classes to hack the dependency. To fix the broken dependency, we're planning to remove platform/ and rearchitect the dependency to modules/ => core/ => wtf/ (very simple!). This means that things in platform/ that are depended on by core/ need to be moved to wtf/. This is why I want to move Timer, LifecycleObserver, Oilpan etc to wtf/. The "Oilpan and Vector" section of the above document would be helpful to understand the problem caused by mixing "things depended on by core/" in wtf/ and platform/ (Note: Oilpan is just one example, and a similar problem is taking place in other places as well).

If we really want to keep the WTF namespace, we can put Timer, LifecycleObserver etc in the WTF namespace when moving them into wtf/. However, IMHO it seems simpler to just drop the WTF namespace.

What's your concern about dropping the WTF namespace?

That code shouldn't be in the same namespace as core. You're proposing the equivalent of merging the cc and content namespaces.

wtf is our standard library, it's not part of core.

I don't fully understand your point, but are you just opposing to dropping the WTF namespace? Or are you opposing to move platform/ things depended on by core/ to wtf/?

Both. Platform is not WTF, and WTF is not core.

 

I understand that you want to keep standard things in the WTF namespace for some philosophical reasons (but the meaning of "standard" is already obscure -- we've already put a bunch of "standard" things in platform/ to hack the dependency), but I think that the advantages of fixing the broken dependency structure and simplifying the code base would be more practical and important.

WTF is the standard library, cc/ should be able to use WTF too.

Then, what is your proposal on the problems caused by the current dependency structure?

 

 

 

 it's a reusable standard library that hopefully we'll be able to merge into base and then start using the same classes in content after the repo merge.

That is our plan (i.e., make wtf/ depend on Chromium's base/ and share more code with Chromium), but I think we'll need to keep lots of code in wtf/ even after the repo merge. Since performance is the most important thing in Blink, we want to keep AtomicString, PassRefPtr, Vector with inline capacity etc in wtf/. Also Oilpan needs to hook allocation points of collections, we'll need to keep Vectors, HashMaps etc in wtf/ as well.

I think we want to be able to use Vectors with inline capacity in content, scheduler, cc, and other things too. 

- E 

--

Kentaro Hara, Tokyo, Japan

[Kinuko Yasuda]

On Wed, May 27, 2015 at 9:09 AM, Elliott Sprehn <esp...@chromium.org> wrote:

On Tue, May 26, 2015 at 5:05 PM, Kentaro Hara <har...@chromium.org> wrote:

On Wed, May 27, 2015 at 8:51 AM, Elliott Sprehn <esp...@chromium.org> wrote:

On Tue, May 26, 2015 at 4:45 PM, Kentaro Hara <har...@chromium.org> wrote:

What's the advantage of removing this namespace?

The advantages of the Blink componentization are explained in this document (https://docs.google.com/document/d/1vtpoNdywErT2wtbE6a64scePBnZycuRhkbuuaIpiu2I/edit). Removing the WTF namespace is the first step for the componentization.

Note that Timer and LifecycleObserver do not belong in wtf. wtf is like base,

One of the main goals of the componentization is to fix the broken dependency structure, which has resulted in a lot of unnecessary abstraction classes to hack the dependency. To fix the broken dependency, we're planning to remove platform/ and rearchitect the dependency to modules/ => core/ => wtf/ (very simple!). This means that things in platform/ that are depended on by core/ need to be moved to wtf/. This is why I want to move Timer, LifecycleObserver, Oilpan etc to wtf/. The "Oilpan and Vector" section of the above document would be helpful to understand the problem caused by mixing "things depended on by core/" in wtf/ and platform/ (Note: Oilpan is just one example, and a similar problem is taking place in other places as well).

If we really want to keep the WTF namespace, we can put Timer, LifecycleObserver etc in the WTF namespace when moving them into wtf/.

Oilpan depends on WebThread, which now depends on WebScheduler, so the dependency graph might become a bit tricky if we want to move Oilpan into wtf/.

 

However, IMHO it seems simpler to just drop the WTF namespace.

What's your concern about dropping the WTF namespace?

That code shouldn't be in the same namespace as core. You're proposing the equivalent of merging the cc and content namespaces.

wtf is our standard library, it's not part of core.

I don't fully understand your point, but are you just opposing to dropping the WTF namespace? Or are you opposing to move platform/ things depended on by core/ to wtf/?

Both. Platform is not WTF, and WTF is not core.

 

I understand that you want to keep standard things in the WTF namespace for some philosophical reasons (but the meaning of "standard" is already obscure -- we've already put a bunch of "standard" things in platform/ to hack the dependency), but I think that the advantages of fixing the broken dependency structure and simplifying the code base would be more practical and important.

WTF is the standard library, cc/ should be able to use WTF too.

This seems to be where things diverge.  If we want to use WTF outside Blink we probably don't want to make WTF dependable on all other blink code.

I was thinking that we could move some of very basic WTF classes that are useful outside Blink into src/base one by one, partly because I've been worrying that just exposing all WTF classes would introduce a lot of confusion (as base doesn't understand WTF types and WTF doesn't understand base types).

Specifically talking about WTF::Vector, if we really want to use WTF::Vector as a standard utility (but without oilpan) it looks viable options (other than merging everything) would be:

• Keep WTF as is, HeapVector could continue to utilize WTF::Vector by specializing its allocator or could be implemented on its own outside WTF.
• WTF::Vector is now blink::Vector, implement yet another similar lightweight vector class in src/base so that all chromium code can use it.
  
  

 it's a reusable standard library that hopefully we'll be able to merge into base and then start using the same classes in content after the repo merge.

That is our plan (i.e., make wtf/ depend on Chromium's base/ and share more code with Chromium), but I think we'll need to keep lots of code in wtf/ even after the repo merge. Since performance is the most important thing in Blink, we want to keep AtomicString, PassRefPtr, Vector with inline capacity etc in wtf/. Also Oilpan needs to hook allocation points of collections, we'll need to keep Vectors, HashMaps etc in wtf/ as well.

I think we want to be able to use Vectors with inline capacity in content, scheduler, cc, and other things too. 

What other containers / utilities do are you thinking you want to use outside Blink?

[Ojan Vafai]

Just to keep the discussion sane, lets move discussion of things to reuse from WTF to this thread and let the original thread focus on the componentization discussion.

[Kentaro Hara]

Oilpan depends on WebThread, which now depends on WebScheduler, so the dependency graph might become a bit tricky if we want to move Oilpan into wtf/.

That is true, but the dependency from Oilpan to the WebThread is very simple, so it would be easy to factor the part out of Oilpan (i.e., factor out glue/MessageLoopInterruptor from wtf/Oilpan).

The current interactions between platform/Oilpan and wtf/ things are much more trickier :)

[Kentaro Hara]

Thanks for coordinating the discussion!

On Wed, May 27, 2015 at 12:34 PM, Ojan Vafai <oj...@chromium.org> wrote:

Just to keep the discussion sane, lets move discussion of things to reuse from WTF to this thread and let the original thread focus on the componentization discussion.

On Tue, May 26, 2015 at 7:47 PM Kinuko Yasuda <kin...@chromium.org> wrote:

On Wed, May 27, 2015 at 9:09 AM, Elliott Sprehn <esp...@chromium.org> wrote:

On Tue, May 26, 2015 at 5:05 PM, Kentaro Hara <har...@chromium.org> wrote:

I understand that you want to keep standard things in the WTF namespace for some philosophical reasons (but the meaning of "standard" is already obscure -- we've already put a bunch of "standard" things in platform/ to hack the dependency), but I think that the advantages of fixing the broken dependency structure and simplifying the code base would be more practical and important.

WTF is the standard library, cc/ should be able to use WTF too.

This seems to be where things diverge.  If we want to use WTF outside Blink we probably don't want to make WTF dependable on all other blink code.

I was thinking that we could move some of very basic WTF classes that are useful outside Blink into src/base one by one, partly because I've been worrying that just exposing all WTF classes would introduce a lot of confusion (as base doesn't understand WTF types and WTF doesn't understand base types).

Specifically talking about WTF::Vector, if we really want to use WTF::Vector as a standard utility (but without oilpan) it looks viable options (other than merging everything) would be:

• Keep WTF as is, HeapVector could continue to utilize WTF::Vector by specializing its allocator or could be implemented on its own outside WTF.
• WTF::Vector is now blink::Vector, implement yet another similar lightweight vector class in src/base so that all chromium code can use it.
  
  

This sounds like a good approach to me.

- If base/X is better than wtf/X', then we can just remove wtf/X'. (e.g., threading primitives etc)

- If wtf/X' is better than base/X, then we can move wtf/X' to base/X' so that Chromium can use X' if they want. (e.g., AtomicString, Vector, hopefully PartitionAlloc etc)

The dependency structure will be: modules/ => core/ => wtf/ => base/. As we move things in wtf/ to base/ incrementally, wtf/ will become smaller.

 

 it's a reusable standard library that hopefully we'll be able to merge into base and then start using the same classes in content after the repo merge.

That is our plan (i.e., make wtf/ depend on Chromium's base/ and share more code with Chromium), but I think we'll need to keep lots of code in wtf/ even after the repo merge. Since performance is the most important thing in Blink, we want to keep AtomicString, PassRefPtr, Vector with inline capacity etc in wtf/. Also Oilpan needs to hook allocation points of collections, we'll need to keep Vectors, HashMaps etc in wtf/ as well.

I think we want to be able to use Vectors with inline capacity in content, scheduler, cc, and other things too. 

What other containers / utilities do are you thinking you want to use outside Blink?

 

[Kentaro Hara]

Thanks all for inputs! I also chatted with tkent-san offline right now.

Based on the inputs, I'm now writing a revised version of the componentization proposal to address the concerns raised here. I'll send it out once it's ready.

[Alex Clarke]

I wonder how much of WTF will stay after blink has been merged into the chromium repo? I doubt it will all go but if it ends up being a lot smaller, the arguments for keeping the WTF namespace seem weaker to me.

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

[Daniel Bratell]

On Wed, 27 May 2015 04:46:48 +0200, Kinuko Yasuda <kin...@chromium.org> wrote:

Keep WTF as is, HeapVector could continue to utilize WTF::Vector by specializing its allocator or could be implemented on its own outside WTF.

I think this is an interesting suggestion (and the same for HashTable), so that Vector can remain a rather abstract class with very little knowledge of anything, and specifically not depend on the garbage collection system in oilpan.

(And could it be renamed from HeapVector to GCVector or something that doesn't use the word heap since that is quite ambigious?)

/Daniel

[Daniel Bratell]

On Wed, 27 May 2015 10:05:32 +0200, 'Alex Clarke' via blink-dev <blin...@chromium.org> wrote:

I wonder how much of WTF will stay after blink has been merged into the chromium repo? I doubt it will all go but if it ends up being a lot smaller, the arguments for keeping the WTF namespace seem weaker to me.

The big bulk is string/text management and collection classes. The collection classes might be replacable by STL collection classes, partly or wholly but considering that so much of a web engine is string management and depending on very specific characteristics of the string class, it seems unlikely that std::string (from more or less random STL libraries) will be able to fully replace the blink String class.

Many of the macros in various WTF header files and other random utility functions should probably be moved/merged into base.

But this is a bit academic. Maybe WTF will become tiny enough that having its own namespace will seem silly. Maybe not. When it feels silly it would be a good time to change it.

And in case it is not clear, I also think there is value in having the utility library in its own namespace. Not critical but a useful feature. The value is somewhat diminished today by all the "using WTF::Foo" statements in the headers though.

/Daniel

[Kinuko Yasuda]

On Wed, May 27, 2015 at 6:30 PM, Daniel Bratell <bra...@opera.com> wrote:

On Wed, 27 May 2015 10:05:32 +0200, 'Alex Clarke' via blink-dev <blin...@chromium.org> wrote:

I wonder how much of WTF will stay after blink has been merged into the chromium repo? I doubt it will all go but if it ends up being a lot smaller, the arguments for keeping the WTF namespace seem weaker to me.

My take here is relatively optimistic: if it gets smaller enough we could reconsider :)

 

The big bulk is string/text management and collection classes. The collection classes might be replacable by STL collection classes, partly or wholly but considering that so much of a web engine is string management and depending on very specific characteristics of the string class, it seems unlikely that std::string (from more or less random STL libraries) will be able to fully replace the blink String class.

WTF collection classes use partition alloc and are highly tuned in terms of memory usage and performance, I think that probably holds a strong reason to keep them around separately from STL collection classes.

[Kinuko Yasuda]

(Moving WTF reuse / merging-to-base discussion from the original thread)

On Wed, May 27, 2015 at 8:27 PM, Kinuko Yasuda <kin...@chromium.org> wrote:

On Wed, May 27, 2015 at 6:30 PM, Daniel Bratell <bra...@opera.com> wrote:

On Wed, 27 May 2015 10:05:32 +0200, 'Alex Clarke' via blink-dev <blin...@chromium.org> wrote:

I wonder how much of WTF will stay after blink has been merged into the chromium repo? I doubt it will all go but if it ends up being a lot smaller, the arguments for keeping the WTF namespace seem weaker to me.

My take here is relatively optimistic: if it gets smaller enough we could reconsider :)

By the way I've been trying to create a list of what WTF classes/utilities can be removed, or merged / subsumed into base/.  It's incomplete (and I could be wrong on many) but since we started to discuss this let me share this now:

https://docs.google.com/spreadsheets/d/1VO9CwSTuQPIaPYCommnjZKBx9yIrU8Q1jJuBpLPTfSM/edit?usp=sharing

Many of utilities that are highly tuned in terms of performance/memory usage are currently marked 'cannot be merged into base/ now' in this spreadsheet but it's purely because I thought it'd need some work / consideration before doing so.

[Kentaro Hara]

Here is a slightly revised version of the proposal :)

Given the complexity of Blink, it's hard to describe the proposal concisely, but I hope the figure attached to this email will help you understand the essence of the proposal. The figure illustrates what files are going to be moved to what directories for several representative files.

- The proposed dependency is modules/ => core/ => wtf/ => base/. Simple!

- In the original proposal, I was planning to move "all files in platform/ that don't depend on core/" to wtf/. However, as Ellioot pointed out, it might be too much. In the revised proposal, I'm planning to move "only files in platform/ that really need to be moved to wtf/" to wtf/. For example, Oilpan will be moved from platform/ to wtf/, but Timer and LifecycleObserver will be moved from platform/ to core/. (Note: In terms of dependency, it is possible to put Timer and LifecycleObserver in wtf/ because they don't depend on core/. In that sense, it is a bit ambiguous what should be in wtf/ and what should be in core/.)

- The revised proposal will reduce the number of files we need to move to wtf/. Thus it is not mandatory to remove the WTF namespace. We can keep the WTF namespace as is.

- As we're discussing in another thread, things in wtf/ that are useful for Chromium will be moved to base/.

- In the original proposal, I was planning to put public APIs (i.e., header files) in public/delegate/ and public/web/. However, there would be little benefit in splitting the APIs in two directories, so I'm now planning to put all public APIs in public/. Their implementations will be put in either of core/web/, core/modules/ or content/.

Does this make more sense? Thanks!

[Kentaro Hara]

On Wed, May 27, 2015 at 9:05 PM, Kentaro Hara <har...@chromium.org> wrote:

Here is a slightly revised version of the proposal :)

Given the complexity of Blink, it's hard to describe the proposal concisely, but I hope the figure attached to this email will help you understand the essence of the proposal. The figure illustrates what files are going to be moved to what directories for several representative files.

- The proposed dependency is modules/ => core/ => wtf/ => base/. Simple!

- In the original proposal, I was planning to move "all files in platform/ that don't depend on core/" to wtf/. However, as Ellioot pointed out, it might be too much. In the revised proposal, I'm planning to move "only files in platform/ that really need to be moved to wtf/" to wtf/. For example, Oilpan will be moved from platform/ to wtf/, but Timer and LifecycleObserver will be moved from platform/ to core/. (Note: In terms of dependency, it is possible to put Timer and LifecycleObserver in wtf/ because they don't depend on core/. In that sense, it is a bit ambiguous what should be in wtf/ and what should be in core/.)

- The revised proposal will reduce the number of files we need to move to wtf/. Thus it is not mandatory to remove the WTF namespace. We can keep the WTF namespace as is.

- As we're discussing in another thread, things in wtf/ that are useful for Chromium will be moved to base/.

- In the original proposal, I was planning to put public APIs (i.e., header files) in public/delegate/ and public/web/. However, there would be little benefit in splitting the APIs in two directories, so I'm now planning to put all public APIs in public/. Their implementations will be put in either of core/web/, core/modules/ or content/.

core/modules/ => modules/web/

Ellioot => Elliott :)

 

Does this make more sense? Thanks!

On Wed, May 27, 2015 at 8:27 PM, Kinuko Yasuda <kin...@chromium.org> wrote:

On Wed, May 27, 2015 at 6:30 PM, Daniel Bratell <bra...@opera.com> wrote:

On Wed, 27 May 2015 10:05:32 +0200, 'Alex Clarke' via blink-dev <blin...@chromium.org> wrote:

I wonder how much of WTF will stay after blink has been merged into the chromium repo? I doubt it will all go but if it ends up being a lot smaller, the arguments for keeping the WTF namespace seem weaker to me.

My take here is relatively optimistic: if it gets smaller enough we could reconsider :)

 

The big bulk is string/text management and collection classes. The collection classes might be replacable by STL collection classes, partly or wholly but considering that so much of a web engine is string management and depending on very specific characteristics of the string class, it seems unlikely that std::string (from more or less random STL libraries) will be able to fully replace the blink String class.

WTF collection classes use partition alloc and are highly tuned in terms of memory usage and performance, I think that probably holds a strong reason to keep them around separately from STL collection classes.

 

Many of the macros in various WTF header files and other random utility functions should probably be moved/merged into base.

But this is a bit academic. Maybe WTF will become tiny enough that having its own namespace will seem silly. Maybe not. When it feels silly it would be a good time to change it.

And in case it is not clear, I also think there is value in having the utility library in its own namespace. Not critical but a useful feature. The value is somewhat diminished today by all the "using WTF::Foo" statements in the headers though.

/Daniel

--

Kentaro Hara, Tokyo, Japan

[Jochen Eisinger]

Hey,

two things that aren't clear to me: why does wtf need to depend on base? couldn't stuff above wtf (continue to) depends on both wtf and base?

And: why are we moving platform concepts such as networking into core? in chrome, net/ is also outside of e.g. content/?

[Kentaro Hara]

two things that aren't clear to me: why does wtf need to depend on base? couldn't stuff above wtf (continue to) depends on both wtf and base?

If we move Vector to base/ while letting wtf/ use Vector, we need to make wtf/ depend on base/.

Do you see any disadvantage in making wtf/ depend on base/? (If there is any disadvantage, we need to consider a way to avoid it.)

And: why are we moving platform concepts such as networking into core? in chrome, net/ is also outside of e.g. content/?

That is a key question. Conceptually we can consider a dependency like web/ => modules/ => core/ => platform/ => wtf/ (e.g., put the network layer in platform/), and we actually have the dependency in the current architecture. However, what we've learned in the Blink code base is that the dependency has resulted in a lot of abstraction classes & hacks to overcome disallowed dependencies (e.g., WebIDB, ServiceWorker, Oilpan etc). Conceptually web/ => modules/ => core/ => platform/ => wtf/ looks cool, but in practice it is hard to implement classes according to the dependency model. Thus I think that overall it is better to simplify the dependency to modules/ => core/ => wtf/ and remove those abstractions & hacks. We will lose some layering concepts, but I believe that the advantages of the simplification would be a clear win. (Note: We will lose some layering concepts in terms of link units, but we still can/should keep a good layering inside the link unit.)

[Kentaro Hara]

> And: why are we moving platform concepts such as networking into core? in chrome, net/ is also outside of e.g. content/?

That is a key question. Conceptually we can consider a dependency like web/ => modules/ => core/ => platform/ => wtf/ (e.g., put the network layer in platform/), and we actually have the dependency in the current architecture. However, what we've learned in the Blink code base is that the dependency has resulted in a lot of abstraction classes & hacks to overcome disallowed dependencies (e.g., WebIDB, ServiceWorker, Oilpan etc). Conceptually web/ => modules/ => core/ => platform/ => wtf/ looks cool, but in practice it is hard to implement classes according to the dependency model. Thus I think that overall it is better to simplify the dependency to modules/ => core/ => wtf/ and remove those abstractions & hacks. We will lose some layering concepts, but I believe that the advantages of the simplification would be a clear win. (Note: We will lose some layering concepts in terms of link units, but we still can/should keep a good layering inside the link unit.)

If we want to keep the platform/ layer, I think we can keep it.

From the perspective of cleaning up the abstraction classes & hacks, the most important part is to move the WebXXX things in web/ and platform/ into core/web/ and modules/web/. As long as we do that, other platform/ things can stay in platform/. In other words, the following dependency would be plausible:

  modules/ => core/ => platform/ (which doesn't contain any WebXXX thing) => wtf/

[Peter Kasting]

On Wed, May 27, 2015 at 7:12 AM, Kentaro Hara <har...@chromium.org> wrote:

two things that aren't clear to me: why does wtf need to depend on base? couldn't stuff above wtf (continue to) depends on both wtf and base?

If we move Vector to base/ while letting wtf/ use Vector, we need to make wtf/ depend on base/.

As a Chromium dev: why would you move Vector to base/?

Chromium has a very consistent style and policy of using std::vector as the One True Vector type everywhere.  I would be loathe to introduce a new vector type at this stage and worry that the confusion and potential inconsistency it could introduce would far outweigh any performance gains.  Unless you've planning to somehow replace all std::vector Chromium-wide?

Unless I'm missing something, it seems much better to keep Vector in wtf/ and not make it available to Chrome.

PK

[Elliott Sprehn]

The wtf containers are much better than the stl ones, all other browsers use something besides the stl because it has better performance and memory usage.

We do plan to use the wtf containers all over the browser in the future for advantages like atomized strings, inline capacity, faster hashing and finely tuned performance.

- E

[Brett Wilson]

I strongly oppose this approach. We can consider adding to the base/containers list for a few cases, but we should not in general be adding WTF classes to base.

Brett

[Dana Jansens]

I'm looking forward to data on a case-by-case basis to replace STL types with WTF types. If they can make chromium faster/smaller that seems like something we can get behind. I'm a little more skeptical than are people coming from the blink side of things, I think, but that's what data is for.

[Peter Kasting]

The STL, in olden days, used to have crappy performance -- back in the MSVC 6 timeframe.  Modern STL implementations are, generally, quite good.  It's always possible for tuned implementations to be better at specific things, but generally you trade off other things to get those wins; you don't normally just outperform the STL in all scenarios.

This is a scenario where, before moving such classes into base/ and using them in Chromium, the people wanting to do such things need to provide case-by-case benchmarks demonstrating that such substitutions are an improvement on perf, memory use, etc.  I am not prepared, and I don't think Chromium in general is prepared, to simply make such wholesale swaps on the assumption that they're wins.

(Indeed, I've long wanted to see such comparative benchmarks for Blink, since I've been skeptical that things like WTF::Vector are perf wins there either, though I'm more prepared to believe that they are, given that Blink is less of a general-purpose codebase.)

PK

[James Robinson]

For things like vector *modern* std::vector is probably equivalent to WTF::Vector in pretty much all cases.  Pre-C++11 std::vector will definitely underperform WTF::Vector, and in fact not even compile in many cases, as it doesn't support move semantics.  The other WTF containers will generally speaking outperform STL containers in most cases, largely because they trade off some bits of flexibility that aren't needed inside Blink.  WTF::String in particular will greatly outperform std::basic_string<> for most operations that Blink wants to do in exchange for having very difficult threading semantics.  The threading semantics that WTF::String has make sense (or at least some sense) for Blink's predominantly single-threaded nature, although they probably wouldn't make sense for many parts of chromium and certainly wouldn't make sense for the STL.  In the past the WTF containers have also benefited from being taught about different classes within Blink to do things like know when an array of a certain type can be safely memcpy()d which is something that's not possible to do with a generic STL or other container library agnostic of the types being contained.  With C++11+ move semantics and forwarding this is probably less valuable than it used to be, but I'd be surprised if the benefit was nonzero.

- James

[Elliott Sprehn]

On Wed, May 27, 2015 at 1:44 PM, James Robinson <jam...@chromium.org> wrote:

For things like vector *modern* std::vector is probably equivalent to WTF::Vector in pretty much all cases.

std::vector doesn't let you set inline capacity to avoid malloc in common cases. That allows blink to avoid doing malloc in a number of cases. It also means better memory locality since the data is right there on the stack.

- E

[James Robinson]

Right, I had forgotten about that.  It's a big deal for sure.  I think that's technically possible with std::allocator shenanagins but it's definitely not convenient and in practice probably never done with std::vector (at least in chromium). 

- James

- E

[John Abd-El-Malek]

Just happened on this thread. For discussions about introducing this to code outside blink, chromium-dev should probably be CC'd.

I completely agree that before anything moves to base, we need some data as it would be disruptive to have two vector classes etc... This should be easy to source, i.e. profile chrome using some of the telemetry benchmarks we have (smooth scrolling?) and see what % of time is spent in the various conversion methods.

[Ojan Vafai]

On Wed, May 27, 2015 at 4:37 AM Kinuko Yasuda <kin...@chromium.org> wrote:

(Moving WTF reuse / merging-to-base discussion from the original thread)

On Wed, May 27, 2015 at 8:27 PM, Kinuko Yasuda <kin...@chromium.org> wrote:

On Wed, May 27, 2015 at 6:30 PM, Daniel Bratell <bra...@opera.com> wrote:

On Wed, 27 May 2015 10:05:32 +0200, 'Alex Clarke' via blink-dev <blin...@chromium.org> wrote:

I wonder how much of WTF will stay after blink has been merged into the chromium repo? I doubt it will all go but if it ends up being a lot smaller, the arguments for keeping the WTF namespace seem weaker to me.

My take here is relatively optimistic: if it gets smaller enough we could reconsider :)

By the way I've been trying to create a list of what WTF classes/utilities can be removed, or merged / subsumed into base/.  It's incomplete (and I could be wrong on many) but since we started to discuss this let me share this now:

https://docs.google.com/spreadsheets/d/1VO9CwSTuQPIaPYCommnjZKBx9yIrU8Q1jJuBpLPTfSM/edit?usp=sharing

Many of utilities that are highly tuned in terms of performance/memory usage are currently marked 'cannot be merged into base/ now' in this spreadsheet but it's purely because I thought it'd need some work / consideration before doing so.

Do you want help sorting this? If so, can you make it editable?

[Kinuko Yasuda]

On Thu, May 28, 2015 at 6:45 AM, Ojan Vafai <oj...@chromium.org> wrote:

On Wed, May 27, 2015 at 4:37 AM Kinuko Yasuda <kin...@chromium.org> wrote:

(Moving WTF reuse / merging-to-base discussion from the original thread)

On Wed, May 27, 2015 at 8:27 PM, Kinuko Yasuda <kin...@chromium.org> wrote:

On Wed, May 27, 2015 at 6:30 PM, Daniel Bratell <bra...@opera.com> wrote:

On Wed, 27 May 2015 10:05:32 +0200, 'Alex Clarke' via blink-dev <blin...@chromium.org> wrote:

I wonder how much of WTF will stay after blink has been merged into the chromium repo? I doubt it will all go but if it ends up being a lot smaller, the arguments for keeping the WTF namespace seem weaker to me.

My take here is relatively optimistic: if it gets smaller enough we could reconsider :)

By the way I've been trying to create a list of what WTF classes/utilities can be removed, or merged / subsumed into base/.  It's incomplete (and I could be wrong on many) but since we started to discuss this let me share this now:

https://docs.google.com/spreadsheets/d/1VO9CwSTuQPIaPYCommnjZKBx9yIrU8Q1jJuBpLPTfSM/edit?usp=sharing

Many of utilities that are highly tuned in terms of performance/memory usage are currently marked 'cannot be merged into base/ now' in this spreadsheet but it's purely because I thought it'd need some work / consideration before doing so.

Do you want help sorting this? If so, can you make it editable?

Sure, made it editable. (Anyone with chromium account can edit)

  

[Kentaro Hara]

Thanks all for comments!

Just to sum up:

- The original intention of this thread was to remove the WTF namespace. It turned out not to be mandatory, so I will drop the proposal.

- Now the real intention of this thread is to get a consensus about the future dependency model in Blink to start the componentization work. I'll start a separate thread to focus on the topic.

- There are a lot of discussions about what should/should not be moved from wtf/ to base/. Let's discuss this in the "Moving WTF classes into base" thread.

[Kinuko Yasuda]

(Moving discussion from the original thread, cc-ing chromium-dev@)

Context: there's a discussion about removing WTF namespace in blink-dev (the proposal was dropped fyi), and there were some voices that we may want to use some of WTF classes out of Blink.  One idea is to move some of move or merge some of very basic WTF classes that could be useful outside Blink into src/base one by one, only when we have concrete proof / data that shows using it across chromium has clear benefit.

A specific request made by esprehn@ (and some other folks) was cc/ and some other out-of-blink components want to use WTF::Vector (which is blink's std::vector), because those WTF container classes are (supposed to be) highly tuned in terms of performance/memory usage. Some discussion about why WTF::Vector could be better than std::vector can be found in the original thread, but esprehn@ summarizes the potential benefit like below:

On Thu, May 28, 2015 at 5:51 AM, Elliott Sprehn <esp...@chromium.org> wrote:

std::vector doesn't let you set inline capacity to avoid malloc in common cases. That allows blink to avoid doing malloc in a number of cases. It also means better memory locality since the data is right there on the stack.

I think we basically agree that we'll need to look into this case-by-case basis, and I started a spreadsheet to list up all WTF classes and see if we can/want to:

1) remove it by replacing its usage with base/'s similar utility, 

2) move it to base/ (or somewhere outside blink/) so that all chromium can use them, or

3) keep it within blink/.

The spreadsheet (now editable by anyone @chromium):

https://docs.google.com/spreadsheets/d/1VO9CwSTuQPIaPYCommnjZKBx9yIrU8Q1jJuBpLPTfSM/edit?usp=sharing

[Brett Wilson]

I would like to decouple this from the Blink merge. I think we need to come up with a policy carefully and I don't think we can rush that. I don't think piecemeal or ad-hoc changes will lead us to a good place. The current Blink API boundary where we have to convert types sucks, and I don't want to make that boundary larger or more complicated by randomly pushing Blink types into higher layers of code.

For now, until there is a more formal policy:

- We should not move things from WTF to base, nor using WTF from non-Blink code. 

- Blink can use base for things if it wants. If we do decide to convert Blink to use something in base, we should do it as quickly as possible to avoid inconsistencies, and then delete the WTF one.

- We should not start using STL or base stuff in Blink when there are WTF equivalents. Just as we don't want a soup of incompatible container types in Chrome, we don't want the inverse soup in Blink.

Brett

[James Robinson]

On Wed, May 27, 2015 at 5:02 PM, Kinuko Yasuda <kin...@chromium.org> wrote:

(Moving discussion from the original thread, cc-ing chromium-dev@)

Context: there's a discussion about removing WTF namespace in blink-dev (the proposal was dropped fyi), and there were some voices that we may want to use some of WTF classes out of Blink.  One idea is to move some of move or merge some of very basic WTF classes that could be useful outside Blink into src/base one by one, only when we have concrete proof / data that shows using it across chromium has clear benefit.

A specific request made by esprehn@ (and some other folks) was cc/ and some other out-of-blink components want to use WTF::Vector (which is blink's std::vector), because those WTF container classes are (supposed to be) highly tuned in terms of performance/memory usage. Some discussion about why WTF::Vector could be better than std::vector can be found in the original thread, but esprehn@ summarizes the potential benefit like below:

On Thu, May 28, 2015 at 5:51 AM, Elliott Sprehn <esp...@chromium.org> wrote:

std::vector doesn't let you set inline capacity to avoid malloc in common cases. That allows blink to avoid doing malloc in a number of cases. It also means better memory locality since the data is right there on the stack.

I think we basically agree that we'll need to look into this case-by-case basis, and I started a spreadsheet to list up all WTF classes and see if we can/want to:

1) remove it by replacing its usage with base/'s similar utility, 

2) move it to base/ (or somewhere outside blink/) so that all chromium can use them, or

3) keep it within blink/.

The spreadsheet (now editable by anyone @chromium):

https://docs.google.com/spreadsheets/d/1VO9CwSTuQPIaPYCommnjZKBx9yIrU8Q1jJuBpLPTfSM/edit?usp=sharing

One thing that spreadsheet doesn't cover that will come up in a surprising number of places is that //base/memory/ref_counted.h has an initial refcount of 0 but WTF::RefCounted has an initial refcount of 1.  This comes up in a surprising amount of code that has to decide whether an instance should be destroyed or wants to initialize a new instance efficiently.  It'd be great to unify behaviors (preferably to an initial refcount of 1 since it avoids a lot of problems) but it'd be a very substantial amount of work.

- James

[Kinuko Yasuda]

I basically agree with these. The spreadsheet was meant to give a better basis for discussion, not meant to imply any hasty action.

One thing I think we should do quickly is set a policy like these and make it clear on our coding style guide (esp. on Blink side).

Using WTF classes outside Blink will need a lot more discussion, but one of the common hope we all had for repository merge is ability to use base/ from Blink.  However we should be very careful doing so too as base/ doesn't know WTF types, and merging them could cause thread-safety issues too (I myself have introduced several racy changes by naively using base::Bind against WebString before).

(These are for Blink-side discussion, I cc'ed chromium-dev but it's just fyi for now)

[Kinuko Yasuda]

Right, personally I feel it may not be feasible unifying these ref-counted templates unless we have a clear win.

 

- James

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

[Brett Wilson]

Sounds good.

Brett

[Kentaro Hara]

I now totally agree with the above proposal of Brett. After the repository merge, we can make wtf/ depend on base/, but that doesn't mean that we want to aggressively port things in wtf/ to base/. On the other hand, wtf/ can start using base/ and remove duplicated code.

P.S.

- We're currently experimenting with replacing tcmalloc with PartitionAlloc (in only Blink or in both Blink and Chromium). Depending on the performance/memory numbers, we may propose moving PartitionAlloc to Chromium.

- When working on Oilpan, I evaluated performance of std::vector and wtf's Vector. It looks like that the inline capacity is very important for performance (as long as you specify a proper inline capacity :-), but for vectors that don't have inline capacity, performance of wtf's Vector is almost the same as std::vector.

[Christian Biesinger]

I'm curious - once we ship Oilpan, will there still be a lot of
refcounted objects left in blink?

-christian

[Kentaro Hara]

I'm curious - once we ship Oilpan, will there still be a lot of
refcounted objects left in blink?

Oilpan won't replace everything; e.g., RenderObjects will keep using PartitionAlloc. But the point is that the current Blink mixes PartitionAlloc, tcmalloc and system allocators. I want to unify them into PartitionAlloc to get Blink objects under control.

[Daniel Bratell]

On Wed, 27 May 2015 21:14:17 +0200, Dana Jansens <dan...@chromium.org> wrote:

I'm looking forward to data on a case-by-case basis to replace STL types with WTF types. If they can make chromium faster/smaller that seems like something we can get behind. I'm a little more skeptical than are people coming from the blink side of things, I think, but that's what data is for.

Me too. And there are at least three factors: Performance (in various use cases), memory usage, and amount of generated machine code ("bloat"). I don't know what would come out on top on either of these, and also, I don't know how reliable a result would be across STL implementations.

If I can show that VS std::vector is very good in a certain case, what does that tell me about iOS or Android std::vector?

Hmm, summer internships coming soon? :-) Or someone that reads this that is just of a curious nature and not too busy that can build a "std collection vs wtf collection" testing framework for measuring all three of performance, memory usage and bloatiness that we can then run across all platforms?

/Daniel

[Antoine Labour]

On Thu, May 28, 2015 at 2:09 AM, Daniel Bratell <bra...@opera.com> wrote:

On Wed, 27 May 2015 21:14:17 +0200, Dana Jansens <dan...@chromium.org> wrote:

I'm looking forward to data on a case-by-case basis to replace STL types with WTF types. If they can make chromium faster/smaller that seems like something we can get behind. I'm a little more skeptical than are people coming from the blink side of things, I think, but that's what data is for.

Me too. And there are at least three factors: Performance (in various use cases), memory usage, and amount of generated machine code ("bloat").

I'd add one more, modern language support & stl compatibility. E.g. movable types, algorithms etc.

Antoine

[Brett Wilson]

On Thu, May 28, 2015 at 11:49 AM, Antoine Labour <pi...@chromium.org> wrote:

On Thu, May 28, 2015 at 2:09 AM, Daniel Bratell <bra...@opera.com> wrote:

On Wed, 27 May 2015 21:14:17 +0200, Dana Jansens <dan...@chromium.org> wrote:

I'm looking forward to data on a case-by-case basis to replace STL types with WTF types. If they can make chromium faster/smaller that seems like something we can get behind. I'm a little more skeptical than are people coming from the blink side of things, I think, but that's what data is for.

Me too. And there are at least three factors: Performance (in various use cases), memory usage, and amount of generated machine code ("bloat").

I'd add one more, modern language support & stl compatibility. E.g. movable types, algorithms etc.

And I would add usability.

My anecdotal experience (from long ago) with WTF's hash table is that it is difficult to use because of the way it handled deleted and empty buckets. It was optimized to avoid heap allocations, which makes it faster, but I wasted an afternoon trying to figure out why my code was crashing. It's good to trade-off speed for usability in some performance critical cases, but for general browser usage I worry that this is not the best approach.

WTF's strings are unambiguously faster for some workloads, but if you use them wrong (pass between threads like the browser process does a lot but Blink does a little) you end up with subtle crashes. This is another example of Blink trading off generality and ease-of-use for speed. If it makes for a crashier product and less productive team, then it's a net negative even if things are some percent faster or smaller.

Especially around string handling, I think Blink's workloads are very different than the rest of the browser. The main question I have is whether there are other significant subsystems where the same tradeoffs that Blink made are also appropriate. And if there are, how can we allow those subsystems to make those tradeoffs without making a soup of similar but incompatible types floating around (which will lead to a slower product overall).

Brett

[Dana Jansens]

Is it possible to use inline capacity and satisfy the same guarantees about iterator validity across mutations of the vector as with std::vector?

 

[Brett Wilson]

I think this is not a problem.

base has StackVector which does this and I don't think there are problems. If Kentaro is correct and that the material difference between std::vector and WTF::Vector is only this inline buffer, one path forward for this container type is to unify on one "vector with a static buffer" object and use it everywhere, dropping the more subtle optimizations WTF::Vector does for optimizing initialization and copies.

I think this path for this particular container would be relatively uncontroversial since we already have that capability. I would want to revisit the base::StackVector API though, I'm not sure it's the best thing (I speak as the author of that object). And unifying will be annoying busywork.

Brett

[Elliott Sprehn]

On Thu, May 28, 2015 at 1:40 PM, Brett Wilson <bre...@chromium.org> wrote:

On Thu, May 28, 2015 at 1:30 PM, Dana Jansens <dan...@chromium.org> wrote:

On Wed, May 27, 2015 at 6:11 PM, Kentaro Hara <har...@chromium.org> wrote:

...

Is it possible to use inline capacity and satisfy the same guarantees about iterator validity across mutations of the vector as with std::vector?

I think this is not a problem.

base has StackVector which does this and I don't think there are problems. If Kentaro is correct and that the material difference between std::vector and WTF::Vector is only this inline buffer, one path forward for this container type is to unify on one "vector with a static buffer" object and use it everywhere, dropping the more subtle optimizations WTF::Vector does for optimizing initialization and copies.

We can't lose the vector optimizations for blink, for example being able to initialize with memset is important. Chromium could just not use those features though.

- E 

[Ojan Vafai]

Another difficulty we ran into with using std::list in Blink was that you can't change the allocator the list uses for it's containers. Only the allocator for the list itself. So we weren't able to have it used PartitionAlloc, which made it show up in CPU profiles.

What do we do in those situations today? Do we replace std::list with our own fork of it?  

[Kentaro Hara]

Also I have a couple of plans to more optimize WTF::Vector for Blink's common workloads, and I believe these efforts are important. So I want to keep those containers in wtf/ and optimize them for Blink.

(Let me reply to Brett's comment in the other thread, since my reply is more related to this topic.)

Brett:

My anecdotal experience (from long ago) with WTF's hash table is that it is difficult to use because of the way it handled deleted and empty buckets. It was optimized to avoid heap allocations, which makes it faster, but I wasted an afternoon trying to figure out why my code was crashing. It's good to trade-off speed for usability in some performance critical cases, but for general browser usage I worry that this is not the best approach. 

 

WTF's strings are unambiguously faster for some workloads, but if you use them wrong (pass between threads like the browser process does a lot but Blink does a little) you end up with subtle crashes. This is another example of Blink trading off generality and ease-of-use for speed. If it makes for a crashier product and less productive team, then it's a net negative even if things are some percent faster or smaller.

I agree with your point, and that is why I'm not really willing to move a lot of things from wtf/ to base/. The assumption in Blink (speed, speed, speed!) is somewhat special and the assumption doesn't necessarily hold in other components in Chromium.

For example, I don't think Chromium wants to use PassRefPtr. PassRefPtr is an optimized version of std::ref_ptr to skip increments/decrements when passing the pointer to other functions etc. This optimization is actually important for Blink's performance (I confirmed the performance impact a while back), but most components in Chromium wouldn't buy that advantage. The same discussion applies for Strings, Vectors, HashTables etc.

Brett:

Especially around string handling, I think Blink's workloads are very different than the rest of the browser. The main question I have is whether there are other significant subsystems where the same tradeoffs that Blink made are also appropriate. And if there are, how can we allow those subsystems to make those tradeoffs without making a soup of similar but incompatible types floating around (which will lead to a slower product overall).

I'm not that negative about having dedicated containers for each subsystem (Blink, V8 etc) as long as it makes sense. Given that the workloads & assumptions are different, it would make more sense to have dedicated containers than generalizing the containers too much. It is indeed nice to generalize things and share more code, but on the other hand, it is important to optimize the subsystem to its workloads & assumptions.

In the particular case of Blink, I think it makes sense to keep dedicated containers in wtf/.

[Daniel Cheng]

On Thu, May 28, 2015 at 4:53 PM Kentaro Hara <har...@chromium.org> wrote:

Also I have a couple of plans to more optimize WTF::Vector for Blink's common workloads, and I believe these efforts are important. So I want to keep those containers in wtf/ and optimize them for Blink.

(Let me reply to Brett's comment in the other thread, since my reply is more related to this topic.)

Brett:

My anecdotal experience (from long ago) with WTF's hash table is that it is difficult to use because of the way it handled deleted and empty buckets. It was optimized to avoid heap allocations, which makes it faster, but I wasted an afternoon trying to figure out why my code was crashing. It's good to trade-off speed for usability in some performance critical cases, but for general browser usage I worry that this is not the best approach. 

 

WTF's strings are unambiguously faster for some workloads, but if you use them wrong (pass between threads like the browser process does a lot but Blink does a little) you end up with subtle crashes. This is another example of Blink trading off generality and ease-of-use for speed. If it makes for a crashier product and less productive team, then it's a net negative even if things are some percent faster or smaller.

I agree with your point, and that is why I'm not really willing to move a lot of things from wtf/ to base/. The assumption in Blink (speed, speed, speed!) is somewhat special and the assumption doesn't necessarily hold in other components in Chromium.

For example, I don't think Chromium wants to use PassRefPtr. PassRefPtr is an optimized version of std::ref_ptr to skip increments/decrements when passing the pointer to other functions etc. This optimization is actually important for Blink's performance (I confirmed the performance impact a while back), but most components in Chromium wouldn't buy that advantage. The same discussion applies for Strings, Vectors, HashTables etc.

FWIW, std::ref_ptr (and the scoped_refptr in //base) do support this using move semantics.

Daniel

[Ojan Vafai]

On Thu, May 28, 2015 at 5:00 PM Daniel Cheng <dch...@chromium.org> wrote:

On Thu, May 28, 2015 at 4:53 PM Kentaro Hara <har...@chromium.org> wrote:

Also I have a couple of plans to more optimize WTF::Vector for Blink's common workloads, and I believe these efforts are important. So I want to keep those containers in wtf/ and optimize them for Blink.

(Let me reply to Brett's comment in the other thread, since my reply is more related to this topic.)

Brett:

My anecdotal experience (from long ago) with WTF's hash table is that it is difficult to use because of the way it handled deleted and empty buckets. It was optimized to avoid heap allocations, which makes it faster, but I wasted an afternoon trying to figure out why my code was crashing. It's good to trade-off speed for usability in some performance critical cases, but for general browser usage I worry that this is not the best approach. 

 

WTF's strings are unambiguously faster for some workloads, but if you use them wrong (pass between threads like the browser process does a lot but Blink does a little) you end up with subtle crashes. This is another example of Blink trading off generality and ease-of-use for speed. If it makes for a crashier product and less productive team, then it's a net negative even if things are some percent faster or smaller.

I agree with your point, and that is why I'm not really willing to move a lot of things from wtf/ to base/. The assumption in Blink (speed, speed, speed!) is somewhat special and the assumption doesn't necessarily hold in other components in Chromium.

For example, I don't think Chromium wants to use PassRefPtr. PassRefPtr is an optimized version of std::ref_ptr to skip increments/decrements when passing the pointer to other functions etc. This optimization is actually important for Blink's performance (I confirmed the performance impact a while back), but most components in Chromium wouldn't buy that advantage. The same discussion applies for Strings, Vectors, HashTables etc.

FWIW, std::ref_ptr (and the scoped_refptr in //base) do support this using move semantics.

Actually, I'm pretty confident that we don't need PassRefPtr. Almost all of the perf win is obviated by NRVO. We've talked for a long time about experimenting with removing it, but noone has owned proving that it won't be a perf regression. I think this would be a valuable simplificaton/cleanup project that noone would object to.

[Ojan Vafai]

My intuition of where the perf benefits (particularly of the allocation savings) will be largest is code inside the renderer. I'm not sure how to reasonably map that to directories. One natural starting place would be to allow WTF classes inside cc. Of course, we should only do so if we can show a measurable performance improvement on some content we care about.

Also, we're considering moving more and more of the logic that's currently in Blink into cc. I don't know how much of the existing code we plan on reusing, but it'd be nice to not be *required* to rewrite it just for this reason.

Does that sound OK?

[Kentaro Hara]

At least, it would be fine in terms of dependency. Since wtf/ will depend on only base/, cc/ can depend on wtf/ without causing a dependency cycle. (This is one of the reasons I don't want to make wtf/ depend on things except base/ (e.g., v8/))

[Daniel Cheng]

NRVO/RVO don't help when passing arguments though, and this is one place PassRefPtr gets used a lot as well.

That being said, I guess Oilpan also removes a lot of the refcounted objects in Blink so maybe it really doesn't matter.

Daniel

[Ojan Vafai]

Yup. I'm skeptical there are that enough places where we use PassRefPtrs as arguments for it to actually matter.

That being said, I guess Oilpan also removes a lot of the refcounted objects in Blink so maybe it really doesn't matter.

That too. But I really expect we'd see no perf regression by just removing PassRefPtr. 

[Nico Weber]

Shouldn't we instead start using std containers in blink, at least in the places where perf isn't critical?

[Nico Weber]

Nevermind, looks like there's another thread about this with more active discussion. Ignore my reply (and this thread unless you want to talk about quantifying the difference between stl and wtf).

[Kentaro Hara]

I think that is our plan.

- Use std containers or base/ things in Blink if it makes sense. Remove redundant containers from Blink.

- Don't aggressively move wtf/ things that are optimized for Blink (e.g., AtomicString, Vector etc) to base/. Keep them in wtf/. We don't need to move them to base/ to use them from cc/, because cc/ can depend on wtf/.

[Daniel Bratell]

On Thu, 28 May 2015 20:49:55 +0200, Antoine Labour <pi...@chromium.org> wrote:

On Thu, May 28, 2015 at 2:09 AM, Daniel Bratell <bra...@opera.com> wrote:

On Wed, 27 May 2015 21:14:17 +0200, Dana Jansens <dan...@chromium.org> wrote:

I'm looking forward to data on a case-by-case basis to replace STL types with WTF types. If they can make chromium faster/smaller that seems like something we can get behind. I'm a little more skeptical than are people coming from the blink side of things, I think, but that's what data is for.

Me too. And there are at least three factors: Performance (in various use cases), memory usage, and amount of generated machine code ("bloat").

I'd add one more, modern language support & stl compatibility. E.g. movable types, algorithms etc.

Move is absolutely an interesting use case. It has the potential to make code faster but right now it's not easy to add move semantics to WTF::Vector because we can't use std::move and std::forward in general until we know that we have good enough stl libraries everywhere. On the other hand we get parts of it for free with STL containers (if new enough). On the other other hand we might get inconsistent performance because of such differences between platforms and we might be better off on average to do something homebaked.

Then I wonder if *full* STL compatibility of collections is really necessary. These collections are for our use and I think we should focus on what we need. Just an example: WTF::HashTable::add() used to return an iterator, just as std::map::insert, and that cost both performance and footprint and callers couldn't care less. If performance is prio #1 then I think STL compatibility should be sacrificable *if* there is a conflict. In most cases there should be no conflict.

/Daniel

[Dana Jansens]

Rvalue references will take you the rest of the way, no? We already support Pass()ing scoped_refptrs by rvalue (ie move() without std::move() yet).

[Ojan Vafai]

Yes. If we find there is an actual perf regression by removing PassRefPtr, we can definitely solve it with rvalue references. I believe this is what they did in WebKit to get rid of PassRefPtr. 

[Yoav Weiss]

That's what is planned in WebKit to get rid of PassRefPtr, but it hasn't yet been done. It's being tracked here.

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

[Kinuko Yasuda]

Interesting. Do you also happen to have a pointer to discussion where getting rid of PassRefPtr is discussed? (The link seems to be for updating the documentation for RefPtr to reflect the discussion)

[Yoav Weiss]

Not a whole lots of discussion, but the thread on webkit-dev started here.

[Kinuko Yasuda]

Thanks!  (Looking)

[Daniel Cheng]

It looks like the WebKit proposal is that arguments that want to pass a reference should be declared as RefPtr<T>&&. I'm curious what people think about that compared to the current Chromium style (moveable types are always passed by value and the call site needs to use std::move() to get move semantics).

Daniel