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 <javascript:>> 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.

Reply via email to