Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 3b2d010

Browse filesBrowse files
fabpotfabpot
committed
bug #16294 [PropertyAccess] Major performance improvement (dunglas)
This PR was squashed before being merged into the 2.3 branch (closes #16294). Discussion ---------- [PropertyAccess] Major performance improvement | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #16179 | License | MIT | Doc PR | n/a This PR improves performance of the PropertyAccess component of ~70%. The two main changes are: * caching the `PropertyPath` initialization * caching the guessed access strategy This is especially important for the `ObjectNormalizer` (Symfony Serializer) and the JSON-LD normalizer ([API Platform](https://api-platform.com)) because they use the `PropertyAccessor` class in large loops (ex: normalization of a list of entities). Here is the Blackfire comparison: https://blackfire.io/profiles/compare/c42fd275-2b0c-4ce5-8bf3-84762054d31e/graph The code of the benchmark I've used (with Symfony 2.3 as dependency): ```php <?php require 'vendor/autoload.php'; class Foo { private $baz; public $bar; public function getBaz() { return $this->baz; } public function setBaz($baz) { $this->baz = $baz; } } use Symfony\Component\PropertyAccess\PropertyAccess; $accessor = PropertyAccess::createPropertyAccessor(); $start = microtime(true); for ($i = 0; $i < 10000; ++$i) { $foo = new Foo(); $accessor->setValue($foo, 'bar', 'Lorem'); $accessor->setValue($foo, 'baz', 'Ipsum'); $accessor->getValue($foo, 'bar'); $accessor->getValue($foo, 'baz'); } echo 'Time: '.(microtime(true) - $start).PHP_EOL; ``` This PR also adds an optional support for Doctrine cache to keep access information across requests and improve the overall application performance (even outside of loops). Commits ------- 284dc75 [PropertyAccess] Major performance improvement
2 parents ebd55fc + 284dc75 commit 3b2d010
Copy full SHA for 3b2d010

File tree

1 file changed

+219
-105
lines changed
Filter options

1 file changed

+219
-105
lines changed

‎src/Symfony/Component/PropertyAccess/PropertyAccessor.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/PropertyAccess/PropertyAccessor.php
+219-105Lines changed: 219 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,21 @@ class PropertyAccessor implements PropertyAccessorInterface
2323
{
2424
const VALUE = 0;
2525
const IS_REF = 1;
26+
const ACCESS_HAS_PROPERTY = 0;
27+
const ACCESS_TYPE = 1;
28+
const ACCESS_NAME = 2;
29+
const ACCESS_REF = 3;
30+
const ACCESS_ADDER = 4;
31+
const ACCESS_REMOVER = 5;
32+
const ACCESS_TYPE_METHOD = 0;
33+
const ACCESS_TYPE_PROPERTY = 1;
34+
const ACCESS_TYPE_MAGIC = 2;
35+
const ACCESS_TYPE_ADDER_AND_REMOVER = 3;
36+
const ACCESS_TYPE_NOT_FOUND = 4;
2637

2738
private $magicCall;
39+
private $readPropertyCache = array();
40+
private $writePropertyCache = array();
2841

2942
/**
3043
* Should not be used by application code. Use
@@ -202,48 +215,31 @@ private function &readProperty(&$object, $property)
202215
throw new NoSuchPropertyException(sprintf('Cannot read property "%s" from an array. Maybe you should write the property path as "[%s]" instead?', $property, $property));
203216
}
204217

205-
$camelProp = $this->camelize($property);
206-
$reflClass = new \ReflectionClass($object);
207-
$getter = 'get'.$camelProp;
208-
$isser = 'is'.$camelProp;
209-
$hasser = 'has'.$camelProp;
210-
$classHasProperty = $reflClass->hasProperty($property);
211-
212-
if ($reflClass->hasMethod($getter) && $reflClass->getMethod($getter)->isPublic()) {
213-
$result[self::VALUE] = $object->$getter();
214-
} elseif ($reflClass->hasMethod($isser) && $reflClass->getMethod($isser)->isPublic()) {
215-
$result[self::VALUE] = $object->$isser();
216-
} elseif ($reflClass->hasMethod($hasser) && $reflClass->getMethod($hasser)->isPublic()) {
217-
$result[self::VALUE] = $object->$hasser();
218-
} elseif ($reflClass->hasMethod('__get') && $reflClass->getMethod('__get')->isPublic()) {
219-
$result[self::VALUE] = $object->$property;
220-
} elseif ($classHasProperty && $reflClass->getProperty($property)->isPublic()) {
221-
$result[self::VALUE] = &$object->$property;
222-
$result[self::IS_REF] = true;
223-
} elseif (!$classHasProperty && property_exists($object, $property)) {
218+
$access = $this->getReadAccessInfo($object, $property);
219+
220+
if (self::ACCESS_TYPE_METHOD === $access[self::ACCESS_TYPE]) {
221+
$result[self::VALUE] = $object->{$access[self::ACCESS_NAME]}();
222+
} elseif (self::ACCESS_TYPE_PROPERTY === $access[self::ACCESS_TYPE]) {
223+
if ($access[self::ACCESS_REF]) {
224+
$result[self::VALUE] = &$object->{$access[self::ACCESS_NAME]};
225+
$result[self::IS_REF] = true;
226+
} else {
227+
$result[self::VALUE] = $object->{$access[self::ACCESS_NAME]};
228+
}
229+
} elseif (!$access[self::ACCESS_HAS_PROPERTY] && property_exists($object, $property)) {
224230
// Needed to support \stdClass instances. We need to explicitly
225231
// exclude $classHasProperty, otherwise if in the previous clause
226232
// a *protected* property was found on the class, property_exists()
227233
// returns true, consequently the following line will result in a
228234
// fatal error.
235+
229236
$result[self::VALUE] = &$object->$property;
230237
$result[self::IS_REF] = true;
231-
} elseif ($this->magicCall && $reflClass->hasMethod('__call') && $reflClass->getMethod('__call')->isPublic()) {
238+
} elseif (self::ACCESS_TYPE_MAGIC === $access[self::ACCESS_TYPE]) {
232239
// we call the getter and hope the __call do the job
233-
$result[self::VALUE] = $object->$getter();
240+
$result[self::VALUE] = $object->{$access[self::ACCESS_NAME]}();
234241
} else {
235-
$methods = array($getter, $isser, $hasser, '__get');
236-
if ($this->magicCall) {
237-
$methods[] = '__call';
238-
}
239-
240-
throw new NoSuchPropertyException(sprintf(
241-
'Neither the property "%s" nor one of the methods "%s()" '.
242-
'exist and have public access in class "%s".',
243-
$property,
244-
implode('()", "', $methods),
245-
$reflClass->name
246-
));
242+
throw new NoSuchPropertyException($access[self::ACCESS_NAME]);
247243
}
248244

249245
// Objects are always passed around by reference
@@ -254,6 +250,77 @@ private function &readProperty(&$object, $property)
254250
return $result;
255251
}
256252

253+
/**
254+
* Guesses how to read the property value.
255+
*
256+
* @param string $object
257+
* @param string $property
258+
*
259+
* @return array
260+
*/
261+
private function getReadAccessInfo($object, $property)
262+
{
263+
$key = get_class($object).'::'.$property;
264+
265+
if (isset($this->readPropertyCache[$key])) {
266+
$access = $this->readPropertyCache[$key];
267+
} else {
268+
$access = array();
269+
270+
$reflClass = new \ReflectionClass($object);
271+
$access[self::ACCESS_HAS_PROPERTY] = $reflClass->hasProperty($property);
272+
$camelProp = $this->camelize($property);
273+
$getter = 'get'.$camelProp;
274+
$isser = 'is'.$camelProp;
275+
$hasser = 'has'.$camelProp;
276+
$classHasProperty = $reflClass->hasProperty($property);
277+
278+
if ($reflClass->hasMethod($getter) && $reflClass->getMethod($getter)->isPublic()) {
279+
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_METHOD;
280+
$access[self::ACCESS_NAME] = $getter;
281+
} elseif ($reflClass->hasMethod($isser) && $reflClass->getMethod($isser)->isPublic()) {
282+
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_METHOD;
283+
$access[self::ACCESS_NAME] = $isser;
284+
} elseif ($reflClass->hasMethod($hasser) && $reflClass->getMethod($hasser)->isPublic()) {
285+
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_METHOD;
286+
$access[self::ACCESS_NAME] = $hasser;
287+
} elseif ($reflClass->hasMethod('__get') && $reflClass->getMethod('__get')->isPublic()) {
288+
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_PROPERTY;
289+
$access[self::ACCESS_NAME] = $property;
290+
$access[self::ACCESS_REF] = false;
291+
} elseif ($classHasProperty && $reflClass->getProperty($property)->isPublic()) {
292+
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_PROPERTY;
293+
$access[self::ACCESS_NAME] = $property;
294+
$access[self::ACCESS_REF] = true;
295+
296+
$result[self::VALUE] = &$object->$property;
297+
$result[self::IS_REF] = true;
298+
} elseif ($this->magicCall && $reflClass->hasMethod('__call') && $reflClass->getMethod('__call')->isPublic()) {
299+
// we call the getter and hope the __call do the job
300+
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_MAGIC;
301+
$access[self::ACCESS_NAME] = $getter;
302+
} else {
303+
$methods = array($getter, $isser, $hasser, '__get');
304+
if ($this->magicCall) {
305+
$methods[] = '__call';
306+
}
307+
308+
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_NOT_FOUND;
309+
$access[self::ACCESS_NAME] = sprintf(
310+
'Neither the property "%s" nor one of the methods "%s()" '.
311+
'exist and have public access in class "%s".',
312+
$property,
313+
implode('()", "', $methods),
314+
$reflClass->name
315+
);
316+
}
317+
318+
$this->readPropertyCache[$key] = $access;
319+
}
320+
321+
return $access;
322+
}
323+
257324
/**
258325
* Sets the value of the property at the given index in the path.
259326
*
@@ -285,96 +352,143 @@ private function writeIndex(&$array, $index, $value)
285352
*/
286353
private function writeProperty(&$object, $property, $singular, $value)
287354
{
288-
$guessedAdders = '';
289-
290355
if (!is_object($object)) {
291356
throw new NoSuchPropertyException(sprintf('Cannot write property "%s" to an array. Maybe you should write the property path as "[%s]" instead?', $property, $property));
292357
}
293358

294-
$reflClass = new \ReflectionClass($object);
295-
$plural = $this->camelize($property);
296-
297-
// Any of the two methods is required, but not yet known
298-
$singulars = null !== $singular ? array($singular) : (array) StringUtil::singularify($plural);
299-
300-
if (is_array($value) || $value instanceof \Traversable) {
301-
$methods = $this->findAdderAndRemover($reflClass, $singulars);
302-
303-
if (null !== $methods) {
304-
// At this point the add and remove methods have been found
305-
// Use iterator_to_array() instead of clone in order to prevent side effects
306-
// see https://github.com/symfony/symfony/issues/4670
307-
$itemsToAdd = is_object($value) ? iterator_to_array($value) : $value;
308-
$itemToRemove = array();
309-
$propertyValue = &$this->readProperty($object, $property);
310-
$previousValue = $propertyValue[self::VALUE];
311-
// remove reference to avoid modifications
312-
unset($propertyValue);
313-
314-
if (is_array($previousValue) || $previousValue instanceof \Traversable) {
315-
foreach ($previousValue as $previousItem) {
316-
foreach ($value as $key => $item) {
317-
if ($item === $previousItem) {
318-
// Item found, don't add
319-
unset($itemsToAdd[$key]);
320-
321-
// Next $previousItem
322-
continue 2;
323-
}
359+
$access = $this->getWriteAccessInfo($object, $property, $singular, $value);
360+
361+
if (self::ACCESS_TYPE_METHOD === $access[self::ACCESS_TYPE]) {
362+
$object->{$access[self::ACCESS_NAME]}($value);
363+
} elseif (self::ACCESS_TYPE_PROPERTY === $access[self::ACCESS_TYPE]) {
364+
$object->{$access[self::ACCESS_NAME]} = $value;
365+
} elseif (self::ACCESS_TYPE_ADDER_AND_REMOVER === $access[self::ACCESS_TYPE]) {
366+
// At this point the add and remove methods have been found
367+
// Use iterator_to_array() instead of clone in order to prevent side effects
368+
// see https://github.com/symfony/symfony/issues/4670
369+
$itemsToAdd = is_object($value) ? iterator_to_array($value) : $value;
370+
$itemToRemove = array();
371+
$propertyValue = &$this->readProperty($object, $property);
372+
$previousValue = $propertyValue[self::VALUE];
373+
// remove reference to avoid modifications
374+
unset($propertyValue);
375+
376+
if (is_array($previousValue) || $previousValue instanceof \Traversable) {
377+
foreach ($previousValue as $previousItem) {
378+
foreach ($value as $key => $item) {
379+
if ($item === $previousItem) {
380+
// Item found, don't add
381+
unset($itemsToAdd[$key]);
382+
383+
// Next $previousItem
384+
continue 2;
324385
}
325-
326-
// Item not found, add to remove list
327-
$itemToRemove[] = $previousItem;
328386
}
329-
}
330-
331-
foreach ($itemToRemove as $item) {
332-
call_user_func(array($object, $methods[1]), $item);
333-
}
334387

335-
foreach ($itemsToAdd as $item) {
336-
call_user_func(array($object, $methods[0]), $item);
388+
// Item not found, add to remove list
389+
$itemToRemove[] = $previousItem;
337390
}
338-
339-
return;
340-
} else {
341-
// It is sufficient to include only the adders in the error
342-
// message. If the user implements the adder but not the remover,
343-
// an exception will be thrown in findAdderAndRemover() that
344-
// the remover has to be implemented as well.
345-
$guessedAdders = '"add'.implode('()", "add', $singulars).'()", ';
346391
}
347-
}
348392

349-
$setter = 'set'.$this->camelize($property);
350-
$classHasProperty = $reflClass->hasProperty($property);
393+
foreach ($itemToRemove as $item) {
394+
call_user_func(array($object, $access[self::ACCESS_REMOVER]), $item);
395+
}
351396

352-
if ($reflClass->hasMethod($setter) && $reflClass->getMethod($setter)->isPublic()) {
353-
$object->$setter($value);
354-
} elseif ($reflClass->hasMethod('__set') && $reflClass->getMethod('__set')->isPublic()) {
355-
$object->$property = $value;
356-
} elseif ($classHasProperty && $reflClass->getProperty($property)->isPublic()) {
357-
$object->$property = $value;
358-
} elseif (!$classHasProperty && property_exists($object, $property)) {
397+
foreach ($itemsToAdd as $item) {
398+
call_user_func(array($object, $access[self::ACCESS_ADDER]), $item);
399+
}
400+
} elseif (!$access[self::ACCESS_HAS_PROPERTY] && property_exists($object, $property)) {
359401
// Needed to support \stdClass instances. We need to explicitly
360402
// exclude $classHasProperty, otherwise if in the previous clause
361403
// a *protected* property was found on the class, property_exists()
362404
// returns true, consequently the following line will result in a
363405
// fatal error.
364-
$object->$property = $value;
365-
} elseif ($this->magicCall && $reflClass->hasMethod('__call') && $reflClass->getMethod('__call')->isPublic()) {
366-
// we call the getter and hope the __call do the job
367-
$object->$setter($value);
406+
407+
$object->{$access[self::ACCESS_NAME]} = $value;
408+
} elseif (self::ACCESS_TYPE_MAGIC === $access[self::ACCESS_TYPE]) {
409+
$object->{$access[self::ACCESS_NAME]}($value);
410+
} else {
411+
throw new NoSuchPropertyException($access[self::ACCESS_NAME]);
412+
}
413+
}
414+
415+
/**
416+
* Guesses how to write the property value.
417+
*
418+
* @param string $object
419+
* @param string $property
420+
* @param string|null $singular
421+
* @param mixed $value
422+
*
423+
* @return array
424+
*/
425+
private function getWriteAccessInfo($object, $property, $singular, $value)
426+
{
427+
$key = get_class($object).'::'.$property;
428+
$guessedAdders = '';
429+
430+
if (isset($this->writePropertyCache[$key])) {
431+
$access = $this->writePropertyCache[$key];
368432
} else {
369-
throw new NoSuchPropertyException(sprintf(
370-
'Neither the property "%s" nor one of the methods %s"%s()", '.
371-
'"__set()" or "__call()" exist and have public access in class "%s".',
372-
$property,
373-
$guessedAdders,
374-
$setter,
375-
$reflClass->name
376-
));
433+
$access = array();
434+
435+
$reflClass = new \ReflectionClass($object);
436+
$access[self::ACCESS_HAS_PROPERTY] = $reflClass->hasProperty($property);
437+
$plural = $this->camelize($property);
438+
439+
// Any of the two methods is required, but not yet known
440+
$singulars = null !== $singular ? array($singular) : (array) StringUtil::singularify($plural);
441+
442+
if (is_array($value) || $value instanceof \Traversable) {
443+
$methods = $this->findAdderAndRemover($reflClass, $singulars);
444+
445+
if (null === $methods) {
446+
// It is sufficient to include only the adders in the error
447+
// message. If the user implements the adder but not the remover,
448+
// an exception will be thrown in findAdderAndRemover() that
449+
// the remover has to be implemented as well.
450+
$guessedAdders = '"add'.implode('()", "add', $singulars).'()", ';
451+
} else {
452+
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_ADDER_AND_REMOVER;
453+
$access[self::ACCESS_ADDER] = $methods[0];
454+
$access[self::ACCESS_REMOVER] = $methods[1];
455+
}
456+
}
457+
458+
if (!isset($access[self::ACCESS_TYPE])) {
459+
$setter = 'set'.$this->camelize($property);
460+
$classHasProperty = $reflClass->hasProperty($property);
461+
462+
if ($reflClass->hasMethod($setter) && $reflClass->getMethod($setter)->isPublic()) {
463+
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_METHOD;
464+
$access[self::ACCESS_NAME] = $setter;
465+
} elseif ($reflClass->hasMethod('__set') && $reflClass->getMethod('__set')->isPublic()) {
466+
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_PROPERTY;
467+
$access[self::ACCESS_NAME] = $property;
468+
} elseif ($classHasProperty && $reflClass->getProperty($property)->isPublic()) {
469+
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_PROPERTY;
470+
$access[self::ACCESS_NAME] = $property;
471+
} elseif ($this->magicCall && $reflClass->hasMethod('__call') && $reflClass->getMethod('__call')->isPublic()) {
472+
// we call the getter and hope the __call do the job
473+
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_MAGIC;
474+
$access[self::ACCESS_NAME] = $setter;
475+
} else {
476+
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_NOT_FOUND;
477+
$access[self::ACCESS_NAME] = sprintf(
478+
'Neither the property "%s" nor one of the methods %s"%s()", '.
479+
'"__set()" or "__call()" exist and have public access in class "%s".',
480+
$property,
481+
$guessedAdders,
482+
$setter,
483+
$reflClass->name
484+
);
485+
}
486+
}
487+
488+
$this->writePropertyCache[$key] = $access;
377489
}
490+
491+
return $access;
378492
}
379493

380494
/**

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.