+Jochen Eisinger <joc...@chromium.org> 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. 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? -Michael On Wed, Jun 13, 2018 at 1:21 AM d.haresign via v8-dev < v8-dev@googlegroups.com> wrote: > On Tuesday, 12 June 2018 04:52:13 UTC-4, mlip...@chromium.org wrote: >> >> On Tue, Jun 12, 2018 at 8:02 AM d.haresign via v8-dev < >> v8-...@googlegroups.com> wrote: >> >>> I work on a product which embeds Chromium. In addition, we host complex >>> C++ objects in V8 directly. Our V8 binding layer supports the tracing of >>> objects to determine references to other objects, rather than simply >>> holding on to all objects with strong 'v8::Persistent's. Up until Chromium >>> 56 / V8 5.6, we were doing this tracing in the GC prologue, using >>> 'Isolate::SetReference' to inform V8 of all the edges. >>> >>> As we look to upgrade past Chromium 59 / V8 5.9, 'Isolate::SetReference' >>> has been removed in favor of the 'EmbedderHeapTracer' APIs. >>> >>> One issue we have with the switch, is that Chromium already sets the >>> 'EmbedderHeapTracer' to its own 'ScriptWrappableVisitor' tracer. In order >>> to work we need to come up with a way to work along side Chromium. >>> >>> To quickly recap on how the tracing works, when an embedder hosts an >>> object it can set internal fields. When V8 is tracing objects, if it >>> discovers an object which has the first and second internal fields both >>> set, it considers it as a wrapper that needs to be traced with the >>> 'EmbedderHeapTracer'. These internal fields let you store information >>> necessary to perform the tracing. >>> >>> In Chromium, the first field is a pointer to a 'WrapperTypeInfo' object, >>> and the second field is a pointer to a 'ScriptWrappable' object. The >>> 'RegisterV8References' implementation in Chromium's tracer expects to only >>> ever be called with these Chromium objects. As such, it does a >>> reinterpret_cast to a 'WrapperTypeInfo *' on the first field. If the >>> 'gin_embedder' field of that object is 'kEmbedderBlink', it does a >>> reinterpret_cast to a 'ScriptWrappable *', and traces that object. >>> >>> If we were to integrate with Chromium's 'EmbedderHeapTracer', we would >>> have to set up our objects to have objects which match the layout of >>> Chromium's 'WrapperTypeInfo', pretend that the are 'kEmbedderBlink' >>> objects, and somehow get it to call our trace methods instead of Chromiums >>> (the exact way this is done seems to vary wildly over the latest 10 or so >>> versions of Chromium). >>> >>> This seems like a rabbit hole full of issues I'd rather avoid. >>> >> >> Just to acknowledge: All of that is correct. >> >> Now, you could still do something differently: >> - Create a generic dispatcher class inheriting from EmbedderHeapTracer. >> - Instead of setting ScriptWrappableVisitor, pass on the generic >> dispatcher to V8. >> - Have your objects match this code >> <https://cs.chromium.org/chromium/src/v8/src/heap/heap.cc?q=TracePossibleWrapper&l=4711&dr=CSs> >> that decides whether something should be passed on or not to the >> EmbedderHeapTracer. In the case that these objects are not critical wrt. to >> memory consumption, tge simplest thing would be to have 3 fields of which >> the first 2 are nullptr. >> - Have your generic dispatcher use Chromium's ScriptWrappable visitor if >> it's a Blink object >> - ... or dispatch to whatever EmbedderHeapTracer, otherwise. >> >> The fragile part is the matching algorithm in V8 which we wanted to >> change but ultimately never needed to. You don't have to deal with >> WrapperTypeInfo or any of the types specified there as the generic visitor >> would just pass on the object. >> > > Yeah, that's another option we considered. Though currently we don't have > any control over this as Chromium sets its own tracer. We could change > Chromium to install a dispatching tracer, and have APIs to let us register > our own tracer with Chromium's dispatching tracer, but I think we'd prefer > to keep our V8 binding library working purely in terms of V8, rather than > adding Chromium into the mix (even if it's linked in elsewhere in the > program). We could also expose an API on V8 to fetch the current tracer > which would allow us to overwrite it with a dispatching tracer, though that > seems more fragile. > > fyi: We were thinking of adding a bit somewhere on V8's hidden class that >> is set through instantiating it through the template info. That bit >> could've been used to signal that the object should be sent to the >> EmbedderHeapTracer instead of doing the pattern matching on fields. >> > > That sounds good. Out of interest, is there any reason why the tracer > only gets the first two embedder fields? Could it be passed a 'v8::Object' > so that the tracer could access additional fields, or would there be > lifetime concerns? Prior to us needing to use the 'EmbedderHeapTracer' > APIs, we were using the 4th field when embedding with Chromium, which > claims the first three. > > The way our tracer works when we're not in a process that links in > Chromium, is that the first embedder field is a pointer to the tracer that > should be used. That allows the tracer to easily decide whether it should > trace the object with a simple check against 'this'. It would be nice if > this could be the implicit contract with Chromium too, perhaps relegating > the 'WrapperTypeInfo' to a third field, or folding it into the > 'ScriptWrappable'. > > >>> Another option I see is to change V8 to support multiple >>> 'EmbedderHeapTracer's. Each 'EmbedderHeapTracer' would be informed of all >>> the wrappers that V8 found, and each one would be responsible for tracing >>> its own objects. From testing with Chromium 62 / V8 6.2, this seems to >>> work, even if there are references between objects from the two embedders. >>> Consider an object graph as follows: >>> >>> (root) -> (E1 O1) -> (E1 O2) -> (E2 O1) -> (E2 O2) -> (E1 O3) >>> >>> Where E1/E2 represents the different embedders, and O1/O2/O3 represents >>> different objects. >>> >>> V8 would discover the wrapper (E1 O1) and inform the >>> 'EmbedderHeapTracer's. E1's tracer would trace (E1 O1), and (E2 O2). E1's >>> tracer doesn't know how to trace (E2 O1), but it informs V8 that it's >>> alive. V8 then informs the tracers about (E2 O1). E2's tracer traces (E2 >>> O1), and (E2 O2). Again, E2's tracer doesn't know how to trace (E1 O3), >>> but it informs V8 that it's alive. V8 then informs the tracers about (E1 >>> O3). E1's tracer traces it, and the tracing is complete. >>> >>> There are a few more details which I am missing out relating to changes >>> in Chromium which would be required to get it to ignore objects, but those >>> should be fairly straight forward. >>> >>> Is there anything I am overlooking which would prevent multiple >>> 'EmbedderHeapTracer's from being a viable option? >>> >> >> Pretty exciting to see such a thing work. And, yes, I would say that this >> is definitely viable. >> >> We were not considering this option as it seemed better to have one >> EmbedderHeapTracer on the embedder side that does the dispatch to other >> tracers as this avoids putting any of the logic that decides on what goes >> where into V8. >> >> In any case: Multiple tracers should work, independently of where you put >> the dispatch and matching logic (V8/Blink). >> >> Keep us posted! >> >> Cheers, -Michael >> > > Ultimately, I think we'd rather have this in V8 to avoid having to pull in > dependencies on Chromium into our code which deals with this. If you > agree, we can continue the discussion over some PRs. > > Thanks, > 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.
