perf(laravel): add a local cache to the metadata cache factories#8417
Open
nstoeckel wants to merge 1 commit into
Open
perf(laravel): add a local cache to the metadata cache factories#8417nstoeckel wants to merge 1 commit into
nstoeckel wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The Laravel metadata cache factories (
CachePropertyMetadataFactory,CachePropertyNameCollectionMetadataFactory,CacheResourceCollectionMetadataFactory) go through the Laravel cache repository on everycreate()call: store resolution via the facade, key hashing (serialize()+xxh3), and aRepository::get()that dispatches aCacheHitevent per lookup.AbstractItemNormalizerresolves metadata once per property per item, so a large collection response makes tens of thousands of trips through that stack in a single request (48,640 property-metadata lookups for a 2,023-item collection in the profiling from #8413). The Symfony implementation guards against exactly this withCachedTrait's$localCachearray. The Laravel port dropped that layer, so the per-request cost isitems × propertiesinstead ofproperties.Fix
Mirror the Symfony implementation: check an in-memory array before consulting the persistent store.
The factories are registered as singletons (
ApiPlatformProvider/ApiPlatformDeferredProvider), so the array lives for the request, exactly likeCachedTrait's$localCacheon the Symfony side. The persistent store keeps its role of amortizing metadata building across requests.The three classes go from
final readonlytofinalwith individuallyreadonlyconstructor properties, since a mutable array property requires it. The classes arefinal, so nothing can have extended them.??=instead ofCachedTrait'sarray_key_exists(): the difference only matters whennullis a legitimate cached value, andnullis unrepresentable here. The decorated factories have non-nullable return types, andrememberForever()treatsnullfrom the store as a miss and falls through to the callback, so neither layer can ever produce it.Measured on the reproduction from #8413 (2,023-item unpaginated collection, 7 scalar properties, plain JSON, v4.3.17): serialization drops from 1.18s to 0.61s.
Tests
Added
Tests/Metadata/CacheMetadataFactoriesTest.php. The memoization proof avoids mocking the cache: perform a lookup, wipe the persistent store, look up again. The decorated factory must have been consulted exactly once, which is only possible if the second lookup was answered from memory. Also covers independent caching per key and the write to the persistent store.The Laravel component suite passes unchanged.