Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switching Enum.HasFlag calls with bitwise operations #953

Merged
merged 4 commits into from
Jan 25, 2025
Merged

Switching Enum.HasFlag calls with bitwise operations #953

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b70c5e8
Removing inefficient calls to Enum.HasFlag
TalZaccai Jan 24, 2025
5dc7a41
format
TalZaccai Jan 24, 2025
4a1925a
renaming props in GOSO
TalZaccai Jan 25, 2025
9515034
Merge branch 'main' into talzacc/enum_hasflag
TalZaccai Jan 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions libs/server/Objects/Hash/HashObject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down
28 changes: 16 additions & 12 deletions libs/server/Objects/SortedSet/SortedSetObjectImpl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,21 @@ bool GetOptions(ref ObjectInput input, ref int currTokenIdx, out SortedSetAddOpt
ReadOnlySpan<byte> 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)
Expand Down Expand Up @@ -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)))
Expand All @@ -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;
Expand All @@ -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));
Expand All @@ -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);
Expand Down
10 changes: 10 additions & 0 deletions libs/server/Objects/Types/GarnetObjectStoreOutput.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,16 @@ public struct GarnetObjectStoreOutput
/// </summary>
public ObjectStoreOutputFlags OutputFlags;

/// <summary>
/// True if output flag WrongType is set
/// </summary>
public bool HasWrongType => (OutputFlags & ObjectStoreOutputFlags.WrongType) == ObjectStoreOutputFlags.WrongType;

/// <summary>
/// True if output flag RemoveKey is set
/// </summary>
public bool HasRemoveKey => (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.
Expand Down
2 changes: 1 addition & 1 deletion libs/server/Resp/RespCommandsInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 4 additions & 4 deletions libs/server/Storage/Functions/ObjectStore/RMWMethods.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.HasWrongType)
return true;

if (output.OutputFlags.HasFlag(ObjectStoreOutputFlags.RemoveKey))
if (output.HasRemoveKey)
{
rmwInfo.Action = RMWAction.ExpireAndStop;
return false;
Expand Down Expand Up @@ -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.HasWrongType)
return true;

if (output.OutputFlags.HasFlag(ObjectStoreOutputFlags.RemoveKey))
if (output.HasRemoveKey)
{
rmwInfo.Action = RMWAction.ExpireAndStop;
return false;
Expand Down
2 changes: 1 addition & 1 deletion libs/server/Storage/Functions/ObjectStore/ReadMethods.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.HasWrongType)
return true;

return opResult;
Expand Down
4 changes: 2 additions & 2 deletions libs/server/Storage/Session/ObjectStore/AdvancedOps.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public GarnetStatus RMW_ObjectStore<TObjectContext>(ref byte[] key, ref ObjectIn

if (status.Found)
{
if (output.OutputFlags.HasFlag(ObjectStoreOutputFlags.WrongType))
if (output.HasWrongType)
return GarnetStatus.WRONGTYPE;

return GarnetStatus.OK;
Expand All @@ -40,7 +40,7 @@ public GarnetStatus Read_ObjectStore<TObjectContext>(ref byte[] key, ref ObjectI

if (status.Found)
{
if (output.OutputFlags.HasFlag(ObjectStoreOutputFlags.WrongType))
if (output.HasWrongType)
return GarnetStatus.WRONGTYPE;

return GarnetStatus.OK;
Expand Down
6 changes: 3 additions & 3 deletions libs/server/Storage/Session/ObjectStore/Common.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ unsafe GarnetStatus ReadObjectStoreOperation<TObjectContext>(byte[] key, ArgSlic
if (status.IsPending)
CompletePendingForObjectStoreSession(ref status, ref _output, ref objectStoreContext);

if (_output.OutputFlags.HasFlag(ObjectStoreOutputFlags.WrongType))
if (_output.HasWrongType)
return GarnetStatus.WRONGTYPE;

Debug.Assert(_output.SpanByteAndMemory.IsSpanByte);
Expand Down Expand Up @@ -533,7 +533,7 @@ unsafe GarnetStatus ReadObjectStoreOperation<TObjectContext>(byte[] key, ref Obj
if (status.IsPending)
CompletePendingForObjectStoreSession(ref status, ref _output, ref objectStoreContext);

if (_output.OutputFlags.HasFlag(ObjectStoreOutputFlags.WrongType))
if (_output.HasWrongType)
return GarnetStatus.WRONGTYPE;

Debug.Assert(_output.SpanByteAndMemory.IsSpanByte);
Expand Down Expand Up @@ -579,7 +579,7 @@ private GarnetStatus CompletePendingAndGetGarnetStatus<TObjectContext>(Status st
if (status.NotFound && !status.Record.Created)
return GarnetStatus.NOTFOUND;

if (status.Found && outputFooter.OutputFlags.HasFlag(ObjectStoreOutputFlags.WrongType))
if (status.Found && outputFooter.HasWrongType)
return GarnetStatus.WRONGTYPE;

return GarnetStatus.OK;
Expand Down
Loading