New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 637155 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Extension Mojo Bindings are instantiated for all extensions

Project Member Reported by esprehn@chromium.org, Aug 12 2016

Issue description

This is very strange, why are we initializing the Mojo stuff when creating the main world?

Total %     Symbol Name
100     blink::FrameLoader::dispatchDidClearDocumentOfWindowObject()
100      blink::ScriptController::initializeMainWorld()
100       blink::WindowProxy::initializeIfNeeded()
100        blink::WindowProxy::initialize()
47.6            blink::WindowProxy::createContext()
47.6             v8::NewContext(...)
23.8            content::RenderFrameImpl::didCreateScriptContext(blink::WebLocalFrame*, v8::Local<v8::Context>, int, int)
23.8             extensions::ExtensionFrameHelper::DidCreateScriptContext(v8::Local<v8::Context>, int, int)
23.8              extensions::Dispatcher::DidCreateScriptContext(blink::WebLocalFrame*, v8::Local<v8::Context> const&, int, int)
23.8               extensions::ModuleSystem::ModuleSystem(extensions::ScriptContext*, extensions::ModuleSystem::SourceMap const*)
23.8                content::RenderFrameImpl::EnsureMojoBuiltinsAreAvailable(v8::Isolate*, v8::Local<v8::Context>)
14.3            blink::OriginTrialContext::initializePendingFeatures()
14          blink::WindowProxy::updateDocumentProperty()
14           blink::Document::wrap(v8::Isolate*, v8::Local<v8::Object>)

https://cs.chromium.org/chromium/src/content/renderer/render_frame_impl.cc?q=EnsureMojoBuiltinsAreAvailable&sq=package:chromium&l=2496

Total %		Symbol Name
100	 	content::RenderFrameImpl::EnsureMojoBuiltinsAreAvailable(v8::Isolate*, v8::Local<v8::Context>)
63	 	 mojo::edk::js::Core::GetModule(v8::Isolate*)
63	 	  v8::ObjectTemplate::NewInstance(v8::Local<v8::Context>)
22.4	 	 content::InterfaceProviderJsWrapper::Create(v8::Isolate*, v8::Local<v8::Context>, shell::InterfaceProvider*)
22.4	 	  gin::WrappableBase::GetWrapperImpl(v8::Isolate*, gin::WrapperInfo*)
22.4	 	   v8::ObjectTemplate::NewInstance(v8::Local<v8::Context>)
14.4	 	 mojo::edk::js::Support::GetModule(v8::Isolate*)
14.4	 	  v8::ObjectTemplate::NewInstance(v8::Local<v8::Context>)

All of the time is spent creating the ObjectTemplate's, I wonder how big the mojo bindings are though. It takes even longer than the |document| property which is huge. That makes me think the mojo bindings must be very large?

ex. https://cs.chromium.org/chromium/src/mojo/edk/js/core.cc?q=Core::GetModule&sq=package:chromium&l=386

There's a lot of stuff in there, how do we make that smaller?

Note: The cost of blink::OriginTrialContext::initializePendingFeatures is  issue 637148 . Once we fix that it'll mean the mojo bindings are taking even more time.

Note(2): This is a regression since we added Mojo, we definitely need to do something here. :)
 
Cc: imch...@chromium.org
Very good question.  I wouldn't think we'd need mojo at all in the main world - the only extension-y thing that I can think of using it is media router.  Derek, is this something MR-related?  Does it need to be in each script context?
I think this might actually be a mistake too. All calls to initializeIfNeeded() do this, but the main world never actually has access to mojo, certainly not web exposed contexts that aren't Web UI, so we're just wasting time down there I think?

I think there's 2 bugs here:
1) The main world creates all this mojo stuff even though we don't need it.
2) Initializing all the mojo stuff is slow.
For MR we only set up the MR mojo interface for the MR extension's render frame. https://cs.chromium.org/chromium/src/chrome/browser/extensions/chrome_mojo_service_registration.cc?sq=package:chromium&rcl=1470626560&l=34. Other render frames that don't have the kMediaRouterPrivate permission won't get it. 

Comment 4 by yzshen@chromium.org, Aug 12 2016

Cc: yzshen@chromium.org
Short term benefit: Since no one seemed to know why we actually inject these mojo bindings, I did a little digging.  It looks like we do use them in a handful of APIs, but we definitely don't need to inject them in every frame.  We can limit it to at least only blessed extension contexts (i.e. only extension background pages or popups).  This doesn't help everything, and can be improved with more digging, but should help for the 99% of web frames that *aren't* extension pages.  I'll try to land a patch for that shortly.
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3cf02f522b6428e4bd30ac55da07657a3e55072c

commit 3cf02f522b6428e4bd30ac55da07657a3e55072c
Author: rdevlin.cronin <rdevlin.cronin@chromium.org>
Date: Mon Aug 15 23:12:31 2016

[Extensions] Don't inject mojo bindings for every context

Currently, the module system injects mojo bindings for every created
script context, because some extension APIs rely on their existence.
However, these bindings are slow and costly to construct. Only create
them for blessed extension contexts.

We should be able to drill this down further and create them even more
rarely, but do this until we can investigate more, since this helps in
the 99% case.

BUG= 637155 

Review-Url: https://codereview.chromium.org/2243323002
Cr-Commit-Position: refs/heads/master@{#412086}

[modify] https://crrev.com/3cf02f522b6428e4bd30ac55da07657a3e55072c/extensions/renderer/module_system.cc

Cc: -rdevlin....@chromium.org
Owner: rdevlin....@chromium.org
Status: Assigned (was: Untriaged)
assigning to myself since this bug is (more or less) specifically about extension mojo bindings.  Should we file a separate one for the fact that mojo bindings should be sped up generally?
Status: Fixed (was: Assigned)
Summary: Extension Mojo Bindings are instantiated for all extensions (was: Extension Mojo Bindings make creating the main world 24% slower)
Mojo bindings are now only installed for the extensions that need them.  Feel free to file a new bug if the mojo binding process itself still needs speeding up.

Sign in to add a comment