Abseil in V8

[Leszek Swirski]

Hi all,

I'm planning on adding Abseil as a dependency in V8, to get rid of some of our hand-rolled base classes and (hopefully) reap the advantages of those engineers' work. An example use would be changing our hand-rolled hashmap to a thin wrapper over absl::flat_hash_map/set: https://chromium-review.googlesource.com/c/v8/v8/+/2464941

This plan is unofficially lgtm'ed by Chromium C++ folks - any objections from our side?

Cheers,

Leszek

[Yang Guo]

You might want to give a heads up to Node.js folks. I don't think they would object, but it would be a courtesy to them.

Cheers,

Yang

--
--
v8-dev mailing list
v8-...@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+un...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/v8-dev/CAGRskv_mgj%2BnH5SHa6m%2Bgsq9U3ZLRwZphzY8zhd_y67jxfbBdg%40mail.gmail.com.

[Leszek Swirski]

+Victor, do you know whom to communicate this to?

To view this discussion on the web visit https://groups.google.com/d/msgid/v8-dev/CAFSTc_gSpJN2sbaExeK2tdcK36oSmxTtKQk5mxvYenjq1PawgQ%40mail.gmail.com.

[Victor Gomes]

I guess Mary Manchini (mmar...@netflix.com) would be a good person to communicate that to. She was investigating the possibility of changing Node build system to a CMake/GN hybrid (https://github.com/nodejs/TSC/issues/901).

To view this discussion on the web visit https://groups.google.com/d/msgid/v8-dev/CAGRskv_2zAJdDPfE_2qCSiWHkNh0hw3g7BjgGmMc9MLfgwe1JA%40mail.gmail.com.

Victor Gomes

Software Engineer

victo...@google.com

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 Ssie 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.

[Leszek Swirski]

+Mary Marchini in that case, thanks!

Mary, FYI, we're planning, if there aren't any strong objections, on adding an Abseil dependency to V8. The Chromium roll of Abseil we're planning on including has a (hand maintained) gn port of Abseil's bazel build files, so I dunno if Node will want to use that or some other Abseil build system port.

To view this discussion on the web visit https://groups.google.com/d/msgid/v8-dev/CAGFuzY2FYtn6ZJC-u07Bi_EK9i-UWYhX%2B_02NnK_jCgh8-U%2Byw%40mail.gmail.com.

[Adam Klein]

What's the full transition plan here? We've done several adhoc transitions between different containers in the past, leaving us currently with a mix of hand-rolled and STL containers. While Abseil seems like it has some definite advantages, it seems like it'd be ideal, both for code size reasons and for uniformity across CL authors & reviewers if there was some agreed-upon end state, at least. Ideally there'd even be a staffed, prioritized effort to get us from the current state to some future state.

- Adam

--

[Leszek Swirski]

I saw this effectively as a cleanup project, with as much staffing/prioritisation as, say, TNodification. If you have spare headcount to offer for this I'll gladly take it :)

The end state I would suggest, is to replace all uses of std::*map with absl, and try to replace our base::TemplateHashMap with it too, turning the former into a thin wrapper over the other. Also, replace any other relevant base implementations with equivalent absl ones where applicable, with transitional aliases in the base namespace. An example here would be base::Optional and a couple of the template traits, like, say, conjunction and disjunction. Using aliases and turning existing containers into thin wrappers should reduce the binary size impact, and then inlining those aliases can be done reasonably ad hoc, or in cleanup Fridays.

How does that sound?

- Leszek

To view this discussion on the web visit https://groups.google.com/d/msgid/v8-dev/CAEvLGcKVpUsVH2ifAUzX0wAN2dgYaPDNoVixVW-%2B%2BYZkOT3o7w%40mail.gmail.com.

[Adam Klein]

Making this a cleanup project makes sense (no, I don't have headcount to allot here), but thanks for making that explicit. Ideally this could be documented somewhere too.

This plan sounds reasonable to me (and your base::Optional alias CL is a nice proof-of-concept). The other thing implicit in my question was: is there buy-in for this approach across the various areas in V8? I don't ask that everyone sign off, but I would like to offer the opportunity for folks to weigh-in if they have opinions (especially since I suspect not everyone follows v8-dev@ closely). I've CCed the relevant folks on this thread.

- Adam

To view this discussion on the web visit https://groups.google.com/d/msgid/v8-dev/CAGRskv_0835CMHLEfOs_jzgRjHtfj08SCq6JjLWXx0LubKSZfQ%40mail.gmail.com.

[Leszek Swirski]

Thanks for the cc's, checking the temperature and buy-in from other team members was indeed one of the reasons I sent out this email.

To view this discussion on the web visit https://groups.google.com/d/msgid/v8-dev/CAEvLGc%2BqyeTE21GT-a2qUP5T02Gd8VX%3D92wOc_pC85FaSDLUHQ%40mail.gmail.com.

[Michael Lippautz]

(Thanks for cc!)

+1

Since V8's copy of base in some areas (not all) is a detached copy of Chromium's base switching to something that's wider used makes a lot of sense.

One caveat: Presumably, this will stay in an internal dependency, so API types will have to go through std equivalents which sometimes means that some internal types would not move to absl to avoid conversion. (I still think this all makes a lot of sense.)

[Leszek Swirski]

On Mon, Oct 19, 2020 at 8:01 AM Michael Lippautz <mlip...@chromium.org> wrote:

One caveat: Presumably, this will stay in an internal dependency, so API types will have to go through std equivalents which sometimes means that some internal types would not move to absl to avoid conversion. (I still think this all makes a lot of sense.)

Good point -- I don't think that necessarily has to be the case, we could make Abseil an externally exposed dependency, but I expect we'd anyway rather prefer types that are on the way to standardisation (e.g. optional or string_view), and anyway we tend to avoid even std:: types on the API boundary where possible.

[Leszek Swirski]

Hi all,

Following up on my previous email discussing Abseil, I've written up a more complete document of how I would see this happening:

https://docs.google.com/document/d/1YJh_lb-V36PE1oebPEb4nj9s1qFvuDlDvvMghHqnPQg/edit?usp=sharing

Please feel free to add any benefits or risks I might have missed.

- Leszek