Rich-T-kid commented on issue #9835: URL: https://github.com/apache/arrow-rs/issues/9835#issuecomment-5028700965
``` 1. Reuse FlatBufferBuilder 2. Reuse arrow_data buffer 3. Reuse metadata vectors 4. Avoid copying flatbuffer data 5. Write directly from existing buffers ``` 1,3,5 were completed in seperate components for #10125 iirc. The core issue with 2 & 4 is the ownership model. I attempted, on two separate occasions, to implement a buffer pool for the FlightData chunks being sent down to the gRPC layer for transmission over the wire. the concept being `split_grpc_response` ideally produces equal-sized chunks, so rather than going through a full `allocation`/`deallocation` cycle each time for memory whose size is known ahead of time, we could just reuse a set of perfectly-sized buffers. This mattered even more given that, past a certain threshold (128KB on Linux), memory chunks are allocated and freed via mmap/munmap directly, rather than being retained by the allocator in the process for potential reuse. By default the`target_grpc_max_size` is set to 2MB's. This was done by creating buffers using [Bytes::from_owner()](https://docs.rs/bytes/latest/bytes/struct.Bytes.html#method.from_owner), so that on drop we could intercept the buffer and place it back into the buffer pool. The exact commit that introduced this is [here](https://github.com/apache/arrow-rs/pull/10137/changes/5ceb08de716c9d3ee5d731b9d6800769e2f28760) I ran into 4 main issues: 1. Negligible benchmark impact. *(for the most part)* The benchmarks showed very little difference in either direction, no matter how much I tuned max_target_size. 2. We're effectively re-implementing allocator behavior. As mentioned above, up to a certain point, allocators already handle memory pooling better, since so much time and engineering effort has gone into them. We'd need a threshold to decide when pooling turns on, or even gate it behind a cfg. 3. Deallocation started showing up in the profile. Say we have 40MB of data split into 4MB batches to send over the wire, and our buffer pool is working as intended, keeping 5 buffers "hot" so we never need to ask the allocator for more memory. Once the last 4MB chunk is sent over the wire and FlightEncoder is dropped, this triggers one large munmap call that deallocates all 20MB (5 × 4MB) of memory at once. 4. We don't control when buffers are dropped. [FlightEncoder pushes buffers down](https://github.com/apache/arrow-rs/blob/0cc07133c659489c2c03a2ad08994b46910e152f/arrow-flight/src/encode.rs#L407) one FlightData chunk at a time, but we have no way to guarantee when a buffer is "consumed" and subsequently dropped. This compounds issue 3: if the buffer pool isn't effective and we have 10 chunks of data alive at the same time, they'll all return to the pool when dropped — and all get deallocated at once when FlightEncoder itself is dropped. > @alamb I think https://github.com/apache/arrow-rs/pull/10262 just avoids reallocations. It doens't prevent the copying in the first place 🤔 Yes, to a degree #10262 avoids intermediate vector re-allocations, but the allocator still needs to call `mmap` & `munmap` repeatedly for however many buffers we need, even if we just gave a perfectly sized buffer back. I hope that make sense, let me know if i missed something -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
