Add a comment explaining why not to switch ImmutableArray.Empty to Array.Empty

[Therzok]

No description provided.

[jkotas]

This will introduce performance regression when Empty is called from generic code and T is reference type.

The current JIT has inlining limitations for cases like these. @davidwrighton is trying to fix them in #31833 . Once it is fixed, it should be ok to make this change - but only for netcoreapp5.0+ builds of this library.

[jkotas]

Actually, scratch my comment. I have not noticed that Empty is a field. It would apply only if Empty was a property.

Still, it is not obvious whether this is an improvement. It saves allocation of one tiny object on GC heap, but introduces more code. It is likely a net regression when looking at the total memory consumption.

[Therzok]

The disabled CA1825 caught my eye, which is why I opened the PR.

Looking at the IL, the Array.Empty<T> version looks one byte shorter: https://sharplab.io/#gist:a3adeca56fd97a06a564ced10c670c32

And from the looks of it, even the JIT is smaller.
https://sharplab.io/#gist:e1b441ef8346b2ece83abf48dd00cf12

[jkotas]

Small generic types and methods have non-trivial overhead in the runtime datastructures that is easy to miss.

Here is a micro-benchmark that demonstrates the impact of this PR https://gist.github.com/jkotas/6b5d2c7088b2a8d8c6332b2c7d74ce74#file-arraywrapper

Without Array.Empty(), the working set is 44,724,224 bytes (Windows x64). With Array.Empty(), the working set is 51,097,600 bytes. So this PR is increasing footprint of typical ImmutableArray instantiation by ~600 bytes. This hit is going to be slightly lower because of there will be Array.Empty() instantiated from other uses that the ImmutableArray gets for "free", but there is no way to turn this into an improvement for a typical app.

[jkotas]

If you agree, I think this PR can be closed.

[Therzok]

@jkotas Thanks for that benchmark, that's really interesting info.

Instead of closing, can I repurpose the PR to add a comment on that behaviour so this won't be PR'd again in the future?

[jkotas]

Yep, adding a comment on this would be great!

[Therzok]

Now it's all a single comment added.

[jkotas]

Thanks