XHProf, a go to tool for profiling, actually impacts the profiling due to huge amount of function/method calls in Drupal 8. @msonnabaum suggested sampling as a lighter weight profiling technique. Initial results are promising. See the interactive SVG below, and annotated screenshot below that, where I list and circle potentially slow methods.

  1. The interactive flame graph
  2. Docs on how to read flame graphs. The gist is that you look for boxes that are wider than what you expect. Then you click on the box to more clearly see where the time is going, within that method. Hover over a box for sampling detail. Colors are not meaningful.
  3. Code for sampling and generating a flame graph
  4. This graph is taken from 1000 requests to the /user/password page of standard profile. I picked this page as it is relatively simple.
  5. Using PHP 5.4 with apc.stat=0 to reduce filesystem noise. Drupal is configured to use APC classloader.



  1. service_container_prod::getStreamWrapperManagerService(). Perhaps a dead end as we take the inevitable hit of loading Module and DB classes here. Module loading can be moved to on-demand after #2353357: hook_stream_wrappers_alter() should be removed as it is broken since modules are not loaded on demand. Indeed a dead end, this takes the hit of opening the DB connection and initializing the module loader service.
  2. service_container_prod::getRouterListenerService(). See #2368275: EntityRouteEnhancer and ContentFormControllerSubscriber implicitly depend on too many services.
  3. Drupal\Core\Block\BlockBase::access() See #2354889: Make block context faster by removing onBlock event and replace it with loading from a ContextManager. More specifically: BlockBase::getConditionContexts() is the culprit. It should be statically cached, but isn't. It dispatches the BlockEvents::CONDITION_CONTEXT event for every block. Epic fail. But also a super easy fix! :) On the default front page, with XDebug and XHProf disabled, this takes 6 to 8 milliseconds. Statically caching would bring this down to ~4.5. Of course, with more blocks, on actual sites, the difference would be much bigger.
  4. \Drupal\Core\Menu\MenuActiveTrail::getActiveTrailCacheKey(). Seems like an unoptimized query in MenuTreeStorage.
  5. Drupal\Core\Routing\RoutePreloader::loadNonAdminRoutes (necessary probably to avoid other stuff)
  6. menu_local_tasks spends a lot of time on initializing necessary services and then even more time on actually getting the local tasks. Hopefully #1805054: Cache localized, access filtered, URL resolved, and rendered menu trees will bring salvation there.
  7. Last but far from least: UserLoginBlock! This is taking up a pretty large amount of the time (>2%). About half of that (1%) is spent on generating a link and finding the alias for a URL. For that, we have #1965074: Add cache wrapper to the UrlGenerator. The other half (1%) is just the building of a form being expensive. If we could render cache the login block, this would go away… and thanks to #2351015: URL generation does not bubble cache contexts we should be able to do that soonish.
Files: 
CommentFileSizeAuthor
#2 flame.png319.79 KBchx

Comments

moshe weitzman’s picture

Issue summary: View changes
chx’s picture

Issue summary: View changes
FileSize
319.79 KB
moshe weitzman’s picture

Issue summary: View changes
benjy’s picture

Issue summary: View changes
dawehner’s picture

Issue summary: View changes
dawehner’s picture

Issue summary: View changes
catch’s picture

Issue summary: View changes
Fabianx’s picture

I just added support for XHProf-Lib / CLI to xhprof-kit, I will also add support for flamegraphs soon. This is really cool.

pwolanin’s picture

Moshe points me to MenuTreeStorage::loadByRoute() as problematic.

Possible it would be faster here to do the sort in PHP (usually should just be 1 item), but the ORDER BY clauses may be confusing mysql.

And/or we should index the full length of route name?

Wim Leers’s picture

Issue summary: View changes

I looked at the flamegraph in detail and added some more conclusions to the IS.

pwolanin’s picture

Quick test with EXPLAIN suggests we should see if there is a clear benefit to pulling the loadByRoute() ordering into PHP, since it's usually 0 or 1 result, and hence a no-op.

Wim Leers’s picture

Looks like #2370667: [Meta] user/password flame graph is addressing point 3: it makes the /user/password page 11% faster. AFAICT solely by dispatching the condition event only once instead of per block.

Fabianx’s picture

#12: Is that link to the right issue?

effulgentsia’s picture

Wim Leers’s picture

Oops, sorry, yes.

Crell’s picture

moshe weitzman’s picture

moshe weitzman’s picture

Issue tags: +flame

FYI, I posted a flame graph of just the bootstrap to #2380417: Bootstrap flame graph

Crell’s picture

If I am reading this correctly (and I may well not be), there may be some performance improvements to be had by speeding up the compiled Container, no? That code is all compiled off of the information provided to the container builder by upstream code.

In particular, it looks like any calls where a service is loaded that needs another service call back up through 2 layers of get() function, one of them ours. I think we could inline some more, no? (That would be best as a PR against Symfony if so.)

I am also not clear on the purpose of Drupal\Core\DependencyInjection\Container::get(). If a service is called but doesn't exist, that should fail with a message, no? So that we know what went wrong and how to fix it? Swallowing an error here seems like a bad idea and a slow-down, as it adds a stack call to every lookup of a service used by a service (which is, let's face it, a lot).

dawehner’s picture

I am also not clear on the purpose of Drupal\Core\DependencyInjection\Container::get(). If a service is called but doesn't exist, that should fail with a message, no? So that we know what went wrong and how to fix it? Swallowing an error here seems like a bad idea and a slow-down, as it adds a stack call to every lookup of a service used by a service (which is, let's face it, a lot).

Note: we do have an issue to micro-optimize that a bit already: #2307869: Remove Drupal's Container::get() but the actual problem seems to be that we do have quite some classes of all those dependencies which needs to be loaded.

I am also not clear on the purpose of Drupal\Core\DependencyInjection\Container::get(). If a service is called but doesn't exist, that should fail with a message, no? So that we know what went wrong and how to fix it? Swallowing an error here seems like a bad idea and a slow-down, as it adds a stack call to every lookup of a service used by a service (which is, let's face it, a lot).

That entirely depends on what you passed in as $invalidBehaviour

One thing we can do, and we have a couple issues doing that already, is to mark services as not public. This will inline the service construction, indeed.
One of them is #2354705: Mark a couple of asset services as non public

Crell’s picture

Yes, if we have a lot of dependencies then there's 2 ways to make the container faster: Fewer dependencies or make the dependency lookup process faster. Both are worth pursuing, IMO.

It looks like our Container::get() is invalidating $invalidBehavior? Because instead of returning null in case of non-exception, you get a string. That... seems ill advised.

dawehner’s picture

It looks like our Container::get() is invalidating $invalidBehavior? Because instead of returning null in case of non-exception, you get a string. That... seems ill advised.

Please, discuss that in some other issue. But seriously, no, NULL is not an object ... you just return the result which is NULL already.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.