Skip to content

Commit

Permalink
Performance optimization extension for RepeatedField<T> (#19115)
Browse files Browse the repository at this point in the history
.NET 8 added support for `AddRange` using the `ReadOnlySpan<T>` data structure. This can significantly improve the performance of `RepeatedField<T>` when adding multiple elements and can avoid the potential need to create extra arrays when using spans.

Here is the code that was added to .NET to support `AddRange` on `List<T>`: https://github.com/dotnet/runtime/blob/85f6d2d9449fb438cd8a5ca41d4beb4c657594e9/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/CollectionExtensions.cs#L85-L103

In the current implementation of `RepeatedField<T>`, the `AddRange` method only operates on `IEnumerable<T>`. Unfortunately, this means that the callers using `Span<T>` have to copy the content of the span to an array or to some other `IEnumerbale<T>` before calling `AddRange`. This is an expensive operation and can be avoided by adding support for `AddRange` that takes a `ReadOnlySpan<T>` directly.

In this proposal, `AddRange` is added as an extension method instead of directly adding it to `RepeatedField<T>` because we want to avoid ambiguity between `RepeatedField<T>.AddRange(IEnumerbale<T>)` and `RepeatedField<T>.AddRange(Span<T>)` for types such as `T[]`. This is the same exact approach taken by .NET runtime itself.

Closes #19115

COPYBARA_INTEGRATE_REVIEW=#19115 from prezaei:pedramr/repeated-field-extensions 8d0ac6a
PiperOrigin-RevId: 700357702
  • Loading branch information
prezaei authored and copybara-github committed Nov 26, 2024
1 parent 343a3f9 commit 70730f5
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 5 deletions.
38 changes: 38 additions & 0 deletions csharp/src/Google.Protobuf.Test/Collections/RepeatedFieldTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,44 @@ public void AddRange_Optimized_NullsProhibited_NullableValueType()
Assert.Catch<ArgumentException>(() => list.AddRange(new List<int?> { 20, null }));
}

[Test]
public void AddRange_ReadOnlySpanPath()
{
var list = new RepeatedField<string>();
list.AddRange(new[] { "foo", "bar" }.AsSpan());
Assert.AreEqual(2, list.Count);
Assert.AreEqual("foo", list[0]);
Assert.AreEqual("bar", list[1]);
}

[Test]
public void AddRange_ReadOnlySpan_NullsProhibited_ReferenceType()
{
// We don't just trust that a collection with a nullable element type doesn't contain nulls
var list = new RepeatedField<string>();
// It's okay for this to throw ArgumentNullException if necessary.
// It's not ideal, but not awful.
Assert.Catch<ArgumentException>(() => list.AddRange(new string[] { "foo", null }.AsSpan()));
}

[Test]
public void AddRange_ReadOnlySpan_NullsProhibited_NullableValueType()
{
// We don't just trust that a collection with a nullable element type doesn't contain nulls
var list = new RepeatedField<int?>();
// It's okay for this to throw ArgumentNullException if necessary.
// It's not ideal, but not awful.
Assert.Catch<ArgumentException>(() => list.AddRange(new int?[] { 20, null }.AsSpan()));
}

[Test]
public void AddRange_ReadOnlySpan_AlreadyNotEmpty()
{
var list = new RepeatedField<int> { 1, 2, 3 };
list.AddRange(new int[] { 4, 5, 6 }.AsSpan());
CollectionAssert.AreEqual(new[] { 1, 2, 3, 4, 5, 6 }, list);
}

[Test]
public void AddRange_AlreadyNotEmpty()
{
Expand Down
42 changes: 37 additions & 5 deletions csharp/src/Google.Protobuf/Collections/RepeatedField.cs
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ public bool Remove(T item)
if (index == -1)
{
return false;
}
}
Array.Copy(array, index + 1, array, index, count - index - 1);
count--;
array[count] = default;
Expand Down Expand Up @@ -429,6 +429,38 @@ public void AddRange(IEnumerable<T> values)
}
}

/// <summary>
/// Adds the elements of the specified span to the end of the collection.
/// </summary>
/// <param name="source">The span whose elements should be added to the end of the collection.</param>
[SecuritySafeCritical]
internal void AddRangeSpan(ReadOnlySpan<T> source)
{
if (source.IsEmpty)
{
return;
}

// For reference types and nullable value types, we need to check that there are no nulls
// present. (This isn't a thread-safe approach, but we don't advertise this is thread-safe.)
// We expect the JITter to optimize this test to true/false, so it's effectively conditional
// specialization.
if (default(T) == null)
{
for (int i = 0; i < source.Length; i++)
{
if (source[i] == null)
{
throw new ArgumentException("ReadOnlySpan contained null element", nameof(source));
}
}
}

EnsureSize(count + source.Length);
source.CopyTo(array.AsSpan(count));
count += source.Length;
}

/// <summary>
/// Adds all of the specified values into this collection. This method is present to
/// allow repeated fields to be constructed from queries within collection initializers.
Expand Down Expand Up @@ -463,7 +495,7 @@ public IEnumerator<T> GetEnumerator()
/// <c>true</c> if the specified <see cref="System.Object" /> is equal to this instance; otherwise, <c>false</c>.
/// </returns>
public override bool Equals(object obj) => Equals(obj as RepeatedField<T>);

/// <summary>
/// Returns an enumerator that iterates through a collection.
/// </summary>
Expand All @@ -476,7 +508,7 @@ public IEnumerator<T> GetEnumerator()
/// Returns a hash code for this instance.
/// </summary>
/// <returns>
/// A hash code for this instance, suitable for use in hashing algorithms and data structures like a hash table.
/// A hash code for this instance, suitable for use in hashing algorithms and data structures like a hash table.
/// </returns>
public override int GetHashCode()
{
Expand Down Expand Up @@ -645,7 +677,7 @@ void IList.Remove(object value)
Remove(t);
}
}
#endregion
#endregion

private sealed class RepeatedFieldDebugView
{
Expand All @@ -660,4 +692,4 @@ public RepeatedFieldDebugView(RepeatedField<T> list)
public T[] Items => list.ToArray();
}
}
}
}
38 changes: 38 additions & 0 deletions csharp/src/Google.Protobuf/RepeatedFieldExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#region Copyright notice and license
// Protocol Buffers - Google's data interchange format
// Copyright 2015 Google Inc. All rights reserved.
//
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file or at
// https://developers.google.com/open-source/licenses/bsd
#endregion

using System;
using System.Security;
using Google.Protobuf.Collections;

// Note: The choice of the namespace here is important because it's rare that people will directly reference Google.Protobuf.Collections
// in their app as they're typically using collections created by generated code. We are keeping the extension method in the Google.Protobuf
// namespace so that it is discoverable by most users.

namespace Google.Protobuf
{
/// <summary>
/// A set of extension methods on <see cref="RepeatedField{T}"/>
/// </summary>
public static class RepeatedFieldExtensions
{
// Note: This method is an extension method to avoid ambiguous overload conflict with existing AddRange(IEnumerable<T>) when the source is an array.
/// <summary>Adds the elements of the specified span to the end of the <see cref="RepeatedField{T}"/>.</summary>
/// <typeparam name="T">The type of elements in the <see cref="RepeatedField{T}"/>.</typeparam>
/// <param name="repeatedField">The list to which the elements should be added.</param>
/// <param name="source">The span whose elements should be added to the end of the <see cref="RepeatedField{T}"/>.</param>
/// <exception cref="ArgumentNullException">The <paramref name="repeatedField"/> is null.</exception>
[SecuritySafeCritical]
public static void AddRange<T>(this RepeatedField<T> repeatedField, ReadOnlySpan<T> source)
{
ProtoPreconditions.CheckNotNull(repeatedField, nameof(repeatedField));
repeatedField.AddRangeSpan(source);
}
}
}

0 comments on commit 70730f5

Please sign in to comment.