-
Notifications
You must be signed in to change notification settings - Fork 583
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
Records cannot be properly used in caches #3008
Comments
There are two ways round this right now:
Index: ehcache-impl/src/main/java/org/ehcache/impl/internal/sizeof/DefaultSizeOfEngine.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/ehcache-impl/src/main/java/org/ehcache/impl/internal/sizeof/DefaultSizeOfEngine.java b/ehcache-impl/src/main/java/org/ehcache/impl/internal/sizeof/DefaultSizeOfEngine.java
--- a/ehcache-impl/src/main/java/org/ehcache/impl/internal/sizeof/DefaultSizeOfEngine.java (revision 8092a5b98f8fa27335377bf1884ac1d708028bd0)
+++ b/ehcache-impl/src/main/java/org/ehcache/impl/internal/sizeof/DefaultSizeOfEngine.java (date 1649711593049)
@@ -25,6 +25,8 @@
import org.ehcache.sizeof.SizeOfFilterSource;
import org.ehcache.core.spi.store.Store;
import org.ehcache.core.spi.store.heap.SizeOfEngine;
+import org.ehcache.sizeof.filters.CombinationSizeOfFilter;
+import org.ehcache.sizeof.impl.ReflectionSizeOf;
/**
* @author Abhilash
@@ -41,7 +43,7 @@
public DefaultSizeOfEngine(long maxObjectGraphSize, long maxObjectSize) {
this.maxObjectGraphSize = maxObjectGraphSize;
this.maxObjectSize = maxObjectSize;
- this.sizeOf = SizeOf.newInstance(new SizeOfFilterSource(true).getFilters());
+ this.sizeOf = new ReflectionSizeOf(new CombinationSizeOfFilter(new SizeOfFilterSource(true).getFilters()));
this.onHeapKeyOffset = sizeOf.deepSizeOf(new CopiedOnHeapKey<>(new Object(), new IdentityCopier<>()));
this.chmTreeBinOffset = sizeOf.deepSizeOf(ConcurrentHashMap.FAKE_TREE_BIN);
} Long term we're going to have to take a long hard look at the whole feature. The JDK is not moving in a direction such that any of this is getting easier. There's some obvious cleanup we can do that will make things better... but I'm wary of sunk-cost fallacies. |
In the long run, should the byte-based cache be spun off as a separate dependency from Ehcache, the base Ehcache only provides a number-based cache and a range of applications? |
In the long term (exactly how long is not clear...) we'll probably be dropping internal support for byte sized caches. I won't drop the ability to configure them... but there will be no built-in store provider that would handle those configurations. If/when this happens it would be associated with a bump of the Java support to 1.11 minimum, and a bunch of overhaul of the underlying code to take advantage of that. |
ehcache/ehcache3#3008 Cache sizes are determined when they are initialized, except for the 2 bill caches, which remain in app.properties.
This will require the productizing and merging of ehcache/sizeof#64. |
Hi, we just ran into this issue, with our DTOs being records :-/ |
|
Hi, thanks for the reply. |
If a cache is sized using MemoryUnit, and the cache key or value contains a record (which were added in Java 17), any attempt to put a value in the cache will fail. The core cause of this failure is this line in sun.misc.Unsafe:
A simple project which shows the issue in practice is linked here.
This seems like a serious issue, because Ehcache gives no clear indication of why this failure happens, and records are natural to use as keys.
The text was updated successfully, but these errors were encountered: