From fbc6b417fadcdb78c239f0ec4497c1ddacd60026 Mon Sep 17 00:00:00 2001 From: Tal Zaccai Date: Mon, 18 Nov 2024 15:07:03 -0800 Subject: [PATCH] Eliminating usage of Enum.Parse/Enum.TryParse from critical performance paths (#806) * Eliminating usage of Enum.Parse/TryParse from critical performance paths * wip * wip * bugfix * format * Fixing comment * Moving parsing logic to SessionParseState extension methods * Removing unnecessary usings * format * Added 2 parsers --- libs/cluster/Server/HashSlot.cs | 20 +- libs/cluster/Session/FailoverCommand.cs | 3 +- .../Session/RespClusterFailoverCommands.cs | 3 +- .../RespClusterSlotManagementCommands.cs | 10 +- libs/cluster/SessionParseStateExtensions.cs | 76 ++++++ libs/common/FailoverOption.cs | 19 +- libs/common/Metrics/InfoMetricsType.cs | 2 + libs/common/Metrics/LatencyMetricsType.cs | 2 + libs/server/ClientType.cs | 18 +- libs/server/Metrics/Info/InfoCommand.cs | 38 ++- .../Metrics/Latency/RespLatencyCommands.cs | 12 +- .../Objects/SortedSet/SortedSetObjectImpl.cs | 38 +-- libs/server/Resp/Bitmap/BitmapCommands.cs | 4 +- libs/server/Resp/ClientCommands.cs | 17 +- libs/server/Resp/KeyAdminCommands.cs | 34 +-- libs/server/Resp/Parser/SessionParseState.cs | 36 --- libs/server/Resp/PurgeBPCommand.cs | 4 +- libs/server/SessionParseStateExtensions.cs | 217 ++++++++++++++++++ .../Functions/MainStore/PrivateMethods.cs | 16 +- .../Storage/Session/MainStore/BitmapOps.cs | 27 ++- 20 files changed, 382 insertions(+), 214 deletions(-) create mode 100644 libs/cluster/SessionParseStateExtensions.cs create mode 100644 libs/server/SessionParseStateExtensions.cs diff --git a/libs/cluster/Server/HashSlot.cs b/libs/cluster/Server/HashSlot.cs index 18578855ed..0e59f8c3ff 100644 --- a/libs/cluster/Server/HashSlot.cs +++ b/libs/cluster/Server/HashSlot.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -using System; using System.Runtime.InteropServices; namespace Garnet.cluster @@ -11,6 +10,8 @@ namespace Garnet.cluster /// public enum SlotState : byte { + // IMPORTANT: Any changes to the values of this enum should be reflected in its parser (SessionParseStateExtensions.TryGetSlotState) + /// /// Slot not assigned /// @@ -41,23 +42,6 @@ public enum SlotState : byte INVALID, } - /// - /// Extension methods for . - /// - public static class SlotStateExtensions - { - /// - /// Validate that the given is legal, and _could_ have come from the given . - /// - /// TODO: Long term we can kill this and use instead of - /// and avoid extra validation. See: https://github.com/dotnet/runtime/issues/81500 . - /// - public static bool IsValid(this SlotState type, ReadOnlySpan fromSpan) - { - return type != SlotState.INVALID && type != SlotState.OFFLINE && Enum.IsDefined(type) && !fromSpan.ContainsAnyInRange((byte)'0', (byte)'9'); - } - } - /// /// Hashslot info /// diff --git a/libs/cluster/Session/FailoverCommand.cs b/libs/cluster/Session/FailoverCommand.cs index 0000313f8d..abd3524772 100644 --- a/libs/cluster/Session/FailoverCommand.cs +++ b/libs/cluster/Session/FailoverCommand.cs @@ -21,7 +21,8 @@ private bool TryFAILOVER() var currTokenIdx = 0; while (currTokenIdx < parseState.Count) { - if (!parseState.TryGetEnum(currTokenIdx, true, out FailoverOption failoverOption) || !failoverOption.IsValid(parseState.GetArgSliceByRef(currTokenIdx).ReadOnlySpan)) + if (!parseState.TryGetFailoverOption(currTokenIdx, out var failoverOption) || + failoverOption == FailoverOption.DEFAULT || failoverOption == FailoverOption.INVALID) { while (!RespWriteUtils.WriteError(CmdStrings.RESP_SYNTAX_ERROR, ref dcurr, dend)) SendAndReset(); diff --git a/libs/cluster/Session/RespClusterFailoverCommands.cs b/libs/cluster/Session/RespClusterFailoverCommands.cs index 42f0616409..c4d2b32aef 100644 --- a/libs/cluster/Session/RespClusterFailoverCommands.cs +++ b/libs/cluster/Session/RespClusterFailoverCommands.cs @@ -30,7 +30,8 @@ private bool NetworkClusterFailover(out bool invalidParameters) if (parseState.Count > 0) { // Try to parse failover option - if (!parseState.TryGetEnum(0, ignoreCase: true, out failoverOption) || !failoverOption.IsValid(parseState.GetArgSliceByRef(0).Span)) + if (!parseState.TryGetFailoverOption(0, out failoverOption) || + failoverOption == FailoverOption.DEFAULT || failoverOption == FailoverOption.INVALID) { var failoverOptionStr = parseState.GetString(0); diff --git a/libs/cluster/Session/RespClusterSlotManagementCommands.cs b/libs/cluster/Session/RespClusterSlotManagementCommands.cs index e136337a3b..137ce3e48c 100644 --- a/libs/cluster/Session/RespClusterSlotManagementCommands.cs +++ b/libs/cluster/Session/RespClusterSlotManagementCommands.cs @@ -439,7 +439,8 @@ private bool NetworkClusterSetSlot(out bool invalidParameters) return true; } - if (!parseState.TryGetEnum(1, ignoreCase: true, out var slotState) || !slotState.IsValid(parseState.GetArgSliceByRef(1).ReadOnlySpan)) + if (!parseState.TryGetSlotState(1, out var slotState) || slotState == SlotState.INVALID || + slotState == SlotState.OFFLINE) { var slotStateStr = parseState.GetString(1); while (!RespWriteUtils.WriteError($"ERR Slot state {slotStateStr} not supported.", ref dcurr, dend)) @@ -531,13 +532,11 @@ private bool NetworkClusterSetSlotsRange(out bool invalidParameters) // CLUSTER SETSLOTRANGE STABLE [slot-start slot-end] string nodeId = default; - // Extract subcommand - var subcommand = parseState.GetString(0); - // Try parse slot state - if (!Enum.TryParse(subcommand, ignoreCase: true, out SlotState slotState)) + if (!parseState.TryGetSlotState(0, out var slotState)) { // Log error for invalid slot state option + var subcommand = parseState.GetString(0); logger?.LogError("The given input '{input}' is not a valid slot state option.", subcommand); slotState = SlotState.INVALID; } @@ -583,6 +582,7 @@ private bool NetworkClusterSetSlotsRange(out bool invalidParameters) break; default: setSlotsSucceeded = false; + var subcommand = parseState.GetString(0); errorMessage = Encoding.ASCII.GetBytes($"ERR Slot state {subcommand} not supported."); break; } diff --git a/libs/cluster/SessionParseStateExtensions.cs b/libs/cluster/SessionParseStateExtensions.cs new file mode 100644 index 0000000000..a836571d90 --- /dev/null +++ b/libs/cluster/SessionParseStateExtensions.cs @@ -0,0 +1,76 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +using Garnet.common; +using Garnet.server; + +namespace Garnet.cluster +{ + /// + /// Extension methods for . + /// + public static class SessionParseStateExtensions + { + /// + /// Parse slot state from parse state at specified index + /// + /// The parse state + /// The argument index + /// Parsed value + /// True if value parsed successfully + public static bool TryGetSlotState(this SessionParseState parseState, int idx, out SlotState value) + { + value = default; + var sbArg = parseState.GetArgSliceByRef(idx).ReadOnlySpan; + + if (sbArg.EqualsUpperCaseSpanIgnoringCase("OFFLINE"u8)) + value = SlotState.OFFLINE; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("STABLE"u8)) + value = SlotState.STABLE; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("MIGRATING"u8)) + value = SlotState.MIGRATING; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("IMPORTING"u8)) + value = SlotState.IMPORTING; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("FAIL"u8)) + value = SlotState.FAIL; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("NODE"u8)) + value = SlotState.NODE; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("INVALID"u8)) + value = SlotState.INVALID; + else return false; + + return true; + } + + /// + /// Parse failover option from parse state at specified index + /// + /// The parse state + /// The argument index + /// Parsed value + /// True if value parsed successfully + public static bool TryGetFailoverOption(this SessionParseState parseState, int idx, out FailoverOption value) + { + value = default; + var sbArg = parseState.GetArgSliceByRef(idx).ReadOnlySpan; + + if (sbArg.EqualsUpperCaseSpanIgnoringCase("DEFAULT"u8)) + value = FailoverOption.DEFAULT; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("INVALID"u8)) + value = FailoverOption.INVALID; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("TO"u8)) + value = FailoverOption.TO; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("FORCE"u8)) + value = FailoverOption.FORCE; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("ABORT"u8)) + value = FailoverOption.ABORT; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("TIMEOUT"u8)) + value = FailoverOption.TIMEOUT; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("TAKEOVER"u8)) + value = FailoverOption.TAKEOVER; + else return false; + + return true; + } + } +} \ No newline at end of file diff --git a/libs/common/FailoverOption.cs b/libs/common/FailoverOption.cs index f7f5f8b849..d4421d63e8 100644 --- a/libs/common/FailoverOption.cs +++ b/libs/common/FailoverOption.cs @@ -12,6 +12,8 @@ namespace Garnet.common /// public enum FailoverOption : byte { + // IMPORTANT: Any changes to the values of this enum should be reflected in its parser (SessionParseStateExtensions.TryGetFailoverOption) + /// /// Internal use only /// @@ -59,21 +61,4 @@ public static class FailoverUtils public static byte[] GetRespFormattedFailoverOption(FailoverOption failoverOption) => infoSections[(int)failoverOption]; } - - /// - /// Extension methods for . - /// - public static class FailoverOptionExtensions - { - /// - /// Validate that the given is legal, and _could_ have come from the given . - /// - /// TODO: Long term we can kill this and use instead of - /// and avoid extra validation. See: https://github.com/dotnet/runtime/issues/81500 . - /// - public static bool IsValid(this FailoverOption type, ReadOnlySpan fromSpan) - { - return type != FailoverOption.DEFAULT && type != FailoverOption.INVALID && Enum.IsDefined(type) && !fromSpan.ContainsAnyInRange((byte)'0', (byte)'9'); - } - } } \ No newline at end of file diff --git a/libs/common/Metrics/InfoMetricsType.cs b/libs/common/Metrics/InfoMetricsType.cs index 70b7f2424f..89c3e58183 100644 --- a/libs/common/Metrics/InfoMetricsType.cs +++ b/libs/common/Metrics/InfoMetricsType.cs @@ -12,6 +12,8 @@ namespace Garnet.common /// public enum InfoMetricsType : byte { + // IMPORTANT: Any changes to the values of this enum should be reflected in its parser (SessionParseStateExtensions.TryGetInfoMetricsType) + /// /// Server info /// diff --git a/libs/common/Metrics/LatencyMetricsType.cs b/libs/common/Metrics/LatencyMetricsType.cs index a74ce6ac8c..107e3d8bf3 100644 --- a/libs/common/Metrics/LatencyMetricsType.cs +++ b/libs/common/Metrics/LatencyMetricsType.cs @@ -8,6 +8,8 @@ namespace Garnet.common /// public enum LatencyMetricsType : byte { + // IMPORTANT: Any changes to the values of this enum should be reflected in its parser (SessionParseStateExtensions.TryGetLatencyMetricsType) + /// /// Latency of processing, per network receive call (server side) - consider batches with only non-admin requests /// Measured from when we start processing the first request in packet, until when we finishing processing the last request in packet (including sending responses). diff --git a/libs/server/ClientType.cs b/libs/server/ClientType.cs index 0c0361e1f7..c7323e8096 100644 --- a/libs/server/ClientType.cs +++ b/libs/server/ClientType.cs @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -using System; - namespace Garnet.server { /// @@ -10,6 +8,8 @@ namespace Garnet.server /// public enum ClientType : byte { + // IMPORTANT: Any changes to the values of this enum should be reflected in its parser (SessionParseStateExtensions.TryGetClientType) + /// /// Default invalid case. /// @@ -39,18 +39,4 @@ public enum ClientType : byte /// SLAVE, } - - public static class ClientTypeExtensions - { - /// - /// Validate that the given is legal, and _could_ have come from the given . - /// - /// TODO: Long term we can kill this and use instead of - /// and avoid extra validation. See: https://github.com/dotnet/runtime/issues/81500 . - /// - public static bool IsValid(this ClientType type, ref ArgSlice fromSlice) - { - return type != ClientType.Invalid && Enum.IsDefined(type) && !fromSlice.ReadOnlySpan.ContainsAnyInRange((byte)'0', (byte)'9'); - } - } } \ No newline at end of file diff --git a/libs/server/Metrics/Info/InfoCommand.cs b/libs/server/Metrics/Info/InfoCommand.cs index 06aab51d34..a90f3874ea 100644 --- a/libs/server/Metrics/Info/InfoCommand.cs +++ b/libs/server/Metrics/Info/InfoCommand.cs @@ -20,31 +20,25 @@ private bool NetworkINFO() if (count > 0) { sections = new HashSet(); - for (int i = 0; i < count; i++) + for (var i = 0; i < count; i++) { - var section = parseState.GetString(i).ToUpper(); + var sbSection = parseState.GetArgSliceByRef(i).ReadOnlySpan; - switch (section) + if (sbSection.EqualsUpperCaseSpanIgnoringCase("RESET"u8)) + reset = true; + else if (sbSection.EqualsUpperCaseSpanIgnoringCase("HELP"u8)) + help = true; + else if (!sbSection.EqualsUpperCaseSpanIgnoringCase("ALL"u8)) { - case InfoHelp.RESET: - reset = true; - break; - case InfoHelp.HELP: - help = true; - break; - case InfoHelp.ALL: - break; - default: - if (Enum.TryParse(section, out InfoMetricsType sectionType)) - { - sections.Add(sectionType); - } - else - { - invalid = true; - invalidSection = section; - } - break; + if (parseState.TryGetInfoMetricsType(i, out var sectionType)) + { + sections.Add(sectionType); + } + else + { + invalid = true; + invalidSection = parseState.GetString(i); + } } } } diff --git a/libs/server/Metrics/Latency/RespLatencyCommands.cs b/libs/server/Metrics/Latency/RespLatencyCommands.cs index c7954aecdd..9c64f7a544 100644 --- a/libs/server/Metrics/Latency/RespLatencyCommands.cs +++ b/libs/server/Metrics/Latency/RespLatencyCommands.cs @@ -50,16 +50,14 @@ private bool NetworkLatencyHistogram() events = new(); for (int i = 0; i < parseState.Count; i++) { - var eventStr = parseState.GetString(i); - - if (Enum.TryParse(eventStr, ignoreCase: true, out LatencyMetricsType eventType)) + if (parseState.TryGetLatencyMetricsType(i, out var eventType)) { events.Add(eventType); } else { invalid = true; - invalidEvent = eventStr; + invalidEvent = parseState.GetString(i); } } } @@ -98,16 +96,14 @@ private bool NetworkLatencyReset() events = new(); for (int i = 0; i < parseState.Count; i++) { - var eventStr = parseState.GetString(i); - - if (Enum.TryParse(eventStr, ignoreCase: true, out LatencyMetricsType eventType)) + if (parseState.TryGetLatencyMetricsType(i, out var eventType)) { events.Add(eventType); } else { invalid = true; - invalidEvent = eventStr; + invalidEvent = parseState.GetString(i); } } } diff --git a/libs/server/Objects/SortedSet/SortedSetObjectImpl.cs b/libs/server/Objects/SortedSet/SortedSetObjectImpl.cs index 7c92e3af3c..522e4b027b 100644 --- a/libs/server/Objects/SortedSet/SortedSetObjectImpl.cs +++ b/libs/server/Objects/SortedSet/SortedSetObjectImpl.cs @@ -30,49 +30,13 @@ private struct ZRangeOptions public bool WithScores { get; set; } }; - bool TryGetSortedSetAddOption(ReadOnlySpan item, out SortedSetAddOption options) - { - if (item.EqualsUpperCaseSpanIgnoringCase("XX"u8)) - { - options = SortedSetAddOption.XX; - return true; - } - if (item.EqualsUpperCaseSpanIgnoringCase("NX"u8)) - { - options = SortedSetAddOption.NX; - return true; - } - if (item.EqualsUpperCaseSpanIgnoringCase("LT"u8)) - { - options = SortedSetAddOption.LT; - return true; - } - if (item.EqualsUpperCaseSpanIgnoringCase("GT"u8)) - { - options = SortedSetAddOption.GT; - return true; - } - if (item.EqualsUpperCaseSpanIgnoringCase("CH"u8)) - { - options = SortedSetAddOption.CH; - return true; - } - if (item.EqualsUpperCaseSpanIgnoringCase("INCR"u8)) - { - options = SortedSetAddOption.INCR; - return true; - } - options = SortedSetAddOption.None; - return false; - } - bool GetOptions(ref ObjectInput input, ref int currTokenIdx, out SortedSetAddOption options, ref byte* curr, byte* end, ref SpanByteAndMemory output, ref bool isMemory, ref byte* ptr, ref MemoryHandle ptrHandle) { options = SortedSetAddOption.None; while (currTokenIdx < input.parseState.Count) { - if (!TryGetSortedSetAddOption(input.parseState.GetArgSliceByRef(currTokenIdx).ReadOnlySpan, out var currOption)) + if (!input.parseState.TryGetSortedSetAddOption(currTokenIdx, out var currOption)) break; options |= currOption; diff --git a/libs/server/Resp/Bitmap/BitmapCommands.cs b/libs/server/Resp/Bitmap/BitmapCommands.cs index 787b168354..17d3f7d3a6 100644 --- a/libs/server/Resp/Bitmap/BitmapCommands.cs +++ b/libs/server/Resp/Bitmap/BitmapCommands.cs @@ -44,6 +44,8 @@ public enum BitmapOperation : byte internal enum BitFieldOverflow : byte { + // IMPORTANT: Any changes to the values of this enum should be reflected in its parser (SessionParseStateExtensions.TryGetBitFieldOverflow) + WRAP, SAT, FAIL @@ -389,7 +391,7 @@ private bool StringBitField(ref TGarnetApi storageApi, bool readOnly isOverflowTypeSet = true; // Validate overflow type - if (!parseState.TryGetEnum(currTokenIdx, true, out BitFieldOverflow _)) + if (!parseState.TryGetBitFieldOverflow(currTokenIdx, out _)) { while (!RespWriteUtils.WriteError( $"ERR Overflow type {parseState.GetString(currTokenIdx)} not supported", diff --git a/libs/server/Resp/ClientCommands.cs b/libs/server/Resp/ClientCommands.cs index 2917264467..36636073c4 100644 --- a/libs/server/Resp/ClientCommands.cs +++ b/libs/server/Resp/ClientCommands.cs @@ -48,12 +48,8 @@ private bool NetworkCLIENTLIST() return AbortWithErrorMessage(CmdStrings.RESP_SYNTAX_ERROR); } - var invalidType = - !parseState.TryGetEnum(1, true, out var clientType) || - (clientType == ClientType.SLAVE) || // SLAVE is not legal as CLIENT|LIST was introduced after the SLAVE -> REPLICA rename - !clientType.IsValid(ref parseState.GetArgSliceByRef(1)); - - if (invalidType) + if (!parseState.TryGetClientType(1, out var clientType) || + clientType == ClientType.SLAVE) // SLAVE is not legal as CLIENT|LIST was introduced after the SLAVE -> REPLICA rename { var type = parseState.GetString(1); return AbortWithErrorMessage(Encoding.UTF8.GetBytes(string.Format(CmdStrings.GenericUnknownClientType, type))); @@ -267,7 +263,8 @@ private bool NetworkCLIENTKILL() ref var filter = ref parseState.GetArgSliceByRef(argIx); var filterSpan = filter.Span; - ref var value = ref parseState.GetArgSliceByRef(argIx + 1); + var valueIx = argIx + 1; + ref var value = ref parseState.GetArgSliceByRef(valueIx); AsciiUtils.ToUpperInPlace(filterSpan); @@ -292,11 +289,7 @@ private bool NetworkCLIENTKILL() return AbortWithErrorMessage(Encoding.ASCII.GetBytes(string.Format(CmdStrings.GenericErrDuplicateFilter, "TYPE"))); } - var unknownType = - !Enum.TryParse(ParseUtils.ReadString(ref value), true, out var typeParsed) || - !typeParsed.IsValid(ref value); - - if (unknownType) + if (!parseState.TryGetClientType(valueIx, out var typeParsed)) { var typeStr = ParseUtils.ReadString(ref value); return AbortWithErrorMessage(Encoding.UTF8.GetBytes(string.Format(CmdStrings.GenericUnknownClientType, typeStr))); diff --git a/libs/server/Resp/KeyAdminCommands.cs b/libs/server/Resp/KeyAdminCommands.cs index 9e7f316f46..8cf1434517 100644 --- a/libs/server/Resp/KeyAdminCommands.cs +++ b/libs/server/Resp/KeyAdminCommands.cs @@ -137,32 +137,6 @@ private bool NetworkEXISTS(ref TGarnetApi storageApi) return true; } - bool TryGetExpireOption(ReadOnlySpan item, out ExpireOption option) - { - if (item.EqualsUpperCaseSpanIgnoringCase("NX"u8)) - { - option = ExpireOption.NX; - return true; - } - if (item.EqualsUpperCaseSpanIgnoringCase("XX"u8)) - { - option = ExpireOption.XX; - return true; - } - if (item.EqualsUpperCaseSpanIgnoringCase("GT"u8)) - { - option = ExpireOption.GT; - return true; - } - if (item.EqualsUpperCaseSpanIgnoringCase("LT"u8)) - { - option = ExpireOption.LT; - return true; - } - option = ExpireOption.None; - return false; - } - /// /// Set a timeout on a key. /// @@ -190,7 +164,7 @@ private bool NetworkEXPIRE(RespCommand command, ref TGarnetApi stora var expireOption = ExpireOption.None; if (parseState.Count > 2) { - if (!TryGetExpireOption(parseState.GetArgSliceByRef(2).ReadOnlySpan, out expireOption)) + if (!parseState.TryGetExpireOption(2, out expireOption)) { var optionStr = parseState.GetString(2); @@ -201,7 +175,7 @@ private bool NetworkEXPIRE(RespCommand command, ref TGarnetApi stora if (parseState.Count > 3) { - if (!TryGetExpireOption(parseState.GetArgSliceByRef(3).ReadOnlySpan, out var additionExpireOption)) + if (!parseState.TryGetExpireOption(3, out var additionExpireOption)) { var optionStr = parseState.GetString(3); @@ -278,7 +252,7 @@ private bool NetworkEXPIREAT(RespCommand command, ref TGarnetApi sto if (parseState.Count > 2) { - if (!TryGetExpireOption(parseState.GetArgSliceByRef(2).ReadOnlySpan, out expireOption)) + if (!parseState.TryGetExpireOption(2, out expireOption)) { var optionStr = parseState.GetString(2); @@ -290,7 +264,7 @@ private bool NetworkEXPIREAT(RespCommand command, ref TGarnetApi sto if (parseState.Count > 3) { - if (!TryGetExpireOption(parseState.GetArgSliceByRef(3).ReadOnlySpan, out var additionExpireOption)) + if (!parseState.TryGetExpireOption(3, out var additionExpireOption)) { var optionStr = parseState.GetString(3); diff --git a/libs/server/Resp/Parser/SessionParseState.cs b/libs/server/Resp/Parser/SessionParseState.cs index f19f09a293..3211d75179 100644 --- a/libs/server/Resp/Parser/SessionParseState.cs +++ b/libs/server/Resp/Parser/SessionParseState.cs @@ -443,42 +443,6 @@ public string GetString(int i) return ParseUtils.ReadString(ref Unsafe.AsRef(bufferPtr + i)); } - /// - /// Get enum argument at the given index - /// Note: this method exists for compatibility with existing code. - /// For best performance use: ReadOnlySpan.EqualsUpperCaseSpanIgnoringCase(""VALUE""u8) to figure out the current enum value - /// - /// True if enum parsed successfully - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public T GetEnum(int i, bool ignoreCase) where T : struct, Enum - { - Debug.Assert(i < Count); - var strRep = GetString(i); - var value = Enum.Parse(strRep, ignoreCase); - // Extra check is to avoid numerical values being successfully parsed as enum value - return string.Equals(Enum.GetName(value), strRep, - ignoreCase ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal) ? value : default; - } - - /// - /// Try to get enum argument at the given index - /// Note: this method exists for compatibility with existing code. - /// For best performance use: ReadOnlySpan.EqualsUpperCaseSpanIgnoringCase(""VALUE""u8) to figure out the current enum value - /// - /// True if integer parsed successfully - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public bool TryGetEnum(int i, bool ignoreCase, out T value) where T : struct, Enum - { - Debug.Assert(i < Count); - var strRep = GetString(i); - var successful = Enum.TryParse(strRep, ignoreCase, out value) && - // Extra check is to avoid numerical values being successfully parsed as enum value - string.Equals(Enum.GetName(value), strRep, - ignoreCase ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal); - if (!successful) value = default; - return successful; - } - /// /// Get boolean argument at the given index /// diff --git a/libs/server/Resp/PurgeBPCommand.cs b/libs/server/Resp/PurgeBPCommand.cs index 8265e87539..9197263a63 100644 --- a/libs/server/Resp/PurgeBPCommand.cs +++ b/libs/server/Resp/PurgeBPCommand.cs @@ -9,6 +9,8 @@ namespace Garnet.server { public enum ManagerType : byte { + // IMPORTANT: Any changes to the values of this enum should be reflected in its parser (SessionParseStateExtensions.TryGetManagerType) + /// /// MigrationManager Buffer Pool /// @@ -50,7 +52,7 @@ private bool NetworkPurgeBP() return AbortWithWrongNumberOfArguments(nameof(RespCommand.PURGEBP)); } - if (!parseState.TryGetEnum(0, ignoreCase: true, out var managerType) || !Enum.IsDefined(managerType)) + if (!parseState.TryGetManagerType(0, out var managerType)) { while (!RespWriteUtils.WriteError(CmdStrings.RESP_SYNTAX_ERROR, ref dcurr, dend)) SendAndReset(); diff --git a/libs/server/SessionParseStateExtensions.cs b/libs/server/SessionParseStateExtensions.cs new file mode 100644 index 0000000000..6a3ac203f1 --- /dev/null +++ b/libs/server/SessionParseStateExtensions.cs @@ -0,0 +1,217 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +using Garnet.common; + +namespace Garnet.server +{ + /// + /// Extension methods for . + /// + public static class SessionParseStateExtensions + { + /// + /// Parse info metrics type from parse state at specified index + /// + /// The parse state + /// The argument index + /// Parsed value + /// True if value parsed successfully + public static bool TryGetInfoMetricsType(this SessionParseState parseState, int idx, out InfoMetricsType value) + { + value = default; + var sbArg = parseState.GetArgSliceByRef(idx).ReadOnlySpan; + + if (sbArg.EqualsUpperCaseSpanIgnoringCase("SERVER"u8)) + value = InfoMetricsType.SERVER; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("MEMORY"u8)) + value = InfoMetricsType.MEMORY; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("CLUSTER"u8)) + value = InfoMetricsType.CLUSTER; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("REPLICATION"u8)) + value = InfoMetricsType.REPLICATION; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("STATS"u8)) + value = InfoMetricsType.STATS; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("STORE"u8)) + value = InfoMetricsType.STORE; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("OBJECTSTORE"u8)) + value = InfoMetricsType.OBJECTSTORE; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("STOREHASHTABLE"u8)) + value = InfoMetricsType.STOREHASHTABLE; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("OBJECTSTOREHASHTABLE"u8)) + value = InfoMetricsType.OBJECTSTOREHASHTABLE; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("STOREREVIV"u8)) + value = InfoMetricsType.STOREREVIV; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("OBJECTSTOREREVIV"u8)) + value = InfoMetricsType.OBJECTSTOREREVIV; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("PERSISTENCE"u8)) + value = InfoMetricsType.PERSISTENCE; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("CLIENTS"u8)) + value = InfoMetricsType.CLIENTS; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("KEYSPACE"u8)) + value = InfoMetricsType.KEYSPACE; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("MODULES"u8)) + value = InfoMetricsType.MODULES; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("BPSTATS"u8)) + value = InfoMetricsType.BPSTATS; + else return false; + + return true; + } + + /// + /// Parse latency metrics type from parse state at specified index + /// + /// The parse state + /// The argument index + /// Parsed value + /// True if value parsed successfully + public static bool TryGetLatencyMetricsType(this SessionParseState parseState, int idx, out LatencyMetricsType value) + { + value = default; + var sbArg = parseState.GetArgSliceByRef(idx).ReadOnlySpan; + + if (sbArg.EqualsUpperCaseSpanIgnoringCase("NET_RS_LAT"u8)) + value = LatencyMetricsType.NET_RS_LAT; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("PENDING_LAT"u8)) + value = LatencyMetricsType.PENDING_LAT; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("TX_PROC_LAT"u8)) + value = LatencyMetricsType.TX_PROC_LAT; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("NET_RS_BYTES"u8)) + value = LatencyMetricsType.NET_RS_BYTES; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("NET_RS_OPS"u8)) + value = LatencyMetricsType.NET_RS_OPS; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("NET_RS_LAT_ADMIN"u8)) + value = LatencyMetricsType.NET_RS_LAT_ADMIN; + else return false; + + return true; + } + + /// + /// Parse client type from parse state at specified index + /// + /// The parse state + /// The argument index + /// Parsed value + /// True if value parsed successfully + public static bool TryGetClientType(this SessionParseState parseState, int idx, out ClientType value) + { + value = ClientType.Invalid; + var sbArg = parseState.GetArgSliceByRef(idx).ReadOnlySpan; + + if (sbArg.EqualsUpperCaseSpanIgnoringCase("NORMAL"u8)) + value = ClientType.NORMAL; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("MASTER"u8)) + value = ClientType.MASTER; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("REPLICA"u8)) + value = ClientType.REPLICA; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("PUBSUB"u8)) + value = ClientType.PUBSUB; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("SLAVE"u8)) + value = ClientType.SLAVE; + + return value != ClientType.Invalid; + } + + /// + /// Parse bit field overflow from parse state at specified index + /// + /// The parse state + /// The argument index + /// Parsed value + /// True if value parsed successfully + internal static bool TryGetBitFieldOverflow(this SessionParseState parseState, int idx, out BitFieldOverflow value) + { + value = default; + var sbArg = parseState.GetArgSliceByRef(idx).ReadOnlySpan; + + if (sbArg.EqualsUpperCaseSpanIgnoringCase("WRAP"u8)) + value = BitFieldOverflow.WRAP; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("SAT"u8)) + value = BitFieldOverflow.SAT; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("FAIL"u8)) + value = BitFieldOverflow.FAIL; + else return false; + + return true; + } + + /// + /// Parse manager type from parse state at specified index + /// + /// The parse state + /// The argument index + /// Parsed value + /// True if value parsed successfully + internal static bool TryGetManagerType(this SessionParseState parseState, int idx, out ManagerType value) + { + value = default; + var sbArg = parseState.GetArgSliceByRef(idx).ReadOnlySpan; + + if (sbArg.EqualsUpperCaseSpanIgnoringCase("MIGRATIONMANAGER"u8)) + value = ManagerType.MigrationManager; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("REPLICATIONMANAGER"u8)) + value = ManagerType.ReplicationManager; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("SERVERLISTENER"u8)) + value = ManagerType.ServerListener; + else return false; + + return true; + } + + /// + /// Parse sorted set add option from parse state at specified index + /// + /// The parse state + /// The argument index + /// Parsed value + /// True if value parsed successfully + internal static bool TryGetSortedSetAddOption(this SessionParseState parseState, int idx, out SortedSetAddOption value) + { + value = SortedSetAddOption.None; + var sbArg = parseState.GetArgSliceByRef(idx).ReadOnlySpan; + + if (sbArg.EqualsUpperCaseSpanIgnoringCase("XX"u8)) + value = SortedSetAddOption.XX; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("NX"u8)) + value = SortedSetAddOption.NX; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("LT"u8)) + value = SortedSetAddOption.LT; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("GT"u8)) + value = SortedSetAddOption.GT; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("CH"u8)) + value = SortedSetAddOption.CH; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("INCR"u8)) + value = SortedSetAddOption.INCR; + else return false; + + return true; + } + + /// + /// Parse expire option from parse state at specified index + /// + /// The parse state + /// The argument index + /// Parsed value + /// True if value parsed successfully + internal static bool TryGetExpireOption(this SessionParseState parseState, int idx, out ExpireOption value) + { + value = ExpireOption.None; + var sbArg = parseState.GetArgSliceByRef(idx).ReadOnlySpan; + + if (sbArg.EqualsUpperCaseSpanIgnoringCase("NX"u8)) + value = ExpireOption.NX; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("XX"u8)) + value = ExpireOption.XX; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("GT"u8)) + value = ExpireOption.GT; + else if (sbArg.EqualsUpperCaseSpanIgnoringCase("LT"u8)) + value = ExpireOption.LT; + else return false; + + return true; + } + } +} \ No newline at end of file diff --git a/libs/server/Storage/Functions/MainStore/PrivateMethods.cs b/libs/server/Storage/Functions/MainStore/PrivateMethods.cs index 3feaa63b2a..7b4ac193eb 100644 --- a/libs/server/Storage/Functions/MainStore/PrivateMethods.cs +++ b/libs/server/Storage/Functions/MainStore/PrivateMethods.cs @@ -711,7 +711,17 @@ void WriteLogDelete(ref SpanByte key, long version, int sessionID) BitFieldCmdArgs GetBitFieldArguments(ref RawStringInput input) { var currTokenIdx = 0; - var cmd = input.parseState.GetEnum(currTokenIdx++, true); + + // Get secondary command. Legal commands: GET, SET & INCRBY. + var cmd = RespCommand.NONE; + var sbCmd = input.parseState.GetArgSliceByRef(currTokenIdx++).ReadOnlySpan; + if (sbCmd.EqualsUpperCaseSpanIgnoringCase("GET"u8)) + cmd = RespCommand.GET; + else if (sbCmd.EqualsUpperCaseSpanIgnoringCase("SET"u8)) + cmd = RespCommand.SET; + else if (sbCmd.EqualsUpperCaseSpanIgnoringCase("INCRBY"u8)) + cmd = RespCommand.INCRBY; + var encodingArg = input.parseState.GetString(currTokenIdx++); var offsetArg = input.parseState.GetString(currTokenIdx++); @@ -724,7 +734,9 @@ BitFieldCmdArgs GetBitFieldArguments(ref RawStringInput input) var overflowType = (byte)BitFieldOverflow.WRAP; if (currTokenIdx < input.parseState.Count) { - overflowType = (byte)input.parseState.GetEnum(currTokenIdx, true); + var overflowTypeParsed = input.parseState.TryGetBitFieldOverflow(currTokenIdx, out var overflowTypeValue); + Debug.Assert(overflowTypeParsed); + overflowType = (byte)overflowTypeValue; } var sign = encodingArg[0] == 'i' ? (byte)BitFieldSign.SIGNED : (byte)BitFieldSign.UNSIGNED; diff --git a/libs/server/Storage/Session/MainStore/BitmapOps.cs b/libs/server/Storage/Session/MainStore/BitmapOps.cs index 458185191b..009e4d9685 100644 --- a/libs/server/Storage/Session/MainStore/BitmapOps.cs +++ b/libs/server/Storage/Session/MainStore/BitmapOps.cs @@ -279,13 +279,15 @@ public unsafe GarnetStatus StringBitField(ArgSlice key, List 0 ? "i"u8 : "u"u8; var encodingSuffix = commandArguments[i].typeInfo & 0x7F; var encodingSuffixLength = NumUtils.NumDigits(encodingSuffix); var offsetLength = NumUtils.NumDigitsInLong(commandArguments[i].offset); - var valueLength = NumUtils.NumDigitsInLong(commandArguments[i].value); + var valueLength = isGet ? 0 : NumUtils.NumDigitsInLong(commandArguments[i].value); var overflowType = ((BitFieldOverflow)commandArguments[i].overflowType).ToString(); // Calculate # of bytes to store parameters @@ -323,10 +325,14 @@ public unsafe GarnetStatus StringBitField(ArgSlice key, List(ArgSlice key, List