Skip to content
This repository was archived by the owner on Dec 14, 2018. It is now read-only.

Add to MemoryCacheOptions the CustomCompactOnMemoryPressureDelegate. #252

Closed
wants to merge 4 commits into from
Closed

Conversation

Wallsmedia
Copy link

Add to MemoryCacheOptions the CustomCompactOnMemoryPressureDelegate.

The reason of adding the custom compact delegate:

  1. I have tried the feature MemoryCacheOptions.CompactOnMemoryPressure = true and found that it is a good concept to trigger of revising the memory cache size when the GC collects generation 2 .

  2. However, the compact implementation is to reduce the cache every time by 10%, it is a weird solution. By the short time it will remove everything from the cache. In the real working .net core rest web service, the cached elements have disappeared before it will be reused. So, I don't have a cache.

  3. I propose to introduce in the MemoryCacheOptions the CustomCompactOnMemoryPressureDelegate as
    public Action<MemoryCache> CustomCompactOnMemoryPressureDelegate { get; set; }
    a. If you don't set this delegate the old Compact(0.10) will be invoked.
    b. If setup you setup the delegate, it will be invoked instead.

For example when the Memory Cache is reached the dedicated number 110000 only in this case the compact method will be called.

Example of the custom delegate:

 private void CustomDelegateCompact(MemoryCache cache)
	        {
	            if(cache.Count > 110000)
	            {
	                cache.Compact(0.1);
	            }
	        }

@dnfclas
Copy link

dnfclas commented Dec 10, 2016

Hi @Wallsmedia, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

@Tratcher
Copy link
Member

We actually have concerns about this trigger mechanic. See #221. I don't think we can evaluate your PR until we've resolved our concerns with the trigger.

@Wallsmedia
Copy link
Author

I came through the thread #221.
My solution has been addressed to solve the similar case.
I had the similar issues in my web rest service.

Good in triggering:

  1. The idea of triggering mechanic had been borrowing from Gc.ReRegisterForFinalize , see example.
  2. The calling of garbage collector for generation MemCache: Initial API structure. #2 and collecting elements from generation MemCache: Initial API structure. #2 it is the best moment to review the size of the memory cache.

Issues in implementation of triggering:

  1. The calling of collect generation MemCache: Initial API structure. #2 can be happened more often in case an ASP Web service. Chances are very high to have non cached objects which are survived to generation MemCache: Initial API structure. #2 and released and collected.
  2. The compact has a hard coded compress ration of 10%
  3. The compact triggers after every generation MemCache: Initial API structure. #2 collection.
  4. The triggering mechanism of compacting can be called very often.
    By other words, the ratio may be "1 added to cache" : "> 1 call of compact "

Solution to the issues:
( Not sure if you don't want support backward compatibility by adding features that are not destroy current contract. )

  1. Add ability to define the custom delegate that will be called by triggering mechanism . In this custom delegate you can define a required logic when to call compact and how match to remove.

If you check the test cases in my pull request, you will see the solution to the #221 issue.

@davidfowl
Copy link
Member

Best to close this issue. We're making some design changes that will allow this and we're moving away from this mechanic in general

@JunTaoLuo
Copy link
Contributor

We decided to remove the GC based eviction trigger in #221. We will revisit the design of MemoryCache in #289.

@JunTaoLuo JunTaoLuo closed this Mar 28, 2017
@Wallsmedia
Copy link
Author

It is very nice and fair decision. I will be happy to take part in the pre-re-view coming changes. I will subscribe on further news/changes.

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

Successfully merging this pull request may close these issues.

6 participants