Skip to content
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

Improve buffer recycling for platforms that have broken SoftReference (Android) #67

Closed
cowtowncoder opened this issue Mar 29, 2013 · 6 comments

Comments

@cowtowncoder
Copy link
Member

Since Google seems unable to provide sane handling for SoftReferences, perhaps we can work-around this issue to some degree, at least for a relatively common case of single-threaded reuse.

The basic idea would be that before checking for combination of ThreadLocal and SoftReference, it'd be possible to use a share AtomicReference for a single buffer. If an instance is found this way, it could avoid use of SoftReference-based alternative; if not, it could use current handling.
This should not add significant overhead over current approach (it might even improve it slightly), but should work better on Android.

@cowtowncoder
Copy link
Member Author

Ok, one complication: the way BufferRecycler is handled, a ThreadLocal instance is accessed by JsonFactory, but never returned (it is set if need be, to allow for full access from same thread). This is problematic wrt one of possible ways to handle this.

@cowtowncoder
Copy link
Member Author

Quick note: Jackson 2.6 adds JsonFactory.Feature.USE_THREAD_LOCAL_FOR_BUFFER_RECYCLING that allows disabling buffer recycling (as per #189), which could help a bit here.
While it would be nice to have some alternate form of recycling (as mentioned, via singleton, AtomicReference), at least removing attempts to use ThreadLocal/SoftReference should offer minor improvement on Android.

@jborgers
Copy link

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.

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/

@cowtowncoder
Copy link
Member Author

cowtowncoder commented Sep 14, 2017

@jborgers Could you file a separate issue, since although it is sort of related, it sounds like separate issue?
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.

@cowtowncoder
Copy link
Member Author

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.

@cowtowncoder
Copy link
Member Author

Solved by #1089 for Jackson 2.16 and later: pluggable buffer recyclers including multipl non-SoftReference based alternatives.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants