Use Encoding.Preamble in StreamReader/Writer

[justinvp]

Use the new Encoding.Preamble property to avoid unnecessary byte[] allocations for encodings that return cached instances from Preamble (e.g. all of the built-in encodings that have preambles).

Reference: dotnet/coreclr#13269

cc: @JeremyKuhne, @ianhays, @stephentoub

[stephentoub]

FYI, this is just a temporary issue, but I've/we've not yet overridden the span-based methods on FileStream, so temporarily this won't be as efficient as it could be. Not something to block on, though.

[stephentoub]

Does the fact that this was being cached mean it's used multiple times? I've not reviewed all of the StreamReader code, but it looks like we might now be accessing Encoding.Preamble multiple times. In theory, could some custom encoding have a preamble and this change could now cause multiple calls to GetPreamble (via Preamble) where previously there was only one? It's probably such a remote case that it's not worth caring about, but just wanted to check.

[justinvp]

Aside from its use in some Debug.Asserts (which we shouldn't care about), it is used in two places: Init and IsPreamble, and IsPreamble can be called multiple times. So it's possible that a custom encoding that hasn't provided an efficient implementation of Preamble could result in more allocations now. I lean towards not caring about it. The upside of this change is that all the built-in encodings won't allocate now (where previously only Encoding.UTF8 avoided the allocation). I'm guessing the built-in encodings are going to be used the vast majority of the time vs. a custom encoding. I'm open to other ideas, though.

[JeremyKuhne]

I'll go with whatever @stephentoub thinks on this one.

[stephentoub]

I think it's probably fine. I'll sleep on it :)

[justinvp]

It may be possible to remove the call in Init, leaving a single call in IsPreamble (which could still be called multiple times, but I suspect multiple calls are rare). I'll play around with it tomorrow and see.

[stephentoub]

I think you're right that the 99.999% case will be the built-in encodings, and for those this change makes sense. Let's do it.

[stephentoub]

Thanks for making all of these Preamble changes :)

[justinvp]

Thank you for driving all the Span<T> API additions throughout :)