Re: [chromium-dev] base/ in Blink - can we do this soon/yet?

[John Abd-El-Malek]

+blink-dev

+dglazkov& kentaro who've been thinking about this lately

On Thu, May 18, 2017 at 6:16 AM, Thiago Farina <tfa...@chromium.org> wrote:

On Thu, May 18, 2017 at 1:54 AM, Yuta Kitamura <yu...@chromium.org> wrote:

On Thu, May 18, 2017 at 6:37 AM, Christian Biesinger <cbies...@chromium.org> wrote:

There's WTF::MakeUnique and WTF::TimeTicks. Include
platform/wtf/PtrUtil.h and platform/wtf/Time.h.

I think they are wrappers around the base/ types. I don't know why we
can't use base/ directly yet.

To prevent undesirable types (std::string or std::vector) from creeping into Blink code base.

std::string and std::vector have performance and memory issues?

--

Thiago Farina

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAFenBsYtSMO2R3ZZuVL%3DxT%2Bf%2BXB2s-HM0pYEA-MCtfEEmoT7tQ%40mail.gmail.com.

[Daniel Cheng]

On Fri, May 19, 2017 at 12:10 AM John Abd-El-Malek <j...@chromium.org> wrote:

+blink-dev

+dglazkov& kentaro who've been thinking about this lately

On Thu, May 18, 2017 at 6:16 AM, Thiago Farina <tfa...@chromium.org> wrote:

On Thu, May 18, 2017 at 1:54 AM, Yuta Kitamura <yu...@chromium.org> wrote:

On Thu, May 18, 2017 at 6:37 AM, Christian Biesinger <cbies...@chromium.org> wrote:

There's WTF::MakeUnique and WTF::TimeTicks. Include
platform/wtf/PtrUtil.h and platform/wtf/Time.h.

I think they are wrappers around the base/ types. I don't know why we
can't use base/ directly yet.

To prevent undesirable types (std::string or std::vector) from creeping into Blink code base.

std::string and std::vector have performance and memory issues?

WTF::String and WTF::Vector have fairly different characteristics from std::string and std::vector. For example, WTF::String is designed to handle 8-bit and 16-bit strings, be cheap to copy, but is also thread unsafe. WTF::Vector has a 1.5x growth factor, so code that pushes elements in a loop should typically reserve capacity first, supports Oilpan types, etc.

FWIW, using base::TimeDelta has come up recently in another CL. That CL currently chooses to #include "base/time/time.h" in the public API header and then uses the fact that WTF::TimeDelta is simply a typedef for it. However, that's depending on an internal implementation detail of WTF::TimeDelta. For instances like this, I think we'll generally be better off if we convert them to be Mojo interfaces.

Daniel

 

--

Thiago Farina

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAFenBsYtSMO2R3ZZuVL%3DxT%2Bf%2BXB2s-HM0pYEA-MCtfEEmoT7tQ%40mail.gmail.com.

--

You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CALhVsw1RPwhd9_ZoN321utBxQ%2B-z907bYQ0R9FRxjfqdLYGAHQ%40mail.gmail.com.

[Nico Weber]

On Thu, May 18, 2017 at 10:34 AM, Daniel Cheng <dch...@chromium.org> wrote:

On Fri, May 19, 2017 at 12:10 AM John Abd-El-Malek <j...@chromium.org> wrote:

+blink-dev

+dglazkov& kentaro who've been thinking about this lately

On Thu, May 18, 2017 at 6:16 AM, Thiago Farina <tfa...@chromium.org> wrote:

On Thu, May 18, 2017 at 1:54 AM, Yuta Kitamura <yu...@chromium.org> wrote:

On Thu, May 18, 2017 at 6:37 AM, Christian Biesinger <cbies...@chromium.org> wrote:

There's WTF::MakeUnique and WTF::TimeTicks. Include
platform/wtf/PtrUtil.h and platform/wtf/Time.h.

I think they are wrappers around the base/ types. I don't know why we
can't use base/ directly yet.

To prevent undesirable types (std::string or std::vector) from creeping into Blink code base.

std::string and std::vector have performance and memory issues?

WTF::String and WTF::Vector have fairly different characteristics from std::string and std::vector. For example, WTF::String is designed to handle 8-bit and 16-bit strings, be cheap to copy, but is also thread unsafe. WTF::Vector has a 1.5x growth factor, so code that pushes elements in a loop should typically reserve capacity first, supports Oilpan types, etc.

FWIW, using base::TimeDelta has come up recently in another CL. That CL currently chooses to #include "base/time/time.h" in the public API header and then uses the fact that WTF::TimeDelta is simply a typedef for it. However, that's depending on an internal implementation detail of WTF::TimeDelta. For instances like this, I think we'll generally be better off if we convert them to be Mojo interfaces.

TimeDelta is a pretty simple class. If we're going to need Mojo for it, we're going to eventually generate a large chunk of the codebase from mojo interface files. That doesn't sound like a desirable end state to me.

 

Daniel

 

--

Thiago Farina

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAFenBsYtSMO2R3ZZuVL%3DxT%2Bf%2BXB2s-HM0pYEA-MCtfEEmoT7tQ%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CALhVsw1RPwhd9_ZoN321utBxQ%2B-z907bYQ0R9FRxjfqdLYGAHQ%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAF3XrKoQg9nc-vCN2dkmROQQxMDL%3DFQ%3DHV%2BaRA-_AxjHuE8KHg%40mail.gmail.com.

[محمد طارق‎]

i wanna leave this group how ?

> TimeDelta is a pretty simple class. If we're going to need Mojo for it, we're going to eventually generate a large chunk of the codebase from mojo interface files. That doesn't sound like a desirable end state to me. To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAF3XrKoQg9nc-vCN2dkmROQQxMDL%3DFQ%3DHV%2BaRA-_AxjHuE8KHg%40mail.gmail.com. To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAMGbLiGTMrqdNQ-SYdseS8%3DBgL2acSPzTTdFcB6E_OjMoTearA%40mail.gmail.com.

[Daniel Cheng]

On Fri, May 19, 2017, 00:36 Nico Weber <tha...@chromium.org> wrote:

On Thu, May 18, 2017 at 10:34 AM, Daniel Cheng <dch...@chromium.org> wrote:

On Fri, May 19, 2017 at 12:10 AM John Abd-El-Malek <j...@chromium.org> wrote:

+blink-dev

+dglazkov& kentaro who've been thinking about this lately

On Thu, May 18, 2017 at 6:16 AM, Thiago Farina <tfa...@chromium.org> wrote:

On Thu, May 18, 2017 at 1:54 AM, Yuta Kitamura <yu...@chromium.org> wrote:

On Thu, May 18, 2017 at 6:37 AM, Christian Biesinger <cbies...@chromium.org> wrote:

There's WTF::MakeUnique and WTF::TimeTicks. Include
platform/wtf/PtrUtil.h and platform/wtf/Time.h.

I think they are wrappers around the base/ types. I don't know why we
can't use base/ directly yet.

To prevent undesirable types (std::string or std::vector) from creeping into Blink code base.

std::string and std::vector have performance and memory issues?

WTF::String and WTF::Vector have fairly different characteristics from std::string and std::vector. For example, WTF::String is designed to handle 8-bit and 16-bit strings, be cheap to copy, but is also thread unsafe. WTF::Vector has a 1.5x growth factor, so code that pushes elements in a loop should typically reserve capacity first, supports Oilpan types, etc.

FWIW, using base::TimeDelta has come up recently in another CL. That CL currently chooses to #include "base/time/time.h" in the public API header and then uses the fact that WTF::TimeDelta is simply a typedef for it. However, that's depending on an internal implementation detail of WTF::TimeDelta. For instances like this, I think we'll generally be better off if we convert them to be Mojo interfaces.

TimeDelta is a pretty simple class. If we're going to need Mojo for it, we're going to eventually generate a large chunk of the codebase from mojo interface files. That doesn't sound like a desirable end state to me.

As the code in question is going over IPC already, using Mojo isn't really an additional cost. I just think code shouldn't depend on the inplementation details of a type like that CL currently does.

There is the additional question of why we have WTF::TimeDelta: I suppose it's for consistency with WTF::Time and WTF::Time ticks which layer a bit of functionality on top. But maybe that functionality should be folded directly into base.

Daniel

[PhistucK]

If you meant to unsubscribe -

Try sending a blank e-mail to blink-dev+unsubscribe@chromium.org (note that +unsubscribe in the address, it is important). If you already did exactly that, skip this.

If that does not work and you are still getting e-mails -

1. Go to https://groups.google.com/a/chromium.org/forum/m/#!forum/blink-dev.

2. Click on the button that has an icon of a head and shoulders.

3. Click on "Leave group" and you are done. Or -

 a. Select in the selection box - "Don't send email updates".

 b. Click on "Save".

☆PhistucK

On Thu, May 18, 2017 at 5:41 PM, محمد طارق <mido...@gmail.com> wrote:

i wanna leave this group how ?

> TimeDelta is a pretty simple class. If we're going to need Mojo for it, we're going to eventually generate a large chunk of the codebase from mojo interface files. That doesn't sound like a desirable end state to me. To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAF3XrKoQg9nc-vCN2dkmROQQxMDL%3DFQ%3DHV%2BaRA-_AxjHuE8KHg%40mail.gmail.com. To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAMGbLiGTMrqdNQ-SYdseS8%3DBgL2acSPzTTdFcB6E_OjMoTearA%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CA%2BvnRtbgyL0cX%3DL6SMSRVBweOZoGh4FKf7Umw7-7wu73CYLmTg%40mail.gmail.com.

[Kentaro Hara]

Short term:

Use WTF::MakeUnique and WTF::TimeTicks. Currently core/ and modules/ are not allowed to use //base types directly.

Mid term:

Onion Soup 2.0 is planning to change the rule because the rule would become problematic as we move more code from //content/ to Blink. I'm thinking about allowing //base types in core/ and modules/ with some whitelisting (checked by the presubmit check). (std::vector, std::string etc won't be allowed for the reason Daniel mentioned.)

--

Kentaro Hara, Tokyo, Japan

[Christian Biesinger]

But... std::string and std::vector are not base/, they're STL. How is
that related one way or another to allowing base/?

Christian

[Nico Weber]

base's APIs use STL containers, blink's APIs don't. So you can't use many base/ APIs from blink without converting back and forth all the time.

[Charles Harrison]

We should also be mindful not to allow //base APIs that lead to creep of //base classes that have WTF counterparts like StringPiece/StringView, in addition to ones that use STL containers.

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

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAMGbLiFqkiChZnc%3DBj1NeGfadau7YjLGhkf%2BHbFBC01R_QhpAg%40mail.gmail.com.

[Kentaro Hara]

Accurately speaking, the plan is to let core/ and modules/ to use basic types like //base, //cc, std:: etc directly without going through the thin wrappers in wtf/ and platform/. The thin wrappers will be gone.

[Kentaro Hara]

We should also be mindful not to allow //base APIs that lead to creep of //base classes that have WTF counterparts like StringPiece/StringView, in addition to ones that use STL containers.

Yes, we're planning to improve the presubmit script and whiltelist the allowed things.