-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
For regular strings avoid igbinary serialization overhead #1786
base: develop
Are you sure you want to change the base?
Conversation
I don't like changes like this. It adds strange unexpected behaviour in exchange for 1-2% performance. Users always may disable serialization via |
I appreciate the feedback, I made it into configuration option specifically so that it can be controlled by the user and turned off/on and is disabled by default. This makes it easier then sprinkling code with setOption() to change value and restore it back, when using a mixed environment where complex values (Arrays & Objects) should be serialized and strings left as they are. The change from logic perspective as far as I can tell is very low risk and adds minimal (if any) cognitive overhead to internal extension code logic. I guess another approach would be to do something like a smart_igbinary serializer that works like igbinary but implements different logic for scalar values and is distinct serialization logic within code flow. If that would be more agreeable I am happy to adjust the patch. |
IMO there should be a division of responsibility. For phpredis all serializers look like a black box that takes a value at the input and gives a string at the output. How it works and how it can be optimized needs to be changed at the level of the serializer code. Something like |
…edis_clone_develop
Presently PhpRedis will segfault if users attempt to clone either the Redis or RedisCluster objects. We plan on investigating proper clone support in future releases, but for now just diable the handlers.
Without this, performing a HMGET call fails to decompress the data before returning it to php.
ZSTD uses two defined error numbers to inform the caller when the compressed data is invalid (e.g. wrong magic number) or the size is unknown. We should always know the size so abort if ZSTD returns either to us. Fixes: phpredis#1936
Rework the session locking unit tests to be less reliant on arbitrary sleep calls which can be very troublesome when running in Travis and especially when running in Travis under valgrind.
…elop_simple_strings
This simple change ensures that string data is stored as is without introducing overhead in speed/memory involved in serialization/deserialization steps.
In benchmarks this shows 1-2% speed improvement in terms of speed, but more critically for shorter strings it eliminates added network overhead resulting from igbinary header.