8000 [Serializer] Add a way to disable ClassDiscriminatorResolver · Issue #28537 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Serializer] Add a way to disable ClassDiscriminatorResolver #28537

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.

Sign up for GitHub

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

Jump to bottom
Closed
franek opened this issue Sep 21, 2018 · 3 comments
Closed

[Serializer] Add a way to disable ClassDiscriminatorResolver #28537

franek opened this issue Sep 21, 2018 · 3 comments
Labels
Feature Serializer

Comments

@franek
Copy link
Contributor
franek commented Sep 21, 2018 •
edited
Loading

Symfony version(s) affected: 4.1.4

Description

I had a performance issue on migrating my project from Symfony3.4 to Symfony4.1.
I mainly use Serializer to serialize data (through ObjectNormalizer).
After doing several blackfire analyses, I have found that problem was due to ClassDiscrimintatorResolver (#24375) which is enable by default whereas I don't need it.

To fix the problem, I had to create a Custom ObjectNormalizer and set $this->classDiscriminatorResolver to null. Not very elegant but I had a huge boost with this modification and SF4 serializer (now 90ms, 120ms without this modification) is now faster than SF3.4 (97ms).

class ObjectNormalizer extends CoreObjectNormalizer implements CacheableSupportsMethodInterface
{

    public function __construct(ClassMetadataFactoryInterface $classMetadataFactory = null, NameConverterInterface $nameConverter = null,
        PropertyAccessorInterface $propertyAccessor = null, PropertyTypeExtractorInterface $propertyTypeExtractor = null, ClassDiscriminatorResolverInterface $classDiscriminatorResolver = null)
    {
        parent::__construct($classMetadataFactory, $nameConverter, $propertyAccessor, $propertyTypeExtractor, $classDiscriminatorResolver);

        /**
         * Disable classDiscriminatorResolver for ObjectNormalizer. It is initialized into 
https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php#L59
         */
        $this->classDiscriminatorResolver = null;
    }

Possible Solution

A possible solution should be to disable classDiscriminatorResolver if framework.serialiazer.discriminator_class_mapping is not set.
@franek
Copy link
Contributor Author
franek commented Sep 24, 2018 •
edited
Loading
8000

Someone asked me to test with Symfony/Serializer: 4.2@dev. I have the same problem (~120ms)

@fbourigault
Copy link
Contributor
fbourigault commented Oct 14, 2018 •
edited
Loading

Here is a profile comparison about using the ClassDiscriminatorFromClassMetadata vs using a no op version: https://blackfire.io/profiles/compare/64e639ed-98a1-40d4-bdec-ae1b0b7cefff/graph

Profiled code: egeloen/ivory-serializer-benchmark#8
Noop implementation:

$noop = class implements ClassDiscriminatorResolverInterface {
    public function getMappingForClass(string $class): ?ClassDiscriminatorMapping
    {
        return null;
    }

    public function getMappingForMappedObject($object): ?ClassDiscriminatorMapping
    {
        return null;
    }

    public function getTypeForMappedObject($object): ?string
    {
        return null;
    }
}

@dunglas
Copy link
Member
dunglas commented Oct 16, 2018

See also #28457

@fbourigault fbourigault mentioned this issue Oct 16, 2018
Merged
nicolas-grekas added a commit that referenced this issue Oct 20, 2018
@nicolas-grekas
bug #28889 [Serializer] Reduce class discriminator overhead (fbouriga…
f1795c0 
…ult)

This PR was merged into the 4.1 branch.

Discussion
----------

[Serializer] Reduce class discriminator overhead

| Q             | A
| ------------- | ---
| Branch?       | 4.1
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #28537
| License       | MIT
| Doc PR        | N/A

This try to fix the overhead added by class discriminator feature.

Here is a 4.1 vs this PR comparison:
https://blackfire.io/profiles/compare/20ead249-0e98-430f-9b8d-906f464b1854/graph

And a 3.4 vs this PR comparison:
https://blackfire.io/profiles/compare/7e402dde-4a54-4053-a12e-d3d6891afc02/graph

Commits
-------

326c267 [Serializer] Reduce class discriminator overhead
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature Serializer
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@dunglas @nicolas-grekas @franek @fbourigault @xabbuh
0