Skip to content
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.

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

Add Array.Fill method for multi-dimensional array #47213

Open
Jimmy-Hu opened this issue Jan 20, 2021 · 8 comments
Open

Add Array.Fill method for multi-dimensional array #47213

Jimmy-Hu opened this issue Jan 20, 2021 · 8 comments
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Runtime
Milestone

Comments

@Jimmy-Hu
Copy link

Jimmy-Hu commented Jan 20, 2021

Background and Motivation

First of all, C# provide Multidimensional Arrays structure. It is good that the API like Array.Copy can deal with multi-dimensional array well. For example, we can copy one multi-dimensional array to another one easily as below.

public class Plane<T>
{
    public int Width { get; }

    public int Height { get; }

    public T[,] Grid { get; set; }

    public int Length => Width * Height;
    
    public Plane(int width, int height)
    {
        Width = width;
        Height = height;
        Grid = new T[width, height];
    }
    
    public Plane(T[,] sourceGrid)
    {
        if (sourceGrid is null)
        {
            throw new ArgumentNullException();
        }

        Width = sourceGrid.GetLength(0);
        Height = sourceGrid.GetLength(1);
        Grid = new T[Width, Height];
        Array.Copy(sourceGrid, Grid, sourceGrid.Length);        //  sourceGrid and Grid are both multi-dimensional array
    }
}

The usage is like this:

Plane<int> test = new Plane<int>(10, 10);
var test2 = new Plane<int>(test.Grid);

When it comes to Array.Fill method, the dimension of input array is specified on only one-dimensional array T[]. I think that it is good to consider to add Array.Fill overloading methods to handle the multi-dimensional array input, i.e. something like T[,].

Proposed API

Similar to the existed Array.Fill<T> method, the proposed API is also with two parameters: an array and a value. The first parameter (array input) is a multi-dimensional array. The second parameter (value input) is the value to assign to each array element. An experimental implementation of a Array.Fill overloading method for two-dimensional array is as follows.

An experimental implementation

public static void Fill<T>(T[,] array, T value)
{
    for (int x = 0; x < array.GetLength(0); x++)
    {
        for (int y = 0; y < array.GetLength(1); y++)
        {
            array.SetValue(value, x, y);
        }
    }
    return;
}

Usage Examples

Based on the experimental implementation above, Array.Fill method can be used like this:

//  Filling int 1 to a two-dimensional array (test here).

int[,] test = new int[10, 10];
Array.Fill(test, 1);
for (int x = 0; x < 10; x++)
{
    for (int y = 0; y < 10; y++)
    {
        Console.Write(test.GetValue(x, y).ToString() + "\t");
    }
    Console.WriteLine("");
}

Alternative Designs

Another possible design of API form is something more similar to Array.Copy. Specifically, make declaration as below.

public static void Fill(Array array, T value);

In this form, the array input is more generic than T[,] (and the other overloading methods for T[,,], T[,,,]...) and a possible implementation of void Fill(Array array, T value) method is like below.

namespace System
{
    public abstract class Array : ICollection, IEnumerable, IList, IStructuralComparable, IStructuralEquatable, ICloneable
    {
+      public static void Fill<T>(Array array, T value)
+          where T : unmanaged
+      {
+          if (ReferenceEquals(array, null))
+          {
+              throw new ArgumentNullException(nameof(array));
+          }
+      
+          Type elementType = array.GetType().GetElementType();
+          if (!elementType.Equals(typeof(T)))
+          {
+              throw new System.InvalidOperationException();
+          }
+      
+          int[] dimensions = new int[array.Rank];
+          for (int i = 0; i < array.Rank; i++)
+              dimensions[i] = array.GetLength(i);
+          T[] tmp = new T[1];
+          int offset = 0;
+          int itemSize = System.Runtime.InteropServices.Marshal.SizeOf(typeof(T));
+          foreach (T item in array)
+          {
+              tmp[0] = value;
+              Buffer.BlockCopy(tmp, 0, array, offset * itemSize, itemSize);
+              offset++;
+          }
+          return;
+      }
    }
}

Risks

Because the proposed Fill method is also a kind of generic type API, some exception cases, including ArgumentNullException and ArgumentException should be considered to make API be more robust.

@Jimmy-Hu Jimmy-Hu added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jan 20, 2021
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jan 20, 2021
@Jimmy-Hu
Copy link
Author

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

I think that the area is area-System.Runtime.

@huoyaoyuan
Copy link
Member

How about allowing MD array to convert to spans?

@tannergooding tannergooding removed the untriaged New issue has not been triaged by the area owner label Jul 12, 2021
@tannergooding tannergooding added this to the Future milestone Jul 12, 2021
@SuperJMN
Copy link

SuperJMN commented Apr 3, 2022

If that helps prioritize this addition, I've just hit a use case for this in my real-life project.

@Saalvage
Copy link
Contributor

Saalvage commented Oct 14, 2022

Also encountered this in a real-world project!

Suggested implementation isn't ideal though, this should be superior, supporting arbitrary rank, managed types and faster performance:

public static void Fill<T>(Array array, T value) {
    ArgumentNullException.ThrowIfNull(array);

    var elementType = array.GetType().GetElementType();
    if (elementType is null)
    {
        throw new ArgumentException("Array without element type", nameof(array));
    }

    if (elementType.IsValueType && value == null)
    {
        throw new ArgumentNullException(nameof(value));
    }

    if (elementType.IsAssignableFrom(value?.GetType()) is not true && value != null)
    {
        throw new ArgumentException("Must be assignable to array's element type", nameof(value));
    }

    if (array.Length == 0) { return; }

    if (!elementType.IsValueType || elementType != value?.GetType())
    {
        var indices = new int[array.Rank];
        while (true)
        {
            array.SetValue(value, indices);
            for (var i = 0; i < array.Rank; i++)
            {
                indices[i]++;
                if (indices[i] < array.GetLength(i)) { break; }
                if (i == array.Rank - 1) { return; }
                indices[i] = 0;
            }
        }
    }
    else
    {
        unsafe
        {
            fixed (void* ptr = &MemoryMarshal.GetArrayDataReference(array))
            {
                new Span<T>(ptr, array.Length).Fill(value);
            }
        }
    }
}

Unsure if it's possible to somehow utilize Span.Fill for managed types, but at least I couldn't figure it out.

Would love to see this get added!

Edit: Tinkered a bit more and if this does get added to the standard library the non-span path should likely look like this:

for (nint i = 0; (nuint)i < array.NativeLength; i++)
{
    array.InternalSetValue(value, i);
}

A lot better than the lib-external solution!

@dakersnar dakersnar added the api-ready-for-review API is ready for review, it is NOT ready for implementation label Dec 13, 2022
@Sergio0694
Copy link
Contributor

Not seeing this mentioned before - as of .NET 6 (courtesy of @GrabYourPitchforks, done in #53562) we now also have a MemoryMarshal.GetArrayDataReference overload for MD arrays, so you can improve the code above in a single fast path for any arbitrary T, without needing to special-case managed types anymore:

public static void Fill<T>(Array array, T value)
{
    // Skipping prologue with checks and whatnot. Also ignoring cases where the size of
    // the array exceeds int.MaxValue, feel free to check/chunk if you need to support that.

    ref byte reference = ref MemoryMarshal.GetArrayDataReference(array);
    Span<T> span = MemoryMarshal.CreateSpan(ref Unsafe.As<byte, T>(ref reference), array.Length);
    
    span.Fill(value);
}

Sidenote, if you're using CommunityToolkit.HighPerformance, for the 2D array case you can also just do:

public static void Fill<T>(T[,] array, T value)
{
    array.AsSpan2D().Fill(value);
}

@bartonjs
Copy link
Member

bartonjs commented Jul 27, 2023

Video

The proposed Array.Fill<T>(Array, T) is too high of a risk of source breaking changes on patterns like filling a long[] or double[] from an int.

During discussion it was suggested that instead the static methods on Array that take an array as the first parameter should/could be transitioned over to MemoryExtensions (or some other destination) as extension methods, and then having one extension method for T[] and another for System.Array will better solve the problem (at least when called from extension syntax). This approach is probably the better path, but needs some investigation.

@bartonjs bartonjs added api-needs-work API needs work before it is approved, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation api-ready-for-review API is ready for review, it is NOT ready for implementation labels Jul 27, 2023
@Neme12
Copy link

Neme12 commented Aug 27, 2023

What about adding an Array<T> type that derives from Array? An array of any arity that is of type T would derive from Array<T> and then you can have type safety for the Fill method and no source breaking changes.

I think that although this would be some amount of work, this would be the best solution for the long term health of the platform, and could help people working with multi-dimensional arrays in other ways as well, not just for one specific API. Such an Array<T> could even implement IEnumerable<T>.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Runtime
Projects
None yet
Development

No branches or pull requests

10 participants