Skip to content

Commit

Permalink
Apply various code analysis improvements (#82)
Browse files Browse the repository at this point in the history
Motivation
----------
Update to use the latest C# 12 features and resolve various analysis
warnings and suggestions.

Modifications
-------------
- Use MemberNotNull attributes
- Use collection initializers
- Resolve some minor performance warnings shown by the latest compiler
- Make some methods static that don't access instance members
- Use more apparent types in many places
  • Loading branch information
brantburnett authored Dec 24, 2023
1 parent 97b2409 commit 777936b
Show file tree
Hide file tree
Showing 12 changed files with 172 additions and 87 deletions.
7 changes: 4 additions & 3 deletions Snappier/Internal/CopyHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ internal class CopyHelpers
// like byte-ordering on various architectures, so we can reference Vector128<byte> directly.
// It is however safe to convert to Vector128<byte> so we'll do that below with some casts
// that are elided by JIT.
private static ReadOnlySpan<byte> PshufbFillPatternsAsBytes => new byte[] {
private static ReadOnlySpan<byte> PshufbFillPatternsAsBytes =>
[
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // Never referenced, here for padding
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1,
Expand All @@ -28,7 +29,7 @@ internal class CopyHelpers
0, 1, 2, 3, 4, 0, 1, 2, 3, 4, 0, 1, 2, 3, 4, 0,
0, 1, 2, 3, 4, 5, 0, 1, 2, 3, 4, 5, 0, 1, 2, 3,
0, 1, 2, 3, 4, 5, 6, 0, 1, 2, 3, 4, 5, 6, 0, 1
};
];

/// <summary>
/// This is a table of shuffle control masks that can be used as the source
Expand All @@ -46,7 +47,7 @@ private static ReadOnlySpan<Vector128<byte>> PshufbFillPatterns
/// <summary>
/// j * (16 / j) for all j from 0 to 7. 0 is not actually used.
/// </summary>
private static ReadOnlySpan<byte> PatternSizeTable => new byte[] {0, 16, 16, 15, 16, 15, 12, 14};
private static ReadOnlySpan<byte> PatternSizeTable => [0, 16, 16, 15, 16, 15, 12, 14];

#endif

Expand Down
43 changes: 23 additions & 20 deletions Snappier/Internal/Crc32CAlgorithm.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,14 @@ internal static class Crc32CAlgorithm

private const uint Poly = 0x82F63B78u;

#pragma warning disable IDE1006
// ReSharper disable once InconsistentNaming
private static readonly uint[] Table;
#pragma warning restore IDE1006

static Crc32CAlgorithm()
{
var table = new uint[16 * 256];
uint[] table = new uint[16 * 256];
for (uint i = 0; i < 256; i++)
{
uint res = i;
Expand Down Expand Up @@ -118,25 +121,25 @@ public static uint Append(uint crc, ReadOnlySpan<byte> source)
uint[] table = Table;
while (source.Length >= 16)
{
var a = table[(3 * 256) + source[12]]
^ table[(2 * 256) + source[13]]
^ table[(1 * 256) + source[14]]
^ table[(0 * 256) + source[15]];

var b = table[(7 * 256) + source[8]]
^ table[(6 * 256) + source[9]]
^ table[(5 * 256) + source[10]]
^ table[(4 * 256) + source[11]];

var c = table[(11 * 256) + source[4]]
^ table[(10 * 256) + source[5]]
^ table[(9 * 256) + source[6]]
^ table[(8 * 256) + source[7]];

var d = table[(15 * 256) + ((byte)crcLocal ^ source[0])]
^ table[(14 * 256) + ((byte)(crcLocal >> 8) ^ source[1])]
^ table[(13 * 256) + ((byte)(crcLocal >> 16) ^ source[2])]
^ table[(12 * 256) + ((crcLocal >> 24) ^ source[3])];
uint a = table[(3 * 256) + source[12]]
^ table[(2 * 256) + source[13]]
^ table[(1 * 256) + source[14]]
^ table[(0 * 256) + source[15]];

uint b = table[(7 * 256) + source[8]]
^ table[(6 * 256) + source[9]]
^ table[(5 * 256) + source[10]]
^ table[(4 * 256) + source[11]];

uint c = table[(11 * 256) + source[4]]
^ table[(10 * 256) + source[5]]
^ table[(9 * 256) + source[6]]
^ table[(8 * 256) + source[7]];

uint d = table[(15 * 256) + ((byte)crcLocal ^ source[0])]
^ table[(14 * 256) + ((byte)(crcLocal >> 8) ^ source[1])]
^ table[(13 * 256) + ((byte)(crcLocal >> 16) ^ source[2])]
^ table[(12 * 256) + ((crcLocal >> 24) ^ source[3])];

crcLocal = d ^ c ^ b ^ a;
source = source.Slice(16);
Expand Down
8 changes: 4 additions & 4 deletions Snappier/Internal/HashTable.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ internal class HashTable : IDisposable

public void EnsureCapacity(int inputSize)
{
var maxFragmentSize = Math.Min(inputSize, (int) Constants.BlockSize);
var tableSize = CalculateTableSize(maxFragmentSize);
int maxFragmentSize = Math.Min(inputSize, (int) Constants.BlockSize);
int tableSize = CalculateTableSize(maxFragmentSize);

if (_buffer is null || tableSize > _buffer.Length)
{
Expand All @@ -50,12 +50,12 @@ public Span<ushort> GetHashTable(int fragmentSize)
}

Span<ushort> hashTable = _buffer.AsSpan(0, hashTableSize);
MemoryMarshal.AsBytes(hashTable).Fill(0);
hashTable.Clear();

return hashTable;
}

private int CalculateTableSize(int inputSize)
private static int CalculateTableSize(int inputSize)
{
if (inputSize > MaxHashTableSize)
{
Expand Down
16 changes: 8 additions & 8 deletions Snappier/Internal/Helpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,13 @@ public static int MaxCompressedLength(int sourceBytes)
return 32 + sourceBytes + sourceBytes / 6 + 1;
}

private static ReadOnlySpan<byte> LeftShiftOverflowsMasks => new byte[]
{
private static ReadOnlySpan<byte> LeftShiftOverflowsMasks =>
[
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x80, 0xc0, 0xe0, 0xf0, 0xf8, 0xfc, 0xfe
};
];

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static bool LeftShiftOverflows(byte value, int shift) =>
Expand Down Expand Up @@ -80,7 +80,7 @@ public static uint ExtractLowBytes(uint value, int numBytes)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static uint UnsafeReadUInt32(ref byte ptr)
{
var result = Unsafe.ReadUnaligned<uint>(ref ptr);
uint result = Unsafe.ReadUnaligned<uint>(ref ptr);
if (!BitConverter.IsLittleEndian)
{
result = BinaryPrimitives.ReverseEndianness(result);
Expand All @@ -92,7 +92,7 @@ public static uint UnsafeReadUInt32(ref byte ptr)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static ulong UnsafeReadUInt64(ref byte ptr)
{
var result = Unsafe.ReadUnaligned<ulong>(ref ptr);
ulong result = Unsafe.ReadUnaligned<ulong>(ref ptr);
if (!BitConverter.IsLittleEndian)
{
result = BinaryPrimitives.ReverseEndianness(result);
Expand Down Expand Up @@ -133,13 +133,13 @@ public static void StoreUnsafe(this Vector128<byte> source, ref byte destination
// or BitOperations. This is the same algorithm used by BitOperations.Log2 when hardware acceleration is unavailable.
// https://github.com/dotnet/runtime/blob/bee217ffbdd6b3ad60b0e1e17c6370f4bb618779/src/libraries/System.Private.CoreLib/src/System/Numerics/BitOperations.cs#L404

private static ReadOnlySpan<byte> Log2DeBruijn => new byte[32]
{
private static ReadOnlySpan<byte> Log2DeBruijn =>
[
00, 09, 01, 10, 13, 21, 02, 29,
11, 14, 16, 18, 22, 25, 03, 30,
08, 12, 20, 28, 15, 17, 24, 07,
19, 27, 23, 06, 26, 05, 04, 31
};
];

/// <summary>
/// Returns the integer (floor) log of the specified value, base 2.
Expand Down
72 changes: 70 additions & 2 deletions Snappier/Internal/NullableAttributes.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#pragma warning disable MA0048 // File name must match type name
#define INTERNAL_NULLABLE_ATTRIBUTES
#define INTERNAL_NULLABLE_ATTRIBUTES
#if NETSTANDARD2_0 || NETCOREAPP2_0 || NETCOREAPP2_1 || NETCOREAPP2_2 || NET45 || NET451 || NET452 || NET6 || NET461 || NET462 || NET47 || NET471 || NET472 || NET48

// https://github.com/dotnet/corefx/blob/48363ac826ccf66fbe31a5dcb1dc2aab9a7dd768/src/Common/src/CoreLib/System/Diagnostics/CodeAnalysis/NullableAttributes.cs
Expand Down Expand Up @@ -137,6 +136,75 @@ sealed class DoesNotReturnIfAttribute : Attribute
/// <summary>Gets the condition parameter value.</summary>
public bool ParameterValue { get; }
}

#if !NET5_0_OR_GREATER
/// <summary>Specifies that the method or property will ensure that the listed field and property members have not-null values.</summary>
[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property, Inherited = false, AllowMultiple = true)]
#if INTERNAL_NULLABLE_ATTRIBUTES
public
#else
internal
#endif
sealed class MemberNotNullAttribute : Attribute
{
/// <summary>Initializes the attribute with a field or property member.</summary>
/// <param name="member">
/// The field or property member that is promised to be not-null.
/// </param>
public MemberNotNullAttribute(string member) => Members = new[] { member };

/// <summary>Initializes the attribute with the list of field and property members.</summary>
/// <param name="members">
/// The list of field and property members that are promised to be not-null.
/// </param>
public MemberNotNullAttribute(params string[] members) => Members = members;

/// <summary>Gets field or property member names.</summary>
public string[] Members { get; }
}

/// <summary>Specifies that the method or property will ensure that the listed field and property members have not-null values when returning with the specified return value condition.</summary>
[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property, Inherited = false, AllowMultiple = true)]
#if INTERNAL_NULLABLE_ATTRIBUTES
public
#else
internal
#endif
sealed class MemberNotNullWhenAttribute : Attribute
{
/// <summary>Initializes the attribute with the specified return value condition and a field or property member.</summary>
/// <param name="returnValue">
/// The return value condition. If the method returns this value, the associated parameter will not be null.
/// </param>
/// <param name="member">
/// The field or property member that is promised to be not-null.
/// </param>
public MemberNotNullWhenAttribute(bool returnValue, string member)
{
ReturnValue = returnValue;
Members = new[] { member };
}

/// <summary>Initializes the attribute with the specified return value condition and list of field and property members.</summary>
/// <param name="returnValue">
/// The return value condition. If the method returns this value, the associated parameter will not be null.
/// </param>
/// <param name="members">
/// The list of field and property members that are promised to be not-null.
/// </param>
public MemberNotNullWhenAttribute(bool returnValue, params string[] members)
{
ReturnValue = returnValue;
Members = members;
}

/// <summary>Gets the return value condition.</summary>
public bool ReturnValue { get; }

/// <summary>Gets field or property member names.</summary>
public string[] Members { get; }
}
#endif
}
#endif

Expand Down
14 changes: 7 additions & 7 deletions Snappier/Internal/SnappyCompressor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,22 @@ public int Compress(ReadOnlySpan<byte> input, Span<byte> output)

while (input.Length > 0)
{
var fragment = input.Slice(0, Math.Min(input.Length, (int)Constants.BlockSize));
ReadOnlySpan<byte> fragment = input.Slice(0, Math.Min(input.Length, (int)Constants.BlockSize));

var hashTable = _workingMemory.GetHashTable(fragment.Length);
Span<ushort> hashTable = _workingMemory.GetHashTable(fragment.Length);

var maxOutput = Helpers.MaxCompressedLength(fragment.Length);
int maxOutput = Helpers.MaxCompressedLength(fragment.Length);

if (output.Length >= maxOutput)
{
var written = CompressFragment(fragment, output, hashTable);
int written = CompressFragment(fragment, output, hashTable);

output = output.Slice(written);
bytesWritten += written;
}
else
{
var scratch = ArrayPool<byte>.Shared.Rent(maxOutput);
byte[] scratch = ArrayPool<byte>.Shared.Rent(maxOutput);
try
{
int written = CompressFragment(fragment, scratch.AsSpan(), hashTable);
Expand Down Expand Up @@ -306,7 +306,7 @@ private static int CompressFragment(ReadOnlySpan<byte> input, Span<byte> output,
// "literal bytes" prior to ip.
ref byte emitBase = ref ip;

var (matchLength, matchLengthLessThan8) =
(int matchLength, bool matchLengthLessThan8) =
FindMatchLength(ref Unsafe.Add(ref candidate, 4), ref Unsafe.Add(ref ip, 4), ref inputEnd, ref data);

int matched = 4 + matchLength;
Expand Down Expand Up @@ -442,7 +442,7 @@ private static ref byte EmitCopyAtMost64LenGreaterThanOrEqualTo12(ref byte op, l

// Write 4 bytes, though we only care about 3 of them. The output buffer
// is required to have some slack, so the extra byte won't overrun it.
var u = unchecked((uint)(Constants.Copy2ByteOffset + ((length - 1) << 2) + (offset << 8)));
uint u = unchecked((uint)(Constants.Copy2ByteOffset + ((length - 1) << 2) + (offset << 8)));
Helpers.UnsafeWriteUInt32(ref op, u);
return ref Unsafe.Add(ref op, 3);
}
Expand Down
12 changes: 6 additions & 6 deletions Snappier/Internal/SnappyDecompressor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public void Decompress(ReadOnlySpan<byte> input)
{
if (!ExpectedLength.HasValue)
{
var readLength = ReadUncompressedLength(ref input);
int? readLength = ReadUncompressedLength(ref input);
if (readLength.HasValue)
{
ExpectedLength = readLength.GetValueOrDefault();
Expand Down Expand Up @@ -94,7 +94,7 @@ public void Reset()
int shift = _uncompressedLengthShift;
bool foundEnd = false;

var i = 0;
int i = 0;
while (input.Length > i)
{
byte c = input[i];
Expand Down Expand Up @@ -126,7 +126,7 @@ public void Reset()
_uncompressedLength = result;
_uncompressedLengthShift = shift;

return foundEnd ? (int?)result : null;
return foundEnd ? result : null;
}

/// <summary>
Expand All @@ -141,7 +141,7 @@ public static int ReadUncompressedLength(ReadOnlySpan<byte> input)
int shift = 0;
bool foundEnd = false;

var i = 0;
int i = 0;
while (input.Length > 0)
{
byte c = input[i];
Expand Down Expand Up @@ -579,7 +579,7 @@ private void Append(ReadOnlySpan<byte> input)
{
ref readonly byte inputPtr = ref input[0];

var lookbackSpan = _lookbackBuffer.Span;
Span<byte> lookbackSpan = _lookbackBuffer.Span;
ref byte op = ref lookbackSpan[_lookbackPosition];

Append(ref op, ref Unsafe.Add(ref lookbackSpan[0], lookbackSpan.Length), in inputPtr, input.Length);
Expand Down Expand Up @@ -634,7 +634,7 @@ private static void AppendFromSelf(ref byte op, ref byte buffer, ref byte buffer

public int Read(Span<byte> destination)
{
var unreadBytes = UnreadBytes;
int unreadBytes = UnreadBytes;
if (unreadBytes == 0)
{
return 0;
Expand Down
Loading

0 comments on commit 777936b

Please sign in to comment.