Bug #63550
closedMenu configuration caching disabled by ineffective type checks
100%
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;
Updated by Gerrit Code Review almost 10 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
Updated by Gerrit Code Review almost 10 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
Updated by Gerrit Code Review almost 10 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
Updated by Gerrit Code Review almost 10 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
Updated by Gerrit Code Review almost 10 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
Updated by Christian Richter over 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
Updated by Christian Richter over 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!
Updated by Gerrit Code Review over 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
Updated by Gerrit Code Review over 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
Updated by Martin Helmich over 9 years ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset 687b26571b73c075c8e0d075a16d3ae647374202.
Updated by Gerrit Code Review over 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
Updated by Martin Helmich over 9 years ago
- Status changed from Under Review to Resolved
Applied in changeset 26bb5e0a8433e1ecbc059c8e61172257771d8175.