Skip to content

Cache improvements: support reference identity and no eviction mode#18859

Merged
T-Gro merged 2 commits into
dotnet:mainfrom
majocha:cache-yield
Aug 28, 2025
Merged

Cache improvements: support reference identity and no eviction mode#18859
T-Gro merged 2 commits into
dotnet:mainfrom
majocha:cache-yield

Conversation

@majocha

@majocha majocha commented Aug 19, 2025

Copy link
Copy Markdown
Contributor

Improve Cache to make it usable in more places where we currently use ConcurrentDictionary.
The interface is now compatible with ConcurrentDictionary so it can be easily swapped now to get telemetry and, if needed, eviction.

Changes include:

  • Added back NoEviction mode, that makes the cache into a thin ConcurrentDictionary wrapper.

  • Added option for custom identity for keys, HashIdentity.Reference is now supported.

  • Added store rebuilding during eviction: if key identity is not 100% reliable, (e.g. type subsumption cache keys) we can end up with unreachable entries that cannot be evicted. If those build up enough, we just drop the old store and build a new one with good keys only.

  • Added graceful disposal for caches of shorter lifetime. Finalizer will now cancel and dispose the background eviction mailbox processor. Telemetry now includes counts of creations and disposals.

  • Factored away the telemetry, so nothing is allocated by the cache itself.

  • Added sensible methods, GetOrAdd, AddOrUpdate. The current interface is:

type internal Cache<'Key, 'Value when 'Key: not null> =
    new: options: CacheOptions<'Key> * ?name: string -> Cache<'Key, 'Value>
    member TryGetValue: key: 'Key * value: outref<'Value> -> bool
    member TryAdd: key: 'Key * value: 'Value -> bool
    member GetOrAdd: key: 'Key * valueFactory: ('Key -> 'Value) -> 'Value
    member AddOrUpdate: key: 'Key * value: 'Value -> unit

Some more tests are added, too.

@github-actions

github-actions Bot commented Aug 19, 2025

Copy link
Copy Markdown
Contributor

⚠️ Release notes required, but author opted out

Warning

Author opted out of release notes, check is disabled for this pull request.
cc @dotnet/fsharp-team-msft

@majocha majocha force-pushed the cache-yield branch 2 times, most recently from 706b850 to ecc77e7 Compare August 25, 2025 12:04
Comment thread src/Compiler/Utilities/Caches.fs Outdated
Comment thread src/Compiler/Utilities/Caches.fs Outdated
@majocha majocha force-pushed the cache-yield branch 2 times, most recently from 6abe415 to 41fd06f Compare August 27, 2025 21:37
@majocha majocha changed the title Cache: support reference identity Cache improvements: support reference identity and no eviction mode Aug 28, 2025
@majocha

majocha commented Aug 28, 2025

Copy link
Copy Markdown
Contributor Author

I removed from this PR any changes that plugged the cache in additional places where ConcurrentDictionary was in use before.

This would be best to do in another PR for easy revert if there are problems.

@majocha majocha marked this pull request as ready for review August 28, 2025 07:07
@majocha majocha requested a review from a team as a code owner August 28, 2025 07:07
Comment thread src/Compiler/Utilities/Caches.fs
@github-project-automation github-project-automation Bot moved this from New to In Progress in F# Compiler and Tooling Aug 28, 2025
@T-Gro T-Gro added the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Aug 28, 2025
@T-Gro T-Gro enabled auto-merge (squash) August 28, 2025 13:39
@T-Gro T-Gro merged commit 9425e4d into dotnet:main Aug 28, 2025
38 of 39 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in F# Compiler and Tooling Aug 28, 2025
@majocha majocha deleted the cache-yield branch August 28, 2025 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants