CryptoStream performs 1-byte reads, leading to performance bottleneck #52651
Comments
|
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label. |
|
Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @GrabYourPitchforks Issue DetailsDescriptionI was using a The below example code simulates a Stream that provides less than the requested datasize, which is common for many streams, such as network streams and whatnot. The console output shows the following: From this I gather that CryptoStream will read in chunks of 128K, but in the event that this buffer isn't filled, it will fill the remainder with 1 byte at a time. IMO, CryptoStream should never read as little as 1 byte at a time, if more is needed. Workarounds:
Example class Program
{
static void Main(string[] args)
{
byte[] data = new byte[1 * 1024 * 1024];
using (Stream myStream = new MyStream(data))
using (SHA1CryptoServiceProvider hasher = new SHA1CryptoServiceProvider())
using (Stream stream = new CryptoStream(myStream, hasher, CryptoStreamMode.Read))
using (Stream target = Stream.Null)
{
// Do some read, e.g. network writing
stream.CopyTo(target);
Console.WriteLine("Completed");
Console.WriteLine(BitConverter.ToString(hasher.Hash));
}
}
}
/// <summary>
/// This represents some stream I've implemented. I provide data as a stream to use it in other areas of the code.
/// The important part is that Read() will not be able to provide the full requested data each time
/// </summary>
class MyStream : Stream
{
private int _position;
private readonly byte[] _backingData;
public MyStream(byte[] backingData)
{
_backingData = backingData;
}
public override int Read(byte[] buffer, int offset, int count)
{
var toRead = Math.Min(count, _backingData.Length - _position);
// Simulate less data available from source
toRead = Math.Min(20000, toRead);
Console.WriteLine("Wanted " + count + ", read " + toRead);
Array.Copy(_backingData, _position, buffer, offset, toRead);
_position += toRead;
return toRead;
}
public override bool CanRead => true;
public override long Length => _backingData.Length;
public override long Position
{
get => _position;
set => throw new NotImplementedException();
}
#region NotImplemented
public override void Flush()
{
throw new NotImplementedException();
}
public override long Seek(long offset, SeekOrigin origin)
{
throw new NotImplementedException();
}
public override void SetLength(long value)
{
throw new NotImplementedException();
}
public override void Write(byte[] buffer, int offset, int count)
{
throw new NotImplementedException();
}
public override bool CanSeek { get; }
public override bool CanWrite { get; }
#endregion
}
|
|
The A much better way to hash a stream if possible would be to do something like: using System.IO;
using System.Security.Cryptography;
using SHA256 sha256 = SHA256.Create();
using Stream stream = Stream.Null; // Your stream you want to hash
byte[] hash = sha256.ComputeHash(stream);For .NET 5, there |
|
Ah, if you need to do something with the stream while hashing it, like copy it to another destination, I suppose you could also read from the stream as well. Perhaps something like this: using System;
using System.Buffers;
using System.IO;
using System.Security.Cryptography;
using IncrementalHash ih = IncrementalHash.CreateHash(HashAlgorithmName.SHA256);
using Stream source = Stream.Null; // Some stream
using Stream destination = Stream.Null; // Some stream
byte[] buffer = ArrayPool<byte>.Shared.Rent(4096);
int bytesRead;
while ((bytesRead = source.Read(buffer)) > 0)
{
ReadOnlySpan<byte> read = buffer.AsSpan(0, bytesRead);
ih.AppendData(read);
destination.Write(read);
}
ArrayPool<byte>.Shared.Return(buffer, clearArray: true);
byte[] hash = ih.GetHashAndReset(); |
|
My workaround involves a I see that #45080 completely covers this issue, so I'll just close it. Thanks for finding it - I think I didn't search for "perf" :/ |
|
Just FYI, with #53644, the earlier repro now outputs: (It's much better, though the last read shouldn't be necessary... I'll see if I can easily fix that before merging.) |
|
Uhm, you probably cannot fix that last read. It's my understanding that Streams' Read() will give you |
|
But yes - way better. |
I already did. To be clear, the issue is this: That last line isn't necessary once the previous read had returned 0. |
Description
I was using a
CryptoStreamwith aSHA1CryptoServiceProviderto hash a stream as it was being read. Through troubleshooting of some slow reads, I noticed thatCryptoStreamwas reading 1 byte at a time, which results in some immensely slow reads.The below example code simulates a Stream that provides less than the requested datasize, which is common for many streams, such as network streams and whatnot. The console output shows the following:
From this I gather that CryptoStream will read in chunks of 128K, but in the event that this buffer isn't filled, it will fill the remainder with 1 byte at a time.
IMO, CryptoStream should never read as little as 1 byte at a time, if more is needed.
Workarounds:
MyStreamsuch that each read isn't "slow"MyStreamin aBufferingStreamExample
The text was updated successfully, but these errors were encountered: