Bug #63550

Menu configuration caching disabled by ineffective type checks

Added by Martin Helmich about 6 years ago. Updated over 2 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

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?Needs Feedback2014-04-15

Actions

Also available in: Atom PDF