-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Span: Add Span<T>.Fill(T value) #19550
Comments
Can we call it memset |
Also an offset+count method? public class Span<T>
{
public void Fill(T value);
public void Fill(T value, int offset, int count);
} Unless this pattern seems ok? span.Slice(1, 12).Fill(42); |
@benaadams I think having an offset+count one is redundant. |
It is, if there is no slice cost, therefore no perf difference between
and
They may end up being the same; e.g. Slice will bounds check; but Fill with extra params will also bounds check; Fill without extra params wouldn't have to bounds check. If they both inlined, it would likely end up the same? |
@benaadams 'syntax sugar' was not the correct term; I meant to say that it was redundant. The whole point of introducing |
Correct. You can tell from the existing Span surface, e.g. none of the Copy methods take offset/counts. |
Just pointing out without offset/count its pushing a lot into inline Slicing. Its a question of granularity and performance of Slice + Action. So if I had two spans that represented a fixed size message packet and I wanted to copy a section from one to another the usage pattern would be:
Rather than
Which may be fine and end up with the same instructions being executed; or it may end up with extra items on the stack and extra checks being done. |
@benaadams If it is less performant then that would probably be considered a problem with the JIT & should be fixed instead of being worked around, no? |
Ideally, then it would hopefully also pick up similar patterns in other code. |
Yes, I think that is the beauty of the E.g.
I have the same concern here, but hope that this would be given attention if that was the case, and until this is proven a problem we should not design around it. |
API review: Approved. |
Note the issue is still up for grabs if anyone wants to take it. It looks like it will be a little bit involved implementation (see attempt in dotnet/coreclr#8498) |
Thanks! Assigned to you. |
Closed via dotnet/corefx#14396 |
As discussed briefly in https://github.com/dotnet/corefx/issues/13427 I propose to add a
Fill(T value)
method toSpan<T>
to make span the goto place for safe and fast memory operations.This is analoguous to the same method approved for arrays in https://github.com/dotnet/corefx/issues/6695 and added in dotnet/coreclr#8137, dotnet/corert#2259 but still lacking optimization https://github.com/dotnet/coreclr/issues/8290, perhaps
Array.Fill<T>
could forward toSpan<T>.Fill
in the future...Rationale and Usage
Provide a safe yet fast way of filling of any type of contiguous memory; managed or unmanaged.
The argumentation for adding this is similar to
Array.Fill<T>
, but here with a wider use case, since this can be used with both managed and unmanaged memory:Proposed API
Add a single method
Fill(T value)
to the existingSpan<T>
API:Open Questions
Open question is whether this should be added as a member method or an extension method. I would argue for a member method, since this will depend on internal representation, and would perhaps allow for the JIT intrinsic version to do optimizations that a generic extension method can't. If that is a valid argument?
@karelz @jkotas @omariom @benaadams @jamesqo
The text was updated successfully, but these errors were encountered: