Problem/Motivation

In shortcut_preprocess_page

     $link = current_path();
    if (!($url = \Drupal::pathValidator()->getUrlIfValid($link))) {
      // Bail out early if we couldn't find a matching route.
      return;
    }

because that can happen how?

Proposed resolution

Replace $url with the route match. Trivial solution, instant win!

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch, shortcut_wat.patch, failed testing.

chx’s picture

Status: Needs work » Needs review
FileSize
1.28 KB
Fabianx’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Performance, +Quick fix

Looks great to me. I assume this has adequate test coverage already.

So RTBC once tests pass.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: shortcut_wat.patch, failed testing.

Fabianx’s picture

dawehner’s picture

For a anonymous user with shortcut permissions this safes itself 1%:

/v/w/d8 (8.0.x) $ ~/bin/xhprof-kit/benchmark-branches.sh 5439b869d9a56 8.0.x shortcut-speedup
=== 8.0.x..shortcut-speedup compared (5439b869d9a56..5439b89f5863f):

ct  : 98,250|97,345|-905|-0.9%
wt  : 245,309|245,720|411|0.2%

benjy’s picture

@dawehner, care to share the contents of benchmark-branches.sh ?

Fabianx’s picture

@benjy:

https://github.com/LionsAd/xhprof-kit/blob/master/benchmark-branches.sh

See also:

https://drupal.org/contributor-tasks/profiling

I have written it originally for the Twig initiative.

You will need to hack it for uprofiler or any non-standard URL for the installation (see https://github.com/LionsAd/xhprof-kit/blob/master/find-min-web.sh and change 127.0.0.1 with drupal8.local or such).

In find-min-web.sh you can also specify the number of runs.

The nicest thing that dawehner currently does not use is:

You can get an API key from me and then others can check the XHProf run online. (see upload-run.sh and upload-bench.php to do it manually)

dawehner’s picture

@benjy
For xhprof have a look at https://github.com/dawehner/xhprof-kit/tree/uprofiler

@Fabianx
Contact message sent.

davidhernandez’s picture

Status: Needs work » Needs review
FileSize
1.3 KB

See if this is green.

Fabianx’s picture

Issue tags: +needs profiling

Nice work!

Fabianx’s picture

Issue tags: -needs profiling

10 shortcuts, Drupal Admin interface exposed for anonymous user, 100 iterations, basic front-page, PHP 5.5 with opcache

TL;DR: Saves 858 function calls, -1%, -1,932 ms

### FINAL REPORT

=== baseline-2351411-8.0.x..8.0.x compared (545228c9d3459..54522a97f422e):

ct : 83,410|83,410|0|0.0%
wt : 193,102|192,792|-310|-0.2%
mu : 23,410,264|23,410,264|0|0.0%
pmu : 23,490,240|23,490,240|0|0.0%

=== baseline-2351411-8.0.x..issue-2351411--shortcut-faster compared (545228c9d3459..54522ab81cc00):

ct : 83,410|82,552|-858|-1.0%
wt : 193,102|191,170|-1,932|-1.0%
mu : 23,410,264|23,367,152|-43,112|-0.2%
pmu : 23,490,240|23,447,008|-43,232|-0.2%

=== SUM: 8_0_x-summary..issue-2351411--shortcut-faster-summary compared (54522ade70ee0..54522ae62234d):

ct : 8,341,000|8,255,200|-85,800|-1.0%
wt : 20,264,594|20,128,144|-136,450|-0.7%
mu : 2,341,026,712|2,336,715,584|-4,311,128|-0.2%
pmu : 2,349,031,488|2,344,693,888|-4,337,600|-0.2%

---
ct = function calls, wt = wall time, cpu = cpu time used, mu = memory usage, pmu = peak memory usage

### XHPROF-LIB REPORT

+--------------------------------+------------+------------+------------+------------+------------+
| namespace                      |        min |        max |       mean |     median |       95th |
+--------------------------------+------------+------------+------------+------------+------------+
| Calls                          |            |            |            |            |            |
|                                |            |            |            |            |            |
| issue-2351411--shortcut-faster |     82,552 |     82,552 |     82,552 |     82,552 |     82,552 |
| 8_0_x                          |     83,410 |     83,410 |     83,410 |     83,410 |     83,410 |
|                                |            |            |            |            |            |
| Wall time                      |            |            |            |            |            |
|                                |            |            |            |            |            |
| issue-2351411--shortcut-faster |    191,170 |    219,081 |    201,281 |    200,438 |    213,130 |
| 8_0_x                          |    192,792 |    218,539 |    202,646 |    201,982 |    216,170 |
|                                |            |            |            |            |            |
| Memory usage                   |            |            |            |            |            |
|                                |            |            |            |            |            |
| issue-2351411--shortcut-faster | 23,367,152 | 23,367,184 | 23,367,156 | 23,367,152 | 23,367,184 |
| 8_0_x                          | 23,410,264 | 23,410,288 | 23,410,267 | 23,410,264 | 23,410,288 |
|                                |            |            |            |            |            |
| Peak memory usage              |            |            |            |            |            |
|                                |            |            |            |            |            |
| issue-2351411--shortcut-faster | 23,446,432 | 23,447,008 | 23,446,939 | 23,447,008 | 23,447,008 |
| 8_0_x                          | 23,490,240 | 23,490,816 | 23,490,315 | 23,490,240 | 23,490,816 |
|                                |            |            |            |            |            |
+--------------------------------+------------+------------+------------+------------+------------+
Fabianx’s picture

Status: Needs review » Reviewed & tested by the community

RTBC

dawehner’s picture

+1!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

This issue is a major task that will improve performance significantly and the disruption it introduces is limited. Per #2350615: [policy, no patch] What changes can be accepted during the Drupal 8 beta phase?, this is a good change to complete during the Drupal 8 beta phase. Committed c0459a1 and pushed to 8.0.x. Thanks!

  • alexpott committed c0459a1 on 8.0.x
    Issue #2351411 by chx, davidhernandez: Fixed [perf] Shortcut reruns...
davidhernandez’s picture

Wow, that was all rather ... fast. :)

Wim Leers’s picture

Yay for improving Shortcut module!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.