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.