[Question] Why not use unsafe code in order to eliminate bounds checking in IndexOf methods?

[OnurGumus]

While I was checking https://github.com/dotnet/coreclr/blob/master/src/mscorlib/src/System/Collections/Generic/EqualityComparer.cs

I noticed that those IndexOf methods don't use any unsafe methods in order to prevent bounds checking of the arrays. While unsafe code is not recommended in general, though coreclr libraries are somewhat "trusted". I just wanted to ask why unsafe code is not used in those places.

[jkotas]

We only use unsafe code where it makes difference that matters. For example, you can see that it is used in ByteEqualityComparer. We do not use it just because of we can.

[OnurGumus]

I understand, though eliminating bounds check by using unsafe code for IndexOf should also matter according to my logic. A typical bounds check elimination usually provides perhaps 10% performance gain for methods like List.Contains at the cost of pinning the array.

[OnurGumus]

In addition to that, one can check if startIndex =0 && endIndex = array.Length, (which would be at least 90% of all cases statistically ) then you can rewrite the loop as
for(var i=0;i<array.Length;i++)... and get the bounds check elimination for free from the JIT.

[mikedn]

A typical bounds check elimination usually provides perhaps 10% performance gain for methods like List.Contains at the cost of pinning the array.

10%? Maybe in worst case scenarios - when T is a value type and Equals is inlined. Otherwise it's unlikely that the cost of the bounds check will be that large when a virtual call to Equals is involved.

In addition to that, one can check if startIndex =0 && endIndex = array.Length, (which would be at least 90% of all cases statistically ) then you can rewrite the loop as
for(var i=0;i<array.Length;i++)... and get the bounds check elimination for free from the JIT.

Some of those loops should be picked up by JIT's loop cloning which is pretty much the same thing that you're describing.

[tannergooding]

I think this is relevant: https://github.com/dotnet/corefx/issues/17318

[mikedn]

I think this is relevant: dotnet/corefx#17318

Not really, that thing doesn't work with loops, at least not with typical ones.

[stephentoub]

@OnurGumus, if there's a case where you believe it would help, you're welcome to try it out, measure it, and if there's a measurable improvement, submit a PR with the changes and showing the perf tests you used and the results. Then we can discuss the more concrete case, and if it's a real win, merge it to get the benefits into the code.