Skip to content

Commit

Permalink
Remove misc. temporary allocations from RESP parsing (#431)
Browse files Browse the repository at this point in the history
* Add TrySliceWithLengthHeader to RespReadUtils

* Will lay ground work for more allocation-free code

* wip: Start removing allocations.

* ReadByteArray => TrySlice
* Avoid some temp arrays in strings I came across

* Move Ascii operations to AsciiUtils and use vectorized BCL where possible

* Remove LINQ From ListRemove

* Remove LINQ From ListInsert

* quick fix to check if ci is green

* Fix ListRemove regression

* Remove unused variable left from previous commits

* More SortedSet to new methods

* Use slice in AdminCommands

* Remove more misc. tiny allocations

* Remove more misc. tiny allocations

* Simplify more RESP parsing..

* Little number parsing consolidation to NumUtils

* Revert some places where no benefit and add some more

* Revert couple more for now

* Apply PR feedback

* and remove couple unnecessary unsafe modifiers

* Apply PR feedback

* oops, forgot this

---------

Co-authored-by: Badrish Chandramouli <[email protected]>
  • Loading branch information
PaulusParssinen and badrishc authored Jun 4, 2024
1 parent a7077a1 commit d15c5a8
Show file tree
Hide file tree
Showing 38 changed files with 494 additions and 517 deletions.
2 changes: 1 addition & 1 deletion libs/cluster/Parsing/ClusterSubCommandParsing.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ _ when param.SequenceEqual(CmdStrings.failreplicationoffset) => ClusterSubcomman
/// <returns>ClusterSubcommand type</returns>
private static ClusterSubcommand ConvertToClusterSubcommandIgnoreCase(ref Span<byte> subcommand)
{
ConvertUtils.MakeUpperCase(subcommand);
AsciiUtils.ToUpperInPlace(subcommand);
var subcmd = subcommand switch
{
_ when subcommand.SequenceEqual(CmdStrings.MEET) => ClusterSubcommand.MEET,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public void CloseDevice()
/// <param name="startAddress"></param>
/// <param name="data"></param>
/// <param name="segmentId"></param>
public void ProcessFileSegments(int segmentId, Guid token, CheckpointFileType type, long startAddress, Span<byte> data)
public void ProcessFileSegments(int segmentId, Guid token, CheckpointFileType type, long startAddress, ReadOnlySpan<byte> data)
{
clusterProvider.replicationManager.UpdateLastPrimarySyncTime();
if (writeIntoCkptDevice == null)
Expand Down Expand Up @@ -75,7 +75,7 @@ public void ProcessFileSegments(int segmentId, Guid token, CheckpointFileType ty
/// <param name="address"></param>
/// <param name="buffer"></param>
/// <param name="size"></param>
private unsafe void WriteInto(IDevice device, ulong address, Span<byte> buffer, int size, int segmentId = -1)
private unsafe void WriteInto(IDevice device, ulong address, ReadOnlySpan<byte> buffer, int size, int segmentId = -1)
{
if (writeCheckpointBufferPool == null)
writeCheckpointBufferPool = new SectorAlignedBufferPool(1, (int)device.SectorSize);
Expand Down
2 changes: 1 addition & 1 deletion libs/cluster/Session/ClusterSlotCheck.cs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ private void WriteClusterSlotVerificationMessage(ClusterConfig config, ClusterSl
/// </summary>
/// <returns>True if redirect, False if can serve</returns>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool NetworkSingleKeySlotVerify(byte[] key, bool readOnly, byte SessionAsking, ref byte* dcurr, ref byte* dend)
public bool NetworkSingleKeySlotVerify(ReadOnlySpan<byte> key, bool readOnly, byte SessionAsking, ref byte* dcurr, ref byte* dend)
{
fixed (byte* keyPtr = key)
return NetworkSingleKeySlotVerify(new ArgSlice(keyPtr, key.Length), readOnly, SessionAsking, ref dcurr, ref dend);
Expand Down
9 changes: 4 additions & 5 deletions libs/cluster/Session/RespClusterBasicCommands.cs
Original file line number Diff line number Diff line change
Expand Up @@ -183,14 +183,13 @@ private bool NetworkClusterMeet(ReadOnlySpan<byte> bufSpan, int count, out bool
}

var ptr = recvBufferPtr + readHead;
if (!RespReadUtils.ReadByteArrayWithLengthHeader(out var ipaddress, ref ptr, recvBufferPtr + bytesRead))
if (!RespReadUtils.ReadStringWithLengthHeader(out var ipaddressStr, ref ptr, recvBufferPtr + bytesRead))
return false;

if (!RespReadUtils.ReadIntWithLengthHeader(out var port, ref ptr, recvBufferPtr + bytesRead))
return false;
readHead = (int)(ptr - recvBufferPtr);

var ipaddressStr = Encoding.ASCII.GetString(ipaddress);
logger?.LogTrace("CLUSTER MEET {ipaddressStr} {port}", ipaddressStr, port);
clusterProvider.clusterManager.RunMeetTask(ipaddressStr, port);
while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_OK, ref dcurr, dend))
Expand Down Expand Up @@ -403,10 +402,10 @@ private bool NetworkClusterGossip(int count, out bool invalidParameters)
var gossipWithMeet = false;
if (count > 1)
{
if (!RespReadUtils.ReadByteArrayWithLengthHeader(out var withMeet, ref ptr, recvBufferPtr + bytesRead))
if (!RespReadUtils.TrySliceWithLengthHeader(out var withMeetSpan, ref ptr, recvBufferPtr + bytesRead))
return false;
Debug.Assert(withMeet.SequenceEqual(CmdStrings.WITHMEET.ToArray()));
if (withMeet.SequenceEqual(CmdStrings.WITHMEET.ToArray()))
Debug.Assert(withMeetSpan.SequenceEqual(CmdStrings.WITHMEET));
if (withMeetSpan.SequenceEqual(CmdStrings.WITHMEET))
gossipWithMeet = true;
}

Expand Down
5 changes: 2 additions & 3 deletions libs/cluster/Session/RespClusterFailoverCommands.cs
Original file line number Diff line number Diff line change
Expand Up @@ -122,13 +122,12 @@ private bool NetworkClusterFailStopWrites(int count, out bool invalidParameters)
}

var ptr = recvBufferPtr + readHead;
if (!RespReadUtils.ReadByteArrayWithLengthHeader(out var nodeIdBytes, ref ptr, recvBufferPtr + bytesRead))
if (!RespReadUtils.ReadStringWithLengthHeader(out var nodeId, ref ptr, recvBufferPtr + bytesRead))
return false;
readHead = (int)(ptr - recvBufferPtr);

if (nodeIdBytes.Length > 0)
if (!string.IsNullOrEmpty(nodeId))
{// Make this node a primary after receiving a request from a replica that is trying to takeover
var nodeId = Encoding.ASCII.GetString(nodeIdBytes);
clusterProvider.clusterManager.TryStopWrites(nodeId);
}
else
Expand Down
15 changes: 7 additions & 8 deletions libs/cluster/Session/RespClusterReplicationCommands.cs
Original file line number Diff line number Diff line change
Expand Up @@ -248,15 +248,15 @@ private bool NetworkClusterInitiateReplicaSync(int count, out bool invalidParame
return false;
if (!RespReadUtils.ReadStringWithLengthHeader(out var primary_replid, ref ptr, recvBufferPtr + bytesRead))
return false;
if (!RespReadUtils.ReadByteArrayWithLengthHeader(out var cEntryByteArray, ref ptr, recvBufferPtr + bytesRead))
if (!RespReadUtils.ReadByteArrayWithLengthHeader(out var checkpointEntryBytes, ref ptr, recvBufferPtr + bytesRead))
return false;
if (!RespReadUtils.ReadLongWithLengthHeader(out var replicaAofBeginAddress, ref ptr, recvBufferPtr + bytesRead))
return false;
if (!RespReadUtils.ReadLongWithLengthHeader(out var replicaAofTailAddress, ref ptr, recvBufferPtr + bytesRead))
return false;
readHead = (int)(ptr - recvBufferPtr);

var remoteEntry = CheckpointEntry.FromByteArray(cEntryByteArray);
var remoteEntry = CheckpointEntry.FromByteArray(checkpointEntryBytes);

if (!clusterProvider.replicationManager.TryBeginReplicaSyncSession(
nodeId, primary_replid, remoteEntry, replicaAofBeginAddress, replicaAofTailAddress, out var errorMessage))
Expand Down Expand Up @@ -291,7 +291,7 @@ private bool NetworkClusterSendCheckpointMetadata(int count, out bool invalidPar
}

var ptr = recvBufferPtr + readHead;
if (!RespReadUtils.ReadByteArrayWithLengthHeader(out var fileTokenBytes, ref ptr, recvBufferPtr + bytesRead))
if (!RespReadUtils.TrySliceWithLengthHeader(out var fileTokenBytes, ref ptr, recvBufferPtr + bytesRead))
return false;
if (!RespReadUtils.ReadIntWithLengthHeader(out var fileTypeInt, ref ptr, recvBufferPtr + bytesRead))
return false;
Expand Down Expand Up @@ -325,14 +325,13 @@ private bool NetworkClusterSendCheckpointFileSegment(int count, out bool invalid
}

var ptr = recvBufferPtr + readHead;
Span<byte> data = default;
if (!RespReadUtils.ReadByteArrayWithLengthHeader(out var fileTokenBytes, ref ptr, recvBufferPtr + bytesRead))
if (!RespReadUtils.TrySliceWithLengthHeader(out var fileTokenBytes, ref ptr, recvBufferPtr + bytesRead))
return false;
if (!RespReadUtils.ReadIntWithLengthHeader(out var ckptFileTypeInt, ref ptr, recvBufferPtr + bytesRead))
return false;
if (!RespReadUtils.ReadLongWithLengthHeader(out var startAddress, ref ptr, recvBufferPtr + bytesRead))
return false;
if (!RespReadUtils.ReadSpanByteWithLengthHeader(ref data, ref ptr, recvBufferPtr + bytesRead))
if (!RespReadUtils.TrySliceWithLengthHeader(out var data, ref ptr, recvBufferPtr + bytesRead))
return false;
if (!RespReadUtils.ReadIntWithLengthHeader(out var segmentId, ref ptr, recvBufferPtr + bytesRead))
return false;
Expand Down Expand Up @@ -376,15 +375,15 @@ private bool NetworkClusterBeginReplicaRecover(int count, out bool invalidParame
return false;
if (!RespReadUtils.ReadStringWithLengthHeader(out var primary_replid, ref ptr, recvBufferPtr + bytesRead))
return false;
if (!RespReadUtils.ReadByteArrayWithLengthHeader(out var cEntryByteArray, ref ptr, recvBufferPtr + bytesRead))
if (!RespReadUtils.ReadByteArrayWithLengthHeader(out var checkpointEntryBytes, ref ptr, recvBufferPtr + bytesRead))
return false;
if (!RespReadUtils.ReadLongWithLengthHeader(out var beginAddress, ref ptr, recvBufferPtr + bytesRead))
return false;
if (!RespReadUtils.ReadLongWithLengthHeader(out var tailAddress, ref ptr, recvBufferPtr + bytesRead))
return false;
readHead = (int)(ptr - recvBufferPtr);

var entry = CheckpointEntry.FromByteArray(cEntryByteArray);
var entry = CheckpointEntry.FromByteArray(checkpointEntryBytes);
var replicationOffset = clusterProvider.replicationManager.BeginReplicaRecover(
recoverMainStoreFromToken,
recoverObjectStoreFromToken,
Expand Down
78 changes: 74 additions & 4 deletions libs/common/AsciiUtils.cs
Original file line number Diff line number Diff line change
@@ -1,17 +1,87 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

using System;
using System.Diagnostics;
using System.Text;

namespace Garnet.common;

/// <summary>
/// Utilites for ASCII parsing and manipulation.
/// </summary>
/// <remarks>
/// This class polyfills various <see cref="char"/> and <c>Ascii</c> methods for .NET 6.
/// </remarks>
public static class AsciiUtils
{
public static byte ToLower(byte value)
/// <summary>Indicates whether a character is within the specified inclusive range.</summary>
/// <param name="c">The character to evaluate.</param>
/// <param name="minInclusive">The lower bound, inclusive.</param>
/// <param name="maxInclusive">The upper bound, inclusive.</param>
/// <returns>true if <paramref name="c"/> is within the specified range; otherwise, false.</returns>
/// <remarks>
/// The method does not validate that <paramref name="maxInclusive"/> is greater than or equal
/// to <paramref name="minInclusive"/>. If <paramref name="maxInclusive"/> is less than
/// <paramref name="minInclusive"/>, the behavior is undefined.
/// </remarks>
public static bool IsBetween(byte c, char minInclusive, char maxInclusive)
{
return (uint)(c - minInclusive) <= (uint)(maxInclusive - minInclusive);
}

public static byte ToLower(byte c)
{
if ((uint)(value - 'A') <= (uint)('Z' - 'A')) // Is in [A-Z]
value = (byte)(value | 0x20);
return value;
if (IsBetween(c, 'A', 'Z'))
c = (byte)(c | 0x20);
return c;
}
public static byte ToUpper(byte c)
{
if (IsBetween(c, 'a', 'z'))
c = (byte)(c & ~0x20);
return c;
}

/// <summary>
/// Convert ASCII Span to upper case
/// </summary>
public static void ToUpperInPlace(Span<byte> command)
{
#if NET8_0_OR_GREATER
Ascii.ToUpperInPlace(command, out _);
#else
foreach (ref var c in command)
if (c > 96 && c < 123)
c -= 32;
#endif
}

/// <inheritdoc cref="EqualsUpperCaseSpanIgnoringCase(ReadOnlySpan{byte}, ReadOnlySpan{byte})"/>
public static bool EqualsUpperCaseSpanIgnoringCase(this Span<byte> left, ReadOnlySpan<byte> right)
=> EqualsUpperCaseSpanIgnoringCase(left, right);

/// <summary>
/// Check if two byte spans are equal, where right is an all-upper-case span, ignoring case if there are ASCII bytes.
/// </summary>
public static bool EqualsUpperCaseSpanIgnoringCase(this ReadOnlySpan<byte> left, ReadOnlySpan<byte> right)
{
if (left.SequenceEqual(right))
return true;
if (left.Length != right.Length)
return false;
for (int i = 0; i < left.Length; i++)
{
var b1 = left[i];
var b2 = right[i];

// Debug assert that b2 is an upper case letter 'A'-'Z'
Debug.Assert(b2 is >= 65 and <= 90);

if (b1 == b2 || b1 - 32 == b2)
continue;
return false;
}
return true;
}
}
44 changes: 0 additions & 44 deletions libs/common/ConvertUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,49 +43,5 @@ public static long MillisecondsFromDiffUtcNowTicks(long ticks)
}
return milliseconds;
}

/// <summary>
/// Convert ASCII Span to upper case
/// </summary>
/// <param name="command"></param>
public static void MakeUpperCase(Span<byte> command)
{
foreach (ref var c in command)
if (c > 96 && c < 123)
c -= 32;
}

/// <summary>
/// Check if two byte spans are equal, where right is an all-upper-case span, ignoring case if there are ASCII bytes.
/// </summary>
/// <param name="left"></param>
/// <param name="right"></param>
/// <returns></returns>
public static bool EqualsUpperCaseSpanIgnoringCase(this ReadOnlySpan<byte> left, ReadOnlySpan<byte> right)
{
if (left.SequenceEqual(right))
return true;
if (left.Length != right.Length)
return false;
for (int i = 0; i < left.Length; i++)
{
var b1 = left[i];
var b2 = right[i];

// Debug assert that b2 is an upper case letter 'A'-'Z'
Debug.Assert(b2 is >= 65 and <= 90);

if (b1 == b2 || b1 - 32 == b2)
continue;
return false;
}
return true;
}

/// <summary>
/// Check if two byte spans are equal, where right is an all-upper-case span, ignoring case if there are ASCII bytes.
/// </summary>
public static bool EqualsUpperCaseSpanIgnoringCase(this Span<byte> left, ReadOnlySpan<byte> right)
=> EqualsUpperCaseSpanIgnoringCase((ReadOnlySpan<byte>)left, right);
}
}
46 changes: 23 additions & 23 deletions libs/common/NumUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ public static bool TryBytesToLong(int length, byte* source, out long result)
return false;

// Parse number and check consumed bytes to avoid alphanumeric strings
if (!Utf8Parser.TryParse(new Span<byte>(beg, len), out result, out var bytesConsumed) || bytesConsumed != len)
if (!TryParse(new ReadOnlySpan<byte>(beg, len), out result))
return false;

// Negate if parsed value has a leading negative sign
Expand Down Expand Up @@ -358,29 +358,29 @@ public static int NumDigitsInLong(long v, ref bool fNeg)
return 19;
}

/// <summary>
/// Try to parse from pointer to integer
/// </summary>
/// <param name="source"></param>
/// <param name="len"></param>
/// <param name="result"></param>
/// <returns></returns>
public static unsafe bool TryBytesToInt(byte* source, int len, out int result)
/// <inheritdoc cref="Utf8Parser.TryParse(ReadOnlySpan{byte}, out int, out int, char)"/>
public static bool TryParse(ReadOnlySpan<byte> source, out int value)
{
bool fNeg = (*source == '-');
var beg = fNeg ? source + 1 : source;
var start = fNeg ? 1 : 0;
result = 0;
for (int i = start; i < len; ++i)
{
if (!(source[i] >= 48 && source[i] <= 57))
{
return false;
}
result = result * 10 + (*beg++ - '0');
}
result = fNeg ? -(result) : result;
return true;
return Utf8Parser.TryParse(source, out value, out var bytesConsumed, default) &&
bytesConsumed == source.Length;
}
/// <inheritdoc cref="Utf8Parser.TryParse(ReadOnlySpan{byte}, out long, out int, char)"/>
public static bool TryParse(ReadOnlySpan<byte> source, out long value)
{
return Utf8Parser.TryParse(source, out value, out var bytesConsumed, default) &&
bytesConsumed == source.Length;
}
/// <inheritdoc cref="Utf8Parser.TryParse(ReadOnlySpan{byte}, out float, out int, char)"/>
public static bool TryParse(ReadOnlySpan<byte> source, out float value)
{
return Utf8Parser.TryParse(source, out value, out var bytesConsumed, default) &&
bytesConsumed == source.Length;
}
/// <inheritdoc cref="Utf8Parser.TryParse(ReadOnlySpan{byte}, out double, out int, char)"/>
public static bool TryParse(ReadOnlySpan<byte> source, out double value)
{
return Utf8Parser.TryParse(source, out value, out var bytesConsumed, default) &&
bytesConsumed == source.Length;
}
}
}
Loading

0 comments on commit d15c5a8

Please sign in to comment.