Sorry about not responding earlier, Groups didn’t send the replies to me. Trying from the web now…
On Thursday, August 29, 2019 at 9:32:24 AM UTC-4, Nico Weber wrote: > > On Thu, Aug 29, 2019 at 6:33 AM Jakob Kummerow <[email protected] > <javascript:>> wrote: > >> I'll give this a shot: >> >> On Thu, Aug 29, 2019 at 12:43 AM Robert Sesek <[email protected] >> <javascript:>> 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! >> > Thanks :) > >> >>> 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 agree with all these concerns for V8. > 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. >> > True, it is vendored in as opposed to using DEPS, but I don’t think that really harms the single source of truth aspect. Chromium vendors in other third-party libraries and, in my experience, that does not typically end up leading to many long-standing local modifications. > >> >>> 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. > I agree that this sounds like the ideal outcome. The one caveat for Chromium is it would introduce a dependency on //v8/public:double_conversion or somesuch, but there is some precedent <https://cs.chromium.org/chromium/src/base/BUILD.gn?l=1619&rcl=414e39a22a5b44fbc0e909c4f50845dcdaf8f422> for that now. > > >> >> Anyone else have any thoughts? >> >> Assuming no objections to that, I can start experimenting with such an API after I get Blink converted to the //base copy of double-conversion. I would probably model it on the API provided by double-conversion (or Chromium’s and Blink’s usage of it). Thanks! - Robert -- -- 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/b8d7c305-9f48-46ad-9efd-62a04590b1b0%40googlegroups.com.
