abseil in chromium

[Ryan Hamilton]

Howdy Folks,

Now that abseil has launched, it would be great to be able to use this in Chromium. For QUIC and HTTP/2, in particular this would help facilitate code sharing.

Is this on the radar? How can I help? :)

Cheers,

Ryan

[Brett Wilson]

I didn't realize it was out. I went to take a look and was disappointed that it included almost nothing interesting. I assume more stuff will be added over time.

One might expect there to be various types of internal classes added over time like StringPiece and URL classes. Chrome has our own version of these and getting new variants will be confusing for everybody. Without really understanding the scope of what will be there, I'm reluctant to say up front we should allow its use.

In some ideal world we would switch to the ABSL versions. But this is a tremendous amount of work, and in some cases there are different goals or historical reasons for certain behaviors (think URL classes).

There is some stuff I'm interested in that I suspect will be added over time. It's possible we could take a case-by-case approach like Blink with base. I'm not technically sure how we would implement this. Like whether we would need to fork some files or if we could bring it in and efficiently use the parts we want.

Brett

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To post to this group, send email to c...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAJ_4DfSWF9%2BhsUQrf8nZbJebJF%2Bo-cZTNYfR4wUO7%3DWqYkxRxg%40mail.gmail.com.

[Peter Kasting]

On Tue, Sep 26, 2017 at 10:25 AM, Brett Wilson <bre...@chromium.org> wrote:

One might expect there to be various types of internal classes added over time like StringPiece and URL classes.

StringPiece is there; it's called string_view to match the C++17 (IIRC) std:: spelling.

PK

[Chris Palmer]

A Piece, or _view, of sadness:

https://twitter.com/kayseesee/status/912712743857221632

--

You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To post to this group, send email to c...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAAHOzFDfaYA6o6g-%2B8htgP%3DT-ooUuuqUSYX8%2B2j0%3Dp9rgY8hFA%40mail.gmail.com.

[dan...@chromium.org]

On Tue, Sep 26, 2017 at 1:25 PM, Brett Wilson <bre...@chromium.org> wrote:

I didn't realize it was out. I went to take a look and was disappointed that it included almost nothing interesting. I assume more stuff will be added over time.

One might expect there to be various types of internal classes added over time like StringPiece and URL classes. Chrome has our own version of these and getting new variants will be confusing for everybody. Without really understanding the scope of what will be there, I'm reluctant to say up front we should allow its use.

In some ideal world we would switch to the ABSL versions. But this is a tremendous amount of work, and in some cases there are different goals or historical reasons for certain behaviors (think URL classes).

There is some stuff I'm interested in that I suspect will be added over time. It's possible we could take a case-by-case approach like Blink with base. I'm not technically sure how we would implement this. Like whether we would need to fork some files or if we could bring it in and efficiently use the parts we want.

I was thinking maybe we could add it to third_party but ban including any headers from it with PRESUBMIT, and then build a whitelist for each thing similar to C++11 library things.

 

Brett

On Tue, Sep 26, 2017 at 10:12 AM, 'Ryan Hamilton' via cxx <c...@chromium.org> wrote:

Howdy Folks,

Now that abseil has launched, it would be great to be able to use this in Chromium. For QUIC and HTTP/2, in particular this would help facilitate code sharing.

Is this on the radar? How can I help? :)

Cheers,

Ryan

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To post to this group, send email to c...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAJ_4DfSWF9%2BhsUQrf8nZbJebJF%2Bo-cZTNYfR4wUO7%3DWqYkxRxg%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To post to this group, send email to c...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CABiGVV-D8uNHvCt1FCMH-ekc7pWh2RJK%2BrTdiPZz%2BX2RxYWuCA%40mail.gmail.com.

[Brett Wilson]

On Tue, Sep 26, 2017 at 10:28 AM, 'Peter Kasting' via cxx <c...@chromium.org> wrote:

Oh I see it.

StringPiece is what I would consider a "bad" example. Although we forked ours from the same original version as the ABSL one, we rewrote ours to be a templatized BasicStringPiece so we can have StringPiece16. ABSL doesn't really have 16-bit strings like we do, so the two StringPieces will remain in that bad range of "similar but different".

Brett

[Brett Wilson]

On Tue, Sep 26, 2017 at 10:31 AM, <dan...@chromium.org> wrote:

On Tue, Sep 26, 2017 at 1:25 PM, Brett Wilson <bre...@chromium.org> wrote:

I didn't realize it was out. I went to take a look and was disappointed that it included almost nothing interesting. I assume more stuff will be added over time.

One might expect there to be various types of internal classes added over time like StringPiece and URL classes. Chrome has our own version of these and getting new variants will be confusing for everybody. Without really understanding the scope of what will be there, I'm reluctant to say up front we should allow its use.

In some ideal world we would switch to the ABSL versions. But this is a tremendous amount of work, and in some cases there are different goals or historical reasons for certain behaviors (think URL classes).

There is some stuff I'm interested in that I suspect will be added over time. It's possible we could take a case-by-case approach like Blink with base. I'm not technically sure how we would implement this. Like whether we would need to fork some files or if we could bring it in and efficiently use the parts we want.

I was thinking maybe we could add it to third_party but ban including any headers from it with PRESUBMIT, and then build a whitelist for each thing similar to C++11 library things.

Yeah, hopefully that can work.

I notice they have a InlinedVector implementation which looks good at first glance. Base's StackVector should really be deleted.

Brett

[David Benjamin]

Adding it would be useful for BoringSSL as well. At some point we plan to move the new PKI stack in //net/cert to BoringSSL so it can be used outside of Chromium, and that would mean severing the link to Chromium //base. This would be useful to Chromium in a vacuum too as WebRTC can't depend on //net. WebRTC is the last link in Chromium to the legacy OpenSSL X.509 code we've been lugging around. We've dropped it in Cronet already with wins in binary size, memory, and performance.

BoringSSL is also starting to convert libssl to C++. abseil would be useful there too. (Though that has a whole larger set of constraints on our end we'd need to clear out.)

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAHtyhaTv%2Bc80ti4RCSBssu9LLk_zstCA4fmHaDtsmt21ku%3DBNg%40mail.gmail.com.

[Brett Wilson]

absl::StrJoin looks really nice too.

But it's only 8-bit. Maybe ABSL is the inspiration we need to finally move away from 16-bit strings :)

Brett

[Jeremy Roman]

I don't feel strongly.

There's a number of things (like absl::Mutex) that I wonder whether they're as tested/tuned on platforms like Windows as the Chromium one. They may well be better, but the likelihood of performance differences in things like that makes a drop-in replacement for all of base/ challenging.

Most of what's in abseil today seems to be stuff that we have a similar implementation of in base/ already. We could possibly adopt (or re-export) the absl:: version of a number of them and shrink the size of base, at the cost of losing direct control over its evolution. I guess in this scenario we would treat Abseil as more or less an "extended standard library", with similar treatment (some things used directly, a few things re-implemented in base/ for a variety of reasons, some things banned).

Importing it isn't zero-cost. At the very least, we'd be responsible for maintaining an updated BUILD.gn that corresponds to the upstream Bazel rules.

--

You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To post to this group, send email to c...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CABiGVV_%3Ddvwr95580OqAYYMBKi9owWs4Su%2BR3%2B7vd1i37n7vHg%40mail.gmail.com.

[Karl Wiberg]

We're considering starting to use this in WebRTC too. Having it available in Chromium would likely be helpful.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CACuR13e56X9PbCZFackO-R3buoef--U4ODnMHg9V5KMkD1KMnw%40mail.gmail.com.

--

Karl Wiberg |

Software Engineer |

kwi...@google.com |

+46 70 696 1024

[jdoe...@chromium.org]

StrCat and StrAppend look nice as well. Maybe it's just me, but I find string concatenation to be a bit cumbersome at the moment. For quick one off operations there is std::to_string, but it is currently still in the "To Be Discussed" section of our C++11 styleguide and comes with it's own performance drawbacks: https://groups.google.com/a/chromium.org/d/msg/cxx/H2uAxR86N_g/YxabMC6nCgAJ For more sophisticated operations we have base::StringPrintf, but given its C based API it is a bit awkward to use when trying to concatenate std::strings. For more information Googlers can see C++ Tip of the Week #3. Non-Googlers hopefully will find it here soon: https://abseil.io/tips/ 

On the matter of string_view and StringPiece16, C++17 comes with a std::basic_string_view, at which point we could replace base::StringPiece with std::string_view and base::StringPiece16 with either std::u16string_view or a custom implementation of std::basic_string_view. I suggest we wait for C++17 instead of trying to adopt absl::string_view in the mean time. While we are on the topic of 16-bit strings: Are there significant differences between the character traits of base::char16 and std::char16_t that would prevent us today from simply typedeffing base::string16 to std::u16string? 

[Daniel Cheng]

On Wed, Sep 27, 2017, 17:24 <jdoe...@chromium.org> wrote:

StrCat and StrAppend look nice as well. Maybe it's just me, but I find string concatenation to be a bit cumbersome at the moment. For quick one off operations there is std::to_string, but it is currently still in the "To Be Discussed" section of our C++11 styleguide and comes with it's own performance drawbacks: https://groups.google.com/a/chromium.org/d/msg/cxx/H2uAxR86N_g/YxabMC6nCgAJ For more sophisticated operations we have base::StringPrintf, but given its C based API it is a bit awkward to use when trying to concatenate std::strings. For more information Googlers can see C++ Tip of the Week #3. Non-Googlers hopefully will find it here soon: https://abseil.io/tips/ 

+1 to StrCat and StrAppend. I keep wanting to add these but haven't had time. However, as brettw pointed out earlier, we'd likely want 8 and 16-bit versions. Unless abseil is willing to accommodate this, it's probably easier to fork a copy =(

On the matter of string_view and StringPiece16, C++17 comes with a std::basic_string_view, at which point we could replace base::StringPiece with std::string_view and base::StringPiece16 with either std::u16string_view or a custom implementation of std::basic_string_view. I suggest we wait for C++17 instead of trying to adopt absl::string_view in the mean time. While we are on the topic of 16-bit strings: Are there significant differences between the character traits of base::char16 and std::char16_t that would prevent us today from simply typedeffing base::string16 to std::u16string? 

We do need to do some work to make base::StringPiece's interface match base::string_view.

As for base::string16, there was a previous thread about this. Unfortunately, it's not as easy as adding a type alias, since base::string16 is currently aliased to sts::wstring on Windows--and wchar_t and char16_t are not convertible. https://groups.google.com/a/chromium.org/forum/m/#!topic/cxx/MrXsU4exAYw has some ideas on how to move forward here, but I don't think anyone has had time to pick it up.

Daniel

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/d34b1923-e147-462b-b84f-cc9cf6253377%40chromium.org.

[Dmitry Skiba]

BTW, protobuf is considering using abseil in the future (for string_view and cord): https://groups.google.com/a/google.com/forum/#!topic/protobuf-discuss/eItJoGwj-pg

I expect others will also use it at some point. I.e. we'll need to have some abseil story in Chrome anyway. 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAF3XrKpVnfawiVT-Ojuz3B2qFThbMn9-QCvKZ-5zBb%3Dtuxa4_Q%40mail.gmail.com.

[Mark Mentovai]

Don’t all of our compilers (or standard libraries) support string_view now anyway?

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CABWS%2BBwOBjXPYoC9tAcstNSrRyBdzsvw8dOGyzWe%2BybWV7efNg%40mail.gmail.com.

[Jeremy Roman]

On Tue, Oct 3, 2017 at 12:31 PM, Mark Mentovai <ma...@chromium.org> wrote:

Don’t all of our compilers (or standard libraries) support string_view now anyway?

I doubt it. std::string_view was introduced in C++17, and we're still missing significant pieces of C++14 support in some of our standard library implementations.

[David Benjamin]

Have we asked the Abseil folks about adding absl::basic_string_view? (Or are we expecting to get C++17 support soon enough that we'd have it by then anyway?)

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CACuR13cbRYBQ09R1M%2Bh3BcdNwxG4ufYVv1Z9iJiNJPvLHJ-M%3Dg%40mail.gmail.com.

[Daniel Cheng]

There's a bug for evaluating C++17 readiness, though I don't know that there's any concrete timeline: https://crbug.com/752720

However, before we could consider switching over to another StringPiece implementation, we'd have to make our StringPiece behavior match the behavior of std::basic_string_view: for example, out-of-bounds behavior differs for the position argument of substr(). It's probably worth doing that independently of whatever string_view implementation we end up deciding to migrate to.

Daniel

[mbon...@webrtc.org]

Hi, as kwiberg@ mentioned in comment #11, WebRTC would like to start using Abseil. Do you think it is possible to add a //third_party/absl folder with a copy of Abseil or do you see any problem in doing this?

[Ryan Hamilton]

This would be a big help to QUIC and SPDY as well!

[Jeremy Roman]

I don't have an objection on principle, but I'd like to understand better. (net in particular generally shares Chromium library usage IIUC, so I'm afraid of Abseil leaking out of there whether we want it to or not).

1. What do WebRTC and net want out of Abseil?

2. Would Abseil completely replace their use of //base, or would they you still need a dependency on base? (If the latter, how does Abseil help?)

At present the public version seems to include (if I subtract the stuff that's already in the C++14 STL):

- container-based algorithms (absl::c_sort etc.)

- thread annotations and miscellaneous other things in absl/base/

- InlineVector and FixedArray containers

- absl::WrapUnique, allocator traits

- void_t and a handful of TMP utilities

- 128-bit ints

- absl::string_view (but not basic_string_view, so limited to 8-bit strings -- which probably makes it unsuitable for Chromium at this point)

- StrCat/StrJoin/StrSplit and other string utilities

- synchronization primitives

- time/duration abstraction

- std::any, std::optional and std::span impls

- std::apply impl

A lot of these would be awkward to use if you still want to interact with Chromium code (e.g. string_view not supporting string16, significantly different sync and time primitives).

[David Benjamin]

SPDY, QUIC (not all of //net, but the core bits of those two), and WebRTC all are used in contexts outside of Chromium and can't depend on //base. They already must carefully handle the interactions across Chromium code. Take a look at net/quic/platform.

BoringSSL is in a similar boat. We even have our own random std::span implementation right now. The certificate verifier library in //net/cert will ultimately get moved into BoringSSL (this is necessary even for Chromium because WebRTC can't depend on //net), at which point all its //base dependencies will need to be severed and replaced with Abseil.

You're right that API boundaries are still a pain, though much fewer of those types would actually be visible across API boundaries. Ultimately, I think we want Chromium to migrate to absl/STL versions of things like string_view, span, etc., so that those may be used directly at API boundaries. Yes, for string_view that requires changes in both Abseil and Chromium, but we should do that.

Note that even those most of those are STL polyfills, waiting for the STL to support them *still* does not remove the need for Abseil. Chromium will be able to depend on std::string_view before some of these other projects can depend on it, because we have other consumers. The absl versions decay to typedefs, so this still works fine. Chromium can refer to std::string_view while other libraries refer to the absl::string_view typedef and everything still interops.

It would be nice for absl and //base to, over time, converge in places, but there's no requirement for it to happen all at once or even everywhere. We can do it incrementally, and only where sensible and valuable.

[Karl Wiberg]

Well said.

WebRTC can't use //base at all—we can't depend on anything in Chromium—so the situation we've ended up with is that we've manually copied various odds and ends from //base into our tree, and in general not kept them up to date, because importing them is a lot of work, because //base isn't a proper library. Abseil on the other hand is a proper library, which we can keep up to date without much effort.

We're primarily looking to use Abseil internally in WebRTC, but some Abseil stuff will end up in our API—e.g. absl::optional, absl::span, that sort of thing—but this shouldn't be much of a problem for Chromium. Chromium code used to have to translate from base::scoped_ptr to rtc::scoped_ptr before we all switched to std::unique_ptr, and Chromium code still has to use rtc::scoped_refptr (instead of base::scoped_refptr) when talking to us. Adding Abseil types to the mix will not really cause any new problems, and may actually get rid of problems by enabling Chromium and WebRTC to converge on their type use. For example, right now Chromium maintains its own base::optional, and WebRTC has its rtc::Optional, but we could both use absl::optional—less maintenance and better interop.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAF8qwaACuu8biVOj94Os_EF5woRj0Mnjni2Vs_SO8-BT8qUU%2BQ%40mail.gmail.com.

[Jeremy Roman]

This sounds reasonable to me (subject to the process in docs/adding_to_third_party.md, a README.md clarifying why it's there, and DEPS and maybe PRESUBMIT checks to constrain other usage in the meantime). I fear that it's not going to entirely solve your problem (as there's a bunch of stuff like that that Abseil doesn't currently cover, such as, as you mention, scoped_refptr).

For general Chromium use, I think we'll probably want to be more concerted about how we make things available (and that probably also ties in to the ultimate timeline for C++17 in Chromium, since large rewrites twice would be painful). base/OWNERS can weigh in, but for simplicity it might be preferable to start by just aliasing things into base, if these limited uses (boringssl, rtc, etc) work out well.

On Thu, Feb 22, 2018 at 2:25 PM, Karl Wiberg <kwi...@webrtc.org> wrote:

Well said.

WebRTC can't use //base at all—we can't depend on anything in Chromium—so the situation we've ended up with is that we've manually copied various odds and ends from //base into our tree, and in general not kept them up to date, because importing them is a lot of work, because //base isn't a proper library. Abseil on the other hand is a proper library, which we can keep up to date without much effort.

We're primarily looking to use Abseil internally in WebRTC, but some Abseil stuff will end up in our API—e.g. absl::optional, absl::span, that sort of thing—but this shouldn't be much of a problem for Chromium. Chromium code used to have to translate from base::scoped_ptr to rtc::scoped_ptr before we all switched to std::unique_ptr, and Chromium code still has to use rtc::scoped_refptr (instead of base::scoped_refptr) when talking to us. Adding Abseil types to the mix will not really cause any new problems, and may actually get rid of problems by enabling Chromium and WebRTC to converge on their type use. For example, right now Chromium maintains its own base::optional, and WebRTC has its rtc::Optional, but we could both use absl::optional—less maintenance and better interop.

Yup, definitely looking forward to that (and I imagine base/OWNERS would like to stop reviewing base::Optional changes :P). What I hope to avoid is a weird intermediate state where we have a random mixture.

[Nico Weber]

(I agree with everything jbroman says here.)

On Thu, Feb 22, 2018 at 1:52 PM, David Benjamin <davi...@chromium.org> wrote:

SPDY, QUIC (not all of //net, but the core bits of those two), and WebRTC all are used in contexts outside of Chromium and can't depend on //base. They already must carefully handle the interactions across Chromium code. Take a look at net/quic/platform.

BoringSSL is in a similar boat. We even have our own random std::span implementation right now. The certificate verifier library in //net/cert will ultimately get moved into BoringSSL (this is necessary even for Chromium because WebRTC can't depend on //net), at which point all its //base dependencies will need to be severed and replaced with Abseil.

You're right that API boundaries are still a pain, though much fewer of those types would actually be visible across API boundaries. Ultimately, I think we want Chromium to migrate to absl/STL versions of things like string_view, span, etc.,

Why wouldn't we use the C++ stdlib types instead?

 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAF8qwaACuu8biVOj94Os_EF5woRj0Mnjni2Vs_SO8-BT8qUU%2BQ%40mail.gmail.com.

[David Benjamin]

On Thu, Feb 22, 2018 at 2:38 PM Nico Weber <tha...@chromium.org> wrote:

(I agree with everything jbroman says here.)

On Thu, Feb 22, 2018 at 1:52 PM, David Benjamin <davi...@chromium.org> wrote:

SPDY, QUIC (not all of //net, but the core bits of those two), and WebRTC all are used in contexts outside of Chromium and can't depend on //base. They already must carefully handle the interactions across Chromium code. Take a look at net/quic/platform.

BoringSSL is in a similar boat. We even have our own random std::span implementation right now. The certificate verifier library in //net/cert will ultimately get moved into BoringSSL (this is necessary even for Chromium because WebRTC can't depend on //net), at which point all its //base dependencies will need to be severed and replaced with Abseil.

You're right that API boundaries are still a pain, though much fewer of those types would actually be visible across API boundaries. Ultimately, I think we want Chromium to migrate to absl/STL versions of things like string_view, span, etc.,

Why wouldn't we use the C++ stdlib types instead?

Right, that's why I said "absl/STL". :-) If the STL version is available and Chromium gets to require it, of course Chromium should use that instead. Though I imagine std::span won't exist for a while, and unifying base::span, absl::Span, and bssl::Span would be nice. Perhaps the QUIC folks could then use that without adding a QuicSpan and cut down on some strings-as-byte-arrays business. :-)

Though note again that, when the STL type exists, the absl type decays to a typedef, so we can switch to the absl type and then later to the STL type, or Chromium can use the STL type while WebRTC uses the absl version, or whatever, depending on each bit of code's minimum.

(Obviously, everyone should strive to update their toolchains as much as possible, and I really wish the rest of even the Google ecosystem were as good as Chromium here, but nodes sufficiently deep in the dependency graph have a hard time. It was a struggle to even get C++11 into BoringSSL. Things chug along at different paces.)

 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAMGbLiGk_Z1C3G0VM%2BiGrAW2egyt2w%3D%2BDi-ODU7ghe94rMwSJg%40mail.gmail.com.

[Karl Wiberg]

Yes—in general, Chromium will be ahead of WebRTC in terms of what C++ standard can be assumed to be available, because being a library, WebRTC can move no faster than its slowest user. But if WebRTC uses absl::optional (for example), then Chromium can interop with us either by also using absl::optional, or by using std::optional and compiling us in C++17 mode (where absl::optional is just a type alias for std::optional).

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAF8qwaAEoFOd5oMJEPZEHb%3DvFdML4bT_7A%3DF-VDpYN614Y-aVQ%40mail.gmail.com.

[dan...@chromium.org]

On Thu, Feb 22, 2018 at 2:38 PM, Nico Weber <tha...@chromium.org> wrote:

(I agree with everything jbroman says here.)

+1 from me as well.

It sounds like implementation wise

- DEPS to absl as a whole would come with a -base pairing so they don't use both.

- Up in src/DEPS we would -absl and then whitelist headers if/as transitioning things out of base.

[Daniel Cheng]

Even if we did DEPS checks, I think it'd be easy for something like absl::optional to leak out and be used in code not dealing with //net or WebRTC due to things leaking from headers. Is it worth explicitly documenting anywhere (in README.chromium or in something similar to chromium-cpp.appspot.com) what the expectations around using constructs in absl are?

Daniel

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAHtyhaTp4M0ovbbt4wqy1MuzeMe4-fxsLmNRpv187%3DwhKmai2Q%40mail.gmail.com.

[Jeremy Roman]

On Thu, Feb 22, 2018 at 3:58 PM, Daniel Cheng <dch...@chromium.org> wrote:

Even if we did DEPS checks, I think it'd be easy for something like absl::optional to leak out and be used in code not dealing with //net or WebRTC due to things leaking from headers. Is it worth explicitly documenting anywhere (in README.chromium or in something similar to chromium-cpp.appspot.com) what the expectations around using constructs in absl are?

I imagine it being mentioned in README.chromium, and at least to start with a presubmit check for \babsl::|\busing namespace\ absl\b would probably suffice to block other leakage.

[John Abd-El-Malek]

On Thu, Feb 22, 2018 at 12:43 PM, <dan...@chromium.org> wrote:

On Thu, Feb 22, 2018 at 2:38 PM, Nico Weber <tha...@chromium.org> wrote:

(I agree with everything jbroman says here.)

+1 from me as well.

It sounds like implementation wise

- DEPS to absl as a whole would come with a -base pairing so they don't use both.

- Up in src/DEPS we would -absl and then whitelist headers if/as transitioning things out of base.

How would we prevent random directories from adding +absl?

 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAHtyhaTp4M0ovbbt4wqy1MuzeMe4-fxsLmNRpv187%3DwhKmai2Q%40mail.gmail.com.

[dani...@webrtc.org]

On Monday, February 26, 2018 at 8:00:04 AM UTC+1, John Abd-El-Malek wrote:

On Thu, Feb 22, 2018 at 12:43 PM, <dan...@chromium.org> wrote:

On Thu, Feb 22, 2018 at 2:38 PM, Nico Weber <tha...@chromium.org> wrote:

(I agree with everything jbroman says here.)

+1 from me as well.

It sounds like implementation wise

- DEPS to absl as a whole would come with a -base pairing so they don't use both.

- Up in src/DEPS we would -absl and then whitelist headers if/as transitioning things out of base.

How would we prevent random directories from adding +absl?

Can .gn  visibility help with that?

absl targets will be visible to webrtc/quic/spdy but not to rest of the chromium.

 

[mbon...@webrtc.org]

Yes, I think GN visibility can help Chromium avoid to start depending on absl as a whole. It will be too strict if Chromium wants to start using something, in that case some absl build targets will need to be visible. This can result in a complex process. Maybe a PRESUBMIT to check Chromium's deps on absl will be enough and easy to maintain (with a list of allowed dependencies).

But if Chromium doesn't want to use absl in the next future, I guess GN visibility is something we should use.

[Karl Wiberg]

Yes, that could definitely work. We already do something very similar with visibility in WebRTC's GN targets, to prevent outsiders from using our non-API build targets.

--

You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To post to this group, send email to c...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/72c45469-68e8-4be5-935c-72fdea3df80e%40chromium.org.

[John Abd-El-Malek]

Sounds like a good solution for now, thanks.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAPXR0RQeRZrJgW0dKCRt1NHaHpY_LJeRD%2B2VN0pW3XpGgSWKEw%40mail.gmail.com.

[Jan Wilken Dörrie]

Bumping this up, since Abseil recently released their SwissTable implementation to GitHub [Blog, Code]. Given that we currently try to avoid std::unordered_ containers, this might be interesting for us. However, we definitely should do a careful performance and memory / binary size analysis, similarly to what brettw@ already did.

Additionally, there are other library features that might be interesting to us:

• absl::optional: This is already used in WebRTC, and adopting it in base would allow us to drop our own base::Optional implementation and make interfacing between Chromium and WebRTC easier. Furthermore, this will make adoption of std::optional easier once we have C++17.

• absl::variant: Modeled after std::variant and quite powerful. Adopting this would simplify the implementation of base::Value quite a bit, and it was also featured in the Promises for Chromium prototype. Due to the heavy use of templates there are some code-bloat concerns with variant, which is why we probably want to restrict usages of this to //base for now.

• Other features listed by jbroman@ above, which would simplify some of the implementations found in //base/stl_util.h and //base/template_util.h.

I suggest we allow some of the abseil features in //base, exporting them in the base:: namespace for client code to use. Also we would discuss on a case-by-case basis what exactly we will expose, similarly to how we discuss adoption of new STL features. WDYT?

Cheers,

Jan

--

Jan Wilken Dörrie

Software Engineer

jdoe...@google.com
 +49 89 839300973

Google Germany GmbH

Erika-Mann-Straße 33

80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado

Registergericht und -nummer: Hamburg, HRB 86891

Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich. Falls sie diese fälschlicherweise erhalten haben sollten, leiten Sie diese bitte nicht an jemand anderes weiter, löschen Sie alle Kopien und Anhänge davon und lassen Sie mich bitte wissen, dass die E-Mail an die falsche Person gesendet wurde.

    

This e-mail is confidential. If you received this communication by mistake, please don't forward it to anyone else, please erase all copies and attachments, and please let me know that it has gone to the wrong person.

[Alex Clarke]

On Thu, 4 Oct 2018 at 11:00, Jan Wilken Dörrie <jdoe...@chromium.org> wrote:

Bumping this up, since Abseil recently released their SwissTable implementation to GitHub [Blog, Code]. Given that we currently try to avoid std::unordered_ containers, this might be interesting for us. However, we definitely should do a careful performance and memory / binary size analysis, similarly to what brettw@ already did.

Additionally, there are other library features that might be interesting to us:

• absl::optional: This is already used in WebRTC, and adopting it in base would allow us to drop our own base::Optional implementation and make interfacing between Chromium and WebRTC easier. Furthermore, this will make adoption of std::optional easier once we have C++17.

• absl::variant: Modeled after std::variant and quite powerful. Adopting this would simplify the implementation of base::Value quite a bit, and it was also featured in the Promises for Chromium prototype. Due to the heavy use of templates there are some code-bloat concerns with variant, which is why we probably want to restrict usages of this to //base for now.

Perhaps we should ban std::visit instead:  https://www.reddit.com/r/cpp/comments/9khij8/stdvariant_code_bloat_looks_like_its_stdvisit/

 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CALB5StadEaDapOus7muSpjXZty7j%2Bda1r4E2FE_NxNQ2vvCe8w%40mail.gmail.com.

[Jeremy Roman]

On Thu, Oct 4, 2018 at 6:00 AM Jan Wilken Dörrie <jdoe...@chromium.org> wrote:

Bumping this up, since Abseil recently released their SwissTable implementation to GitHub [Blog, Code]. Given that we currently try to avoid std::unordered_ containers, this might be interesting for us. However, we definitely should do a careful performance and memory / binary size analysis, similarly to what brettw@ already did.

Indeed. In particular, I thought SwissTable was optimized primarily for x86_64 (for datacentre use), but we have ARM/ARM64 platforms as well.

Additionally, there are other library features that might be interesting to us:

• absl::optional: This is already used in WebRTC, and adopting it in base would allow us to drop our own base::Optional implementation and make interfacing between Chromium and WebRTC easier. Furthermore, this will make adoption of std::optional easier once we have C++17.

This strikes me as a good place to start if we want to rely on Abseil more, since base::Optional already aims to be compatible with std::optional, much like absl::optional.

[Nico Weber]

The listed reasons for using absl in base sound fairly small to me at this point, so I'd probably suggest waiting until there's more upside.

[dan...@chromium.org]

On Thu, Oct 4, 2018 at 1:45 PM Nico Weber <tha...@chromium.org> wrote:

The listed reasons for using absl in base sound fairly small to me at this point, so I'd probably suggest waiting until there's more upside.

I'd personally like to see us alias base::Optional to absl's so we can stop maintaining our copy. (I don't have a well formed opinion on keeping it aliased vs using absl:: directly thereafter yet.)

 

On Thu, Oct 4, 2018 at 10:55 AM Jeremy Roman <jbr...@chromium.org> wrote:

On Thu, Oct 4, 2018 at 6:00 AM Jan Wilken Dörrie <jdoe...@chromium.org> wrote:

Bumping this up, since Abseil recently released their SwissTable implementation to GitHub [Blog, Code]. Given that we currently try to avoid std::unordered_ containers, this might be interesting for us. However, we definitely should do a careful performance and memory / binary size analysis, similarly to what brettw@ already did.

Indeed. In particular, I thought SwissTable was optimized primarily for x86_64 (for datacentre use), but we have ARM/ARM64 platforms as well.

I look forward to learning more about the performance of SwissTable!

 

Additionally, there are other library features that might be interesting to us:

• absl::optional: This is already used in WebRTC, and adopting it in base would allow us to drop our own base::Optional implementation and make interfacing between Chromium and WebRTC easier. Furthermore, this will make adoption of std::optional easier once we have C++17.

This strikes me as a good place to start if we want to rely on Abseil more, since base::Optional already aims to be compatible with std::optional, much like absl::optional.

• absl::variant: Modeled after std::variant and quite powerful. Adopting this would simplify the implementation of base::Value quite a bit, and it was also featured in the Promises for Chromium prototype. Due to the heavy use of templates there are some code-bloat concerns with variant, which is why we probably want to restrict usages of this to //base for now.

If we can simplify base::Value /and/ reduce binary size then that seems like a good motivation to me. If we can't get both then maybe we should steer clear of this particular widget for now.

[Brett Wilson]

On Thu, Oct 4, 2018 at 10:56 AM <dan...@chromium.org> wrote:

On Thu, Oct 4, 2018 at 1:45 PM Nico Weber <tha...@chromium.org> wrote:

The listed reasons for using absl in base sound fairly small to me at this point, so I'd probably suggest waiting until there's more upside.

I'd personally like to see us alias base::Optional to absl's so we can stop maintaining our copy. (I don't have a well formed opinion on keeping it aliased vs using absl:: directly thereafter yet.)

My team just started using C++17. I wonder how far away it is for Chromium? My impression is it's blocked on Nacl or something, but once that's unblocked it might be relatively faster to move to 14 and 17 than it has been to update C++ versions. If true, it might be better to wait.

 

 On Thu, Oct 4, 2018 at 10:55 AM Jeremy Roman <jbr...@chromium.org> wrote:

On Thu, Oct 4, 2018 at 6:00 AM Jan Wilken Dörrie <jdoe...@chromium.org> wrote:

Bumping this up, since Abseil recently released their SwissTable implementation to GitHub [Blog, Code]. Given that we currently try to avoid std::unordered_ containers, this might be interesting for us. However, we definitely should do a careful performance and memory / binary size analysis, similarly to what brettw@ already did.

Indeed. In particular, I thought SwissTable was optimized primarily for x86_64 (for datacentre use), but we have ARM/ARM64 platforms as well.

I look forward to learning more about the performance of SwissTable!

 

Additionally, there are other library features that might be interesting to us:

• absl::optional: This is already used in WebRTC, and adopting it in base would allow us to drop our own base::Optional implementation and make interfacing between Chromium and WebRTC easier. Furthermore, this will make adoption of std::optional easier once we have C++17.

This strikes me as a good place to start if we want to rely on Abseil more, since base::Optional already aims to be compatible with std::optional, much like absl::optional.

• absl::variant: Modeled after std::variant and quite powerful. Adopting this would simplify the implementation of base::Value quite a bit, and it was also featured in the Promises for Chromium prototype. Due to the heavy use of templates there are some code-bloat concerns with variant, which is why we probably want to restrict usages of this to //base for now.

If we can simplify base::Value /and/ reduce binary size then that seems like a good motivation to me. If we can't get both then maybe we should steer clear of this particular widget for now.

Also watch out because any public-facing changes to base::Value take *forever*.

Brett

[Alex Clarke]

On Thu, 4 Oct 2018 at 19:08, Brett Wilson <bre...@chromium.org> wrote:

On Thu, Oct 4, 2018 at 10:56 AM <dan...@chromium.org> wrote:

On Thu, Oct 4, 2018 at 1:45 PM Nico Weber <tha...@chromium.org> wrote:

The listed reasons for using absl in base sound fairly small to me at this point, so I'd probably suggest waiting until there's more upside.

I'd personally like to see us alias base::Optional to absl's so we can stop maintaining our copy. (I don't have a well formed opinion on keeping it aliased vs using absl:: directly thereafter yet.)

My team just started using C++17. I wonder how far away it is for Chromium? My impression is it's blocked on Nacl or something, but once that's unblocked it might be relatively faster to move to 14 and 17 than it has been to update C++ versions. If true, it might be better to wait.

Migrating to C++17 would be ideal,  but I'm under the impression we're blocked until NaCl is depreciated which will be several years :(

If we wanted to go ahead with promises we'd need to land a base::Variant in the mean time.

 

 

 On Thu, Oct 4, 2018 at 10:55 AM Jeremy Roman <jbr...@chromium.org> wrote:

On Thu, Oct 4, 2018 at 6:00 AM Jan Wilken Dörrie <jdoe...@chromium.org> wrote:

Bumping this up, since Abseil recently released their SwissTable implementation to GitHub [Blog, Code]. Given that we currently try to avoid std::unordered_ containers, this might be interesting for us. However, we definitely should do a careful performance and memory / binary size analysis, similarly to what brettw@ already did.

Indeed. In particular, I thought SwissTable was optimized primarily for x86_64 (for datacentre use), but we have ARM/ARM64 platforms as well.

I look forward to learning more about the performance of SwissTable!

 

Additionally, there are other library features that might be interesting to us:

• absl::optional: This is already used in WebRTC, and adopting it in base would allow us to drop our own base::Optional implementation and make interfacing between Chromium and WebRTC easier. Furthermore, this will make adoption of std::optional easier once we have C++17.

This strikes me as a good place to start if we want to rely on Abseil more, since base::Optional already aims to be compatible with std::optional, much like absl::optional.

• absl::variant: Modeled after std::variant and quite powerful. Adopting this would simplify the implementation of base::Value quite a bit, and it was also featured in the Promises for Chromium prototype. Due to the heavy use of templates there are some code-bloat concerns with variant, which is why we probably want to restrict usages of this to //base for now.

If we can simplify base::Value /and/ reduce binary size then that seems like a good motivation to me. If we can't get both then maybe we should steer clear of this particular widget for now.

Also watch out because any public-facing changes to base::Value take *forever*.

Brett

--

You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To post to this group, send email to c...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CABiGVV9fHLxHLenNr8wify%3DxurzFoojjxcUwM_456KhhBTCrLg%40mail.gmail.com.

[Nico Weber]

On Fri, Oct 5, 2018 at 3:54 AM Alex Clarke <alexc...@google.com> wrote:

On Thu, 4 Oct 2018 at 19:08, Brett Wilson <bre...@chromium.org> wrote:

On Thu, Oct 4, 2018 at 10:56 AM <dan...@chromium.org> wrote:

On Thu, Oct 4, 2018 at 1:45 PM Nico Weber <tha...@chromium.org> wrote:

The listed reasons for using absl in base sound fairly small to me at this point, so I'd probably suggest waiting until there's more upside.

I'd personally like to see us alias base::Optional to absl's so we can stop maintaining our copy. (I don't have a well formed opinion on keeping it aliased vs using absl:: directly thereafter yet.)

My team just started using C++17. I wonder how far away it is for Chromium? My impression is it's blocked on Nacl or something, but once that's unblocked it might be relatively faster to move to 14 and 17 than it has been to update C++ versions. If true, it might be better to wait.

Migrating to C++17 would be ideal,  but I'm under the impression we're blocked until NaCl is depreciated which will be several years :(

Discussions are ongoing, but yes, at the moment it looks like it's going to be a while.

[Brett Wilson]

On Fri, Oct 5, 2018 at 4:35 AM Nico Weber <tha...@chromium.org> wrote:

On Fri, Oct 5, 2018 at 3:54 AM Alex Clarke <alexc...@google.com> wrote:

On Thu, 4 Oct 2018 at 19:08, Brett Wilson <bre...@chromium.org> wrote:

On Thu, Oct 4, 2018 at 10:56 AM <dan...@chromium.org> wrote:

On Thu, Oct 4, 2018 at 1:45 PM Nico Weber <tha...@chromium.org> wrote:

The listed reasons for using absl in base sound fairly small to me at this point, so I'd probably suggest waiting until there's more upside.

I'd personally like to see us alias base::Optional to absl's so we can stop maintaining our copy. (I don't have a well formed opinion on keeping it aliased vs using absl:: directly thereafter yet.)

My team just started using C++17. I wonder how far away it is for Chromium? My impression is it's blocked on Nacl or something, but once that's unblocked it might be relatively faster to move to 14 and 17 than it has been to update C++ versions. If true, it might be better to wait.

Migrating to C++17 would be ideal,  but I'm under the impression we're blocked until NaCl is depreciated which will be several years :(

Discussions are ongoing, but yes, at the moment it looks like it's going to be a while.

Bummer!