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

ArrayRange #27424

Closed
moien007 opened this issue Sep 19, 2018 · 6 comments
Closed

ArrayRange #27424

moien007 opened this issue Sep 19, 2018 · 6 comments
Milestone

Comments

@moien007
Copy link

In most cases ArrayPool returns an array that its length is bigger than the requested length.
So we usually use ArraySegment but its Offset field is unneeded. So having ArrayRange will be nice.

It already implemented here: https://github.com/aspnet/Blazor/blob/master/src/Microsoft.AspNetCore.Blazor/RenderTree/ArrayRange.cs

@gfoidl
Copy link
Member

gfoidl commented Sep 19, 2018

Do Memory<T> / Span<T> not suffice?

@moien007
Copy link
Author

ArrayRange is less complex than Memory<T> and Span<T>. Also Memory<T> contains _index which is (if I'am not wrong) same as ArraySegment<T>.Offset .

@svick
Copy link
Contributor

svick commented Sep 19, 2018

its Offset field is unneeded

Memory<T> contains _index

Why is that an issue?

@moien007
Copy link
Author

Why is that an issue?

If we use Memory<T> with ArrayPool<T>, _index is always zero, isn't it ?
Only difference between ArrayRange and ArraySegment<T> or Memory<T> is Offset or _index removal which makes less stack copy.

@svick
Copy link
Contributor

svick commented Sep 19, 2018

In that case, do you have a benchmark that shows a realistic scenario (i.e. not a microbenchmark), where using ArrayRange would significantly improve performance?

In other words: if you want to introduce a new API whose sole reason for existing is improved performance, I think you should first show that it actually improves performance.

@moien007
Copy link
Author

In that case, do you have a benchmark that shows a realistic scenario (i.e. not a microbenchmark), where using ArrayRange would significantly improve performance?

In other words: if you want to introduce a new API whose sole reason for existing is improved performance, I think you should first show that it actually improves performance.

I don't think ArrayRange give a significant performance improvement, so I'm convinced.

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 3.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants