Bug #63550

Menu configuration caching disabled by ineffective type checks

Added by Martin Helmich almost 5 years ago. Updated about 1 year ago.

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

100%

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 menus Closed 2015-03-25
Related to TYPO3 Core - Bug #57953: Rendering time of HMENU is really bad, maybe bug? New 2014-04-15

Associated revisions

Revision 687b2657 (diff)
Added by Martin Helmich over 4 years ago

[BUGFIX] Re-enable menu configuration caching

It is ensured that the menu configuration is always an array.

The PageRepository::getHash() method is extended to return the
raw cache content. This allows to cache empty arrays.

Change-Id: If92c80feabb8e68b66497827667b9fd0ab1c214e
Resolves: #63550
Releases: master, 6.2
Reviewed-on: http://review.typo3.org/35002
Reviewed-by: Markus Klein <>
Reviewed-by: Benjamin Mack <>
Reviewed-by: Arjen Hoekema <>
Tested-by: Arjen Hoekema <>
Tested-by: Markus Klein <>

Revision 26bb5e0a (diff)
Added by Martin Helmich over 4 years ago

[BUGFIX] Re-enable menu configuration caching

It is ensured that the menu configuration is always an array.

The PageRepository::getHash() method is extended to return the
raw cache content. This allows to cache empty arrays.

Change-Id: If92c80feabb8e68b66497827667b9fd0ab1c214e
Resolves: #63550
Releases: master, 6.2
Reviewed-on: http://review.typo3.org/35001
Reviewed-by: Markus Klein <>
Tested-by: Markus Klein <>

History

#1 Updated by Gerrit Code Review almost 5 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

#2 Updated by Gerrit Code Review almost 5 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

#3 Updated by Gerrit Code Review almost 5 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

#4 Updated by Gerrit Code Review almost 5 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

#5 Updated by Gerrit Code Review almost 5 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

#6 Updated by Christian Richter over 4 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

#7 Updated by Christian Richter over 4 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!

#8 Updated by Gerrit Code Review over 4 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

#9 Updated by Gerrit Code Review over 4 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

#10 Updated by Martin Helmich over 4 years ago

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

#11 Updated by Gerrit Code Review over 4 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

#12 Updated by Martin Helmich over 4 years ago

  • Status changed from Under Review to Resolved

#13 Updated by Benni Mack about 1 year ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF