-
Notifications
You must be signed in to change notification settings - Fork 217
Remove GC Compaction trigger #221
Comments
@KKhurin Are you able to help us with this? |
Let's have a discussion. |
I just hit what I think is a down side of the current implementation in that it appears we purge the cache on every Gen2 collection, which in my case was occurring during app start as I was forcibly priming my cache. |
Currently compaction is triggered by a gen2 collection by the GC. This is not the best trigger since it may occur frequently and does not correlate to the actual memory usage of the cache. The proposal is to remove the current trigger and instead rely on the user or external triggers to decide when to compact the cache. This will require us to expose the Compact on the interface. Note that there's currently no way of accurately determining the memory usage by the cache so it's difficult to figure out if or how much compaction should occur. |
@JunTaoLuo can you update the bug title to reflect the decisions that were made (e.g. change Foo to X because Y). |
After the latest discussions, we decided to remove the memory compaction trigger based on Gen2 GC. To compact, downcast to |
Will file follow up issue for revisiting the memory cache design. |
@JunTaoLuo is this change announcement-worthy? |
Yup that's why there is a breaking changes tag. Will get to it. |
@JunTaoLuo Do you want to keep the bug open in that case? |
I will open a different issue in the Discussion milestone for the discussion to the announcement. This one just tracks the work item. |
Just checking the workaround for version 1.0.0 is to set CompactOnMemoryPressure to false for the MemoryCache instance? (FYI, what we think we are experiencing as a result of this design bug is poor caching and only 30% memory utilisation on a server with 16GB RAM. Another server with 32GB RAM and the same application is also at 30% memory utilisation.) |
@code-mogwai yes, if this is affecting you, you should set |
@KKhurin @davidfowl
Looks like we may have room for improvement here.
The text was updated successfully, but these errors were encountered: