CoreExtension::getAttribute: small improvement regarding getter/isser/hasser #4662
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
b173e57 to
f8d742e
Compare
f8d742e to
45cd6ff
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR optimizes the method name caching in Twig's CoreExtension by improving how getter/is/has method names are cached to reduce unnecessary string operations during attribute resolution. The main goal is to cache method names in their camelCase form (e.g., firstName instead of FirstName) so that common property access patterns like person.firstName can be resolved more efficiently.
Key changes:
- Refactored prefix handling for getter/is/has methods to use a common
$prefixLengthvariable - Modified the cache key generation to store camelCase names (e.g.,
firstName) instead of PascalCase names (e.g.,FirstName) - Updated comments to reflect the improved caching strategy
|
I suppose we're looking for a performance improvement here. --- a/src/Extension/CoreExtension.php
+++ b/src/Extension/CoreExtension.php
@@ -1826,17 +1826,13 @@ final class CoreExtension extends AbstractExtension
$lcMethods = array_map('strtolower', $methods);
$classCache = [];
foreach ($methods as $i => $method) {
- $classCache[$method] = $method;
$classCache[$lcName = $lcMethods[$i]] = $method;
if ('g' === $lcName[0] && str_starts_with($lcName, 'get')) {
- $name = substr($method, 3);
$lcName = substr($lcName, 3);
} elseif ('i' === $lcName[0] && str_starts_with($lcName, 'is')) {
- $name = substr($method, 2);
$lcName = substr($lcName, 2);
} elseif ('h' === $lcName[0] && str_starts_with($lcName, 'has')) {
- $name = substr($method, 3);
$lcName = substr($lcName, 3);
if (\in_array('is'.$lcName, $lcMethods, true)) {
continue;
@@ -1846,23 +1842,15 @@ final class CoreExtension extends AbstractExtension
}
// skip get() and is() methods (in which case, $name is empty)
- if ($name) {
- if (!isset($classCache[$name])) {
- $classCache[$name] = $method;
- }
-
- if (!isset($classCache[$lcName])) {
- $classCache[$lcName] = $method;
- }
+ if ($lcName && !isset($classCache[$lcName])) {
+ $classCache[$lcName] = $method;
}
}
$cache[$class] = $classCache;
}
$call = false;
- if (isset($cache[$class][$item])) {
- $method = $cache[$class][$item];
- } elseif (isset($cache[$class][$lcItem = strtolower($item)])) {
+ if (isset($cache[$class][$lcItem = strtolower($item)])) {
$method = $cache[$class][$lcItem];
} elseif (isset($cache[$class]['__call'])) {
$method = $item;Memory should be a bit less and I expect the performance to be (almost) the same. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For a getter method like
getFirstNameit is common to call it in twig viaperson.firstName.But at the moment twig is adding these variants to the class method cache:
getFirstName,getfirstname,FirstNameandfirstname.So when resolving the name, it uses the first
elseifhere with additionalstrtolowercall, becausefirstNameis missing:Twig/src/Extension/CoreExtension.php
Lines 1863 to 1867 in 403bd9d
This PR replaces
FirstNamewithfirstNamein the method cache.So
person.firstNameis resolved via firstifbranch (butperson.FirstNamewould use theelseifwithstrtolowernow).