From e24a798d8f84f71b8d3f34063ca6856d715a1076 Mon Sep 17 00:00:00 2001 From: Michael Lee Date: Mon, 3 Aug 2015 15:13:18 +0800 Subject: [PATCH] [property-access] Improvement for Accessing Reference Chain Improve performance for the following scenarios: - Example 1: ```php $a = array( 'a' => array( 'b' => array( 'c' => 'old-value' ) ) ); $pa->setValue($a, '[a][b][c]', 'new-value'); // The PropertyAccessor will try to set values for // $a['a']['b']['c'], $a['a']['b'] and $a['a'], // but in fact it may terminate the loop // right after the value of $a[a][b][c] is set, // because $a, $[a], $[a][b] and $[a][b][c] // are all passed as reference - the reference chain is not broken. ``` - Example 2 ```php $b = array( 'a' => array( 'b' => array( 'c' => 'old-value' ) ) ) $a = new Foo($b); // In this example, the reference chain of $b is broken, // because it's passed to $a.value as value // But its elements are all passed as reference, // so after setting the value for $b[a][b][c], there is no need // to set value for $b[a][b] and $b[a] $pa->setValue($a, 'value[a][b][c]', 'new-value'); ``` | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a --- .../PropertyAccess/PropertyAccessor.php | 45 +++++++++++++++---- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php index d4e8c78a9b..3b7a50e2fe 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php @@ -25,6 +25,7 @@ class PropertyAccessor implements PropertyAccessorInterface { const VALUE = 0; const IS_REF = 1; + const IS_REF_CHAINED = 2; /** * @var bool @@ -75,6 +76,7 @@ class PropertyAccessor implements PropertyAccessorInterface array_unshift($propertyValues, array( self::VALUE => &$objectOrArray, self::IS_REF => true, + self::IS_REF_CHAINED => true, )); for ($i = count($propertyValues) - 1; $i >= 0; --$i) { @@ -82,14 +84,33 @@ class PropertyAccessor implements PropertyAccessorInterface $property = $propertyPath->getElement($i); - if ($propertyPath->isIndex($i)) { - $this->writeIndex($objectOrArray, $property, $value); - } else { - $this->writeProperty($objectOrArray, $property, $value); - } + // You only need set value for current element if: + // 1. it's the parent of the last index element + // OR + // 2. its child is not passed by reference + // + // This may avoid uncessary value setting process for array elements. + // For example: + // '[a][b][c]' => 'old-value' + // If you want to change its value to 'new-value', + // you only need set value for '[a][b][c]' and it's safe to ignore '[a][b]' and '[a]' + // + if ($i === count($propertyValues) - 1 || !$propertyValues[$i + 1][self::IS_REF]) { + if ($propertyPath->isIndex($i)) { + $this->writeIndex($objectOrArray, $property, $value); + } else { + $this->writeProperty($objectOrArray, $property, $value); + } + + // if current element is an object + // OR + // if current element's reference chain is not broken - current element + // as well as all its ancients in the property path are all passed by reference, + // then there is no need to continue the value setting process + if (is_object($propertyValues[$i][self::VALUE]) || $propertyValues[$i][self::IS_REF_CHAINED]) { + return; + } - if ($propertyValues[$i][self::IS_REF] && is_object($objectOrArray)) { - return; } $value = &$objectOrArray; @@ -132,6 +153,7 @@ class PropertyAccessor implements PropertyAccessorInterface array_unshift($propertyValues, array( self::VALUE => $objectOrArray, self::IS_REF => true, + self::IS_REF_CHAINED => true, )); for ($i = count($propertyValues) - 1; $i >= 0; --$i) { @@ -149,7 +171,7 @@ class PropertyAccessor implements PropertyAccessorInterface } } - if ($propertyValues[$i][self::IS_REF] && is_object($objectOrArray)) { + if (is_object($propertyValues[$i][self::VALUE]) || $propertyValues[$i][self::IS_REF_CHAINED]) { return true; } } @@ -232,6 +254,13 @@ class PropertyAccessor implements PropertyAccessorInterface throw new UnexpectedTypeException($objectOrArray, $propertyPath, $i + 1); } + // Set the IS_REF_CHAINED flag to true if: + // current property is passed by reference and + // it is the first element in the property path or + // the IS_REF_CHAINED flag of its parent element is true + // Basically, this flag is true only when the reference chain from the top element to current element is not broken + $propertyValue[self::IS_REF_CHAINED] = $propertyValue[self::IS_REF] && ($i == 0 || $propertyValues[$i - 1][self::IS_REF_CHAINED]); + $propertyValues[] = &$propertyValue; }