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

[API Proposal]: Add span based apis for Array.CreateInstance/GetValue/SetValue #81826

Closed
afxres opened this issue Feb 8, 2023 · 5 comments
Closed
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Memory

Comments

@afxres
Copy link

afxres commented Feb 8, 2023

Background and motivation

Avoid extra memory allocate during reflections/serialization

API Proposal

Array.CreateInstance(Type, int[])
Array.CreateInstance(Type, int[], int[])
Array.GetValue(int[])
Array.SetValue(object, int[])
+ Array.CreateInstance(Type, Span<int>)
+ Array.CreateInstance(Type, Span<int>, Span<int>)
+ Array.GetValue(Span<int>)
+ Array.SetValue(object, Span<int>)
...

API Usage

var lengths = (stackalloc int[] { 1, 2 });
var lowerBounds = (stackalloc int[] { 3, 4 });
var array = Array.CreateInstance(typeof(int), lengths, lowerBounds);
// we can use 'MemoryMarshal.GetArrayDataReference' instead of 'GetValue/SetValue'
// but 'CreateInstance' still have to allocate extra memories
var reference = Unsafe.As<byte, int>(ref MemoryMarshal.GetArrayDataReference(array));

Alternative Designs

No response

Risks

No response

@afxres afxres added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Feb 8, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Feb 8, 2023
@ghost
Copy link

ghost commented Feb 8, 2023

Tagging subscribers to this area: @dotnet/area-system-memory
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

Avoid extra memory allocate during reflections/serialization

API Proposal

Array.CreateInstance(Type, int[])
Array.CreateInstance(Type, int[], int[])
Array.GetValue(int[])
Array.SetValue(object, int[])
+ Array.CreateInstance(Type, Span<int>)
+ Array.CreateInstance(Type, Span<int>, Span<int>)
+ Array.GetValue(Span<int>)
+ Array.SetValue(object, Span<int>)
...

API Usage

var lengths = (stackalloc int[] { 1, 2 });
var lowerBounds = (stackalloc int[] { 3, 4 });
var array = Array.CreateInstance(typeof(int), lengths, lowerBounds);
// we can use 'MemoryMarshal.GetArrayDataReference' instead of 'GetValue/SetValue'
// but 'CreateInstance' still have to allocate extra memories
var reference = Unsafe.As<byte, int>(ref MemoryMarshal.GetArrayDataReference(array));

Alternative Designs

No response

Risks

No response

Author: afxres
Assignees: -
Labels:

api-suggestion, area-System.Memory, untriaged

Milestone: -

@tannergooding
Copy link
Member

CC. @jkotas, @BruceForstall

This seems like general goodness and helps make certain type of multi-dimensional array handling "better".

However, Array is special so I wanted to confirm that there aren't any concerns or considerations that might block such functionality from being exposed.

I imagine if it is, we may want CreateInstance<T>(...) to match the other Array proposal that Jan has open.

@jkotas
Copy link
Member

jkotas commented Feb 10, 2023

This seems like general goodness and helps make certain type of multi-dimensional array handling "better".

It makes it better, but it is not going to be by much. These APIs are still going to be slow APIs that do a lot of work even after this change.

we may want CreateInstance(...) to match the other Array proposal that Jan has open.

Do you mean #76478? There is no CreateInstance<T> in that proposal. If you know T, you can just use new T[...].

@tannergooding
Copy link
Member

Yes, that's the one I was thinking of.

@tannergooding
Copy link
Member

Going to close this based on the feedback above. We already have overloads that avoid allocating for the common cases of 1/2/3 lengths. For more lengths we may end up adding params Span<T> support as part of a more holistic effort when the language feature becomes available.

@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jul 21, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Memory
Projects
None yet
Development

No branches or pull requests

3 participants