dotnet / dotnet/machinelearning

DataFrame GetMutableBuffer method and ReadOnlyBuffer issues

Open
#6,715 0 comments 0 reactions 0 assignees View on GitHub
area-DataFrame enhancement
Dominant language
C#
Stars
9.4k
Forks
2k
Avg merge
2d 20h
Merged PRs (30d)
11

Description

As it was mentioned in #6642 DataFrame has a lot of boilerplate code like:

```
DataFrameBuffer resultMutableBuffer = DataFrameBuffer.GetMutableBuffer(resultBuffer);
resultContainer.Buffers[b] = resultMutableBuffer;
Span resultSpan = resultMutableBuffer.Span;
DataFrameBuffer resultMutableNullBitMapBuffer = DataFrameBuffer.GetMutableBuffer(resultContainer.NullBitMapBuffers[b]);
resultContainer.NullBitMapBuffers[b] = resultMutableNullBitMapBuffer;
Span resultNullBitMapSpan = resultMutableNullBitMapBuffer.Span;
```

It looks like it's used for working with readonly memory, when dataframe column is created from the Apache Arrow RecordBatch in

`public static DataFrame FromArrowRecordBatch(RecordBatch recordBatch)`

It's also used in constructor of PrimitiveDataFrameColumn:

`public PrimitiveDataFrameColumn(string name, ReadOnlyMemory buffer, ReadOnlyMemory nullBitMap, int length = 0, int nullCount = 0) `

which I think shouldn't be public, as it highly depends on internal implementation of PrimitiveColumn.

There is an issue with FromArrowRecordBatch factory method:

RecordBatch is a disposable object. Apache Arrow by default uses NativeMemoryAllocator to allocate unmanaged memory (for example, this default allocator is used in Spark.Net to create RecorBatch and pass it to DataFrame.FromArrowRecordBatch factory method).
So it's up to a DataFrame to hold the link to the RecordBatch and correctly Dispose it. Or it has to copy the unmanaged readonly memory from the RecordBatch into managed buffers (that exactly what is happening in GetMutableBuffer on attempt to edit data), but in this case we can avoid using ReadOnlyBuffers at all or at least limit it usage to ReadOnlyDataFrame class.

The suggestion is:
1) Avoid using GetMutableBuffer and ReadOnlyBuffers in the DataFrame, copy memory from Apache Arrow Record Batch on DataFrame creation (anyway we have to do on any attempt to edit DataFrame)
2) Introduce ReadOnlyDataFrame with limit set of operation (only readonly like Sort, GroupBy, Filter and etc and other with inPlace set to false). ReadOnlyDataFrame will also implement ML.IDataView. So it will be the way to create ReadOnlyDataFrame from Apache Arrow RecordsBatch without copy operation and use it in ML .Net
3) Make ReadOnlyDataFrameBuffer to implement IDisposable interface. For example, similar to Apache ArrowBuffer:

```
public readonly partial struct ArrowBuffer : IEquatable, IDisposable
{
private readonly IMemoryOwner _memoryOwner;
private readonly ReadOnlyMemory _memory;

public static ArrowBuffer Empty => new ArrowBuffer(Memory.Empty);

public ArrowBuffer(ReadOnlyMemory data)
{
_memoryOwner = null;
_memory = data;
}

internal ArrowBuffer(IMemoryOwner memoryOwner)
{
// When wrapping an IMemoryOwner, don't cache the Memory
// since the owner may be disposed, and the cached Memory would
// be invalid.

_memoryOwner = memoryOwner;
_memory = Memory.Empty;
}

public ReadOnlyMemory Memory =>
_memoryOwner != null ? _memoryOwner.Memory : _memory;
...

public void Dispose()
{
_memoryOwner?.Dispose();
}
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.