fix: use Clear(key) instead of ClearByKey(key) for exact-key cache removal - #1022
Open
AndersKaae wants to merge 1 commit into
Open
fix: use Clear(key) instead of ClearByKey(key) for exact-key cache removal#1022AndersKaae wants to merge 1 commit into
AndersKaae wants to merge 1 commit into
Conversation
…moval SyncEntityCache.AddName/GetEntity and ContentTypeBaseSerializer's alias cache all called DictionaryAppCache.ClearByKey(exactKey), which does a full LINQ scan of every entry in the cache looking for a prefix match, even though the key passed is always an exact id/guid string with no other entries sharing that prefix. DictionaryAppCache already exposes Clear(key), a direct ConcurrentDictionary.TryRemove - O(1), no scan. nameCache in particular is shared across an entire Export/Report run and grows by one entry per resolved ancestor name, so ClearByKey's O(n) scan compounded into genuine O(n^2) cost as the cache grew - confirmed via CPU profiling (~91% of wall time in the LINQ Where/ToArray + ConcurrentDictionary enumeration) and an isolated benchmark (60k items: 27.4s vs 27ms, ~1000x). Left 5 other ClearByKey call sites unchanged (uSyncService_Handlers, SyncHandlerRoot x2 incl. PrepCaches/CleanCaches) - those rely on genuine multi-key prefix matching, one explicitly documented in a comment as "a starts with call" for clearing a related folder cache. Changing those to Clear() would silently break that behavior. Local fix for LegalDesk V2's uSync migration validation work. Not yet upstreamed to Jumoo - see LegalDesk-V2 task history for the diagnosis.
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
2 participants
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.
SyncEntityCache.AddName/GetEntityandContentTypeBaseSerializer's alias cache all callDictionaryAppCache.ClearByKey(exactKey), which does a full scan of every cache entry looking for a prefix match, even though the key passed is always an exact id/guid with no other entry sharing that prefix.DictionaryAppCachealready exposesClear(key), a direct dictionary removal with no scan.nameCachein particular is shared across an entire Export/Report run and grows by one entry per resolved ancestor name, so the O(n) scan compounds into O(n^2) as the run progresses. Confirmed via CPU profiling (~91% of wall time in theWhere/ToArrayscan) and an isolated benchmark: 60k cached items, 27.4s vs 27ms (~1000x) for a single clear-and-reinsert.Left the other 5
ClearByKeycall sites (uSyncService_Handlers,SyncHandlerRoot:PrepCaches/CleanCaches) unchanged since those rely on genuine multi-key prefix matching (one is explicitly documented as "a starts with call").Local fix for LegalDesk V2's uSync migration validation work, moving our real content tree (~440k items) between environments. We'd struggled with this since a support thread in July ("Usync complete issues"), where the Publisher web UI was timing out in minutes on even tiny syncs; we eventually moved to the CLI tool instead (SignalR in the web UI looks like the more likely cause of those particular fast timeouts, separate from this). The CLI then ran into the plain
HttpClient100s timeout on our real-scale Export/Report, and profiling that long-running process is what surfaced thisClearByKeyscan as a genuine O(n^2) bottleneck.Rebased onto current
v13/main; builds clean.