Prefer Span<T>.Clear() over Span<T>.Fill(default)

[GrabYourPitchforks]

(Copied from #30740 (comment).)

Consider replacing this:

Span<T> theSpan = GetSpan();
theSpan.Fill(default(T));

With this:

Span<T> theSpan = GetSpan();
theSpan.Clear();

I see a handful of uses of Fill(0) or Fill(default) throughout the codebase (see callers). The Clear() method is potentially much more optimized than the Fill method depending on the T in use.

Category: Performance

[gfoidl]

Aside from the analyzer I can submit a PR changing Fill(default) to Clear.

[Joe4evr]

Is there a rule-of-thumb on when Clear() could be more optimized than Fill(default)?

[GrabYourPitchforks]

@Joe4evr currently Clear is always more optimized than Fill(default) for all T. Maybe there's an exception when the span contains only 1 or 2 elements, but those are rarities.

[jeffhandley]

I'm going to remove this from the Roslyn Analyzers project for the time-being; we will add it back to the project once we get our triage process in place for prioritizing newly filed code-analyzer issues.

[GrabYourPitchforks]

@jeffhandley Any issues with me marking this ready for review? Seems straightforward enough.

[davidfowl]

I feel like Span<T>.Fill(default) would only be written by someone coming from C\C++, memset(0, sizeof(...)).

[GrabYourPitchforks]

I count 9 references to Span<T>.Fill(default) tracked by source.dot.net.

[jeffhandley]

Any issues with me marking this ready for review?

No objections from me; thanks!

[bartonjs]

Video

Level: Info
Category: Performance

Looks good as proposed.

[carlossanlop]

@huoyaoyuan Consider this issue assigned to you.

Edit: Can't assign it to you because you need to participate in this discussion. But consider it yours.

[huoyaoyuan]

@carlossanlop You can assign me now, and you can tell the owners to review the PR.

[NN---]

@GrabYourPitchforks Probably also Fill could be optimized internally when default value is passed.

[huoyaoyuan]

I'm afraid a branch will cause performance regression on small spans.

[huoyaoyuan]

For VB, please confirm on the dotnet/runtime issue whether VB support is desired or not? If desired, and you wish to postpone it to a separate PR, please file a tracking issue in this repo for VB part.

Originally posted by @mavasani in dotnet/roslyn-analyzers#5181 (review)

@bartonjs
I think support for VB shouldn't be desired, since using span in VB will cause error anyway.

[stephentoub]

Thanks. We don't need VB support for analyzers about the use of span until such a time that VB has more than minimal support for spans.