Visit node/1

Check for database updates in xhprof.

Note that there are three cache->set() from MenuTree.

Patch attached.

Comments

berdir’s picture

Issue tags: +Needs tests

I thought that's why we do unit tests? :)

dawehner’s picture

Issue tags: +PHPUnit
StatusFileSize
new31.09 KB

This is emberassing tbh.
This still obviously does not cover all of the menu tree building but at least a little bit more.

Status: Needs review » Needs work

The last submitted patch, 2: menu_tree-2229283-2.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review

2: menu_tree-2229283-2.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 2: menu_tree-2229283-2.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
StatusFileSize
new4.49 KB

Rerolled.

Status: Needs review » Needs work

The last submitted patch, 6: menu_tree-2229283-6.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
StatusFileSize
new4.21 KB
new704 bytes

Maybe this is it.

wim leers’s picture

8: menu_tree-2229283-8.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 8: menu_tree-2229283-8.patch, failed testing.

rajendar reddy’s picture

Status: Needs work » Needs review
StatusFileSize
new4.35 KB

Updating the patch. Please review.

wim leers’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests +Performance
dawehner’s picture

Thank you

webchick’s picture

+++ b/core/modules/menu_link/tests/Drupal/menu_link/Tests/MenuTreeTest.php
@@ -321,6 +328,78 @@ public function testGetActiveTrailIdsWithoutPreferredLink() {
+    $this->menuTree->buildTree('test_menu');
+    $this->menuTree->buildTree('test_menu');

Is that intentional..? If so, it could use a comment as to why we do that.

webchick’s picture

Status: Reviewed & tested by the community » Needs review
dawehner’s picture

// Ensure that the static caching triggered.

$this->cacheBackend->expects($this->exactly(1))
  ->method('get');

$this->menuTree->buildTree('test_menu');
$this->menuTree->buildTree('test_menu');
catch’s picture

Status: Needs review » Fixed

Yes that's the right comment, it's just not immediately next to those two lines. Committed/pushed to 8.x, thanks!

  • Commit 519e98b on 8.x by catch:
    Issue #2229283 by dawehner, Rajendar Reddy, catch: Menu tree caching is...

Status: Fixed » Closed (fixed)

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