Skip to content
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

netcore GetOrAdd doesn't work with SizeLimit set #38

Closed
dozysleeper opened this issue Jun 5, 2018 · 8 comments
Closed

netcore GetOrAdd doesn't work with SizeLimit set #38

dozysleeper opened this issue Jun 5, 2018 · 8 comments
Labels

Comments

@dozysleeper
Copy link

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
Copy link

CCludts commented Sep 30, 2018

Any updates/workaround on this?

@alastairtree
Copy link
Owner

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
Copy link

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
Copy link

simeyla commented Oct 16, 2019

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
Copy link

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
Copy link
Owner

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
Copy link

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants