Skip to content

Commit

Permalink
Addressing review comments - 4.
Browse files Browse the repository at this point in the history
  • Loading branch information
@AR-May
AR-May committed Jan 12, 2022
1 parent 19c60f4 commit d0462e3
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 24 deletions.
2 changes: 2 additions & 0 deletions src/Tasks.UnitTests/Hash_Tests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ public void HashTaskLargeInputCountAndSizeTest()
}

#pragma warning disable CA5350
// This test verifies that hash computes correctly for various numbers of characters.
// We would like to process edge of the buffer use cases regardless on the size of the buffer.
[Fact]
public void HashTaskDifferentInputSizesTest()
{
Expand Down
47 changes: 23 additions & 24 deletions src/Tasks/Hash.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.Security.Cryptography;
using System.Text;
using Microsoft.Build.Framework;
using Microsoft.Build.Shared;

namespace Microsoft.Build.Tasks
{
Expand All @@ -18,19 +17,17 @@ namespace Microsoft.Build.Tasks
/// </remarks>
public class Hash : TaskExtension
{
private const char s_itemSeparatorCharacter = '\u2028';

private const char ItemSeparatorCharacter = '\u2028';
private static readonly Encoding s_encoding = Encoding.UTF8;

private static readonly byte[] s_itemSeparatorCharacterBytes = s_encoding.GetBytes(new char[] { s_itemSeparatorCharacter });
private static readonly byte[] s_itemSeparatorCharacterBytes = s_encoding.GetBytes(new char[] { ItemSeparatorCharacter });

// Size of buffer where bytes of the strings are stored until sha1.TransformBlock is be run on them.
// It is needed to get a balance between amount of costly sha1.TransformBlock calls and amount of allocated memory.
private const int sha1BufferSize = 512;
private const int Sha1BufferSize = 512;

// Size of chunks in which ItemSpecs would be cut.
// String of size 169 gives no more than ~512 bytes in utf8 encoding.
private const int maxInputChunkLength = 169;
// We have chosen this length so itemSpecChunkByteBuffer rented from ArrayPool will be close but not bigger than 512.
private const int MaxInputChunkLength = 169;

/// <summary>
/// Items from which to generate a hash.
Expand Down Expand Up @@ -63,28 +60,28 @@ public override bool Execute()
byte[] sha1Buffer = null;

// Buffer in which bytes of items' ItemSpec are to be stored.
byte[] byteBuffer = null;
byte[] itemSpecChunkByteBuffer = null;

try
{
sha1Buffer = System.Buffers.ArrayPool<byte>.Shared.Rent(sha1BufferSize);
byteBuffer = System.Buffers.ArrayPool<byte>.Shared.Rent(s_encoding.GetMaxByteCount(maxInputChunkLength));
sha1Buffer = System.Buffers.ArrayPool<byte>.Shared.Rent(Sha1BufferSize);
itemSpecChunkByteBuffer = System.Buffers.ArrayPool<byte>.Shared.Rent(s_encoding.GetMaxByteCount(MaxInputChunkLength));

int sha1BufferPosition = 0;
for (int i = 0; i < ItemsToHash.Length; i++)
{
string itemSpec = IgnoreCase ? ItemsToHash[i].ItemSpec.ToUpperInvariant() : ItemsToHash[i].ItemSpec;

// Slice the itemSpec string into chunks of reasonable size and add them to sha1 buffer.
for (int itemSpecPosition = 0; itemSpecPosition < itemSpec.Length; itemSpecPosition += maxInputChunkLength)
for (int itemSpecPosition = 0; itemSpecPosition < itemSpec.Length; itemSpecPosition += MaxInputChunkLength)
{
int charsToProcess = Math.Min(itemSpec.Length - itemSpecPosition, maxInputChunkLength);
int byteCount = s_encoding.GetBytes(itemSpec, itemSpecPosition, charsToProcess, byteBuffer, 0);
int charsToProcess = Math.Min(itemSpec.Length - itemSpecPosition, MaxInputChunkLength);
int byteCount = s_encoding.GetBytes(itemSpec, itemSpecPosition, charsToProcess, itemSpecChunkByteBuffer, 0);

AddBytesToSha1Buffer(sha1, sha1Buffer, ref sha1BufferPosition, sha1BufferSize, byteBuffer, byteCount);
sha1BufferPosition = AddBytesToSha1Buffer(sha1, sha1Buffer, sha1BufferPosition, Sha1BufferSize, itemSpecChunkByteBuffer, byteCount);
}

AddBytesToSha1Buffer(sha1, sha1Buffer, ref sha1BufferPosition, sha1BufferSize, s_itemSeparatorCharacterBytes, s_itemSeparatorCharacterBytes.Length);
sha1BufferPosition = AddBytesToSha1Buffer(sha1, sha1Buffer, sha1BufferPosition, Sha1BufferSize, s_itemSeparatorCharacterBytes, s_itemSeparatorCharacterBytes.Length);
}

sha1.TransformFinalBlock(sha1Buffer, 0, sha1BufferPosition);
Expand All @@ -104,9 +101,9 @@ public override bool Execute()
{
System.Buffers.ArrayPool<byte>.Shared.Return(sha1Buffer);
}
if (byteBuffer != null)
if (itemSpecChunkByteBuffer != null)
{
System.Buffers.ArrayPool<byte>.Shared.Return(byteBuffer);
System.Buffers.ArrayPool<byte>.Shared.Return(itemSpecChunkByteBuffer);
}
}
}
Expand All @@ -123,9 +120,9 @@ public override bool Execute()
/// <param name="sha1BufferSize">The size of sha1 buffer.</param>
/// <param name="byteBuffer">Bytes buffer which contains bytes to be written to sha1 buffer.</param>
/// <param name="byteCount">Amount of bytes that are to be added to sha1 buffer.</param>
private void AddBytesToSha1Buffer(SHA1 sha1, byte[] sha1Buffer, ref int sha1BufferPosition, int sha1BufferSize, byte[] byteBuffer, int byteCount)
private int AddBytesToSha1Buffer(SHA1 sha1, byte[] sha1Buffer, int sha1BufferPosition, int sha1BufferSize, byte[] byteBuffer, int byteCount)
{
int bytesProcessedNumber = 0;
int bytesProcessed = 0;
while (sha1BufferPosition + byteCount >= sha1BufferSize)
{
int sha1BufferFreeSpace = sha1BufferSize - sha1BufferPosition;
Expand All @@ -134,21 +131,23 @@ private void AddBytesToSha1Buffer(SHA1 sha1, byte[] sha1Buffer, ref int sha1Buff
{
// If sha1 buffer is empty and bytes number is big enough there is no need to copy bytes to sha1 buffer.
// Pass the bytes to TransformBlock right away.
sha1.TransformBlock(byteBuffer, bytesProcessedNumber, sha1BufferSize, null, 0);
sha1.TransformBlock(byteBuffer, bytesProcessed, sha1BufferSize, null, 0);
}
else
{
Array.Copy(byteBuffer, bytesProcessedNumber, sha1Buffer, sha1BufferPosition, sha1BufferFreeSpace);
Array.Copy(byteBuffer, bytesProcessed, sha1Buffer, sha1BufferPosition, sha1BufferFreeSpace);
sha1.TransformBlock(sha1Buffer, 0, sha1BufferSize, null, 0);
sha1BufferPosition = 0;
}

bytesProcessedNumber += sha1BufferFreeSpace;
bytesProcessed += sha1BufferFreeSpace;
byteCount -= sha1BufferFreeSpace;
}

Array.Copy(byteBuffer, bytesProcessedNumber, sha1Buffer, sha1BufferPosition, byteCount);
Array.Copy(byteBuffer, bytesProcessed, sha1Buffer, sha1BufferPosition, byteCount);
sha1BufferPosition += byteCount;

return sha1BufferPosition;
}
}
}

0 comments on commit d0462e3

Please sign in to comment.