Reduce memory usage during decryption #1318
Conversation
| @@ -202,14 +202,13 @@ public static byte[] SymmetricEncryptWithHMACIV( byte[] input, byte[] key, byte[ | |||
| var random = GenerateRandomBlock( 3 ); | |||
| Array.Copy( random, 0, iv, iv.Length - random.Length, random.Length ); | |||
|
|
|||
| using ( var hmac = new HMACSHA1( hmacSecret ) ) | |||
| using ( var ms = new MemoryStream() ) | |||
| using ( var ms = new MemoryStream( random.Length + input.Length ) ) | |||
There was a problem hiding this comment.
I feel like ArrayPool.Shared should be used in these methods.
There was a problem hiding this comment.
I found that if it is a small array, ArrayPool will consume more performance, but stackalloc is worth trying
There was a problem hiding this comment.
Are you sure that will hold true for long running processes? Also take a look at #683
There was a problem hiding this comment.
I think it's fine, the memory will always be collect in time
There was a problem hiding this comment.
Can you benchmark the old against the new please, including memory stats? I’d expect the change in allocations to reflect a change in CPU time, but it would be good to have actual figures.
There was a problem hiding this comment.
I ran a couple benchmarks on my laptop too, just to confirm.
This PR drops execution time about 10-30%. If we use ArrayPool<byte>.Shared instead of new MemoryStream then we can get a further 15-25% savings.
However, considering that on my laptop these calls are all measured in microseconds, it's hardly worth the effort to argue over it.
I'm happy to merge this if you are @xPaw.
There was a problem hiding this comment.
However, its memory usage is much reduced compared to the original, especially in multi-threading. Multiple threads download chunkdata and decrypt it at the same time, which can reduce the memory by more than 200MB (500MB->300MB).
There was a problem hiding this comment.
Okay I looked at this code again, a temporary array is not even required.
Make an instance of HMACSHA1, and then use TransformBlock / TransformFinalBlock if I remember correctly.
But this code also allocates other random arrays like iv and GenerateRandomBlock, this could also be refactored (and I would use ArrayPool).
If the entire encryption flow was refactored, I'm almost positive it's possible to get it down to zero allocations (besides arraypool where it really is needed). But this requires changing code not to return byte arrays, but rather take an existing output array is a parameter.
No description provided.