DefaultCache.calculateSize does not try very hard to deal with Collections and other
not serializable components. Please note that an object does not have to be java.io.Serializable
to be cached.
A patch attached improves the situation described in the dicussion in two ways:
java.util.ArrayList$SubList and collections are handled properly now
not serializable objects (within a collection or not) will issue a warning:
org.jivesoftware.util.cache.DefaultCache - Unable to determine size of class java.util.ArrayList instance
with a stacktrace.
A small jython script to demonstrate the issue is attached.
Assigning to Guus for review, please keep the patches coming!
This patch introduces a new exception everywhere (previous one was breaking the build)
Hello Marcin, thank you for your patch! It looks solid, but is missing some details:
a serialVersionUID in the new Exception class that you added
updated metadata for the clustering plugin. The modifications that you made make the plugin no longer backwards compatible, so we must be sure to enforce that in the plugin.xml! Also, please update the changelog for that plugin.
Other than those two remarks, I've got nothing to add. I was pleasantly surprised by the quality of the patch. Thanks for taking the time to contribute!
Can you please add the last few bits?
Attached please find another try. Yeah, my Java got a bit rusty it seems.
org.jivesoftware.util.cache.CannotCalculateSizeException has now a nice, positive serialVersionUID;
updated metadata of the plugin to require Openfire 3.7.2, bumped pluin version to 1.2.1 with the date as of today;
move DefaultCache.calculateSize() to CacheSizes.sizeOfAnything() and update consumers to use it as well. Sorry for the name - sizeOfObject() was already taken.
replace sizeOfMap(Map<String, String>) with a generic one.
The only downsize of the patch is that it discards all partial correct calculations in case of collections, because we can't throw an exception and keep the calculation result at the same time.
Thanks for the update! I've just committed your patch. Have you tried this in production? I wonder how it affects performance (lots of iterations versus better cache management).