Task #55167

Epic #55070: Workpackages

Epic #55065: WP: Overall System Performance (Backend and Frontend)

Epic #55656: Optimize overall Extbase performance

Story #55168: Optimize Extbase generic persistence

Extbase: avoid parseQuery() / buildQuery() to be called multiple times

Added by Felix Oertel over 5 years ago. Updated almost 2 years ago.

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

100%

TYPO3 Version:
6.2
PHP Version:
Tags:
Complexity:
hard
Sprint Focus:

Description

At the moment the extbase persistence runs buildQuery() and parseQuery() for every single getObjectDataByQuery() / getObjectCountByQuery(). When fetching child objects, this is called for every single object. This results in a lot of re-run code. In a simple blog_example page, this is called up to 200 times.

We will try to cache the built query as in an live environment always the same queries should be run, just with different parameters. Special care is to be taken for which parts of the query are "static" and about to be built into the cache identifier.

We will have to give the developer the chance to disable query caching though, because in some cases it might result in real big data piles. (e. g. matrix-like queries, allowing the front-end to influence the number of queried properties (search)).


Subtasks

Task #55661: Add functional tests for extbase persistenceClosedFelix Oertel

Task #57173: Use of cf_extbase_typo3dbbackend_queries results in fatalClosedXavier Perseguers

Task #57130: Extbase Queries with NULL comparison don't work anymoreClosed

Task #57104: Regression: Query building fails for LazyObjectStorageClosed

Task #57263: Typo3DbQueryParser does not handle LowerCaseInterface correctlyClosedFelix Oertel

Task #57338: Make Typo3DbQueryParser SingletonClosed


Related issues

Related to TYPO3 Core - Task #53514: Extbase: DataMapper causes redundant queries Closed 2013-11-11
Related to TYPO3 Core - Bug #57053: Regression: Extbase query builder uses invalid parameter in parseComparison() Closed 2014-03-18
Related to TYPO3 Core - Task #57280: Introduce setting to be able to disable Extbase query cache for all queries. Closed 2014-03-25
Related to TYPO3 Core - Task #57275: Add first level cache for the query cache Closed 2014-03-25
Follows TYPO3 Core - Task #55360: Extbase: use preparedStatement in generic persistence Closed 2014-03-19

Associated revisions

Revision cf42649b (diff)
Added by Felix Oertel over 5 years ago

[TASK] Add functional tests for generic persistence

To backup the changes in Extbase's generic persistence,
we add some functional tests.

Resolves: #55661
Related: #55360
Related: #55167
Releases: 6.2
Change-Id: I0b845618b1cec9788ca36ac59e25ea6779f01cb6
Reviewed-on: https://review.typo3.org/28262
Reviewed-by: Marc Bastian Heinrichs
Tested-by: Marc Bastian Heinrichs
Reviewed-by: Steffen Ritter
Tested-by: Steffen Ritter

Revision 57caf5a4 (diff)
Added by Ernesto Baschny over 5 years ago

[TASK] Follow-up to "Add functional tests for generic persistence"

Test directory changed from "Relation" to "Persistence".

Resolves: #55661
Related: #55360
Related: #55167
Releases: 6.2

Change-Id: I422dbfb6bdad4215bdc70dea775917cf3159605f
Reviewed-on: https://review.typo3.org/28297
Reviewed-by: Ernesto Baschny
Tested-by: Ernesto Baschny

Revision 2583fd2a (diff)
Added by Felix Oertel over 5 years ago

[TASK] Use queryCache in generic persistence

At the moment extbase persistence is quite slow. To some degree,
the query building is responsible for that. For each and every
query the parseQuery() and buildQuery() methods are called.

