Task #90104

Introduce prepared statement for BackendUtility::getPageForRootline

Added by Markus Klein 9 months ago. Updated 8 months ago.

Status:
Closed
Priority:
Should have
Assignee:
Category:
Backend API
Target version:
-
Start date:
2020-01-13
Due date:
% Done:

100%

TYPO3 Version:
9
PHP Version:
Tags:
Complexity:
Sprint Focus:

Description

This function is called a lot, hence the DB statement should be reused.


Related issues

Related to TYPO3 Core - Epic #88474: Page tree performance in 9.5 New 2018-11-16
Related to TYPO3 Core - Bug #90434: getPageForRootline() caches the SQL statement too greedy Closed 2020-02-19
Related to TYPO3 Core - Bug #90535: 10.3.0 throws PHP Warning: mysqli_stmt::bind_param(): Couldn't fetch mysqli_stmt in /vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php line 236 Closed 2020-02-25
Related to TYPO3 Core - Bug #91206: PDOException "You cannot serialize or unserialize PDOStatement instances" in PopulatePageSlugs wizard Closed 2020-04-27

Associated revisions

Revision cd0c343a (diff)
Added by Markus Klein 9 months ago

[TASK] Use prepared statement in BackendUtility::getPageForRootline

The function is already caching the result for individual pages.
Still it is called for a lot of pages, so even more speed
can be gained by also re-using the database statement.

Specifically the page-tree rendering time reduces a lot with this.

Resolves: #90104
Releases: master, 9.5
Change-Id: I35308e163cd93b22c12ecf743759b2f19e2a7ad9
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/62867
Tested-by: TYPO3com <>
Tested-by: Benni Mack <>
Tested-by: Susanne Moog <>
Reviewed-by: Benni Mack <>
Reviewed-by: Susanne Moog <>

Revision a8116815 (diff)
Added by Markus Klein 9 months ago

[TASK] Use prepared statement in BackendUtility::getPageForRootline

The function is already caching the result for individual pages.
Still it is called for a lot of pages, so even more speed
can be gained by also re-using the database statement.

Specifically the page-tree rendering time reduces a lot with this.

Resolves: #90104
Releases: master, 9.5
Change-Id: I35308e163cd93b22c12ecf743759b2f19e2a7ad9
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/62902
Tested-by: TYPO3com <>
Tested-by: Susanne Moog <>
Reviewed-by: Susanne Moog <>

History

#1 Updated by Gerrit Code Review 9 months ago

  • Status changed from New to Under Review

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

#2 Updated by Markus Klein 9 months ago

  • Related to Epic #88474: Page tree performance in 9.5 added

#3 Updated by Gerrit Code Review 9 months ago

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

#4 Updated by Gerrit Code Review 9 months ago

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

#5 Updated by Gerrit Code Review 9 months ago

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

#6 Updated by Gerrit Code Review 9 months ago

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

#7 Updated by Gerrit Code Review 9 months ago

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

#8 Updated by Gerrit Code Review 9 months ago

Patch set 1 for branch 9.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/62902

#9 Updated by Markus Klein 9 months ago

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

#10 Updated by Wolfgang Klinger 8 months ago

This introduces a big (at least for me) regression, as the 'additionalFields' are not taken into account here!

So it's impossible to fetch a rootline with any other fields than the default fields hardcoded in the class.

#11 Updated by Markus Klein 8 months ago

@Wolfgang: Can you please explain why this change should be responsible? I did not change the query itself.

#12 Updated by Wolfgang Klinger 8 months ago

            $statement = $runtimeCache->get('getPageForRootlineStatement');

statement is fetched without taking the additionalFields into account.
So when it's cached once, you can pass whatever additionalFields parameter you want, you'll always only get an array of the hardcoded fields of the first call.

So imho this

            $statement = $runtimeCache->get('getPageForRootlineStatement');

should be (only drafted quickly):

            $statement = $runtimeCache->get('getPageForRootlineStatement' . md5(implode(',', $additionalFields)));


and
              $runtimeCache->set('getPageForRootlineStatement' . md5(implode(',', $additionalFields)), $statement);

#13 Updated by Markus Klein 8 months ago

  • Related to Bug #90434: getPageForRootline() caches the SQL statement too greedy added

#14 Updated by Georg Ringer 8 months ago

  • Related to Bug #90535: 10.3.0 throws PHP Warning: mysqli_stmt::bind_param(): Couldn't fetch mysqli_stmt in /vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php line 236 added

#15 Updated by Benni Mack 8 months ago

  • Status changed from Resolved to Closed

#16 Updated by Markus Klein 6 months ago

  • Related to Bug #91206: PDOException "You cannot serialize or unserialize PDOStatement instances" in PopulatePageSlugs wizard added

Also available in: Atom PDF