On Thu, Aug 29, 2019 at 6:33 AM Jakob Kummerow <[email protected]> wrote:
> I'll give this a shot: > > On Thu, Aug 29, 2019 at 12:43 AM Robert Sesek <[email protected]> wrote: > >> I’m replacing >> <https://chromium-review.googlesource.com/c/chromium/src/+/1698345> >> Chromium’s >> //base double conversion routines with >> https://github.com/google/double-conversion. That project is an >> extracted version, with a few API tweaks, of the V8 double conversion >> routines <https://cs.chromium.org/chromium/src/v8/src/numbers/>. It >> turns out that Blink also uses this library >> <https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/wtf/dtoa/>, >> so I plan on at least unifying Blink on the new copy in //base. >> > > Cool! > > >> But I have some questions for V8 that could lead to more unification: >> >> 1) Is the github.com/google/double-conversion project backed by V8, or >> is it only loosely affiliated? >> > > Loosely affiliated. I'm not aware of any involvement of current V8 team > members with that repository. > > >> 2) Would V8 consider making the external library its source of truth for >> double-conversion? >> > > *Consider*, sure, but I can't promise any particular outcome of the > consideration ;-) > It would certainly be nice not to duplicate maintenance/improvement > efforts between the external library and V8's internal version, so I'm > generally supportive of consolidating on one source of truth. FWIW, we have > at various times toyed with the idea of splitting off various subsystems of > V8 as reusable libraries; this would set a nice example of doing that. > Concerns I can see include: > (1) any kind of change (whether it's a bug fix, a performance improvement, > or a refactoring) is easier to do when it doesn't involve multiple > repositories. While changes to this code are rare, burdening ourselves with > extra difficulty should ideally be outweighed by some benefit (beyond > "would be nice"). > (2) we need that code to implement JavaScript's semantics, which may or > may not be aligned with the goals of the external library. > (3) we need that code to compile with GN, whereas the external library > seems to only maintain SCons and CMake builds. We could upstream a BUILD.gn > file, but see point (1). Considering integration with project-wide build > configuration infrastructure, it's likely preferable to keep the BUILD.gn > file in V8's repository. > (4) we need that code to have test coverage we can trust. I see no CI > system on the external library. It does seem to contain a fork of V8's > original "cctest" coverage for that code, so presumably we could > re-integrate that with our test driver, but it's likely going to be a bit > of work. > > I see that in your CL, you're (for now?) checking a copy of the external > github code into the Chromium repository, as opposed to DEPS'ing it in. > That addresses some of the concerns, but compromises a bit on the goal of > having a single source of truth. In particular, you're running the risk of > divergence over time, unless you manually make sure that any change > contributed to one copy is also applied to the other. > > >> 3) Would V8 consider making v8/src/numbers public API, rather than >> keeping it in v8::internal, so that Chromium could consume it directly? >> > > I think we would be fine with that. One way to do it would be to introduce > a new public header file (strawman: "<v8>/include/v8-double-conversion.h") > exposing an API we'll design based on Chrome's (and other embedders'?) > needs, and then internally map that onto the various bits in src/numbers/. > Alternatively, we could put the code into its own subdirectory (maybe > "<v8>/double-conversion/", similar to > <chrome>/base/third_party/double-conversion/ in your CL), and make that > accessible (through namespace, build system, and documentation) to > consumers outside of V8. > This sounds like the ideal outcome for Chromium. V8 would still have some of downside 1 you list, but hopefully the double conversion API is fairly stable by now. It'd address 3, 4, and it'd enable chrome to ship just one copy of this code for chrome, blink, and v8. > > Anyone else have any thoughts? > > -- -- v8-dev mailing list [email protected] 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 [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/v8-dev/CAMGbLiGyYcfU56TDRcsHeU1xOX5exGAPVpQQwvcR%3DMhP3V81VA%40mail.gmail.com.