This patch introduces a query cache to store the query structure.
This way a query can be reused, even when fired with different
parameters.

  • Introduce a parameterIdentifier to the comparison
    In the way the extbase query object model works, it's hard to
    identify an object in a way which is unique and still compre-
    hensible. To ease this, an identifier is introduced.
  • Move parsing to an external QueryParser
    All the query parsing is now seperated from the storageBackend.
    The query parsing is mostly left the way it was, it should be
    re-profiled later.
  • introduce cache extbase_typo3dbbackend_queries
  • No more statement handling by reference
    Make getStatementParts() return the query and parameters in an
    array instead of handing down the parameters by reference.
  • Split up parseQuery in preparsing and real parsing with caching
    In every call the query has to be preparsed to determine the
    identifier and extract the parameters. The identifier can then
    be used to look up the query structure in the cache. If it is
    not (yet) cached, Extbase will do the real parsing which takes
    some more time.
  • Introduce useQueryCache to querySettings
    Like preparedStatements, caching the queries does not always
    make sense and will spam your cache under certain conditions.
    It's enabled by default and can be disabled on a per-query-
    basis.
  • Use named parameter's placeholders
    Instead of using "?" as parameter's placeholder and relying
    on the right order of parameters in the array, named place-
    holders are used to ensure the correct association.
  • Move unit tests around to fit the new queryParser

Resolves: #55167
Releases: 6.2
Change-Id: I517c0500cad75cae96fd739f2ff7222db80b42a5
Reviewed-on: https://review.typo3.org/27555
Reviewed-by: Anja Leichsenring
Tested-by: Anja Leichsenring
Tested-by: Philipp Gampe
Reviewed-by: Wouter Wolters
Tested-by: Wouter Wolters

Revision b8edbf13 (diff)
Added by Alexander Stehlik over 5 years ago

[BUGFIX] Named parameters in Extbase comparison queries

Instead of using unnamend parameters (?) in parseComparison() in the
Extbase Typo3DbQueryParser the matching parameter identifier will be
used for building the comparison SQL.

Additionally the non existing "$parameters" method parameter is removed
from a lot of method calls.

Resolves: #57053
Related: #55167
Releases: 6.2
Change-Id: I1123275af57e8803ecd792d65f91627cef54f501
Reviewed-on: https://review.typo3.org/28511
Reviewed-by: Felix Oertel
Tested-by: Felix Oertel
Reviewed-by: Wouter Wolters
Reviewed-by: Markus Klein
Reviewed-by: Xavier Perseguers
Tested-by: Xavier Perseguers

Revision 8326a71f (diff)
Added by Klaas Johan Kooistra over 5 years ago

[BUGFIX] Queries with NULL don't work anymore

See change: I517c0500cad75cae96fd739f2ff7222db80b42a5
At the end of the Typo3DbQueryParser::parseComparison() method the updated
operator for NULL values was no longer passed to the parseDynamicOperand,
generating = NULL instead of IS NULL.

Solved the issue by adding the operator parameter again.

Change-Id: I13c3cecf0bf4c95789032f31ead192b945794484
Resolves: #57130
Related: #55167
Releases: 6.2
Reviewed-on: https://review.typo3.org/28592
Reviewed-by: Markus Klein
Reviewed-by: Felix Oertel
Tested-by: Felix Oertel

Revision 449c81e4 (diff)
Added by Felix Oertel over 5 years ago

[BUGFIX] FOLLOWUP Queries with NULL don't work

In https://review.typo3.org/28592 we apparently did not
fix the whole problem. This followup will deal with
the edge-case of using a similar built statement, which
results in the same hash being generated from the
QueryParser.

Resolves: #57130
Related: #55167
Releases: 6.2
Change-Id: I02a478fcabb89d11054f8f0a3c800965edfd7844
Reviewed-on: https://review.typo3.org/28715
Reviewed-by: Marc Bastian Heinrichs
Tested-by: Marc Bastian Heinrichs
Reviewed-by: Markus Klein
Tested-by: Markus Klein

History

#1 Updated by Felix Oertel over 5 years ago

  • Parent task set to #55168

#2 Updated by Felix Oertel over 5 years ago

  • Subject changed from avoid buildQuery() to be called multiple times to avoid parseQuery() / buildQuery() to be called multiple times

#3 Updated by Alexander Schnitzler over 5 years ago

Did you already have a look at ticket #53514? Saves a lot of queries, too.

Edit: wrong ticket number.

#4 Updated by Felix Oertel over 5 years ago

yep, got this on my list, thanks.

#5 Updated by Gerrit Code Review over 5 years ago

  • Status changed from Accepted to Under Review
  • Estimated time set to 0.00 h

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/27555

