8000 [Cache] Added support for Redis igbinary serializer by kmadejski · Pull Request #27709 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Cache] Added support for Redis igbinary serializer #27709

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
wants to merge 1 commit into from
Closed

[Cache] Added support for Redis igbinary serializer #27709

wants to merge 1 commit into from

Conversation

kmadejski
Copy link
Contributor
Q A
Branch? 3.4, 4.0, 4.1, master
Bug fix? no
New feature? yes/no
BC breaks? no
Deprecations? no
Tests pass? yes
License MIT
Doc PR *

Since Redis supports igbinary serializer it would be great to be able to benefit from it.
(ref: https://github.com/phpredis/phpredis#setoption)
Using igbinary can lower Redis memory usage from 15% to 45% (depends on use case). In our, eZ Platform, case it's a lot because we store a lot of cache data in Redis.

  • docs PR will be provided if the community agrees with the idea shipped with this PR.

@nicolas-grekas
Copy link
Member

There is already a PR that will hopefully provide igbinary support to all backends, see #27645. For your info, the approach here doesn't work because serialization already happens in native form, see https://github.com/kmadejski/symfony/blob/73125f02702f4bdb631324c1ee8bb561bc1b2526/src/Symfony/Component/Cache/Traits/RedisTrait.php#L292
I'm therefor closing, but thank you anyway for submitting!
If you want to try #27645, please do! Note that this is a new feature so it should be part of 4.2 and cannot be merged on 3.4.

@nicolas-grekas nicolas-grekas added this to the next milestone Jun 25, 2018
@nicolas-grekas
Copy link
Member

By the way, see also #27646

@kmadejski
Copy link
Contributor Author

@nicolas-grekas I've noticed your PR right after I've created my own, thanks for closing.

For your info, the approach here doesn't work because serialization already happens in native form, see https://github.com/kmadejski/symfony/blob/73125f02702f4bdb631324c1ee8bb561bc1b2526/src/Symfony/Component/Cache/Traits/RedisTrait.php#L292

You are partially right, indeed serialization happens here, but if you use igbinary on once serialized data, then you can still save between 15 - 40% of memory.

If you want to try #27645, please do!

I'll take a look and review ;)

@nicolas-grekas nicolas-grekas modified the milestones: next, 4.2 Nov 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Cache Feature Status: Needs Review
Projects
None yet
Milestone
4.2
Development

Successfully merging this pull request may close these issues.
3 participants
@kmadejski @nicolas-grekas @carsonbot
0