ReflectionCache takes a lot of memory

[jcsdt]

Hello,

We noticed on one of our production system that the ReflectionCache retains indirectly a lot of classes which adds up to a lot of memory.

In our case, after a full GC, we still have close to 2 millions kotlin.reflect.jvm.internal.impl.metadata.ProtoBuf$Type along with other internal kotlin reflect classes taking close to 1 GB on the heap.

Analysing a heap dump, the culprit seems to be javaConstructorToValueCreator which by retaining a list of KParameter indirectly retains kotlin.reflect.jvm.internal.impl.serialization.deserialization.descriptors.DeserializedClassConstructorDescriptor which in turns has a lot of references to other classes.

I'm not knowledgeable enough with kotlin reflection to suggest a solution there. But we could wrap the values of all the maps inside ReflectionCache with SoftReference to let the GC collect them when needed.

Happy to submit a PR if you agree with this solution.

Thank you

Versions
Kotlin: 1.5.31
Jackson-module-kotlin: 2.13.3
Jackson-databind: 2.13.3

Additional context
I link a screenshot of Eclipse Memory Analyzer which show the merged shortest path excluding any soft/weak references.

[cowtowncoder]

I don't maintain this module, but one thing to also consider -- a simpler question -- is whether the default size -- 100,000?!?! -- is way too big.

But I am also surprised that anything would use Method as key etc: this looks.... bad. I wish I knew the reasons for doing this, and whether it has actually been measured to help. On Java side I have found caching of Reflection types to be of little value: JDK does aggressive caching and trying to do it on library side tends to lead to nasty object retention like you see here.

One challenge here is that this module does not currently have an active maintainer it seems. We have had active maintainers occasionally but time to spend on OSS varies. So while we'd welcome PRs, I am also not sure who could actually review it.

But FWTW, I'd be happy to merge a patch to reduce default sizes at very least; and that could go in 2.14 branch. I really doubt big caches make sense (or maybe not even small ones but I digress).

[jcsdt]

Hi @cowtowncoder ,

Thanks for your swift reply. The cache size defaults to 512 entries. Even if we reduce this, a few entries can still accumulate a lot of classes.

At least it's possible to configure this value when instantiating the module and disable the cache with a zero value. But like you said I don't know the consequences performance side.

[cowtowncoder]

@jcsdt Ok I must have misread the stats here. 512 sounds ok. I do like your idea of configurable size, including 0 to disable.
In absence of more advanced handling that seems like a good start -- my suspicion is that caching might be help a lot.
I am bit unsure about SoftReferences as their use tends to bring alternative complications wrt lifecycle and do not work very well on platforms like Android (where AFAIK they are almost immediately drop by GC...).

So I think as the first step, configurable size contribution would be well received.

[k163377]

I worked on reducing cache space in #627 and #628.

I have the following other ideas.

1. remove javaMemberIsRequired.
2. remove javaConstructorIsCreatorAnnotated.
3. merge javaConstructorToKotlin and javaMethodToKotlin.

The rationale for the idea is that the overall processing results are cached on the Jackson side and are not likely to be accessed often enough to affect performance.

However, I believe that micro-benchmark performance validation is essential for these changes (as well as for SoftReference conversion).
I would appreciate it if someone could help me with these verifications.
kogera-benchmark could be used as a base for verification.

[k163377]

@jcsdt
I realized that I had not scrutinized the image you shared with me, so I did a little research.

First, there appears to be a problem with the implementation of ValueCreator.valueParameters.
Since KFunction parameters are managed by SoftReference, if they are initialized again after GC is done, there will be two of them, which seems to double the memory consumption.
This situation occurs in the case of constructors, in calls that use default arguments.

Also, it looks like KParameterImpl.descriptor, which accounts for much of the use in the image you shared, is already SoftReference on the Kotlin side.
Therefore, it may be possible to reduce the final memory usage by caching only the necessary values extracted from KParameterImpl in jackson-module-kotlin.

At least the former is easy to fix and I will create a PR soon.
I am not familiar with SoftReference, so please let me know if there are any mistakes.

[k163377]

Some of the measures related to this issue will be published in 2.15.
If 2.15 shows improvements, this issue will be resolved.

[k163377]

@jcsdt
Now that 2.15.0-rc1 has been released, can you please confirm that memory consumption has been improved?
https://mvnrepository.com/artifact/com.fasterxml.jackson.module/jackson-module-kotlin/2.15.0-rc1

[k163377]

The corrections to the areas raised as an issue have been completed and will be closed.
If you still have problem, please submit a new issue.