#6 Updated by Ernesto Baschny over 5 years ago

  • Subject changed from avoid parseQuery() / buildQuery() to be called multiple times to Extbase: avoid parseQuery() / buildQuery() to be called multiple times
  • Category set to Extbase
  • Estimated time set to 0.00 h
  • TYPO3 Version set to 6.2

#7 Updated by Gerrit Code Review over 5 years ago

  • Estimated time set to 0.00 h

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/27555

#8 Updated by Gerrit Code Review over 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 https://review.typo3.org/27555

#9 Updated by Gerrit Code Review over 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 https://review.typo3.org/27555

#10 Updated by Gerrit Code Review over 5 years ago

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

#11 Updated by Gerrit Code Review over 5 years ago

  • Estimated time set to 0.00 h

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

#12 Updated by Gerrit Code Review over 5 years ago

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

#13 Updated by Gerrit Code Review over 5 years ago

  • Estimated time set to 0.00 h

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

#14 Updated by Gerrit Code Review over 5 years ago

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

#15 Updated by Gerrit Code Review over 5 years ago

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

#16 Updated by Gerrit Code Review over 5 years ago

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

#17 Updated by Anonymous over 5 years ago

  • Status changed from Under Review to Resolved

#18 Updated by Alexander Stehlik over 5 years ago

After the patch was applied I get this SQL error with the news extension:

SELECT COUNT(*) 
FROM tx_news_domain_model_news
WHERE
    (
        tx_news_domain_model_news.uid IN (
            SELECT uid_local FROM tx_news_domain_model_news_category_mm WHERE uid_foreign=?
        )
        AND tx_news_domain_model_news.pid IN ('99999999')
    )
    AND tx_news_domain_model_news.deleted=0 AND tx_news_domain_model_news.t3ver_state<=0
    AND tx_news_domain_model_news.pid<>-1 AND tx_news_domain_model_news.hidden=0
    AND tx_news_domain_model_news.starttime<=1395170580 AND (tx_news_domain_model_news.endtime=0 OR tx_news_domain_model_news.endtime>1395170580)
    AND (tx_news_domain_model_news.fe_group=''
        OR tx_news_domain_model_news.fe_group IS NULL
        OR tx_news_domain_model_news.fe_group='0'
        OR FIND_IN_SET('0',tx_news_domain_model_news.fe_group)
        OR FIND_IN_SET('-1',tx_news_domain_model_news.fe_group)
    )
    AND (tx_news_domain_model_news.sys_language_uid IN (0,-1))

This is a var_dump of the sql query parts:

array (size=9)
  'keywords' => 
    array (size=0)
      empty
  'tables' => 
    array (size=1)
      'tx_news_domain_model_news' => string 'tx_news_domain_model_news' (length=25)
  'unions' => 
    array (size=0)
      empty
  'fields' => 
    array (size=1)
      'tx_news_domain_model_news' => string 'tx_news_domain_model_news.*' (length=27)
  'where' => 
    array (size=5)
      0 => string '(' (length=1)
      1 => string 'tx_news_domain_model_news.uid IN (SELECT uid_local FROM tx_news_domain_model_news_category_mm WHERE uid_foreign=?)' (length=114)
      2 => string ' AND ' (length=5)
      3 => string 'tx_news_domain_model_news.pid IN (:AND1pid)' (length=43)
      4 => string ')' (length=1)
  'additionalWhereClause' => 
    array (size=0)
      empty
  'orderings' => 
    array (size=1)
      0 => string 'tx_news_domain_model_news.datetime DESC' (length=39)
  'limit' => null
  'offset' => null

This is the error:

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '?) AND tx_news_domain_model_news.pid IN ('99999999')) AND tx_news_domain_model_n' at line 1

I think the problem is the unnamed parameter (?) in the uid_foreign query.

I also pointed out some problems in the code in Gerrit.

#19 Updated by Alexander Stehlik over 5 years ago

Think I found it. Regression in https://forge.typo3.org/issues/57053

#20 Updated by Alexander Stehlik over 5 years ago

  • Estimated time set to 0.00 h

Found another regression in #57104 :(

#21 Updated by Peter Beernink over 5 years ago

Also #57130 seems to be another regression..

#22 Updated by Alexander Stehlik over 5 years ago

  • Estimated time deleted (0.00 h)

#23 Updated by Riccardo De Contardi almost 2 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF