Task #54085

Epic #55070: Workpackages

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

Story #55078: Optimize PHP code performance in TYPO3 methods

Replace all strcmp() calls with ===

Added by Markus Klein over 1 year ago. Updated about 1 year ago.

Status:Resolved Start date:2013-11-29
Priority:Should have Due date:
Assigned To:- % Done:

100%
Category:Performance Spent time: -
Target version:6.2.0
TYPO3 Version:6.2 Complexity:easy
PHP Version: Sprint Focus:

Description

When testing for the identity of strings using the strcmp() function has worse performance than the === operator. (3-5x)

Replace all ~300 calls to strcmp() with the operator.

Attention: This is replacement only works when the return value of strcmp() is checked for being a boolean value, otherwise a string ordering is performed and the call MUST NOT be replaced!

The operator is also slower than the = operator, so use the latter one if possible.

Related issues

related to Core - Task #54265: Use (int) instead of intval() or (integer) Resolved 2013-12-07
related to Core - Task #54517: Replace substr() and strpos() with GeneralUtility::isFirs... Accepted 2013-12-19
related to Core - Bug #56248: You can not add new records (TCA) in the edit mode Resolved 2014-02-24
related to Core - Bug #58525: First usergroup from BE-user no longer applied to new pages Closed 2014-05-05
related to Core - Task #56393: creation on new pages have broken permissions (perms_grou... Resolved 2014-02-27

Associated revisions

Revision 0a761995
Added by Jo Hasenau over 1 year ago

[TASK] Replace all strcmp() calls with ===

This patch replaces about 300 places using strcmp()
in the whole core. There are different contexts for strcmp()
within this set, i.e. checking for strings being '0',
'', not '' and the like.

strcmp() has to stay when it comes to real sorting of strings,
which is a rather rare case, otherwise it can be replaced with
faster alternatives.

The following 'rules' were used for the replacement:

  • Use a type cast if you can't be sure about the incoming values.
    We do not need type casts if the types are implicitly defined
    before by another function. i.e. intval(), trim(), substr()
  • Use int-cast whenever the values to be compared are numbers only.
  • Use string-cast for any other combination. i.e
    (string)$len === '0' when $len can be NULL, which is different
    to (int)$len === 0

Resolves: #54085
Releases: 6.2
Change-Id: I88fb294ae20d8c23ff58d8296fbb37925d5213c8
Reviewed-on: https://review.typo3.org/25843
Reviewed-by: Markus Klein
Tested-by: Markus Klein

Revision 630b877b
Added by André Haehnel over 1 year ago

[BUGFIX] Adding new records in edit mode broken

It was not possible anymore to use the "add new record" button
in TCA select fields on records with pid > 0.

This patch makes it work again; the redirect to the original page
gets prevented when a pid is set. This was handled the same way
in 4.6 and 6.1.

Change-Id: Iec058818405385efdacaebf5080f339371356810
Resolves: #56248
Related: #54085
Releases: 6.2
Reviewed-on: https://review.typo3.org/27935
Reviewed-by: Helmut Hummel
Tested-by: Helmut Hummel

History

#1 Updated by Jo Hasenau over 1 year ago

I will take care of it the next days - thanks for opening the issue :-)

#2 Updated by Markus Klein over 1 year ago

@Jo: Maybe http://forge.typo3.org/issues/54091 is relevant too.

#3 Updated by Gerrit Code Review over 1 year 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/25843

#4 Updated by Gerrit Code Review over 1 year 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/25843

#5 Updated by Gerrit Code Review over 1 year 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/25843

#6 Updated by Jo Hasenau over 1 year ago

Attention: This is replacement only works when the return value of strcmp() is checked for being zero, otherwise a string ordering is performed and the call MUST NOT be replaced!

Shouldn't it be: "is checked for being zero or not being zero"

Actually strcmp is meant to be used for sorting of strings, since it can return positive or negative values for cases when string A is "larger" or "smaller" than string B. It will return 0 when string "A" and string "B" are equal.

So any case that is just checking for the result of strcmp being TRUE or FALSE could IMHO be safely replaced.
In some cases we might have to add isset() before the === operator since NULL could be !== '' but still might have to be considered ''.

Anyway even

isset($foo) && $foo !== ''

will perform better, since it will just skip the operator in cases when $foo is NULL and it will no create any copies, so the memory footprint will be significantly smaller.

#7 Updated by Markus Klein over 1 year ago

I updated the description to be more precise about the boolean usage.

And it would be great to see some memory footprint in the end. The question surely is not if we have a reduction, but rather how much it finally really is. ;-)

#8 Updated by Gerrit Code Review over 1 year 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/25843

#9 Updated by Gerrit Code Review over 1 year 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/25843

#10 Updated by Gerrit Code Review over 1 year ago

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

#11 Updated by Gerrit Code Review over 1 year 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/25843

#12 Updated by Gerrit Code Review over 1 year ago

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

#13 Updated by Gerrit Code Review over 1 year 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/25843

#14 Updated by Gerrit Code Review over 1 year 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/25843

#15 Updated by Gerrit Code Review over 1 year 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/25843

#16 Updated by Gerrit Code Review over 1 year ago

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

#17 Updated by Gerrit Code Review over 1 year ago

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

#18 Updated by Gerrit Code Review over 1 year ago

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

#19 Updated by Gerrit Code Review over 1 year ago

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

#20 Updated by Gerrit Code Review over 1 year ago

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

#21 Updated by Gerrit Code Review over 1 year ago

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

#22 Updated by Gerrit Code Review over 1 year ago

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

#23 Updated by Gerrit Code Review over 1 year ago

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

#24 Updated by Gerrit Code Review over 1 year ago

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

#25 Updated by Gerrit Code Review over 1 year ago

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

#26 Updated by Gerrit Code Review over 1 year ago

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

#27 Updated by Gerrit Code Review over 1 year ago

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

#28 Updated by Gerrit Code Review over 1 year ago

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

#29 Updated by Gerrit Code Review over 1 year ago

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

#30 Updated by Gerrit Code Review over 1 year ago

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

#31 Updated by Gerrit Code Review over 1 year ago

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

#32 Updated by Gerrit Code Review over 1 year ago

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

#33 Updated by Gerrit Code Review over 1 year ago

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

#34 Updated by Jo Hasenau over 1 year ago

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

#35 Updated by Ernesto Baschny over 1 year ago

  • Target version changed from next-patchlevel to 6.2.0
  • Parent task changed from #52949 to #55078

#36 Updated by Markus Klein about 1 year ago

Regression in #56393

Also available in: Atom PDF