Skip to content

StringViewArray: enable nullable reference types and optimize encoding with stackalloc/ArrayPool #400

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kronic wants to merge 2 commits into
base: main
Choose a base branch
from
+69 −26
Open

StringViewArray: enable nullable reference types and optimize encoding with stackalloc/ArrayPool #400

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e794898
update StringViewArray
kronic Aug 4, 2026
a4286b0
- Use stackalloc for small strings (<=1024 bytes) and ArrayPool for l…
kronic Aug 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 69 additions & 26 deletions src/Apache.Arrow/Arrays/StringViewArray.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#nullable enable

using System;
using System.Buffers;
using System.Collections;
using System.Collections.Generic;
using System.Runtime.InteropServices;
Expand All @@ -22,33 +25,63 @@

namespace Apache.Arrow
{
public class StringViewArray : BinaryViewArray, IReadOnlyList<string>
public class StringViewArray(ArrayData data) : BinaryViewArray(ArrowTypeId.StringView, data), IReadOnlyList<string?>

@CurtHagenlocher CurtHagenlocher Aug 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a codebase is easier to read when it's consistent in its use of syntax and personally found this new form jarring. I'm curious what other people think.

@adamreeve adamreeve Aug 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also found it a bit jarring but I haven't been doing as much work in C# lately so maybe I'm just not used to this. I noticed Rider always suggests refactoring to this form.

{
public static readonly Encoding DefaultEncoding = Encoding.UTF8;
public static Encoding DefaultEncoding { get; } = new UTF8Encoding(false);

@adamreeve adamreeve Aug 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little concerned that this could be a breaking behaviour change, but I checked that decoding is forgiving of whether or not a BOM is present, regardless of the encoderShouldEmitUTF8Identifier parameter. So I think this is OK.

I notice that we disable testing the StringView type in our C Data Interface Python integration tests:

arrow-dotnet/test/Apache.Arrow.Tests/CDataInterfacePythonTests.cs

Lines 772 to 773 in dc2c566

// TODO: Enable these once this the version of pyarrow referenced during testing supports them
HashSet<ArrowTypeId> unsupported = new HashSet<ArrowTypeId> { ArrowTypeId.ListView, ArrowTypeId.BinaryView, ArrowTypeId.StringView, ArrowTypeId.Decimal32, ArrowTypeId.Decimal64 };

Did you find that emitting the BOM was causing a problem when integrating with another Arrow implementation?

@kronic kronic Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adamreeve Yes, BOM causes problems with DataFusion.


public new class Builder : BuilderBase<StringViewArray, Builder>
public new class Builder() : BuilderBase<StringViewArray, Builder>(StringViewType.Default)
{
public Builder() : base(StringViewType.Default) { }

protected override StringViewArray Build(ArrayData data)
{
return new StringViewArray(data);
}

public Builder Append(string value, Encoding encoding = null)
public Builder Append(string? value, Encoding? encoding = null)
{
if (value == null)
if (value is null)
{
return AppendNull();
}
encoding = encoding ?? DefaultEncoding;
byte[] span = encoding.GetBytes(value);
return Append(span.AsSpan());

encoding ??= DefaultEncoding;
int maxByteCount = encoding.GetMaxByteCount(value.Length);
#if NETCOREAPP

@CurtHagenlocher CurtHagenlocher Aug 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please put preprocessor directives in flush-left like the rest of the codebase.

byte[]? buffer = null;

Span<byte> span = maxByteCount <= 1024

@CurtHagenlocher CurtHagenlocher Aug 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been using 256 as a conservative upper bound for stackalloc in libraries; see VariantValueWriter.StackAllocThreshold.

? stackalloc byte[maxByteCount]
: buffer = ArrayPool<byte>.Shared.Rent(maxByteCount);

try
{
int encodeBytes = encoding.GetBytes(value, span);
return Append(span.Slice(0, encodeBytes));
}
finally
{
if (buffer is not null)
{
ArrayPool<byte>.Shared.Return(buffer);
}
}
#else
byte[] buffer = ArrayPool<byte>.Shared.Rent(maxByteCount);

try
{
int encodeBytes = encoding.GetBytes(value, 0, value.Length, buffer, 0);
return Append(buffer.AsSpan(0, encodeBytes));
}
finally
{
ArrayPool<byte>.Shared.Return(buffer);
}
#endif
}

public Builder AppendRange(IEnumerable<string> values, Encoding encoding = null)
public Builder AppendRange(IEnumerable<string?> values, Encoding? encoding = null)
{
foreach (string value in values)
foreach (string? value in values)
{
Append(value, encoding);
}
Expand All @@ -57,31 +90,41 @@ public Builder AppendRange(IEnumerable<string> values, Encoding encoding = null)
}
}

public StringViewArray(ArrayData data)
: base(ArrowTypeId.StringView, data) { }

public StringViewArray(int length,
public StringViewArray
(
int length,
ArrowBuffer valueOffsetsBuffer,
ArrowBuffer dataBuffer,
ArrowBuffer nullBitmapBuffer,
int nullCount = 0, int offset = 0)
: this(new ArrayData(StringViewType.Default, length, nullCount, offset,
new[] { nullBitmapBuffer, valueOffsetsBuffer, dataBuffer }))
{ }
int nullCount = 0,
int offset = 0
) : this
(
new ArrayData
(
StringViewType.Default,
length,
nullCount,
offset,
[nullBitmapBuffer, valueOffsetsBuffer, dataBuffer]
)
)
{
}

public override void Accept(IArrowArrayVisitor visitor) => Accept(this, visitor);

public string GetString(int index, Encoding encoding = default)
public string? GetString(int index, Encoding? encoding = null)
{
encoding ??= DefaultEncoding;

ReadOnlySpan<byte> bytes = GetBytes(index, out bool isNull);

if (isNull)
{
return null;
}
if (bytes.Length == 0)

if (bytes.Length is 0)
{
return string.Empty;
}
Expand All @@ -93,11 +136,11 @@ public string GetString(int index, Encoding encoding = default)
}
}

int IReadOnlyCollection<string>.Count => Length;
int IReadOnlyCollection<string?>.Count => Length;

string IReadOnlyList<string>.this[int index] => GetString(index);
string? IReadOnlyList<string?>.this[int index] => GetString(index);

IEnumerator<string> IEnumerable<string>.GetEnumerator()
IEnumerator<string?> IEnumerable<string?>.GetEnumerator()
{
for (int index = 0; index < Length; index++)
{
Expand Down
Loading