-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[API Proposal]: HybridCache: span-based fetch #112866
Comments
/cc @jodydonetti |
Tagging subscribers to this area: @dotnet/area-extensions-caching |
@mgravell is the idea here that whenever there is a cache hit, it doesn't end up allocating the If that's the case, wouldn't it still be a better idea to create a dedicated Using your example: var value = hybridCache.GetOrCreateAsync(new MyCustomKeyStruct(key, id), ...); Or is there a major downside to doing something like that? Sorry if this is a dumb question, I'm just not as familiar with interpolation handlers yet and was curious. |
@julealgon the idea of HybridCache here is to radically simplify dealing with Now contrast Basically: the string-like API is a very practical and efficient way of avoiding a wide raft of problems; this last change allows us to avoid the actual
Don't forget:
There is also the "no IDistributedCache" scenario; we'd still need to box the struct, which isn't free, but is probably cheaper than the string, I'll grant - but we'd also lose the dictionary rehashing feature; this is specific to Also, to underline the point: |
As someone who is currently heavily using only
Yeah, we've been creating dedicated key
I feel dumb for not realizing this. Of course... because it's a value type and the key is not generic. 🤦♂️
Noted. Thanks!
Interesting. So, at the end of the day, I believe in our current scenario (pure
👍 |
Not really, it's |
Hi @mgravell
Dumb question (I'm still thinking about it), but in the case of L1+L2 this will allocate a string every time L2 is needed, correct? Not that it is worse than now, mind you, just pointing it out for me to get the full picture. |
@jodydonetti you might've misunderstood what I meant there. I was precisely talking about I was just making a point that we are not yet using I was not talking about HybridCache's interface there. |
Yes I got that, I referenced one of When Marc mentioned "IMemoryCache talks object, and IDistributedCache talks string" I think he was referring to the keys being Anyway it seems like I got that wrong, my bad. |
Alternatively, instead of public abstract partial class HybridCache
{
+ public abstract ValueTask<T> GetOrCreateAsync<TKey, T>(
+ TKey key, Func<CancellationToken, ValueTask<T>> factory, HybridCacheEntryOptions? options = null,
+ IEnumerable<string>? tags = null, CancellationToken cancellationToken = default)
+ where TKey : ISpanFormattable, allows ref struct;
} In this case you could make a custom type like var value = hybridCache.GetOrCreateAsync(new MyCustomKeyStruct(key, id), ...); However, Example extensionpublic static class DefaultHybridCacheExtensions
{
// Extension for DefaultInterpolatedStringHandler
public static ValueTask<T> GetOrCreateAsync<T>(this HybridCache cache,
ref DefaultInterpolatedStringHandler key,
Func<CancellationToken, ValueTask<T>> factory,
HybridCacheEntryOptions? options = null, IEnumerable<string>? tags = null,
CancellationToken cancellationToken = default)
{
var wrapper = new DefaultInterpolatedStringHandlerWrapper(key);
return cache.GetOrCreateAsync(wrapper, factory, options, tags, cancellationToken);
}
// Wraps DefaultInterpolatedStringHandler to ISpanFormattable
private ref struct DefaultInterpolatedStringHandlerWrapper(DefaultInterpolatedStringHandler handler) : ISpanFormattable
{
private DefaultInterpolatedStringHandler _handler = handler;
public bool TryFormat(Span<char> destination, out int charsWritten, ReadOnlySpan<char> format, IFormatProvider? provider)
{
// Workaround: Text is internal. PR #112171
var span = GetText(ref _handler);
if (span.Length > destination.Length)
{
charsWritten = 0;
return false;
}
span.CopyTo(destination);
charsWritten = span.Length;
return true;
[UnsafeAccessor(UnsafeAccessorKind.Method, Name = "get_Text")]
static extern ReadOnlySpan<char> GetText(ref DefaultInterpolatedStringHandler handler);
}
public string ToString(string? format, IFormatProvider? formatProvider) => _handler.ToString();
public override string ToString() => _handler.ToString();
}
} Down-side: the key has to be copied from |
Maybe that makes sense for an additional/extension convenience API, but we could only use it to generate a string-like key - we couldn't use that as the key (can't store an allows ref-struct, and even if we remove that: boxing, etc) - and I think it complicates the key-generation mechanism - it becomes an interpolated string with extra steps |
This is smart, and I like the automatic lifting. Right now I'm thinking about the possible intersections of the different overloads, including the ones with the stateful factory, which when used is typically done to pass along the key to the factory itself.
Yes, this is very important to avoid surprises.
👍 One question: there's no plan for new overloads of Set/Remove/etc, right?
There's a lot going on already, and 10 will be LTS, so I would stick to that only. My 2 cents.
Good call, preferential treatment for the happy path.
Chef's kiss. Closing remarks: this is all really nifty stuff, and it can make a difference with really high perf scenarios. I'f I'm being honest, the thing I'm worried about is how all of this will play out with the rest of the codebase, overloads and scenarios. For example logging/OTEL: if they are enabled, and the key is (presumably) written, would it still be possible to zero-allocate? And let's say it's not possible: we may think "no big deal, we would have had the string anyway" right? Eh, no sure about that, since if we don't keep around the string key for the lifetime of the entire method (and sub-methods), at every log/trace point we would need to allocate a new string, every time, or store that at first use and pass that around. I hope the problem is that I just don't have imagined every little piece in a clear way right now, but just wanted to share these second-order thinking with you and the team. |
Re logging/OTEL, I can think of some ways of doing that without allocating, but I suspect you'd usually not want to not log extensive data in the "hit" path - so maybe a more pragmatic approach here is to ensure you have a string if full debug logging is enabled? My intention here (for the default implementation) is to use a single code path between the two versions via
Using
That is correct for now. If there's a real need for them: then maybe! Perhaps questionable for Add, for the simple reason that you're always going to need to materialize a string for add, to store the key - unless perhaps it already exists and you're throwing! But remove, upsert, etc... not necessarily a problem. |
Being genuinely serious: I'm totally fine with that being an upsell "here's a scenario where FusionCache offers a more tailored API for these nuanced scenarios" - I don't think HybridCache is ever going to tick every box for every niche. |
Really appreciate the transparency... even though even FusionCache is not there yet 😅 (see here). Thanks! |
Background and motivation
This is follow-on work related to DefaultInterpolatedStringHandler and MemoryCache; the core idea here is that
HybridCache
should be able to satisfy local "hit" requests synchronously without the code of astring key
parameter, instead leaving the key unmaterialized. This is achieved by taking the existing 2HybridCache.GetOrCreateAsync
methods that take astring key
parameter, and add an overload that hasref DefaultInterpolatedStringHandler key
instead. Since Roslyn prefers this signature, existingHybridCache
code of the form:will automatically update (at next recompile) to the new usage, with zero code changes required. We will also ensure documentation is clear about preferring this API.
Since this is a public abstraction with 1st and 3rd party implementations, and
DefaultInterpolatedStringHandler
is a dangerous type, we do not blindly let implementations deal with this; the two new methods are non-virtual
, with a newvirtual
method that takes the key asReadOnlySpan<char> key
. That way, implementations are not exposed to the fiddlyDefaultInterpolatedStringHandler
API and we do not risk leaks / double-pool-restore.API compatibility:
This should probably target net10+ only, since it requires related features. If the
MemoryCache
API gets backported to net9 (unlikely), in theory we could use theDefaultInterpolatedStringHandler
via unsafe-accessor, but: it seems preferable to consider this as net10+.API Proposal
Notes:
abstract
rulesfinally
withoutawait
does not in this instance represent premature cleanup; the span cannot traverse theasync
/await
boundary, so we know it is safe to callClear()
in all exit scenariosThe bottom two methods are the
public
API surface, usingtry
/finally
to ensure correct handling of thekey
. The first two method is theprotected virtual
method that specific implementations may choose tooverride
to exploit allocation-free L1 lookups. If they do not choose to do so, we simply allocate thestring
as we would have originally. This ensures the default behaviour is valid on existing implementations.A note is added to clarify the slightly unusual usage of a span on a
*Async
method:This atypical usage is because of the high probability of local synchronous "hit" results, hence
ValueTask<T>
as the return value.API Usage
No change at the caller:
The text was updated successfully, but these errors were encountered: