From b70c5e811f69b537b9ad4eed7539c5fe315bd50d Mon Sep 17 00:00:00 2001 From: Tal Zaccai Date: Fri, 24 Jan 2025 15:50:35 -0700 Subject: [PATCH 1/3] Removing inefficient calls to Enum.HasFlag --- libs/server/Objects/Hash/HashObject.cs | 9 +++--- .../Objects/SortedSet/SortedSetObjectImpl.cs | 28 +++++++++++-------- .../Objects/Types/GarnetObjectStoreOutput.cs | 10 +++++++ libs/server/Resp/RespCommandsInfo.cs | 2 +- .../Functions/ObjectStore/RMWMethods.cs | 8 +++--- .../Functions/ObjectStore/ReadMethods.cs | 2 +- .../Session/ObjectStore/AdvancedOps.cs | 4 +-- .../Storage/Session/ObjectStore/Common.cs | 6 ++-- 8 files changed, 42 insertions(+), 27 deletions(-) diff --git a/libs/server/Objects/Hash/HashObject.cs b/libs/server/Objects/Hash/HashObject.cs index 215cb1010c7..cad2de4210f 100644 --- a/libs/server/Objects/Hash/HashObject.cs +++ b/libs/server/Objects/Hash/HashObject.cs @@ -501,9 +501,9 @@ private int SetExpiration(byte[] key, long expiration, ExpireOption expireOption if (expirationTimes.TryGetValue(key, out var currentExpiration)) { - if (expireOption.HasFlag(ExpireOption.NX) || - (expireOption.HasFlag(ExpireOption.GT) && expiration <= currentExpiration) || - (expireOption.HasFlag(ExpireOption.LT) && expiration >= currentExpiration)) + if ((expireOption & ExpireOption.NX) == ExpireOption.NX || + ((expireOption & ExpireOption.GT) == ExpireOption.GT && expiration <= currentExpiration) || + ((expireOption & ExpireOption.LT) == ExpireOption.LT && expiration >= currentExpiration)) { return (int)ExpireResult.ExpireConditionNotMet; } @@ -515,7 +515,8 @@ private int SetExpiration(byte[] key, long expiration, ExpireOption expireOption } else { - if (expireOption.HasFlag(ExpireOption.XX) || expireOption.HasFlag(ExpireOption.GT)) + if ((expireOption & ExpireOption.XX) == ExpireOption.XX || + (expireOption & ExpireOption.GT) == ExpireOption.GT) { return (int)ExpireResult.ExpireConditionNotMet; } diff --git a/libs/server/Objects/SortedSet/SortedSetObjectImpl.cs b/libs/server/Objects/SortedSet/SortedSetObjectImpl.cs index d18d3e99f76..a17db761ff5 100644 --- a/libs/server/Objects/SortedSet/SortedSetObjectImpl.cs +++ b/libs/server/Objects/SortedSet/SortedSetObjectImpl.cs @@ -47,17 +47,21 @@ bool GetOptions(ref ObjectInput input, ref int currTokenIdx, out SortedSetAddOpt ReadOnlySpan optionsError = default; // XX & NX are mutually exclusive - if (options.HasFlag(SortedSetAddOption.XX) && options.HasFlag(SortedSetAddOption.NX)) + if ((options & SortedSetAddOption.XX) == SortedSetAddOption.XX && + (options & SortedSetAddOption.NX) == SortedSetAddOption.NX) optionsError = CmdStrings.RESP_ERR_XX_NX_NOT_COMPATIBLE; // NX, GT & LT are mutually exclusive - if ((options.HasFlag(SortedSetAddOption.GT) && options.HasFlag(SortedSetAddOption.LT)) || - ((options.HasFlag(SortedSetAddOption.GT) || options.HasFlag(SortedSetAddOption.LT)) && - options.HasFlag(SortedSetAddOption.NX))) + if (((options & SortedSetAddOption.GT) == SortedSetAddOption.GT && + (options & SortedSetAddOption.LT) == SortedSetAddOption.LT) || + (((options & SortedSetAddOption.GT) == SortedSetAddOption.GT || + (options & SortedSetAddOption.LT) == SortedSetAddOption.LT) && + (options & SortedSetAddOption.NX) == SortedSetAddOption.NX)) optionsError = CmdStrings.RESP_ERR_GT_LT_NX_NOT_COMPATIBLE; // INCR supports only one score-element pair - if (options.HasFlag(SortedSetAddOption.INCR) && (input.parseState.Count - currTokenIdx > 2)) + if ((options & SortedSetAddOption.INCR) == SortedSetAddOption.INCR && + (input.parseState.Count - currTokenIdx > 2)) optionsError = CmdStrings.RESP_ERR_INCR_SUPPORTS_ONLY_SINGLE_PAIR; if (!optionsError.IsEmpty) @@ -131,7 +135,7 @@ private void SortedSetAdd(ref ObjectInput input, ref SpanByteAndMemory output) if (!sortedSetDict.TryGetValue(member, out var scoreStored)) { // Don't add new member if XX flag is set - if (options.HasFlag(SortedSetAddOption.XX)) continue; + if ((options & SortedSetAddOption.XX) == SortedSetAddOption.XX) continue; sortedSetDict.Add(member, score); if (sortedSet.Add((score, member))) @@ -143,7 +147,7 @@ private void SortedSetAdd(ref ObjectInput input, ref SpanByteAndMemory output) else { // Update new score if INCR flag is set - if (options.HasFlag(SortedSetAddOption.INCR)) + if ((options & SortedSetAddOption.INCR) == SortedSetAddOption.INCR) { score += scoreStored; incrResult = score; @@ -155,9 +159,9 @@ private void SortedSetAdd(ref ObjectInput input, ref SpanByteAndMemory output) // Don't update existing member if NX flag is set // or if GT/LT flag is set and existing score is higher/lower than new score, respectively - if (options.HasFlag(SortedSetAddOption.NX) || - (options.HasFlag(SortedSetAddOption.GT) && scoreStored > score) || - (options.HasFlag(SortedSetAddOption.LT) && scoreStored < score)) continue; + if ((options & SortedSetAddOption.NX) == SortedSetAddOption.NX || + ((options & SortedSetAddOption.GT) == SortedSetAddOption.GT && scoreStored > score) || + ((options & SortedSetAddOption.LT) == SortedSetAddOption.LT && scoreStored < score)) continue; sortedSetDict[member] = score; var success = sortedSet.Remove((scoreStored, member)); @@ -166,12 +170,12 @@ private void SortedSetAdd(ref ObjectInput input, ref SpanByteAndMemory output) Debug.Assert(success); // If CH flag is set, add changed member to final count - if (options.HasFlag(SortedSetAddOption.CH)) + if ((options & SortedSetAddOption.CH) == SortedSetAddOption.CH) addedOrChanged++; } } - if (options.HasFlag(SortedSetAddOption.INCR)) + if ((options & SortedSetAddOption.INCR) == SortedSetAddOption.INCR) { while (!RespWriteUtils.TryWriteDoubleBulkString(incrResult, ref curr, end)) ObjectUtils.ReallocateOutput(ref output, ref isMemory, ref ptr, ref ptrHandle, ref curr, ref end); diff --git a/libs/server/Objects/Types/GarnetObjectStoreOutput.cs b/libs/server/Objects/Types/GarnetObjectStoreOutput.cs index c6dba3be90d..da1af63c8be 100644 --- a/libs/server/Objects/Types/GarnetObjectStoreOutput.cs +++ b/libs/server/Objects/Types/GarnetObjectStoreOutput.cs @@ -50,6 +50,16 @@ public struct GarnetObjectStoreOutput /// public ObjectStoreOutputFlags OutputFlags; + /// + /// True if output flag WrongType is set + /// + public bool IsWrongType => (OutputFlags & ObjectStoreOutputFlags.WrongType) == ObjectStoreOutputFlags.WrongType; + + /// + /// True if output flag RemoveKey is set + /// + public bool RemoveKey => (OutputFlags & ObjectStoreOutputFlags.RemoveKey) == ObjectStoreOutputFlags.RemoveKey; + public void ConvertToHeap() { // Does not convert to heap when going pending, because we immediately complete pending operations for object store. diff --git a/libs/server/Resp/RespCommandsInfo.cs b/libs/server/Resp/RespCommandsInfo.cs index 0624f75d39f..37b5dd5295d 100644 --- a/libs/server/Resp/RespCommandsInfo.cs +++ b/libs/server/Resp/RespCommandsInfo.cs @@ -326,7 +326,7 @@ public static bool TryGetRespCommandInfo(RespCommand cmd, tmpRespCommandInfo = FlattenedRespCommandsInfo[cmd]; if (tmpRespCommandInfo == default || - (txnOnly && tmpRespCommandInfo.Flags.HasFlag(RespCommandFlags.NoMulti))) return false; + (txnOnly && (tmpRespCommandInfo.Flags & RespCommandFlags.NoMulti) == RespCommandFlags.NoMulti)) return false; respCommandsInfo = tmpRespCommandInfo; return true; diff --git a/libs/server/Storage/Functions/ObjectStore/RMWMethods.cs b/libs/server/Storage/Functions/ObjectStore/RMWMethods.cs index d53ade1bfef..88def9dd3b8 100644 --- a/libs/server/Storage/Functions/ObjectStore/RMWMethods.cs +++ b/libs/server/Storage/Functions/ObjectStore/RMWMethods.cs @@ -142,10 +142,10 @@ bool InPlaceUpdaterWorker(ref byte[] key, ref ObjectInput input, ref IGarnetObje if ((byte)input.header.type < CustomCommandManager.CustomTypeIdStartOffset) { var operateSuccessful = value.Operate(ref input, ref output, out sizeChange); - if (output.OutputFlags.HasFlag(ObjectStoreOutputFlags.WrongType)) + if (output.IsWrongType) return true; - if (output.OutputFlags.HasFlag(ObjectStoreOutputFlags.RemoveKey)) + if (output.RemoveKey) { rmwInfo.Action = RMWAction.ExpireAndStop; return false; @@ -239,10 +239,10 @@ public bool PostCopyUpdater(ref byte[] key, ref ObjectInput input, ref IGarnetOb if ((byte)input.header.type < CustomCommandManager.CustomTypeIdStartOffset) { value.Operate(ref input, ref output, out _); - if (output.OutputFlags.HasFlag(ObjectStoreOutputFlags.WrongType)) + if (output.IsWrongType) return true; - if (output.OutputFlags.HasFlag(ObjectStoreOutputFlags.RemoveKey)) + if (output.RemoveKey) { rmwInfo.Action = RMWAction.ExpireAndStop; return false; diff --git a/libs/server/Storage/Functions/ObjectStore/ReadMethods.cs b/libs/server/Storage/Functions/ObjectStore/ReadMethods.cs index 3f10f18ec60..d525630d507 100644 --- a/libs/server/Storage/Functions/ObjectStore/ReadMethods.cs +++ b/libs/server/Storage/Functions/ObjectStore/ReadMethods.cs @@ -49,7 +49,7 @@ public bool SingleReader(ref byte[] key, ref ObjectInput input, ref IGarnetObjec if ((byte)input.header.type < CustomCommandManager.CustomTypeIdStartOffset) { var opResult = value.Operate(ref input, ref dst, out _); - if (dst.OutputFlags.HasFlag(ObjectStoreOutputFlags.WrongType)) + if (dst.IsWrongType) return true; return opResult; diff --git a/libs/server/Storage/Session/ObjectStore/AdvancedOps.cs b/libs/server/Storage/Session/ObjectStore/AdvancedOps.cs index 123334490c0..77720ea9ed6 100644 --- a/libs/server/Storage/Session/ObjectStore/AdvancedOps.cs +++ b/libs/server/Storage/Session/ObjectStore/AdvancedOps.cs @@ -21,7 +21,7 @@ public GarnetStatus RMW_ObjectStore(ref byte[] key, ref ObjectIn if (status.Found) { - if (output.OutputFlags.HasFlag(ObjectStoreOutputFlags.WrongType)) + if (output.IsWrongType) return GarnetStatus.WRONGTYPE; return GarnetStatus.OK; @@ -40,7 +40,7 @@ public GarnetStatus Read_ObjectStore(ref byte[] key, ref ObjectI if (status.Found) { - if (output.OutputFlags.HasFlag(ObjectStoreOutputFlags.WrongType)) + if (output.IsWrongType) return GarnetStatus.WRONGTYPE; return GarnetStatus.OK; diff --git a/libs/server/Storage/Session/ObjectStore/Common.cs b/libs/server/Storage/Session/ObjectStore/Common.cs index 4658d401a49..83a66f2da56 100644 --- a/libs/server/Storage/Session/ObjectStore/Common.cs +++ b/libs/server/Storage/Session/ObjectStore/Common.cs @@ -498,7 +498,7 @@ unsafe GarnetStatus ReadObjectStoreOperation(byte[] key, ArgSlic if (status.IsPending) CompletePendingForObjectStoreSession(ref status, ref _output, ref objectStoreContext); - if (_output.OutputFlags.HasFlag(ObjectStoreOutputFlags.WrongType)) + if (_output.IsWrongType) return GarnetStatus.WRONGTYPE; Debug.Assert(_output.SpanByteAndMemory.IsSpanByte); @@ -533,7 +533,7 @@ unsafe GarnetStatus ReadObjectStoreOperation(byte[] key, ref Obj if (status.IsPending) CompletePendingForObjectStoreSession(ref status, ref _output, ref objectStoreContext); - if (_output.OutputFlags.HasFlag(ObjectStoreOutputFlags.WrongType)) + if (_output.IsWrongType) return GarnetStatus.WRONGTYPE; Debug.Assert(_output.SpanByteAndMemory.IsSpanByte); @@ -579,7 +579,7 @@ private GarnetStatus CompletePendingAndGetGarnetStatus(Status st if (status.NotFound && !status.Record.Created) return GarnetStatus.NOTFOUND; - if (status.Found && outputFooter.OutputFlags.HasFlag(ObjectStoreOutputFlags.WrongType)) + if (status.Found && outputFooter.IsWrongType) return GarnetStatus.WRONGTYPE; return GarnetStatus.OK; From 5dc7a41eb62297857a6e4c21ba2ddb002a8100ab Mon Sep 17 00:00:00 2001 From: Tal Zaccai Date: Fri, 24 Jan 2025 16:05:58 -0700 Subject: [PATCH 2/3] format --- libs/server/Objects/SortedSet/SortedSetObjectImpl.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libs/server/Objects/SortedSet/SortedSetObjectImpl.cs b/libs/server/Objects/SortedSet/SortedSetObjectImpl.cs index a17db761ff5..f22b7c68c72 100644 --- a/libs/server/Objects/SortedSet/SortedSetObjectImpl.cs +++ b/libs/server/Objects/SortedSet/SortedSetObjectImpl.cs @@ -47,20 +47,20 @@ bool GetOptions(ref ObjectInput input, ref int currTokenIdx, out SortedSetAddOpt ReadOnlySpan optionsError = default; // XX & NX are mutually exclusive - if ((options & SortedSetAddOption.XX) == SortedSetAddOption.XX && + if ((options & SortedSetAddOption.XX) == SortedSetAddOption.XX && (options & SortedSetAddOption.NX) == SortedSetAddOption.NX) optionsError = CmdStrings.RESP_ERR_XX_NX_NOT_COMPATIBLE; // NX, GT & LT are mutually exclusive - if (((options & SortedSetAddOption.GT) == SortedSetAddOption.GT && + if (((options & SortedSetAddOption.GT) == SortedSetAddOption.GT && (options & SortedSetAddOption.LT) == SortedSetAddOption.LT) || - (((options & SortedSetAddOption.GT) == SortedSetAddOption.GT || + (((options & SortedSetAddOption.GT) == SortedSetAddOption.GT || (options & SortedSetAddOption.LT) == SortedSetAddOption.LT) && (options & SortedSetAddOption.NX) == SortedSetAddOption.NX)) optionsError = CmdStrings.RESP_ERR_GT_LT_NX_NOT_COMPATIBLE; // INCR supports only one score-element pair - if ((options & SortedSetAddOption.INCR) == SortedSetAddOption.INCR && + if ((options & SortedSetAddOption.INCR) == SortedSetAddOption.INCR && (input.parseState.Count - currTokenIdx > 2)) optionsError = CmdStrings.RESP_ERR_INCR_SUPPORTS_ONLY_SINGLE_PAIR; From 4a1925aab800a0dceb4d376db63a41ed6a0ed72b Mon Sep 17 00:00:00 2001 From: Tal Zaccai Date: Fri, 24 Jan 2025 17:06:33 -0700 Subject: [PATCH 3/3] renaming props in GOSO --- libs/server/Objects/Types/GarnetObjectStoreOutput.cs | 4 ++-- libs/server/Storage/Functions/ObjectStore/RMWMethods.cs | 8 ++++---- libs/server/Storage/Functions/ObjectStore/ReadMethods.cs | 2 +- libs/server/Storage/Session/ObjectStore/AdvancedOps.cs | 4 ++-- libs/server/Storage/Session/ObjectStore/Common.cs | 6 +++--- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/libs/server/Objects/Types/GarnetObjectStoreOutput.cs b/libs/server/Objects/Types/GarnetObjectStoreOutput.cs index da1af63c8be..60b61fd04e3 100644 --- a/libs/server/Objects/Types/GarnetObjectStoreOutput.cs +++ b/libs/server/Objects/Types/GarnetObjectStoreOutput.cs @@ -53,12 +53,12 @@ public struct GarnetObjectStoreOutput /// /// True if output flag WrongType is set /// - public bool IsWrongType => (OutputFlags & ObjectStoreOutputFlags.WrongType) == ObjectStoreOutputFlags.WrongType; + public bool HasWrongType => (OutputFlags & ObjectStoreOutputFlags.WrongType) == ObjectStoreOutputFlags.WrongType; /// /// True if output flag RemoveKey is set /// - public bool RemoveKey => (OutputFlags & ObjectStoreOutputFlags.RemoveKey) == ObjectStoreOutputFlags.RemoveKey; + public bool HasRemoveKey => (OutputFlags & ObjectStoreOutputFlags.RemoveKey) == ObjectStoreOutputFlags.RemoveKey; public void ConvertToHeap() { diff --git a/libs/server/Storage/Functions/ObjectStore/RMWMethods.cs b/libs/server/Storage/Functions/ObjectStore/RMWMethods.cs index 88def9dd3b8..c8d7ce0f212 100644 --- a/libs/server/Storage/Functions/ObjectStore/RMWMethods.cs +++ b/libs/server/Storage/Functions/ObjectStore/RMWMethods.cs @@ -142,10 +142,10 @@ bool InPlaceUpdaterWorker(ref byte[] key, ref ObjectInput input, ref IGarnetObje if ((byte)input.header.type < CustomCommandManager.CustomTypeIdStartOffset) { var operateSuccessful = value.Operate(ref input, ref output, out sizeChange); - if (output.IsWrongType) + if (output.HasWrongType) return true; - if (output.RemoveKey) + if (output.HasRemoveKey) { rmwInfo.Action = RMWAction.ExpireAndStop; return false; @@ -239,10 +239,10 @@ public bool PostCopyUpdater(ref byte[] key, ref ObjectInput input, ref IGarnetOb if ((byte)input.header.type < CustomCommandManager.CustomTypeIdStartOffset) { value.Operate(ref input, ref output, out _); - if (output.IsWrongType) + if (output.HasWrongType) return true; - if (output.RemoveKey) + if (output.HasRemoveKey) { rmwInfo.Action = RMWAction.ExpireAndStop; return false; diff --git a/libs/server/Storage/Functions/ObjectStore/ReadMethods.cs b/libs/server/Storage/Functions/ObjectStore/ReadMethods.cs index d525630d507..35f69bbd4dc 100644 --- a/libs/server/Storage/Functions/ObjectStore/ReadMethods.cs +++ b/libs/server/Storage/Functions/ObjectStore/ReadMethods.cs @@ -49,7 +49,7 @@ public bool SingleReader(ref byte[] key, ref ObjectInput input, ref IGarnetObjec if ((byte)input.header.type < CustomCommandManager.CustomTypeIdStartOffset) { var opResult = value.Operate(ref input, ref dst, out _); - if (dst.IsWrongType) + if (dst.HasWrongType) return true; return opResult; diff --git a/libs/server/Storage/Session/ObjectStore/AdvancedOps.cs b/libs/server/Storage/Session/ObjectStore/AdvancedOps.cs index 77720ea9ed6..2d98a5ae71e 100644 --- a/libs/server/Storage/Session/ObjectStore/AdvancedOps.cs +++ b/libs/server/Storage/Session/ObjectStore/AdvancedOps.cs @@ -21,7 +21,7 @@ public GarnetStatus RMW_ObjectStore(ref byte[] key, ref ObjectIn if (status.Found) { - if (output.IsWrongType) + if (output.HasWrongType) return GarnetStatus.WRONGTYPE; return GarnetStatus.OK; @@ -40,7 +40,7 @@ public GarnetStatus Read_ObjectStore(ref byte[] key, ref ObjectI if (status.Found) { - if (output.IsWrongType) + if (output.HasWrongType) return GarnetStatus.WRONGTYPE; return GarnetStatus.OK; diff --git a/libs/server/Storage/Session/ObjectStore/Common.cs b/libs/server/Storage/Session/ObjectStore/Common.cs index 83a66f2da56..e5df24178a8 100644 --- a/libs/server/Storage/Session/ObjectStore/Common.cs +++ b/libs/server/Storage/Session/ObjectStore/Common.cs @@ -498,7 +498,7 @@ unsafe GarnetStatus ReadObjectStoreOperation(byte[] key, ArgSlic if (status.IsPending) CompletePendingForObjectStoreSession(ref status, ref _output, ref objectStoreContext); - if (_output.IsWrongType) + if (_output.HasWrongType) return GarnetStatus.WRONGTYPE; Debug.Assert(_output.SpanByteAndMemory.IsSpanByte); @@ -533,7 +533,7 @@ unsafe GarnetStatus ReadObjectStoreOperation(byte[] key, ref Obj if (status.IsPending) CompletePendingForObjectStoreSession(ref status, ref _output, ref objectStoreContext); - if (_output.IsWrongType) + if (_output.HasWrongType) return GarnetStatus.WRONGTYPE; Debug.Assert(_output.SpanByteAndMemory.IsSpanByte); @@ -579,7 +579,7 @@ private GarnetStatus CompletePendingAndGetGarnetStatus(Status st if (status.NotFound && !status.Record.Created) return GarnetStatus.NOTFOUND; - if (status.Found && outputFooter.IsWrongType) + if (status.Found && outputFooter.HasWrongType) return GarnetStatus.WRONGTYPE; return GarnetStatus.OK;