Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Serializer] Cache the normalizer to use when possible #27049

Merged
merged 2 commits into from Apr 29, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/Symfony/Component/Serializer/CHANGELOG.md
Expand Up @@ -4,6 +4,8 @@ CHANGELOG
4.1.0
-----

* added `CacheableSupportsMethodInterface` for normalizers and denormalizers that use
only the type and the format in their `supports*()` methods
* added `MissingConstructorArgumentsException` new exception for deserialization failure
of objects that needs data insertion in constructor
* added an optional `default_constructor_arguments` option of context to specify a default data in
Expand Down
Expand Up @@ -30,15 +30,14 @@
*
* @author Kévin Dunglas <dunglas@gmail.com>
*/
abstract class AbstractObjectNormalizer extends AbstractNormalizer
abstract class AbstractObjectNormalizer extends AbstractNormalizer implements CacheableSupportsMethodInterface
{
const ENABLE_MAX_DEPTH = 'enable_max_depth';
const DEPTH_KEY_PATTERN = 'depth_%s::%s';
const DISABLE_TYPE_ENFORCEMENT = 'disable_type_enforcement';

private $propertyTypeExtractor;
private $attributesCache = array();
private $cache = array();

/**
* @var callable|null
Expand Down Expand Up @@ -225,11 +224,7 @@ public function setMaxDepthHandler(?callable $handler): void
*/
public function supportsDenormalization($data, $type, $format = null)
{
if (!isset($this->cache[$type])) {
$this->cache[$type] = class_exists($type) || (interface_exists($type) && null !== $this->classDiscriminatorResolver && null !== $this->classDiscriminatorResolver->getMappingForClass($type));
}

return $this->cache[$type];
return \class_exists($type) || (\interface_exists($type, false) && $this->classDiscriminatorResolver && null !== $this->classDiscriminatorResolver->getMappingForClass($type));
}

/**
Expand Down
@@ -0,0 +1,25 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Serializer\Normalizer;

/**
* Marker interface for normalizers and denormalizers that use
* only the type and the format in their supports*() methods.
*
* By implementing this interface, the return value of the
* supports*() methods will be cached by type and format.
*
* @author Kévin Dunglas <dunglas@gmail.com>
*/
interface CacheableSupportsMethodInterface
{
}
Expand Up @@ -22,7 +22,7 @@
* @author Grégoire Pineau <lyrixx@lyrixx.info>
* @author Kévin Dunglas <dunglas@gmail.com>
*/
class ConstraintViolationListNormalizer implements NormalizerInterface
class ConstraintViolationListNormalizer implements NormalizerInterface, CacheableSupportsMethodInterface
{
/**
* {@inheritdoc}
Expand Down
14 changes: 2 additions & 12 deletions src/Symfony/Component/Serializer/Normalizer/CustomNormalizer.php
Expand Up @@ -17,13 +17,11 @@
/**
* @author Jordi Boggiano <j.boggiano@seld.be>
*/
class CustomNormalizer implements NormalizerInterface, DenormalizerInterface, SerializerAwareInterface
class CustomNormalizer implements NormalizerInterface, DenormalizerInterface, SerializerAwareInterface, CacheableSupportsMethodInterface
{
use ObjectToPopulateTrait;
use SerializerAwareTrait;

private $cache = array();

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -67,14 +65,6 @@ public function supportsNormalization($data, $format = null)
*/
public function supportsDenormalization($data, $type, $format = null)
{
if (isset($this->cache[$type])) {
return $this->cache[$type];
}

if (!class_exists($type)) {
return $this->cache[$type] = false;
}

return $this->cache[$type] = is_subclass_of($type, 'Symfony\Component\Serializer\Normalizer\DenormalizableInterface');
return \is_subclass_of($type, DenormalizableInterface::class);
}
}
Expand Up @@ -23,7 +23,7 @@
*
* @author Kévin Dunglas <dunglas@gmail.com>
*/
class DataUriNormalizer implements NormalizerInterface, DenormalizerInterface
class DataUriNormalizer implements NormalizerInterface, DenormalizerInterface, CacheableSupportsMethodInterface
{
private static $supportedTypes = array(
\SplFileInfo::class => true,
Expand Down
Expand Up @@ -20,7 +20,7 @@
*
* @author Jérôme Parmentier <jerome@prmntr.me>
*/
class DateIntervalNormalizer implements NormalizerInterface, DenormalizerInterface
class DateIntervalNormalizer implements NormalizerInterface, DenormalizerInterface, CacheableSupportsMethodInterface
{
const FORMAT_KEY = 'dateinterval_format';

Expand Down
Expand Up @@ -20,7 +20,7 @@
*
* @author Kévin Dunglas <dunglas@gmail.com>
*/
class DateTimeNormalizer implements NormalizerInterface, DenormalizerInterface
class DateTimeNormalizer implements NormalizerInterface, DenormalizerInterface, CacheableSupportsMethodInterface
{
const FORMAT_KEY = 'datetime_format';
const TIMEZONE_KEY = 'datetime_timezone';
Expand Down
Expand Up @@ -35,22 +35,21 @@
class GetSetMethodNormalizer extends AbstractObjectNormalizer
{
private static $setterAccessibleCache = array();
private $cache = array();

/**
* {@inheritdoc}
*/
public function supportsNormalization($data, $format = null)
{
return parent::supportsNormalization($data, $format) && (isset($this->cache[$type = \get_class($data)]) ? $this->cache[$type] : $this->cache[$type] = $this->supports($type));
return parent::supportsNormalization($data, $format) && $this->supports(\get_class($data));
}

/**
* {@inheritdoc}
*/
public function supportsDenormalization($data, $type, $format = null)
{
return parent::supportsDenormalization($data, $type, $format) && (isset($this->cache[$type]) ? $this->cache[$type] : $this->cache[$type] = $this->supports($type));
return parent::supportsDenormalization($data, $type, $format) && $this->supports($type);
}

/**
Expand Down
Expand Up @@ -19,7 +19,7 @@
*
* @author Fred Cox <mcfedr@gmail.com>
*/
class JsonSerializableNormalizer extends AbstractNormalizer
class JsonSerializableNormalizer extends AbstractNormalizer implements CacheableSupportsMethodInterface
{
/**
* {@inheritdoc}
Expand Down
Expand Up @@ -30,22 +30,20 @@
*/
class PropertyNormalizer extends AbstractObjectNormalizer
{
private $cache = array();

/**
* {@inheritdoc}
*/
public function supportsNormalization($data, $format = null)
{
return parent::supportsNormalization($data, $format) && (isset($this->cache[$type = \get_class($data)]) ? $this->cache[$type] : $this->cache[$type] = $this->supports($type));
return parent::supportsNormalization($data, $format) && $this->supports(\get_class($data));
}

/**
* {@inheritdoc}
*/
public function supportsDenormalization($data, $type, $format = null)
{
return parent::supportsDenormalization($data, $type, $format) && (isset($this->cache[$type]) ? $this->cache[$type] : $this->cache[$type] = $this->supports($type));
return parent::supportsDenormalization($data, $type, $format) && $this->supports($type);
}

/**
Expand Down
67 changes: 60 additions & 7 deletions src/Symfony/Component/Serializer/Serializer.php
Expand Up @@ -26,6 +26,7 @@
use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
use Symfony\Component\Serializer\Exception\LogicException;
use Symfony\Component\Serializer\Normalizer\CacheableSupportsMethodInterface;

/**
* Serializer serializes and deserializes data.
Expand Down Expand Up @@ -55,10 +56,14 @@ class Serializer implements SerializerInterface, ContextAwareNormalizerInterface
protected $decoder;

/**
* @var array
* @internal since Symfony 4.1
*/
protected $normalizers = array();

private $cachedNormalizers;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolas-grekas what is this property for? it it set below but never used again.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, $this->normalizers is protected so could be edited by extensions. in which case you'd want to reset the cache?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that's the purpose, allowing efficient caching without breaking BC of the protected property (which is now internal so will be removed in 5.0

private $denormalizerCache = array();
private $normalizerCache = array();

public function __construct(array $normalizers = array(), array $encoders = array())
{
foreach ($normalizers as $normalizer) {
Expand Down Expand Up @@ -200,10 +205,35 @@ public function supportsDenormalization($data, $type, $format = null, array $con
*
* @return NormalizerInterface|null
*/
private function getNormalizer($data, $format, array $context)
private function getNormalizer($data, ?string $format, array $context)
{
foreach ($this->normalizers as $normalizer) {
if ($normalizer instanceof NormalizerInterface && $normalizer->supportsNormalization($data, $format, $context)) {
if ($this->cachedNormalizers !== $this->normalizers) {
$this->cachedNormalizers = $this->normalizers;
$this->denormalizerCache = $this->normalizerCache = array();
}
$type = \is_object($data) ? \get_class($data) : 'native-'.\gettype($data);

if (!isset($this->normalizerCache[$format][$type])) {
$this->normalizerCache[$format][$type] = array();

foreach ($this->normalizers as $k => $normalizer) {
if (!$normalizer instanceof NormalizerInterface) {
continue;
}

if (!$normalizer instanceof CacheableSupportsMethodInterface) {
$this->normalizerCache[$format][$type][$k] = false;
} elseif ($normalizer->supportsNormalization($data, $format)) {
$this->normalizerCache[$format][$type][$k] = true;

return $normalizer;
}
}
}

foreach ($this->normalizerCache[$format][$type] as $k => $cached) {
$normalizer = $this->normalizers[$k];
if ($cached || $normalizer->supportsNormalization($data, $format, $context)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify maybe you can replace $cached by $isCacheableAndSupportsNormalization?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bof, this is a local variable, and the concern is separated by 5 lines...

return $normalizer;
}
}
Expand All @@ -219,10 +249,33 @@ private function getNormalizer($data, $format, array $context)
*
* @return DenormalizerInterface|null
*/
private function getDenormalizer($data, $class, $format, array $context)
private function getDenormalizer($data, string $class, ?string $format, array $context)
{
foreach ($this->normalizers as $normalizer) {
if ($normalizer instanceof DenormalizerInterface && $normalizer->supportsDenormalization($data, $class, $format, $context)) {
if ($this->cachedNormalizers !== $this->normalizers) {
$this->cachedNormalizers = $this->normalizers;
$this->denormalizerCache = $this->normalizerCache = array();
}
if (!isset($this->denormalizerCache[$format][$class])) {
$this->denormalizerCache[$format][$class] = array();

foreach ($this->normalizers as $k => $normalizer) {
if (!$normalizer instanceof DenormalizerInterface) {
continue;
}

if (!$normalizer instanceof CacheableSupportsMethodInterface) {
$this->denormalizerCache[$format][$class][$k] = false;
} elseif ($normalizer->supportsDenormalization(null, $class, $format)) {
$this->denormalizerCache[$format][$class][$k] = true;

return $normalizer;
}
}
}

foreach ($this->denormalizerCache[$format][$class] as $k => $cached) {
$normalizer = $this->normalizers[$k];
if ($cached || $normalizer->supportsDenormalization($data, $class, $format, $context)) {
return $normalizer;
}
}
Expand Down