[Serializer] Add methods `getSupportedTypes` to allow better performance #49291

tucksaun · 2023-02-07T22:58:57Z

Q	A
Branch?	6.3
Bug fix?	no
New feature?	yes
Deprecations?	yes (new method in the interfaces, one interface deprecated)
License	MIT
Doc PR	to be written

The PRs allows normalizers or denormalizers to expose their supported types and the associated cacheability to the Serializer.
With this info, even if the supports* call is not cacheable, the Serializer can skip a ton of method calls to supports* improving performance substaintially in some cases:

I found this design while working on a customer project performance (a big app built around API Platform): we reached the point where the slowest part of main application endpoint was `Symfony\Component\Serializer\Serializer::getNormalizer`.

After some digging, we found out we were experiencing the conjunction of two phenomenons:

the application is quite complex and returns deep nested and repeating structures, exposing the underlying bottleneck;
and a lot of custom non-cacheable normalizers.
Because most of the normalizers are not cacheable, the Serializer has to call every normalizer over and over again leading getNormalizer to account for 20% of the total wall time:

We first tried to improve cacheability based on context without much success, then an approach similar to #45779 with some success but still feeling this could be faster.
We then thought that even if the supportsNormalization could not be cached (because of the context), maybe we could avoid the calls at the origin by letting the Normalizers expose the types they support and came to this PR with pretty good results.

The perfornance improvement was only measured by adapting Symfony's normalizers as well as the project ones, proper third party normalizers updates should improve performance even more.

This should effectively replaces the CacheableSupportsMethodInterface as the cacheability can now be returned by getSupportedTypes.

nicolas-grekas

Nice improvement!

I would go as far as suggesting to add getSupportedTypes to both NormalizerInterface and DenormalizerInterface (with a BC layer of course)? That'd make a lot of sense to me.

src/Symfony/Component/Serializer/Normalizer/BackedEnumNormalizer.php

src/Symfony/Component/Serializer/Normalizer/CacheableSupportsMethodInterface.php

src/Symfony/Component/Serializer/Normalizer/MimeMessageNormalizer.php

src/Symfony/Component/Serializer/Normalizer/SupportedTypesMethodInterface.php

src/Symfony/Component/Serializer/Serializer.php

src/Symfony/Component/Serializer/Normalizer/DateTimeZoneNormalizer.php

src/Symfony/Component/Serializer/Serializer.php

stof · 2023-02-08T11:50:06Z

the new interface needs to be implemented in the TraceableNormalizer, so that the benefits of this caching are not lost (and hidden from devs) in dev environments when the profiler is enabled.

src/Symfony/Component/Serializer/Serializer.php

dunglas · 2023-02-08T22:54:50Z

This looks very promising! +1 on my side for the general approach as well as for making supportsNormalization() optional.

tucksaun · 2023-02-18T01:02:10Z

@nicolas-grekas PR has been updated to move getSupportedTypes to NormalizerInterface and DenormalizerInterface

fabpot · 2023-03-10T13:55:05Z

Thank you @tucksaun.

tucksaun

Thanks @nicolas-grekas for taking over the last edits regarding your comment 👍
Also I like the idea or the wildcard: stating that a normalizer supports any type and is cacheable was indeed something my PR was lacking!

tucksaun · 2023-03-13T02:59:16Z

src/Symfony/Component/Serializer/Normalizer/DenormalizerInterface.php

+     * For each supported formats (if applicable), the supported types should be
+     * returned as keys, and each type should be mapped to a boolean indicating
+     * if the result of supportsDenormalization() can be cached or not
+     * (a result cannot be cached when it depends on the context or on the data.)


"(a result cannot be cached when it depends on the context or on the data)." (dot after the parenthesis) ?

tucksaun · 2023-03-13T03:01:09Z

src/Symfony/Component/Serializer/Normalizer/NormalizerInterface.php

+     * For each supported formats (if applicable), the supported types should be
+     * returned as keys, and each type should be mapped to a boolean indicating
+     * if the result of supportsNormalization() can be cached or not
+     * (a result cannot be cached when it depends on the context or on the data.)


"(a result cannot be cached when it depends on the context or on the data)." (dot after the parenthesis) ?

