Change byte allocation to be pooled for performance improvements

[mithileshz]

Fixes #4735

[OsirisTerje]

The StreamsComparer does not have direct unit tests. It could be an idea to add this since it is being directly modified here.

[OsirisTerje]

The BUFFER_SIZE is 4096. That is a bit more than what is recommended for stackalloc, which says up to 1KB. Should this be a concern here?

[stevenaw]

I agree, even 1KB is on the higher side as I've seen 256 bytes often as a limit to stackalloc.

One other option here could be ArrayPool.

[mithileshz]

I'll look into ArrayPool - haven't used it before so will investigate and come back later today.

[stevenaw]

Thanks for adding me here @OsirisTerje

@mithileshz Welcome back and thank you as well for noticing the room for improvement here 🙂
I'm away on vacation without a laptop but can participate better upon my return. For now I've left a suggestion above in response @OsirisTerje 's comment.

[mithileshz]

The StreamsComparer does not have direct unit tests. It could be an idea to add this since it is being directly modified here.

@OsirisTerje There are tests inside EqualConstraintTests inside the region StreamEquality. As this change doesn't change the behaviour of the StreamComparer, I didn't add any extra tests as those would have been covered by the tests in class?

[OsirisTerje]

4 tests failing.

1) Failed : NUnit.Framework.Tests.Constraints.EqualConstraintTests+StreamEquality.ShortReadingMemoryStream_AssertErrorMessageFailurePointIsCorrect
  Assert.That(ex?.Message, Does.Contain("Stream lengths are both 9. Streams differ at offset 8."))
  Expected: String containing "Stream lengths are both 9. Streams differ at offset 8."
  But was:  "  Assert.That(entryStream, Is.EqualTo(expectedStream))
  Stream lengths are both 9. Streams differ at offset 2552.
"
   at NUnit.Framework.Tests.Constraints.EqualConstraintTests.StreamEquality.ShortReadingMemoryStream_AssertErrorMessageFailurePointIsCorrect() in /_/src/NUnitFramework/tests/Constraints/EqualConstraintTests.cs:line 257

2) Failed : NUnit.Framework.Tests.Constraints.EqualConstraintTests+StreamEquality.UnSeekableActualAndExpectedStreamsEqual
  Assert.That(actualStream, Is.EqualTo(expectedStream))
  Streams differ at offset 2552.
   at NUnit.Framework.Tests.Constraints.EqualConstraintTests.StreamEquality.UnSeekableActualAndExpectedStreamsEqual() in /_/src/NUnitFramework/tests/Constraints/EqualConstraintTests.cs:line 197

3) Failed : NUnit.Framework.Tests.Constraints.EqualConstraintTests+StreamEquality.UnSeekableActualStreamEqual
  Assert.That(entryStream, Is.EqualTo(expectedStream))
  Streams differ at offset 2552.
   at NUnit.Framework.Tests.Constraints.EqualConstraintTests.StreamEquality.UnSeekableActualStreamEqual() in /_/src/NUnitFramework/tests/Constraints/EqualConstraintTests.cs:line 147

4) Failed : NUnit.Framework.Tests.Constraints.EqualConstraintTests+StreamEquality.UnSeekableExpectedStreamEqual
  Assert.That(actualStream, Is.EqualTo(expectedStream))
  Streams differ at offset 2552.
   at NUnit.Framework.Tests.Constraints.EqualConstraintTests.StreamEquality.UnSeekableExpectedStreamEqual() in /_/src/NUnitFramework/tests/Constraints/EqualConstraintTests.cs:line 171
```

[stevenaw]

Great work @mithileshz , results are looking promising so far!
I've proactively left a few helpful pointers since you mentioned you were a little new to using ArrayPool.

[OsirisTerje]

lgtm

[stevenaw]

Fantastic, looks like your change speeds things up and lowers allocations.
LGTM, thanks for your contribution @mithileshz