Project

General

Profile

Actions

Bug #63550

closed

Menu configuration caching disabled by ineffective type checks

Added by Martin Helmich over 9 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
Caching
Target version:
-
Start date:
2014-12-03
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
6.2
PHP Version:
5.6
Tags:
Complexity:
easy
Is Regression:
No
Sprint Focus:

Description

We just encountered a bug that under certain circumstances prevented the caching of multi-level menu configurations.

We encountered this issue due to massive performance problems after a TYPO3 6.2 migration. In our case, we have a nested menu with 3 levels; however, only a small subset of menu items actually has a 3rd menu level. For every other 2nd level menu item without sub menu items, the entire menu configuration was completely rebuilt from scratch, because the (empty) menu configuration was not cached by the caching framework.

This is actually due to two issues in the TYPO3\Core\Frontend\ContentObject\Menu\TextMenuContentObject and TYPO3\Core\Frontend\Page\PageRepository class.

First of all, the TextMenuContentObject class generates a NULL menu configuration when no pages are rendered with in the menu. The AbstractMenuContentObject class then tries to cache this NULL value. However, when rendering the NEXT menu item, the menu configuration will be re-built when the cached value is !is_array(...) (i.e. always, when the cached value is NULL).

Secondly, the TYPO3\Core\Frontend\Page\PageRepository class contains a typing error that effectively prevents empty arrays from being cached by the caching framework.

Both of these bugs in conjunction not only effectively disable the caching of the menu configuration, but also put a lot of load on the database, because the AbstractMenuContentObject actually tries to read the cache AND rebuild it for each single menu item.

Proposed solution:

- Store an (empty) array in cache for empty menus instead of NULL.
- Allow the TYPO3\Core\Frontend\Page\PageRepository class to cache empty arrays (this class actually wraps TYPO3's caching framework, which in itself handles those values correctly).

Caveats:

- Not sure if the proposed change is backwards-compatible. Comments are welcome.

We propose the following patch as a solution (change request on Gerrit will follow shortly):

diff --git a/typo3/sysext/frontend/Classes/ContentObject/Menu/TextMenuContentObject.php b/typo3/sysext/frontend/Classes/ContentObject/Menu/TextMenuContentObject.php
index 44098de..779e5ad 100644
--- a/typo3/sysext/frontend/Classes/ContentObject/Menu/TextMenuContentObject.php
+++ b/typo3/sysext/frontend/Classes/ContentObject/Menu/TextMenuContentObject.php
@@ -33,7 +33,10 @@ class TextMenuContentObject extends \TYPO3\CMS\Frontend\ContentObject\Menu\Abstr
                $splitCount = count($this->menuArr);
                if ($splitCount) {
                        list($NOconf) = $this->procesItemStates($splitCount);
+               } else {
+                       $NOconf = array();
                }
+
                if ($this->mconf['debugItemConf']) {
                        echo '<h3>$NOconf:</h3>';
                        debug($NOconf);
diff --git a/typo3/sysext/frontend/Classes/Page/PageRepository.php b/typo3/sysext/frontend/Classes/Page/PageRepository.php
index c876d34..beb3f39 100644
--- a/typo3/sysext/frontend/Classes/Page/PageRepository.php
+++ b/typo3/sysext/frontend/Classes/Page/PageRepository.php
@@ -879,7 +879,7 @@ class PageRepository {
                $hashContent = NULL;
                $contentHashCache = GeneralUtility::makeInstance('TYPO3\\CMS\\Core\\Cache\\CacheManager')->getCache('cache_hash');
                $cacheEntry = $contentHashCache->get($hash);
-               if ($cacheEntry) {
+               if ($cacheEntry !== FALSE) {
                        $hashContent = $cacheEntry;
                }
                return $hashContent;

Related issues 2 (0 open2 closed)

Related to TYPO3 Core - Bug #66023: Performance issue with caching empty text menusClosedOliver Hader2015-03-25

Actions
Related to TYPO3 Core - Bug #57953: Rendering time of HMENU is really bad, maybe bug?Closed2014-04-15

Actions
Actions #1

Updated by Gerrit Code Review over 9 years ago

  • Status changed from New to Under Review

Patch set 1 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/35001

Actions #2

Updated by Gerrit Code Review over 9 years ago

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/35002

Actions #3

Updated by Gerrit Code Review over 9 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/35002

Actions #4

Updated by Gerrit Code Review over 9 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/35002

Actions #5

Updated by Gerrit Code Review over 9 years ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/35002

Actions #6

Updated by Christian Richter about 9 years ago

I can confirm, that the provided patch set 1 (I know I should have tried set 4, but the merge with patch 1 was so easy...) is really helping me out in my 4-Lang, 4-menu site using TYPO3 6.2.9!
I think this is indeed relevant! I would like to provide more details if needed!
Best, Christian

Actions #7

Updated by Christian Richter about 9 years ago

I just updated from 6.2.9 (patch-set-1 applied) to 6.2.10 (no patch-set applied, still 4-Lang, 4-menu, expAll) and the cache generation speed/performance dropped clearly.
I then applied patch set 4 to a 6.2.10 Core and the speed improvement on non-ssd environments are really significant!

Actions #8

Updated by Gerrit Code Review about 9 years ago

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/35002

Actions #9

Updated by Gerrit Code Review about 9 years ago

Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/35002

Actions #10

Updated by Martin Helmich about 9 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
Actions #11

Updated by Gerrit Code Review about 9 years ago

  • Status changed from Resolved to Under Review

Patch set 2 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/35001

Actions #12

Updated by Martin Helmich about 9 years ago

  • Status changed from Under Review to Resolved
Actions #13

Updated by Benni Mack over 5 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF