Hi, I gave a quick look at your patches but to be honest, it's not easy to figure out the problem you're trying to solve in the first place. Or is it simply to discard the use of ref counting to track buffer usages and relying on Java references to keep track of buffers at the expense of creating more pressure on the GC itself to collect and free buffers?
On Wed, Sep 29, 2021 at 11:58 PM Hongze Zhang <notify...@126.com> wrote: > Hi, > > I would like to discuss on the potential of introducing a GC-based > reference management strategy to Arrow Java, and we > have already been working on an implementation in our own project. I have > put the related codes in following branch and > if it makes sense to upstream Apache Arrow I can open a PR for it: > > > https://github.com/zhztheplayer/arrow-1/commits/wip-chunk-cleaner > > In the branch, regarding Commit 1, Commit 2 and Commit 3: > > Commit 1: To break AllocationManager to two components: > MemoryChunkManager[1] which maintains BufferLedger- > BufferAllocator mappings, and MemoryChunkAllocator[2] which performs the > underlying allocate/destory operations. The > previous customizations such as NettyAllocationManager, > UnsafeAllocationManager, are moved to MemoryChunkAllocator API, > as NettyMemoryChunkAllocator and UnsafeMemoryChunkAllocator. > > Commit 2: To introduce a new implementation of MemoryChunkManager, > MemoryChunkCleaner[3]. By default, MemoryChunkCleaner > converts all managed chunks to WeakReferences, and a global thread will > observe on the assigned reference queue to > release the unused garbage chunks which are enqueued by JVM GC. Some > modes[4] are there to provide different strategies, > such as to disable manual reference management (default), or to enable > manual and GC-based reference management at the > same time, or to report leaks only. > > Commit 3: Add API "ArrowBuf buffer(MemoryChunk chunk)" to BufferAllocator. > This makes some special workloads, e.g. > cross-JNI buffer sharing much easier as we will no longer need an > Allocator directly binding to the shared memory > chunks, and will still be able to leverage all of Allocator's advantages > to manage the chunks (E.g. to use > MemoryChunkCleaner). > > Some possible Q&As about this implementation: > > 1. Why adding MemoryChunkAllocator, rather than just customizing > AllocationManager? > It is to reuse all of current underlying chunk allocation strategies, > Netty, Unsafe, and others. Also, with the layer of > MemoryChunk separated from AllocationManager we will be able to create > MemoryChunks actively in some special cases, e.g. > cross-JNI buffer sharing, C data interface buffer importing, then populate > the chunks to a manager BufferAllocator. > > 2. Why using WeakReference rather than PhantomReference? > In Java 8, PhantomReference has some issue against its referent object. > The object cannot be garbage collected before > being enqueued. In WeakReference we don't have this issue. See ref[5]. > > 3. Why not sun.misc.Cleaner? > Cleaner API maintains a global doubly linked list to keep the Cleaner > instances alive. This brings overheads to us since > we will create local doubly linked list for cleaning up all the buffers on > Allocator close. See a unit test[6]. > > 4. Why closing all buffers on Allocator close? > This behavior can be customizabale within Mode[7]s. A principle is, when > relying on GC, we should allow closing buffers > manually, or at least closing all of them on Allocator close. Or the > actual release time of the underlying chunks will > be unpredictable. > > 5. Can we use heap based buffers? > If I am not wrong, no. The heap objects can be physically moved around by > JVM. The addresses can vary. > > 6. When should GC happen? > A new AllocationListener implementation, GCTriger[8] is introduced. GC > will be performed when BufferAllocator is full. > > 7. Performance? > Based on previous measurement, it doesn't bring overheads on the legacy > path (by using default MemoryChunkManager). For > GC-based path (MemoryChunkCleaner + Mode.GC_ONLY), allocations can be > comparatively slower due to higher GC pressure > (For example, in out Arrow-based query engine, to run TPC-DS SF1000, the > overhead can be up to 3%). I can collect more > benchmark results in future, maybe under a JIRA ticket or a PR. > > 8. Breaking changes? > It doesn't break library-based allocation strategy selection like directly > including arrow-memory-unsafe.jar to > projects. However it breaks use of the previous AllocationManager.Factory > API. Users should migrate to > MemoryChunkAllocator API. Which in my opinion is simple to do. > > The implementation is still evolving, so if the links get out-of-date you > can check on the newest branch head. Any > suggestions welcome. > > Some other discussions that may be related to this topic: > > > https://lists.apache.org/thread.html/r09b2e7dd8e342818190c332ee20d97f68816241d0c683e17c4f35dc6%40%3Cdev.arrow.apache.org%3E > > > https://lists.apache.org/thread.html/rc956b16b7ed9768cce8eaecafdc363f7b3808dafe6edf1940b139ecb%40%3Cuser.arrow.apache.org%3E > > Best, > Hongze > > > > [1] > https://github.com/zhztheplayer/arrow-1/blob/52e5def7a239164b5c13457c8be38e227bb2907d/java/memory/memory-core/src/main/java/org/apache/arrow/memory/ > MemoryChunkManager.java > [2] > > https://github.com/zhztheplayer/arrow-1/blob/52e5def7a239164b5c13457c8be38e227bb2907d/java/memory/memory-core/src/main/java/org/apache/arrow/memory/MemoryChunkAllocator.java > > [3] > > https://github.com/zhztheplayer/arrow-1/blob/52e5def7a239164b5c13457c8be38e227bb2907d/java/memory/memory-core/src/main/java/org/apache/arrow/memory/MemoryChunkCleaner.java > [4] > > https://github.com/zhztheplayer/arrow-1/blob/52e5def7a239164b5c13457c8be38e227bb2907d/java/memory/memory-core/src/main/java/org/apache/arrow/memory/MemoryChunkCleaner.java#L503 > [5] > > https://stackoverflow.com/questions/56684150/why-since-java-9-phantomreference-java-doc-states-that-it-is-dedicated-to-the-po > [6] > > https://github.com/zhztheplayer/arrow-1/blob/52e5def7a239164b5c13457c8be38e227bb2907d/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestMemoryChunkCleaner.java#L204 > [7] > > https://github.com/zhztheplayer/arrow-1/blob/52e5def7a239164b5c13457c8be38e227bb2907d/java/memory/memory-core/src/main/java/org/apache/arrow/memory/MemoryChunkCleaner.java#L503 > [8] > > https://github.com/zhztheplayer/arrow-1/blob/52e5def7a239164b5c13457c8be38e227bb2907d/java/memory/memory-core/src/main/java/org/apache/arrow/memory/MemoryChunkCleaner.java#L402 > > >
