Problem/Motivation
Core's 8 philosophy has always been to improve performance automatically _WITHOUT_ configuration.
Making use of igbinary is giving us performance for free (similar to how APCu improves performance - if detected - transparently). It saves 50% time on unserialize and memory footprint
Related Symfony docs are https://symfony.com/doc/current/components/cache.html#marshalling-serial... as they support igbinary since SF 4+
Proposed resolution
- - Add igbinary serialization component (can copy from igbinary module)
- - Use a factory for serialization.phpserialize service
- - If igbinary is present use it, else use the standard component
For concerns that igbinary could be switched on/off on the fly and hence lead to invalid data Plan B is:
- - Detect igbinary during container build
- - Add DefaultPhpSerialize class, which extends from Drupal\Component\Serialization\PhpSerialize
- - Replace serialization.phpserialize service with other class (only if it's the default class, so it can still be overridden)
Remaining tasks
- Create patch
- Review
- Commit
User interface changes
- None
API changes
- None
Data model changes
- None
Release notes snippet
Igbinary is now used to serialize objects stored in the database cache and expirable key/value backends when the igbinary PHP extension is available, with transparent fallback to PHP serialization otherwise.
| Comment | File | Size | Author |
|---|---|---|---|
| #25 | Screenshot from 2025-02-03 15-12-46.png | 171.49 KB | berdir |
| #21 | compression_test.php_.txt | 3.87 KB | berdir |
Issue fork drupal-3014514
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
catchHow does this resolve a situation such as restoring a database from an environment that has/doesn't have igbinary on one that doesn't have/has it?
Comment #3
ndobromirov commentedIt will need to have some meta-data on the serialized object to keep how it was serialized, so in reality it can read from any serialized source and write to the existing default one. Hopefully that's bot a big overhead...
Comment #4
berdir#839444: Make serializer customizable for Cache\DatabaseBackend discussed having different database tables for that, if we would do it transparently like I proposed there, based on the injected service, then we could switch out the serializer.
I guess we can't just switch out the serializer service class dynamically as that would implicitly change the format on all things that use that service, so I suppose it would be up to the factory class to pick the other one and so we'd just change the argument of that?
Comment #6
andypostDatabase layer is also stuck on feature detection (capabilities) of connection so answer to @catch #2 is core needs feature detection for each subsystem...
if we allow to pgsql 9.5 we got JSONB serialization for free, then a question of PHP feature detection should be able to find database-compatible serialization as well (meantime 8.0 require php*-json)
Comment #8
mradcliffeI am not sure there is anything actionable for a novice contributor to do on this task yet so I am removing the novice tag.
Comment #12
andypostComment #17
catch#839444: Make serializer customizable for Cache\DatabaseBackend just landed so we're unblocked here.
However, I'm still stuck on how we'll deal with the issues from #2/#4. Could we maybe put this in $settings and write it out in the installer? That way, if you install on an environment with igbinary enabled, you would get that serializer, and it's up to you to then make sure that other environments your site gets migrated to also have igbinary available (which is not unlike a lot of other issues with php extensions). But then existing sites would not have things changed under their feet then.
But also, do we want to provide some kind of way to migrate?
Or instead of a settings flag, should we just bring igbinary module into core - but prevent installing it/uninstalling it on existing sites at least until a migration path is worked out?
Should we have a fallback serializer than can read PHP string serialization but only writes igbinary?
Comment #18
andypostIf the module enabled then all caches should be cleaned at its install or dump will contain serialized with it.
So instead of container param it could be a call to moduleExists()
Comment #19
andypostbtw when cache is stored in APCu it can use igbinary without core https://www.php.net/manual/en/apcu.configuration.php#ini.apcu.serializer
So the new question is how to deal with this option when cache in chained-fast...
Comment #20
berdirLooking at the igbinary module, it contains a check if the returned value is igbinary: https://git.drupalcode.org/project/igbinary/-/blob/2.0.x/src/Component/S...
It won't work for other serializer implementations, but what if we introduce a new IgBinaryIfAvailableSerializer that on encode has a function exists check and on decode checks if the returned value is igbinary-encoded and and a function exists. The problem is if it's igbinary and the function doesn't exist. We might need to extend the interface with an isValid() option or something or allow for it to throw an exception.
Comment #21
berdirI wanted to review the possible benefits of using igbinary based on real world examples, including gz compression (both the redis module offers that as an option based on cache data size and the igbinary module does too as a separate serializer but then always.
Based on an umami demo install, I picked a few example cache entries, views_data, entity types, module list and some small ones and compared serialize, serialize with compression level 1, igbinary and also that with gz compression level 1,6 and 9. Both redis and igbinary default to 1, redis has it a a configurable setting. All of that in terms of size and speed of serialize and unserialize. For speed, I did run each operation 1000x times and then reported the total in ms (microtime() * 1000), absolute numbers aren't meant to be meaningful, just as a baseline for the relative speed. doing it 1000x seemed useful to even out random stuff, reported times seem to be vary +/- 10% (views_data:en unserialize was between ~90 and ~110). note that compression numbers are always *including* the respective serialize/unserialize call.
The script I used is attached. Results probably vary quite a bit between different systems, and I directly accessed the cache entries, so it relies on having warm caches. use
select cid, length(data) as length from cache_default order by length asc;to get list of available cache entries and their size.compact strings on: https://gist.githubusercontent.com/Berdir/e0bbdbf3922fdc9c8ae905fd80ac2d...
compact strings off: https://gist.githubusercontent.com/Berdir/c35f0007efba8c50cd896689290758...
This is on DDEV PHP 8.3, igbinary 3.2.16. WSL2 on Windows 10, i9-11900K @ 3.50GHz.
Takeways:
* reduction especially on large cache entries is massive. views_data on igbinary is only 18% of serialize. The compact strings setting is doing the heavy lifting here, it's 60% with that turned off. Combined with gz1, it's only 4% of the serialize size (gz1 on serialize is 9%). views data is extremely repetitive. For most other cache entries, it's around 30% with igbinary and 10% combined with gz1
* igbinary serialize speed is almost the same, up to 10% slower on most sizes with compact string. it's up to 30% faster with compact strings off, but the size benefit seem to outweigh this easily, see also unserialize performance next.
* compression is fairly expensive
* igbinary unserialize is not quite as much faster as the issue title claims, but it seems to be a fairly stable 60-70% of unserialize() time on most sizes. it's actually slower with compact strings off, probably because there's a lot more data to work through?
* combined with uncompression, igbinary unserialize is about the same as serialize(). I haven't done any comparisons with how much we save in network/communication with the cache backend, but I would assume that igbinary + uncompression having the same speed as unserialize() seems like a huge win when we only have to transfer 4-20% of the data instead from redis and can store much more in the cache
* didn't think too much about absolute numbers, but the compression cost increases a lot in relative numbers on small cache entries. around 8x (3x for unserialize) for that 1300 length ckeditor cache and and 12x (5x for unserialize) for that tiny 300 length image toolkit cache. for comparison, empty/small/redirect render caches start around 250 length. Redis currently documents a length of 100 for the compress flag. I just picked that number fairly randomly and documented that people should do their own tests. I doubt any/many did. Anyway, I think that is clearly too low. maybe 1000? or higher? maybe someone who is better at math than me could calculate the network vs cpu overhead and what value makes sense.
* as expected, higher compression levels only result in minor improvements in size, with a massive cost in serialize speed (6 is 2x as slow as 1), so doesn't make sense to go higher. unserialize on higher compression level is actually slightly faster, but not enough to justify it I think.
* compression and uncompression is faster on igbinary than serialize, probably because the input string is already way shorter. for example views_data unserialize is 87 vs 32 overhead.
* When I was about to submit, I realized that having a mostly string cache, specifically page cache could also be interesting. Not for igbinary because serialize() and igbinary are pretty much identical there but for compression. So I added that as well (umami frontpage) and did rerun the script. compression in relative numbers looks very expensive there, but that's just because serialize of a very long string is very fast. the html too gets reduced to ~18% of the size, at a cost of 35 (compress) and 17 (uncompress). That's pretty consistent in regards to size with other caches, unsurprisingly.
Comment #22
catchShould this be 60%?
Comment #23
berdirYes, I meant to say 60-70% with that x, edited to make that clearer.
Comment #24
catchThanks that makes sense.
I think this is very likely to be the case for the database cache backend too.
Also, when looking at memory issues I often see quite a lot from database queries and unserialize for large cache items. it's possible that uncompression means that would get transferred elsewhere but we might get lucky and it ends up a net reduction.
Also I wonder if it's worth looking into gzip compressing tags?
Comment #25
berdirFWIW, looking at blackfire data does show that in some cases, the cost of a gzuncompress is considerable, specifically on page cache hits:
Not sure but I would assume that blackfire doesn't add a huge overhead to a function call like that. That's 3.8ms. That's still with plain serialize, about to do compare that with igbinary and also without compression.
I've tried to do some testing with either blackfire or just plain ab on all combinations of serialize/igbinary/compression, but it's tricky, variations are too high between runs to really see a clear pattern. It's also not always that high I think.
Comment #26
berdirAs expected, variation is too high for page cache to really compare in terms of speed/IO wait, without compression the response time was a bit lower, but it also claims to have less IO wait, which doesn't make sense obviously.
Network is probably the most reliable metric change:
Network+547 kB (+419%)
131 kB → 678 kB
Memory also went up a bit, but only 2%.
On a dynamic page cache hit, the gzuncompress is at 2% or so, so way less visible and comparing with and without compress gives me:
Network-1.37 MB (-86%)
1.58 MB → 215 kB
That's pretty neat.
One random but completely unrelated thing that I saw pop up is Drupal\help\HelpTopicTwigLoader, that adds all modules as extension folder and does an is_dir() check on them. that's enough to account for about 5ms in Blackfire and happens even on a dynamic page cache hit it seems. Will try to create a separate issue for that.
Last unrelated side note: If I'm seeing this correctly, then all the performance things I've been working on in redis and core (where we've included patches so far) resulted a reduction of Redis::hgetAll() calls from 54 to 27 in this project, and combined with the switch to igbinary, from 400kb network to 200kb. And I'm working on more, such as route preloading.
Comment #27
fgmI think there is one important issue here : is the Redis under test remote from the Drupal instance, or local to it ?
In most - if not all - enterprise setups I see doing audits, the Redis/Valkey servers are always either on the DB instances or completely standalone, not on the web servers (unlike memcached). This means that the impact of bandwidth reduction is much more relevant to overall performance in those setups than it is when benchmarking on a local instance.
These tests being run on DDEV make me suspect the measurements are for a local instance, however. Maybe it would be useful to try the same on two separate AWS/GCP/Azure/OVHCloud instances instead ? Or even a container and something like Elasticache for Redis in the same AZ, which will likely not co-locate the Redis on the same instance.
Comment #28
catchAdding #3421234: Json and PHP serializers should throw exception on failure as related. Didn't review that issue properly yet but from the summary looks like we should already be catching and ignoring that exception in the cache backends.
Comment #29
berdirRe #27: Yes, redis being on the same server or not can make a huge difference. FWIW, all data in comment #27 is based on tests on a regular platform.sh project. The question I'm trying to answer/provide data for an answer in #25/#26 is whether core should use compression by default or not.
We also have a dedicated Gen2 project on platform.sh that uses multiple servers and would be more interesting for the impact/advantage of compression in such a scenario, but I don't have enough insights/blackfire there to be able to do that kind of profiling.
Either way, our default configuration probably shouldn't be optimized for that scenario at the cost of a less "enterprise" setup, that said, at the other end of the scale you have "classic" webhosting that often also has separate database servers.
Comment #30
catchFor the database cache there's also the issue that cache tables can end up holding more data than the rest of the database itself. We did https://www.drupal.org/node/2891281 but there was someone in slack actually trying to use that with what sounded like a medium-traffic site and running into lots of problems with the delete queries and similar. So if it's neutral or a very small regression with the database cache, it might be worth it anyway.
Comment #31
fgmI think at some point Platform tried to maximize co-locating containers in a project to close instance, from what DamZ told me long ago. I wonder if that is the case in these experiments.
Comment #32
hanoiiI recently stumbled upon igbinary and this great thread, and I wanted to share some recent real-life improvements that really surprised me.
This is a very VERY heavy setup (in terms of entity relationships, site building, lots of Layout Builder and custom stuff) with significant technical debt, so Drupal is doing a lot. For example: 8k block plugins, ~80 MB of RAM just for PHP deserializing this. (The site has a moderate amount of content — not tiny, but not massive either. It used to run as a multisite with 12 sites on a Platform.sh large plan, and the sites frequently stalled and timed out. I recently moved each site to its own medium plan on Platform.sh — 1 GB RAM Redis/Valkey, 1.25 GB RAM MySQL, 256 MB RAM app container with only 2 FPM processes.)
On Redis I only keep the entity and render cache bins plus the smaller ones. I had to move page_cache and dynamic_page_cache to the DB because Redis was evicting too many keys (before igbinary; I might revisit this after things stabilize). I’m also being very lax about when I clear the page rendering–related bins (they don’t normally get cleared on every deploy).
The figures below are from php.access.log on Platform.sh, which logs response time, RAM, and CPU. I collected these over a few days — I specifically wanted to see how it behaved on a non-weekend day. This sample is from one of the sites with the most traffic. Even after a fully cleared cache (everything, including an empty Redis service) it performed A LOT better.
I honestly wasn’t expecting this improvement. Response times improved — less dramatically, since most of the traffic is anonymous and once pages are cached they already respond fast enough (<100 ms). But there’s still a clear shift of the distribution towards the lower end. RAM, on the other hand, shows an even greater improvement. Before igbinary I was seeing cached responses take over 150 MB of RAM. I’m not exactly sure why, but I think it was mostly due to large deserializations. CPU is less conclusive — I don’t think the peak info from the logs is a very meaningful metric. CPU will vary a lot depending on many other factors, especially in a containerized app like this.
I’m open to any suggestions, questions, or follow-up!
Comment #34
alexpottRe #2 / #4 how about adding something to the cache key in \Drupal\Core\Cache\DatabaseBackend::normalizeCid() depending on what serializer we're using? Then we wouldn't get error if igbinary becomes available or goes away?
Comment #35
berdirWe could also invalidate all cache data if we fail to unserialize it, #3421234: Json and PHP serializers should throw exception on failure should help with that. I think a bigger concern would be non-cache data that we can not rebuild that is currently serialized, but we should probably never have used serialize/unserialize for those in the first place, but it's been a very long and so far not very effective journey to change that (think keyvalue with things like installed entity type/field definitions, link fields and other field types storing serialized data, ...)
Comment #36
longwaveWe could break this down into multiple issues: just add the igbinary serializer here, and document how to switch to it; then it's up to site owners if they actually want to make the change to start with, because they know it is safe for their environment. Then we can defer the problem of auto detection and fallbacks to a followup.
Comment #37
berdirSounds good to me as a first step.
redis has docs on how to use igbinary, process for it being in core/database backend should be very similar: https://project.pages.drupalcode.org/redis/#using-igbinary-serialization
I also did some more testing around compression, without igbinary but that shouldn't have a huge impact: https://www.drupal.org/project/redis/issues/3570218#comment-16461143
That is I think valuable if we also combine igbinary with compression. tldr, the length setting I have in redis is IMHO worth it and 1000 a sensible threshold to start compression.
Comment #38
andypost@berdir do you mean to split serializer and cache item compression?
Comment #39
berdirNo, I neither said that we should split nor that we shouldn't. Just that if we support compression (which we do not yet in core either in the cache or as a seriailzer), that should have a threshold setting as to when we apply that.
the igbinary project provides separate serializers with/without compression and they all can detect and apply the appropriate uncompression/unserialization but only from more "advanced" to less, meaning, when you configure it to use igbinary+compress, it can handle existing that that's not compressed, aand then the igbinary parent can handle data that used phpserialize(). However, the igbinary serializer can't handle compressed data and the php serializer (in core) can't handle igbinary. So, going phpserialize => igbinary => igbinary+compress can handle existing data, the other direction not so much:
https://git.drupalcode.org/project/igbinary/-/blob/2.0.x/src/Component/S...
I'm not sure what the best approach for this is. Possibly a single new serializer that can detect and support all formats on decode and applies the best possible option on encode, possibly with some setting flags?
I also saw https://wiki.php.net/rfc/modern_compression, didn't get into 8.5 sadly, but in the future we could possibly then switch to zstandard assuming we can detect zstandard vs zlib on decode.
Comment #40
andypost@berdir thank you - good idea so probably the way ahead is #1281408: Add a compressing serializer decorator
Comment #42
andypostAdded minimally required implementation, locally it brings 7-20% boost for serialization and even storage
Comment #43
cmlaraVisual review only:
MR !14948 appears to not be honoring SerializationInterface by failing to properly raise exceptions when faults occur.
See #3421234: Json and PHP serializers should throw exception on failure for where this fault has occurred in the past and reasons why it should not be repeated.
Comment #44
andypost@cmlare good point! added wrapper to catch warnings and errors
Comment #45
catchI did some manual testing locally. I think the performance/memory gains here are well demonstrated already, although I have a specific site I can test that on a bit later today too.
First of all did a standard install via the UI.
Then applied the MR, and verified (via xhprof because it's easy) that igbinary was being used.
Then I reverted the MR - unserialize() exploded, but we don't really need to worry about that because we wouldn't do a straight revert without handling things.
Then I reapplied the MR, but manually broke the function_exists() check for igbinary so it would fall back to unserialize(), to simulate moving to a host with no ig_binary, this also blew up with
InvalidDataTypeException.So then I added a try/catch block in the DatabaseBackend, and I initially got a missing class error for UserSession, changing the deployment identifier fixed this for me and everything else worked. I've pushed a commit for this.
Then I tried to reproduce the UserSession error (which I assume was failing to load the container) and couldn't get it to break the same way again.
So the cache backend needs to handle invalid serialization formats, which previously it didn't, but once we add that it mostly works give or take the container issue I ran into. But even that container issue was quickly fixed by changing the deployment_identifier which seems OK to have to change if you move your site to a completely different server with different configuration. Or the other way to do this would be for the ibinary unserializer to transparently handle the case where igbinary has gone away and return FALSE instead of throwing an exception - due to the prefix added here that specific case can be detected.
I didn't make the same try/catch change for expirable key value store and for me I think we should open a separate issue for that - it's not in the critical path for performance, and I think we need to discuss whether it's OK for that to get essentially 'wiped' if igbinary goes away (for cache backends it obviously is OK).
Comment #46
catchComment #47
catchGot the UserSession error again - switching from HEAD to the branch:
Visiting update.php and clicking through fixed that, so that's another way than the deployment identifier, but would be good to figure out what's going on there and if possible make it more fault tolerant.
Even on a dynamic page cache hit on the standard front page, which only uses about 5mb of memory and 70ms anyway, I'm seeing maybe 5-10ms CPU/wall time saving, which works out at around 10%. Too much variation between requests for that to be accurate but it does look consistently lower.
Comment #48
catchNot for this issue, but we could open a separate issue to recommend setting
apc.serializer=igbinarywhen both igbinary and apcu are available in the status report.I tried that and at least on a dynamic page cache hit it has a very small positive effect, might be more on cache misses or partial cache hits where a lot more config and discovery caches are getting hit.
Comment #49
andypost@catch probably you hit this missing class issue because my branch was outdated (before UserRepository) so the only way I can guess as new serializer added to default bootstrap of
DrupalKernelAlso let's keep cached checks for function availability in local static to prevent abuse API and serialization issues
Comment #50
catchOhhhh if so that's great news because otherwise this is looking very solid to me.
I can live with the local static if there's a good reason for it - but we should mention it in an inline comment.
Comment #51
andypostLooks like it's a good idea to introduce
serialization.defaultComment #52
grimreaperHi,
Thanks for the work done on this issue!
MR looks pretty good to me.
Halfway to make https://www.drupal.org/project/igbinary obsolete. The other half will be #1281408: Add a compressing serializer decorator but that will be for later. :)
Comment #53
grimreaperComment #54
andypostrebased and addressed feedback, also did search in contrib
- https://search.tresbien.tech/search?q=ObjectAwareSerializationInterface
- https://search.tresbien.tech/search?q=serialization.phpserialize&num=50&...
Now sure
serialization.defaulthas any value as contrib should transition to use autowire so new alias seems uselessComment #55
andypostAdded release note snippet and CR (asked Opus to generate detailed one)
Comment #56
andypostUpdated IS with Symfony state
Comment #57
grimreaperTo not forget, Slack discussion: https://drupal.slack.com/archives/C079NQPQUEN/p1777106593402379
Comment #58
quietone commentedWhen referencing Slack discussion it is preferred that the thread is summarized, including who participated. We can't rely on Slack history being available.
Comment #59
grimreaperOk.
Slack discussion involved: @andypost, @longwave @berdir, @grimreaper and @kingdutch.
Discussion was mainly about aliasing service or not. And declaring a default serialization service.
This @berdir's comment potentially increase the scope of this issue:
Please feel free to put other important comment back in the issue.
Comment #60
catchI think we could do this with just a name change - any additional features could happen in follow-ups, but if we make it purpose-specific rather than implementation-specific it would allow for more things to be added later.
Comment #61
andypostAs we stuck on naming and string instead of interface then maybe introduce
serialization.cache_dataand use it for cache backends, then title could be "Add an optimized cache data serializer using igbinary when available" and put the rest into follow-up?Comment #62
catchYeah I like #61 and
serialization.cache_dataseems fine for naming.Comment #63
andypostCreated follow-up #3590507: Align SerializationInterface::decode() error contract across implementations
I think it needs following work
- remove Expirable k/v from the MR and file follow-up for it
- another issue to audit
ObjectAwareSerializationInterfaceconsumersComment #64
andypostComment #65
catchThis looks good to me, I think everything from #62 and #64 was dealt with.
Comment #66
quietone commented@grimreaper, thanks.
A shorter title should suffice, anyone who wants to know why a change was made can read the issue.
Comment #67
longwaveTo throw in a curveball here, Symfony has just dropped a new Deep Cloner feature which can serialize with a claimed 30-40% size reduction and 4x-15x speed improvement, along with a native PHP extension that performs even better: https://symfony.com/blog/new-in-symfony-8-1-deep-cloner
While that probably shouldn't interrupt this issue it's another good reason to think about how to make the choice of serializer configurable.
Comment #68
catchI think that means picking a generic service name that we can potentially support more implementations in is definitely a good idea. We could open a follow-up to compare with igbinary.
Comment #69
berdirAdded one comment to the MR.
Also setting to needs work to update the CR, because that's outdated.
I like the generic service name and not automatically replacing the existing one.
On the compression topic, I found #1281408: Add a compressing serializer decorator, quite old and the issue summary/title actually doesn't reflect what the earlier patches there are doing (the decorator was proposed by fabianx), but we could pick that up as a follow-up and then implement that idea on top of this. That's better than inheritance to combine that together I guess, but we'll need to see how that works with the early bootstrap container, means another change for that.
Comment #71
longwaveTried to move this along a bit. Fixed the case where igbinary is no longer available, we throw an exception in the decoder and catch it in the cache backend and convert it to a cache miss. Also renamed the service to serialization.igbinary and aliased cache_data to that, so it should be swappable if anyone wants to.
Comment #72
longwaveI almost wonder if we need a separate deserializer service which can detect the format somehow and delegate to the correct deserializer if available. Thinking forward to if we introduce a third serializer and want to allow all three types to be used simultaneously for backward compatibility, but maybe that's just not worth the effort.
Comment #73
berdirI think I'd rather not use igbinary in the service or class name to cover more advanced use cases but something like OptimizedSerializer, maybe even explicitly OptimizedCacheSerializer, because this is designed for data that *can* be thrown away, it shouldn't be used for example for key value where we want to keep it.
Not sure what a third format might be, but something we absolutely should add is compression, with #1281408: Add a compressing serializer decorator, we could use a decorator s mentioned there, but we could also go with a all-in-one optimized serializer.
Comment #74
longwaveSee #67 for the possible third option
We could optionally compress it and/or add a custom format header before storing.