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 almost 6 years ago. Updated almost 2 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
Performance
Target version:
Start date:
2013-11-29
Due date:
% Done:

100%

TYPO3 Version:
6.2
PHP Version:
Tags:
Complexity:
easy
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 TYPO3 Core - Task #54265: Use (int) instead of intval() or (integer) Closed 2013-12-07
Related to TYPO3 Core - Task #54517: Replace substr() and strpos() with GeneralUtility::isFirstPartOfString when it makes sense Rejected 2013-12-19
Related to TYPO3 Core - Bug #56248: You can not add new records (TCA) in the edit mode Closed 2014-02-24
Related to TYPO3 Core - Bug #58525: First usergroup from BE-user no longer applied to new pages Closed 2014-05-05
Related to TYPO3 Core - Task #56393: creation on new pages have broken permissions (perms_groupid set wrong) Closed 2014-02-27

Associated revisions

Revision 0a761995 (diff)
Added by Jo Hasenau almost 6 years 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 (diff)
Added by André Haehnel over 5 years 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 almost 6 years ago

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

#2 Updated by Markus Klein almost 6 years ago

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

#3 Updated by Gerrit Code Review almost 6 years 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 almost 6 years 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 almost 6 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/25843

#6 Updated by Jo Hasenau almost 6 years 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 almost 6 years 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 almost 6 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/25843

#9 Updated by Gerrit Code Review almost 6 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/25843

#10 Updated by Gerrit Code Review almost 6 years 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 almost 6 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/25843

#12 Updated by Gerrit Code Review almost 6 years 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 almost 6 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/25843

#14 Updated by Gerrit Code Review almost 6 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/25843

#15 Updated by Gerrit Code Review almost 6 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/25843

#16 Updated by Gerrit Code Review almost 6 years 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 almost 6 years 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 almost 6 years 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 almost 6 years 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 almost 6 years 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 almost 6 years 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 almost 6 years 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 almost 6 years 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 almost 6 years 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 almost 6 years 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 almost 6 years 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 almost 6 years 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 almost 6 years 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 almost 6 years 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 almost 6 years 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 almost 6 years 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 almost 6 years 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 almost 6 years 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 almost 6 years ago

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

#35 Updated by Ernesto Baschny over 5 years ago

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

#36 Updated by Markus Klein over 5 years ago

Regression in #56393

#37 Updated by Riccardo De Contardi almost 2 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF