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

Normalizers should be cacheable with CacheableSupportsMethodInterface #54

Conversation

Korbeil
Copy link
Member

@Korbeil Korbeil commented Jan 18, 2019

Second hand on #52

With Symfony 4.1, a new interface CacheableSupportsMethodInterface was given to explicitly say that:

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

@see https://symfony.com/blog/new-in-symfony-4-1-faster-serializer

It seems that all the normalizers generated by Jane are eligible to this cache and given that on a serious project we have tenth of normalizers, we can get an important performance boost.

Fixed some stuff that wasn't working.

@Korbeil Korbeil force-pushed the feature/4-generate-cacheableSupportsMethodInterface-normalizers branch from 0e35346 to 5b46f41 Compare January 23, 2019 00:01
@Korbeil
Copy link
Member Author

Korbeil commented Jan 23, 2019

All ready to merge 🎉

  • So I rebased since PHP CS update fixes #55 as been merged (and fix all tests).
  • Added a test for this new setting
  • Added some documentation

@joelwurtz
Copy link
Member

Why doing an option for cacheable support ? It should be always enable imo, no need for an option here. And if we must support old versions of serializer, then this interface should be implemented only if it exist

@Korbeil
Copy link
Member Author

Korbeil commented Jan 23, 2019

After quick talk with @joelwurtz & @bastnic we decided to keep the option but default value would be null.
If value is null, we will check if cacheable exists and will add it if it exists. And you can also force to true or false if you always want it or not.

@Korbeil Korbeil force-pushed the feature/4-generate-cacheableSupportsMethodInterface-normalizers branch from 5b06fed to 7330cc9 Compare January 23, 2019 12:16
@Korbeil Korbeil force-pushed the feature/4-generate-cacheableSupportsMethodInterface-normalizers branch from 7330cc9 to 7c207f8 Compare January 23, 2019 12:26
CHANGELOG.md Outdated Show resolved Hide resolved
Co-Authored-By: Korbeil <baptiste.leduc@gmail.com>
@joelwurtz joelwurtz merged commit 40a38d8 into janephp:4.x Jan 24, 2019
@Korbeil Korbeil deleted the feature/4-generate-cacheableSupportsMethodInterface-normalizers branch June 19, 2019 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants