Task #54265
closedEpic #55070: Workpackages
Epic #55065: WP: Overall System Performance (Backend and Frontend)
Story #55078: Optimize PHP code performance in TYPO3 methods
Use (int) instead of intval() or (integer)
Added by Jo Hasenau over 10 years ago. Updated almost 7 years ago.
100%
Description
Description
When testing for the identity of integers using the intval() function has worse performance than a type cast with (int). (3-5x)
Replace all ~1600 calls to intval() with a type cast.
Updated by Jo Hasenau over 10 years ago
Actually the impact is around factor 2.5 not 3 to 5 but still it should be worth switching from intval() to (int).
There is one execpetion though, which is the "base" parameter of the intval() function.
Don't know yet, if this is actually used within the TYPO3 core, but if it is, then intval() should not be replaced.
Updated by Markus Klein over 10 years ago
Hi! Thanks for fiddling around with this.
Before we invest that much time here (the replacement with a regexp is fast, but the review is very cumbersome), we'd discuss this on the core list first.
I've a gut feeling that not too many devs will support this and therefor it might be hard to get it merged finally.
Cheers Markus
Updated by Steffen Ritter over 10 years ago
We discuessed this shortly on the release team meeting as we are concepting the performance improvements.
We are in - but need to make sure that it's really the same :)
Updated by Mathias Schreiber over 10 years ago
Hi joey,
in general: great idea.
I suggest changing the topic to "replace intval() with (int) where it makes sense" in order to make it sound less ranty
Updated by Steffen Ritter over 10 years ago
- Subject changed from Replace all intval() with (int) to Replace all intval() with (int) wherever it makes sense
Updated by Jo Hasenau over 10 years ago
So I will go for it but do it the "semiautomatic" way.
It might take a bit more patchsets than #54085 but IMHO it was working quite well to have a limited number of changes per patchset. So reviewing will be easier and discussing can happen on the way through the patchsets. After all there might be less context related things to discuss than with strcmp().
BTW: According to some PHP benchmarking pages I have some more candidates like array_unique or array_key_exists on my list. So if you guys already found some others, feel free to assign the tasks to me.
Updated by Stefan Froemken over 10 years ago
Please correct me, but I may read that we want to use (integer) instead of (int). Same with (boolean) instead of (bool). I think we should use the same style everywhere.
Updated by Jo Hasenau over 10 years ago
Agreed to use the same style, but in that case it should be (int) and (bool) for everything, since these are less "bloaty" while still easily understandable.
Updated by Stefan Neufeind over 10 years ago
Please see current thread on the core-mailinglist about CGL-decisions like these ("integer or int"). There is a notes-document to sum up decisions, their pros/cons etc.
Updated by Jo Hasenau over 10 years ago
So could someone take care of that document and confirm one or the other way.
https://notes.typo3.org/p/CGL-decisions
It's not necessary to decide everything but at least the (bool) (int) vs. (boolean) (integer) thingie should be decided before I go on with this task.
Updated by Gerrit Code Review over 10 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/26740
Updated by Gerrit Code Review over 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 https://review.typo3.org/26740
Updated by Gerrit Code Review over 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 https://review.typo3.org/26740
Updated by Gerrit Code Review over 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 https://review.typo3.org/26740
Updated by Gerrit Code Review over 10 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/26740
Updated by Chris topher over 10 years ago
- Subject changed from Replace all intval() with (int) wherever it makes sense to Use (integer) instead of intval() or (int)
Updated by Gerrit Code Review over 10 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/26740
Updated by Gerrit Code Review over 10 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/26740
Updated by Gerrit Code Review over 10 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/26740
Updated by Gerrit Code Review over 10 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/26740
Updated by Gerrit Code Review over 10 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/26740
Updated by Gerrit Code Review over 10 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/26740
Updated by Gerrit Code Review over 10 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/26740
Updated by Gerrit Code Review over 10 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/26740
Updated by Gerrit Code Review over 10 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/26740
Updated by Gerrit Code Review over 10 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/26740
Updated by Ernesto Baschny over 10 years ago
- Parent task changed from #52949 to #55078
Updated by Ingo Schmitt over 10 years ago
Core decision as from today: it should be written (int)$variable
Updated by Gerrit Code Review over 10 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/26740
Updated by Gerrit Code Review over 10 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/26740
Updated by Gerrit Code Review over 10 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/26740
Updated by Bernhard Kraft over 10 years ago
What are the (php internal?) differences between intval() and a (int) typecast?
Is it only that intval() takes a base argument (defaulting to 10) and the difference in speed or are there other specialities?
Could someone point me to a document on that case?
Updated by Jo Hasenau over 10 years ago
It's exactly that: A difference in speed and memory consumption.
The rule of thumb is:
- The right operator or typecast is better than
- the right native function is better than
- your own optimized method for the same purpose
So as long as there is no need for a different base, you should do a typecast to (int) instead of intval().
This might be irrelevant with future PHP versions, since the PHP guys might improve performance as well, but still we can be sure that the function call will never outperform the typecast.
http://www.php.net/manual/de/language.types.type-juggling.php
http://www.php.net/manual/de/language.types.integer.php#language.types.integer.casting
<quote>
To explicitly convert a value to integer, use either the (int) or (integer) casts. However, in most cases the cast is not needed, since a value will be automatically converted if an operator, function or control structure requires an integer argument. A value can also be converted to integer with the intval() function.
</quote>
Updated by Gerrit Code Review over 10 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/26740
Updated by Gerrit Code Review over 10 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/26740
Updated by Gerrit Code Review over 10 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/26740
Updated by Gerrit Code Review over 10 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/26740
Updated by Jo Hasenau over 10 years ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset 9646d26bac991dd23b5d58e7b85ab72d7a4af870.
Updated by Simon Schaufelberger over 10 years ago
can somebody please share the script how to do that automatically or was everything done manually? I want to use that for my own extensions as well!
Updated by Jo Hasenau over 10 years ago
This was done manually, since I wanted to take care of surrounding issues as well, which would have been impossible with an automated search and replace.
The goal was to get rid of superfluous method calls and typecasts additionally. So the only helper was PHPstorm to find the occurrences and provide a local history of the changes.
Updated by Chris topher over 10 years ago
- Subject changed from Use (integer) instead of intval() or (int) to Use (int) instead of intval() or (integer)
Updated by Riccardo De Contardi almost 7 years ago
- Status changed from Resolved to Closed