CurtHagenlocher opened a new issue, #464:
URL: https://github.com/apache/arrow-dotnet/issues/464
### Describe the bug
`CArrowArrayStreamExporter.GetNext` reads a `RecordBatch` from the
underlying `IArrowArrayStream`, exports it with
`CArrowArrayExporter.ExportRecordBatch`, and then never disposes it:
```csharp
RecordBatch recordBatch =
arrayStream.ArrowArrayStream.ReadNextRecordBatchAsync().Result;
if (recordBatch != null)
{
CArrowArrayExporter.ExportRecordBatch(recordBatch, cArray);
}
```
For allocator-backed buffers, `ArrowBuffer.TryExport` takes its own
reference (`_handle.Retain()`), so the exported `ArrowArray` keeps the memory
alive independently of the batch. When the consumer calls `release`, only the
exported reference is dropped. The batch's own reference is never released, so
with the default `NativeMemoryAllocator` the native memory is only freed when
the `NativeMemoryManager` finalizer eventually runs.
In practice this means every batch exported through the C stream interface
depends on finalization to free its buffers. Reclamation is driven entirely by
GC activity in the exporting runtime, which is mostly the `AddMemoryPressure`
calls in `NativeMemoryAllocator` and `ExportedAllocationOwner`.
This is especially visible in NativeAOT-compiled libraries (for example,
ADBC drivers). They have their own private GC heap, and nothing collects that
heap once the library stops allocating.
### Measurements
I used a NativeAOT-compiled ADBC driver loaded by a C host. The host drains
a stream of 6 batches × 4M rows twice and releases every array, the stream and
the statement/connection/database. All numbers are process private bytes; the
baseline is about 3 MB.
| | as-is | stream disposes each batch after export |
|---|---|---|
| after release, no GC | 395 MB | 335 MB |
| after `GC.Collect(2, Aggressive, true, true)` | 138 MB | 5.8 MB |
| after Collect + `WaitForPendingFinalizers` + Aggressive | 5.5 MB | — |
The 138 MB left behind in the "as-is" column is native memory that only the
finalizer queue frees.
### Suggested fix
Have `GetNext` dispose the batch once it has been exported. Disposing is
safe for buffers that `TryExport` retains (`_handle != null`).
There is one caveat. Buffers without a shared handle are exported by pinning
`_memory`, and some `MemoryManager` implementations (e.g.
`NativeMemoryManager`) throw when disposed while pinned. A complete fix
therefore needs one of these:
- make the export path always take an owning reference, or
- only dispose the batch when every buffer was exported by reference, or
- move ownership of the batch into the `ExportedAllocationOwner`, so it is
disposed when the exported array is released.
The third option is probably the cleanest, since it also covers the
non-shared case.
### Component(s)
C#
--
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]