…portedTypes()" (nicolas-grekas) This PR was merged into the 6.3 branch. Discussion ---------- [Serializer] Allow filtering "object" when using "getSupportedTypes()" | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Follows #49291 This allows telling when only objects are accepted, to skip calling `supportsNormalization()` when native types are considered. /cc `@tucksaun` Commits ------- 6984dce [Serializer] Allow filtering "object" when using "getSupportedTypes()"

| Q | A | ------------- | --- | Branch? | 6.3 for features | Bug fix? | no | New feature? | no | Deprecations? | yes | Tickets | symfony#49291 (review) | License | MIT | Doc PR | n/a As mentioned in symfony#49291 (review).

This PR was merged into the 6.3 branch. Discussion ---------- [Serializer] Add missing upgrade note | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | no | New feature? | no  | Deprecations? | no  | Tickets | none | License | MIT | Doc PR | none Forgot during #49291 /cc `@nicolas`-grekas Commits ------- a28ff01 [Serializer] Add missing upgrade note

| Q | A | ------------- | --- | Branch? | 6.3 for features | Bug fix? | no | New feature? | no | Deprecations? | yes | Tickets | symfony#49291 (review) | License | MIT | Doc PR | n/a As mentioned in symfony#49291 (review).

…(tucksaun) This PR was merged into the 6.3 branch. Discussion ---------- [Serializer] Mark `ObjectNormalizer` as final for 7.0 | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | no | New feature? | no | Deprecations? | yes | Tickets | #49291 (review) | License | MIT | Doc PR | none Extracted from #49950 and related to #49291 (review) Commits ------- b2828e9 [Serializer] Mark ObjectNormalizer as final for 7.0

| Q | A | ------------- | --- | Branch? | 6.3 for features | Bug fix? | no | New feature? | no | Deprecations? | yes | Tickets | symfony#49291 (review) | License | MIT | Doc PR | n/a As mentioned in symfony#49291 (review).

…(tucksaun) This PR was merged into the 6.3 branch. Discussion ---------- [Serializer] Marking some Normalizer classes as final | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | no | New feature? | no | Deprecations? | yes | Tickets | #49291 (review) | License | MIT | Doc PR | n/a As mentioned in #49291 (review). Making those classes final will allow to get rid of all the `__CLASS__ === static::class` for cacheability checks in 7.0 Commits ------- d976fc5 [Serializer] Marking some Normalizer classes as final

| Q | A | ------------- | --- | Branch? | 6.3 for features | Bug fix? | no | New feature? | no | Deprecations? | yes | Tickets | symfony/symfony#49291 (review) | License | MIT | Doc PR | n/a As mentioned in symfony/symfony#49291 (review).

…rmalizableInterface (spideyfusion) This PR was merged into the 6.3 branch. Discussion ---------- [Serializer] Fix ignoring objects that only implement DenormalizableInterface | Q | A | ------------- | --- | Branch? | 6.3  | Bug fix? | yes | New feature? | no  | Deprecations? | no  | Tickets | N/A  | License | MIT | Doc PR | N/A  When the #49291 optimization got introduced, the `DenormalizableInterface` type was left out of the `CustomNormalizer` normalizer's `getSupportedTypes` implementation. This prevents us for example, implementing custom deserialization logic for API objects where we can't describe all deserialization rules via attributes.  Commits ------- a0a3ce1 [Serializer] Fix ignoring objects that only implement DenormalizableInterface

tucksaun requested a review from dunglas as a code owner February 7, 2023 22:58

carsonbot added Status: Needs Review Deprecation Feature Serializer labels Feb 7, 2023

carsonbot added this to the 6.3 milestone Feb 7, 2023

nicolas-grekas reviewed Feb 8, 2023

View reviewed changes

nicolas-grekas mentioned this pull request Feb 8, 2023

[Serializer] Improve performance by exposing supports-never/-always #45779

Closed

Jeroeny reviewed Feb 8, 2023

View reviewed changes

src/Symfony/Component/Serializer/Serializer.php Outdated Show resolved Hide resolved

Jeroeny reviewed Feb 8, 2023

View reviewed changes

src/Symfony/Component/Serializer/Normalizer/DateTimeZoneNormalizer.php Outdated Show resolved Hide resolved

Jeroeny reviewed Feb 8, 2023

View reviewed changes

src/Symfony/Component/Serializer/Serializer.php Outdated Show resolved Hide resolved

Jeroeny reviewed Feb 8, 2023

View reviewed changes

src/Symfony/Component/Serializer/Serializer.php Outdated Show resolved Hide resolved

tucksaun force-pushed the serializer/speed-improvement branch 3 times, most recently from e11f040 to 25b94bc Compare February 10, 2023 20:57

tucksaun force-pushed the serializer/speed-improvement branch 2 times, most recently from d103aa6 to 84b2667 Compare February 18, 2023 00:29

tucksaun requested review from OskarStark and jderusse as code owners February 18, 2023 00:29

tucksaun force-pushed the serializer/speed-improvement branch 6 times, most recently from db028b5 to 2e9daf0 Compare February 18, 2023 00:47

tucksaun removed request for dunglas and jderusse February 18, 2023 01:02

carsonbot added Status: Reviewed and removed Status: Needs Review labels Mar 9, 2023

nicolas-grekas added the Ready label Mar 9, 2023

fabpot approved these changes Mar 10, 2023

View reviewed changes

tucksaun and others added 2 commits March 10, 2023 14:52

[Serializer] Add methods getSupportedTypes to allow better performance

400685a

[Serializer] Add wildcard support to getSupportedTypes()

e5af24a

fabpot force-pushed the serializer/speed-improvement branch from 25cf787 to e5af24a Compare March 10, 2023 13:55

fabpot merged commit 58052e9 into symfony:6.3 Mar 10, 2023

fabpot mentioned this pull request Mar 10, 2023

[Serializer] Add methods getSupportedTypes to allow better performance symfony/symfony-docs#18042

Closed

tucksaun commented Mar 13, 2023

View reviewed changes

nicolas-grekas mentioned this pull request Mar 19, 2023

[Serializer] Allow filtering "object" when using "getSupportedTypes()" #49735

Merged

tucksaun mentioned this pull request Apr 5, 2023

[Serializer] Marking some Normalizer classes as final #49950

Merged

This was referenced Apr 5, 2023

[Serializer] Mark ObjectNormalizer as final for 7.0 #49953

Merged

[Serializer] Add missing upgrade note #49954

Merged

fabpot mentioned this pull request May 1, 2023

Release v6.3.0-BETA1 #50199

Merged

javiereguiluz added the Performance label May 5, 2023

spideyfusion mentioned this pull request Jun 1, 2023

[Serializer] Fix ignoring objects that only implement DenormalizableInterface #50521

Merged

MaSpeng mentioned this pull request Jun 7, 2023

[WIP] Add getSupportedTypes method to Normalizers api-platform/core#5611

Closed

mtarld mentioned this pull request Jan 7, 2024

[Serializer] [WIP] Add cache warmer for normalizer cache #39853

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Serializer] Add methods `getSupportedTypes` to allow better performance #49291

[Serializer] Add methods `getSupportedTypes` to allow better performance #49291

tucksaun commented Feb 7, 2023 •

edited

Loading

nicolas-grekas left a comment

stof commented Feb 8, 2023

dunglas commented Feb 8, 2023

tucksaun commented Feb 18, 2023

fabpot commented Mar 10, 2023

tucksaun left a comment

tucksaun Mar 13, 2023

tucksaun Mar 13, 2023

[Serializer] Add methods getSupportedTypes to allow better performance #49291

[Serializer] Add methods getSupportedTypes to allow better performance #49291

Conversation

tucksaun commented Feb 7, 2023 • edited Loading

nicolas-grekas left a comment

Choose a reason for hiding this comment

stof commented Feb 8, 2023

dunglas commented Feb 8, 2023

tucksaun commented Feb 18, 2023

fabpot commented Mar 10, 2023

tucksaun left a comment

Choose a reason for hiding this comment

tucksaun Mar 13, 2023

Choose a reason for hiding this comment

tucksaun Mar 13, 2023

Choose a reason for hiding this comment

[Serializer] Add methods `getSupportedTypes` to allow better performance #49291

[Serializer] Add methods `getSupportedTypes` to allow better performance #49291

tucksaun commented Feb 7, 2023 •

edited

Loading