Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Use Array.Fill in Enumerable.Repeat.ToArray #16122

Merged
merged 2 commits into from
Feb 13, 2017

Conversation

jamesqo
Copy link
Contributor

@jamesqo jamesqo commented Feb 13, 2017

As the title says. Currently the implementation is the same (using a for-loop) and there's no difference, but in the future it could possibly be optimized by e.g. implementing it natively for reference types to avoid the checks to account for covariant arrays on every store.

/cc @JonHanna @stephentoub @VSadov

Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Less duplication makes me happy. Thanks.

@@ -79,10 +79,7 @@ public TResult[] ToArray()
TResult[] array = new TResult[_count];
if (_current != null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jkotas, is there any efficient and built-in mechanism for checking whether a value is default(TResult)? This check will handle reference types; I'm wondering if there's something that would more generally help with any type.

Copy link
Contributor

@JonHanna JonHanna Feb 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

System.Runtime.CompilerServices.RuntimeHelpers.Equals(_current, default(TResult)) is identity-equality for reference types, internal representation equality for value types, ignoring all Equals and == overrides.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But unless there's special recognition of it by the JIT (?), it'll box both value type arguments. Hence the "efficient" part of my question.

Copy link
Contributor Author

@jamesqo jamesqo Feb 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JonHanna That is not particularly efficient though because structs will be boxed and a call will be made. By contrast _current != null has non-existent overhead for value types because it's eliminated by the JIT. (edit: beaten by @stephentoub)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it'll box. And now I think about it, I wrote that skip on null, and that's precisely why I didn't test for default generally. I'd certainly love to learn of a non-boxing mechanism.

@stephentoub stephentoub merged commit 28ffdd6 into dotnet:master Feb 13, 2017
@jamesqo jamesqo deleted the use.array.fill branch February 13, 2017 23:37
@karelz karelz modified the milestone: 2.0.0 Feb 22, 2017
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
* Use Array.Fill in Enumerable.Repeat.ToArray


Commit migrated from dotnet/corefx@28ffdd6
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants