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.

Reply via email to