Dont use the ThreadDeathWatcher to release memory back to PoolArena a…#7446
Dont use the ThreadDeathWatcher to release memory back to PoolArena a…#7446normanmaurer wants to merge 2 commits intono_thread_death_watcher_fast_thread_local_threadfrom
Conversation
…ocalThread with wrapped Runnable is used Motivation: We dont need to use the ThreadDeathWatcher if we use a FastThreadLocalThread for which we wrap the Runnable and ensure we call FastThreadLocal.removeAll() once the Runnable completes. Modifications: - Dont use a ThreadDeathWatcher if we are sure we will call FastThreadLocal.removeAll() - Add unit test. Result: Less overhead / running theads if you only allocate / deallocate from FastThreadLocalThreads.
…fter PoolThreadCache is not used anymore Motivation: At the moment we use the ThreadDeathWatcher to release memory back to the PoolArena after the PoolThreadCache is not used anymore if the cache is used by a non-FastThreadLocalThread. This will need to spawn on extra thread and iterate over all the Threads each second. Modifications: Better just use a finalizer to return the memory back to the PoolArena after the cache is not used anymore and the cache belonged to a non-FastThreadLocalThread. Result: Less running threads / overhead.
|
PTAL... This depends on #7445. |
|
Hmm... since finalize is deprecated in Java 9, do we want to do something else? We could keep the cleaner thread around and use |
|
@ejona86 hmm yeah we could have some kind of "ReaperThread" that will cleanup this stuff... Even this would be better then the So I think a WDYT ? |
|
I've found adding threads within a library to be difficult. If we remove it now, then depending how much time passes I'd anticipate re-adding it to cause breakage. Using Is there a reason to try hard to kill |
|
@ejona86 |
|
@ejona86 actually I would argue that if we would run on java9 only we should use the WYTD ? |
|
"on java9 only", I'd agree with that, but given support is currently Java 6, it's a pretty moot. Cleaner when on Java 9 sounds good, but we will need a fallback. If we chose Hmm... classloader leak... I guess the failure case is the ThreadLocal instance could be thrown away, and let the storage evaporate (since Thread uses WeakReference for the mapping), but the Thread itself remain running. So ThreadDeathWatcher continues running unawares, and in fact maybe even transitively retains the ThreadLocal because it retains the ClassLoader. So it seems the problem is the false assumption that the ThreadLocal storage's life ends at the same time as the thread. I guess that's why Cleaner has "The context class loader of the thread is set to the system class loader." Aaaand... we couldn't do the same approach, because the Cleaner trick relies on the fact that Cleaner is in the system class loader. Does my analysis feel right? If so, then it seems |
| }; | ||
| } | ||
|
|
||
| // We create an instanceof this if we need to call free() as part of the GC of the cache. |
|
Wait... if my analysis is correct, then even I feel like something else needs to give. Either the caching needs to go, or an API to clear the cache, or some slow caching path, or something. For the slow caching path, maybe we could force all non- |
| // Wait for the ThreadDeathWatcher to have destroyed all thread caches | ||
| // Wait for the GC to have destroyed all thread caches | ||
| while (allocator.metric().numThreadLocalCaches() > 0) { | ||
| System.gc(); |
There was a problem hiding this comment.
Also System.runFinalization()
| @Override | ||
| protected void finalize() throws Throwable { | ||
| try { | ||
| free(); |
There was a problem hiding this comment.
is it safe to call free multiple times?
132758b to
7e131b8
Compare
|
@carl-mastrangelo @ejona86 WDYT of the approach: #7463 ? |
|
Let me close this.. as we should better use #7463 |
…fter PoolThreadCache is not used anymore
Motivation:
At the moment we use the ThreadDeathWatcher to release memory back to the PoolArena after the PoolThreadCache is not used anymore if the cache is used by a non-FastThreadLocalThread. This will need to spawn on extra thread and iterate over all the Threads each second.
Modifications:
Better just use a finalizer to return the memory back to the PoolArena after the cache is not used anymore and the cache belonged to a non-FastThreadLocalThread.
Result:
Less running threads / overhead.