Add mechanism for forcing `BufferRecycler` released (to call on shutdown)

[jborgers]

We see a class loader memory leak by using Jackson: on redeployment of our application in WebSphere we see an increase of heap usage of a couple of hundred MB's en after several redeployments heap usage becomes close to the heap size and garbage collection takes a lot of CPU. Note that most extra heap is taken by the BufferRecyclers retaining char[][]'s.

SoftReferences may help to prevent out of memory errors, it doesn't help for gc overhead (including long compaction pauses.)
In addition, the BufferRecycler classes of previous deployed versions of the app are still in the ThreadLocals of all threads of the threadpool and prevent the classloader with all its classes to be unloaded.
See here: https://stackoverflow.com/questions/17968803/threadlocal-memory-leak for more on classloader leaks.

We would like Jackson to release/remove the BufferRecyclers from the ThreadLocals on shutdown, by calling a shutdown method on e.g. JsonFactory.
See also: http://java.jiderhamn.se/2012/02/26/classloader-leaks-v-common-mistakes-and-known-offenders/

[jborgers]

See #67 and #189

[jborgers]

From #67 posted by @cowtowncoder:
I can see potential issues with hot reloading, although TBH I didn't think anyone would really want to use those in production systems these days (but rather just during development).
If there are convenient hooks for clearing state (and if ThreadLocal allows purging across Threads -- I know that under the hood there are probably means, but JsonFactory does not keep track of anything by itself) adding those would be an improvement.

Thank you for the links that might be helpful here.

One thought on BufferRecycler usage: while bit ugly, it would probably be possible to replace its use with old-skool combination of recycler "class", for static utility methods, but passing Object[] that contains buffers to be recycled. These would need to be cast, but doing this would remove any Jackson provided classes -- buffers are just byte[]s and char[]s after all.
If this is the only class remaining it could help remove ClassLoader reference itself I assume.

[jborgers]

Thanks for your reply.
We actually do hot re-deployments in production.
I can imagine a solution with WeakReferences in stead of SoftReferences in ThreadLocal, to BufferRecycler, and in addition a (strong) reference from e.g. JsonFactory to all BufferRecycler objects, which can be dereferenced/released by invoking a shutDown method. On application stop/undeploy event we could then invoke this JsonFactory.shutDown method.

[jborgers]

In your thought, if you don't put any objects of jackson classes on the ThreadLocal, this would solve the classloading leak. However, if you reference a BufferRecycler by an Object reference (or in array Object[]) the objects itself still reference BufferRecycler class and thus the classloader. Therefore, it doesn't solve the issue.

[cowtowncoder]

@jborgers Use of ThreadLocal is a core feature and rather important for performance. It works really nicely, allows memory to be cleared via GC (with soft references), and I don't have much interest in implementing more complicated buffer reuse scheme.
I am not 100% sure I understand reference wrt weak vs soft references in this context. My recollection was that downside of soft references was that they really disappear very quickly and do not work as well for caches; but that they are indeed cleared when memory pressure so dictates.
So to me this choice does not seem like it would affect retention of ClassLoaders via class.

But as to BufferRecycler, what I meant was that it would be perfectly possible to only recycle an Object[] that contains actual buffers, and either make BufferRecycler only consist of static methods, or just create wrapper from Object[]. This way reference from ThreadLocal would only be to JDK classes (Object[] and char[] / byte[] buffers retained).
It would be slightly ugly but nothing too bad, and if still using recycler instance seems like it could actually be hidden as implementation detail.

[jborgers]

@cowtowncoder I agree that use of ThreadLocal is a nice feature for performance. Only I have an issue with SoftReferences. They prevent out-of-memory however, cause high heap usage and high gc overhead.

As I understand it, having no BufferRecycler instances, so char[][] and byte[][] directly in Object[] in ThreadLocal will indeed solve the classloader leak: there is no reference from the system classloader to a class loaded by the classloader of the undeployed application, so that classloader and its classes can be gc-ed.

However, since each application creates its own ThreadLocal which ends up in a Map in Thread, the buffers created for the undeployed applications are still there. This is most of the added heap usage by the undeployed application.

In my proposed solution, WeakReferences are used instead of SoftReferences, so if no other (stronger) reference references the BufferRecycler, it will be gc-ed. We keep a reference from JsonFactory to all BufferRecycler's in order to be able to release them (dereference) in a shutdown method. I think this list of references should be SoftReferences, to keep the same behavior in case of memory pressure, when not using the shutdown.

[jborgers]

@cowtowncoder I cloned the source, added and changed a few lines of code in JsonFactory as prototype implementation of my proposed solution. Would that be helpful? Should I create a fork?

[cowtowncoder]

Sure, fork is usually a good way, even if just reproducing a problem or showing approach.

[jborgers]

Right, we are going to test this forked version in a load test and do heap analysis to see if it shows the desired effect when doing re-deployments: no more classloader leak and releasing of the unneeded BufferRecyclers in ThreadLocals, resulting in lower, stable heap usage.

[cowtowncoder]

@jborgers That would be great. I think there may be one or two other places (jackson-dataformat-smile has additional recycler, and JsonStringEncoder with this project too. However, eliminating latter should be easy if you can verify that approach works (i.e. if you can see what are the references blocking GC of ClassLoaders). It should also be possible to locally hack JsonStringEncoder to elminiate recycling for testing; and if you don't use smile format it's not relevant yet.

I can go ahead and check other format modules as well; these are the only components with buffer recycling within Jackson components.

[jborgers]

My colleague had good results with our improved version of JsonFactory/jackson-core with a small webservice running on HotSpot: buffers were gc-ed quickly after shutdown. Friday they had a version ready for the whole application running in WebSphere, I expect it will be fully load tested beginning of next week.

[cowtowncoder]

Excellent, looking forward to seeing more!

[jborgers]

@cowtowncoder We have good results from the load test on WebSphere/J9-JVM! The BufferRecyclers are released after the shutdown method is called on undeploy and the classloader is also released.
I put the change in JsonFactory in my fork: https://github.com/jborgers/jackson-core.
How can we go forward?

[cowtowncoder]

One possibility would be a PR; I could either merge it directly, or cut'n paste depending on how things go (I'd want to get this in 2.9 branch most likely, but master is diverging from it quite rapidly due to 3.0 being major API-incompatible change).
Anyway, I just need to see diffs.

Regardless of mode of merging changes one thing I would need is CLA:

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

(or Corporate CLA variant that's found next to it).

This is usually printed, filled & signed, scanned and email to info at fasterxml dot com.
Only needs to be done once for all Jackson contributions, but we need it before first one. Keeps corporate lawyers less unhappy wrt distribution etc.