I'd rather move tracing functionality into gin than add more required embedder fields. Adding the tracing to gin might also be nice once we move PDFium to use gin bindings.
I think checking out gin in addition to v8 is a negligible cost, and I'd much rather have embedders go through gin than trying to write their own bindings. On Wed, Jun 13, 2018 at 7:04 PM d.haresign via v8-dev < v8-dev@googlegroups.com> wrote: > On Tuesday, 12 June 2018 22:15:01 UTC-4, mlip...@chromium.org wrote: >> >> +Jochen Eisinger who is gin >> <https://cs.chromium.org/chromium/src/gin/?g=0> owner and also has a >> good overview of the whole Chromium architecture. >> >> The Chromium dispatch for the EmbedderHeapTracer currently needs two >> internal embedder fields and no lookup for a tracer. It requires a >> suboptimal pattern match on embedder fields that we should optimize though. >> >> I am happy to discuss any improvements on that, especially the matching >> for whether we should send or not, but the performance for using it in >> Chromium should stay similar as this is used everywhere for the DOM and >> Blink is the main embedder. >> > > Understood. You mentioned having a bit somewhere in the hidden class, can > you elaborate a bit on that? I'd be happy to investigate doing this > instead. > > I think that if a project wants to live as an embedder in the >> Chromium/Blink environment, then it should at least depend on gin. We can >> then think of adding maybe adding a the dispatcher to gin that then >> forwards based on the GinEmbedder >> <https://cs.chromium.org/chromium/src/gin/public/gin_embedders.h?q=GinEmbedder&g=0&l=14> >> and >> set that to V8 to generalize this for the Chromium world. Jochen, wdyt? >> > > I have two concerns about this. First, from what I can tell, gin seems to > be part of Chromium, at least in terms of the source distribution, so we'd > effectively be depending on Chromium. But second and more importantly, we > don't always embed alongside Chromium. Our library is also used on the > server-side. For that reason we'd rather just depend on V8 and have the > same code work in both environments. > > Whilst you can put the dispatching tracer somewhere other than V8, I'm > don't think it's unreasonable to have the logic in V8 itself. > > Let me enumerate the changes I think are required in V8: > > - Add AddEmbedderHeapTracer and RemoveEmbedderHeapTracer > - Deprecate SetEmbedderHeapTracer (make it forward to > AddEmbedderHeapTracer) > - LocalEmbedderHeapTracer to store container of remote tracers > - LocalEmbedderHeapTracer to call all tracers in a loop > - NumberOfWrappersToTrace needs to take a sum > - AdvanceTracing needs some thought: does it attempt to finish one > remote tracer before moving on to the next, or round-robin? > - Everything else can just iterate and call > > That's basically it. However there are potentially more changes (e.g. to > TracePossibleWrappers) when you consider how a third party tracer would > work in conjunction with Chromium's. > > I envisaged that V8 would simply tell all remote tracers about all > wrappers. Each tracer would be responsible for filtering out the wrappers > that don't belong to it. If there was a "contract" that the first embedder > field was set to point to the tracer to be used, then the implementation > would be dead simple: > > void ScriptWrappableMarkingVisitor::RegisterV8References( > const std::vector<std::pair<void *, void *>>& > internal_fields_of_potential_wrappers) { > CHECK(ThreadState::Current()); > for (auto& pair : internal_fields_of_potential_wrappers) { > if (pair.first == this) { > RegisterV8Reference(pair); > } > } > } > > But there are other options on how to do this, if adding a third field > would be an unacceptable cost. We could adjust V8's TracePossibleWrappers > to not check if the first field is nullptr, and adjust Chromium's > RegisterV8References > to filter out nullptrs. We can probably come up with a scheme for our > tracer that works with only one field (though it would be nicer if we had > access to more than just the first two...) > > Thoughts? > Daryl. > > -- > -- > v8-dev mailing list > v8-dev@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+unsubscr...@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. > -- -- v8-dev mailing list v8-dev@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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
