alastairtree / LazyCache

An easy to use thread safe in-memory caching service with a simple developer friendly API for c#
https://nuget.org/packages/LazyCache
MIT License
1.72k stars 159 forks source link

netcore GetOrAdd doesn't work with SizeLimit set #38

Closed dozysleeper closed 4 years ago

dozysleeper commented 6 years ago

Hi alastairtree,

We have tried setting the SizeLimit on the cache using the .netcore version of IAppCache, and it doesn't set the size on the entry before GetOrCreate is called on the provider.

We think this is because of the use of the Lazy in GetOrAdd in the CachingService.cs

Here is some test code which fails for GetOrAdd (but works for Add):

IAppCache cacheWithSizeLimit = new CachingService(
                new MemoryCacheProvider(new MemoryCache(new MemoryCacheOptions { SizeLimit = 5 })));

            cacheWithSizeLimit.Add("key1", "item1", new MemoryCacheEntryOptions().SetSize(1));

            cacheWithSizeLimit.GetOrAdd("key2", () => "item2", new MemoryCacheEntryOptions().SetSize(1));

When we test the calls to MemoryCacheProvider directly, it fails only when a lazy is passed to GetOrCreate, we believe because it's using a Lazy it's not executing the call back to SetOptions on the entry.

The example below works for item1 and item2 but not item3.

var memoryCacheProvider =
                new MemoryCacheProvider(new MemoryCache(new MemoryCacheOptions { SizeLimit = 5 }));

            memoryCacheProvider.Set("key1", "item1", new MemoryCacheEntryOptions().SetSize(1));

            memoryCacheProvider.GetOrCreate("key2", entry =>
                                                    {

                                                        entry.SetOptions(new MemoryCacheEntryOptions().SetSize(1));
                                                        return "item2";
                                                    });

            memoryCacheProvider.GetOrCreate("key3", entry =>
                                                    new Lazy<string>(() =>
                                                               {
                                                                   entry.SetOptions(new MemoryCacheEntryOptions().SetSize(1));
                                                                   return "item3";
                                                               }
                                                    ));
CCludts commented 6 years ago

Any updates/workaround on this?

alastairtree commented 5 years ago

Yeah this is indeed a bug, LazyCache cannot support the size limit feature currently. I will be really tough to fix because of the API design, we cannot easily support Lazy initialisation and calling SetSize at the right time. May need to offer a workaround like extra optional arguments to set size directly. Doubt i am gonna have a chance to look at this in the near future but open to sugestions and PRs.

monoclex commented 5 years ago

This would be very helpful. I wouldn't mind if you set the initial size of the task/lazy to 1, and then updated the policy later (if that's allowed)

simeyla commented 5 years ago

I ran into a similar issue with EFCore - which some some reason sets a size on the global IMemoryCache. It basically broke my existing LazyCache usage.

It's SUPER puzzling to me that EF Core thinks it OK to do this.

Anyway I just made my own implementation to make a non-SizeLimit cache.


 // https://docs.microsoft.com/en-us/aspnet/core/performance/caching/memory?view=aspnetcore-3.0
    public interface IMemoryCache2 : IMemoryCache
    {

    }

    public class MyMemoryCache : MemoryCache, IMemoryCache2
    {
        public MyMemoryCache(IOptions<MemoryCacheOptions> optionsAccessor) : base(optionsAccessor)
        {

        }
    }

    // LazyCache
    public class MemoryCache2Provider : ICacheProvider
    {
        internal readonly IMemoryCache2 cache;

        public MemoryCache2Provider(IMemoryCache2 cache)
        {
            this.cache = cache;
        }

        public void Set(string key, object item, MemoryCacheEntryOptions policy)
        {
            cache.Set(key, item, policy);
        }

        public object Get(string key)
        {
            return cache.Get(key);
        }

        public object GetOrCreate<T>(string key, Func<ICacheEntry, T> factory)
        {
            return cache.GetOrCreate(key, factory);
        }

        public void Remove(string key)
        {
            cache.Remove(key);
        }

        public Task<T> GetOrCreateAsync<T>(string key, Func<ICacheEntry, Task<T>> factory)
        {
            return cache.GetOrCreateAsync(key, factory);
        }

        public void Dispose()
        {
            cache?.Dispose();
        }
    }

Then add these:

        services.AddSingleton<ICacheProvider, MemoryCache2Provider>();
        services.AddSingleton<IMemoryCache2, MyMemoryCache>();
        services.AddLazyCache();
AndersMad commented 4 years ago

as @SirJosh3917 said - just setting it to 1 would help - as knowing the actual size of the cached object it impossible before the actual creation and even then it's very likely it will be an estimate anyways.. so maybe just go:

object CacheFactory(ICacheEntry entry) =>
    entry.Size = 1;
alastairtree commented 4 years ago

This is fixed in LazyCache 2.1 because we now allow you to pass the MemoryCacheEntryOptions so they are set on the cache entry object before the cached func is lazily evaluated. The following should now work fine:

cacheWithSizeLimit.GetOrAdd("key2", () => "item2", new MemoryCacheEntryOptions().SetSize(1));
aranthana commented 1 year ago
new MemoryCacheEntryOptions().SetSize(1)

This create new entry in cache for a key every time.
After set like this IAppCache cacheWithSizeLimit = new CachingService( new MemoryCacheProvider(new MemoryCache(new MemoryCacheOptions { SizeLimit = 5 }))); , The GetOrAdd does not check for Get instead adding new entry every time.

DanielTulpWE commented 1 year ago

This is fixed in LazyCache 2.1 because we now allow you to pass the MemoryCacheEntryOptions so they are set on the cache entry object before the cached func is lazily evaluated. The following should now work fine:

cacheWithSizeLimit.GetOrAdd("key2", () => "item2", new MemoryCacheEntryOptions().SetSize(1));

but this does not set the sizeLimit of the overall cache, it sets the size of the cache object being added