Skip to content

[netcore] Simplify Array.Length/LongLength implementation - #17287

Closed
filipnavara wants to merge 2 commits into
mono:masterfrom
filipnavara:array-length
Closed

filipnavara wants to merge 2 commits into
mono:masterfrom
filipnavara:array-length

Conversation

@filipnavara

Copy link
Copy Markdown
Contributor

No description provided.

return length;
}
}
public int Length => checked ((int)Unsafe.As<RawData>(this).Count);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I guess you will need to remove the intrinsic then 🙂

@filipnavara filipnavara Oct 11, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh right, may not be worth it afterall.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why only netcore?

@filipnavara filipnavara Oct 14, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The motivation for this was dotnet/coreclr#27123. I was testing the grounds to see if we can share more of the code with CoreCLR eventually. The only reason to port it to non-netcore is code parity and removing the native code altogether. It doesn't generate better or worse code in the end so I am not sure if it's worth it.

@filipnavara
filipnavara marked this pull request as ready for review October 11, 2019 15:19
@filipnavara filipnavara changed the title [netcore] Optimize Array.Length/LongLength implementation [netcore] Simplify Array.Length/LongLength implementation Oct 11, 2019
@lambdageek

Copy link
Copy Markdown
Member

I want to hear what @vargaz thinks, but I think it's worth moving this to managed in non-netcore Mono and cleaning up the unmanaged code.

@marek-safar

Copy link
Copy Markdown
Member

Closing as we have already intrinsic for this code paths

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants