-
Notifications
You must be signed in to change notification settings - Fork 550
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.
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
Perf Improvements to Custom Commands' Execution Path #940
Open
TalZaccai
wants to merge
10
commits into
main
Choose a base branch
from
talzacc/perf_mem_impr2
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
98eea93
perf fixes
TalZaccai c1c8fa1
small fix
TalZaccai 4c50166
Comment fixes, adding documentation
TalZaccai faab5c4
format
TalZaccai 34b96eb
wip
TalZaccai 1495b3d
wip
TalZaccai f970b36
merging from latest main
TalZaccai d0c94ee
Merge branch 'main' into talzacc/perf_mem_impr2
TalZaccai 224effd
Adressing comments
TalZaccai 922a964
Merge branch 'main' into talzacc/perf_mem_impr2
TalZaccai File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,100 +1,190 @@ | ||
// Copyright (c) Microsoft Corporation. | ||
// Licensed under the MIT license. | ||
|
||
using System.Diagnostics; | ||
using System; | ||
using System.Collections.Generic; | ||
using Garnet.common; | ||
|
||
namespace Garnet.server | ||
{ | ||
/// <summary> | ||
/// Server session for RESP protocol - basic commands are in this file | ||
/// Session-specific access to custom command data held by CustomCommandManager | ||
/// For both custom procedures and custom transactions, this class maintains a cached instance of the transaction / procedure that can be called directly per-session | ||
/// This class also maintains a cache of custom command info and docs to avoid the cost of synchronization when calling CustomCommandManager | ||
/// </summary> | ||
internal sealed class CustomCommandManagerSession | ||
{ | ||
// Initial size of expandable maps | ||
private static readonly int MinMapSize = 8; | ||
|
||
// The instance of the CustomCommandManager that this session is associated with | ||
readonly CustomCommandManager customCommandManager; | ||
|
||
// These session specific arrays are indexed by the same ID as the arrays in CustomCommandManager | ||
ExpandableMap<CustomTransactionProcedureWithArity> sessionTransactionProcMap; | ||
// These session specific maps are indexed by the same ID as the arrays in CustomCommandManager | ||
// Maps between the transaction ID and a tuple of the per-session transaction procedure and its arity | ||
ExpandableMap<(CustomTransactionProcedure, int)> sessionTransactionProcMap; | ||
// Maps between the custom procedure ID and the per-session custom procedure instance | ||
ExpandableMap<CustomProcedure> sessionCustomProcMap; | ||
|
||
public CustomCommandManagerSession(CustomCommandManager customCommandManager) | ||
{ | ||
this.customCommandManager = customCommandManager; | ||
sessionTransactionProcMap = new ExpandableMap<CustomTransactionProcedureWithArity>(CustomCommandManager.MinMapSize, 0, byte.MaxValue); | ||
sessionCustomProcMap = new ExpandableMap<CustomProcedure>(CustomCommandManager.MinMapSize, 0, byte.MaxValue); | ||
|
||
sessionTransactionProcMap = new ExpandableMap<(CustomTransactionProcedure, int)>(MinMapSize, 0, byte.MaxValue); | ||
sessionCustomProcMap = new ExpandableMap<CustomProcedure>(MinMapSize, 0, byte.MaxValue); | ||
} | ||
|
||
/// <summary> | ||
/// Get a custom procedure by ID | ||
/// </summary> | ||
/// <param name="id">The procedure ID</param> | ||
/// <param name="respServerSession">The current session</param> | ||
/// <returns>The per-session instance of the procedure</returns> | ||
/// <exception cref="GarnetException"></exception> | ||
public CustomProcedure GetCustomProcedure(int id, RespServerSession respServerSession) | ||
{ | ||
// Check if we already have a cached entry of the per-session custom procedure instance | ||
if (!sessionCustomProcMap.TryGetValue(id, out var customProc)) | ||
{ | ||
// If not, get the custom procedure from the CustomCommandManager | ||
if (!customCommandManager.TryGetCustomProcedure(id, out var entry)) | ||
throw new GarnetException($"Custom procedure {id} not found"); | ||
|
||
// Create the session-specific instance and add it to the cache | ||
customProc = entry.CustomProcedureFactory(); | ||
customProc.respServerSession = respServerSession; | ||
var setSuccessful = sessionCustomProcMap.TrySetValue(id, ref customProc); | ||
Debug.Assert(setSuccessful); | ||
sessionCustomProcMap.TrySetValue(id, ref customProc); | ||
} | ||
|
||
return customProc; | ||
} | ||
|
||
/// <summary> | ||
/// Get a custom transaction procedure by ID | ||
/// </summary> | ||
/// <param name="id">The transaction ID</param> | ||
/// <param name="respServerSession">The current session</param> | ||
/// <param name="txnManager">txnManager</param> | ||
/// <param name="scratchBufferManager">scratchBufferManager</param> | ||
/// <param name="arity">The arity of the transaction</param> | ||
/// <returns>The per-session instance of the transaction</returns> | ||
/// <exception cref="GarnetException"></exception> | ||
public CustomTransactionProcedure GetCustomTransactionProcedure(int id, RespServerSession respServerSession, TransactionManager txnManager, ScratchBufferManager scratchBufferManager, out int arity) | ||
{ | ||
if (sessionTransactionProcMap.Exists(id)) | ||
// Check if we already have a cached entry of the per-session custom transaction instance | ||
if (sessionTransactionProcMap.TryGetValue(id, out var tranToArity)) | ||
{ | ||
ref var customTranProc = ref sessionTransactionProcMap.GetValueByRef(id); | ||
if (customTranProc.Procedure != null) | ||
if (tranToArity.Item1 != null) | ||
{ | ||
arity = customTranProc.Arity; | ||
return customTranProc.Procedure; | ||
arity = tranToArity.Item2; | ||
return tranToArity.Item1; | ||
} | ||
} | ||
|
||
if (!customCommandManager.TryGetCustomTransactionProcedure(id, out var entry)) | ||
throw new GarnetException($"Transaction procedure {id} not found"); | ||
_ = customCommandManager.customCommandsInfo.TryGetValue(entry.NameStr, out var cmdInfo); | ||
arity = cmdInfo?.Arity ?? 0; | ||
return GetCustomTransactionProcedureAndSetArity(entry, respServerSession, txnManager, scratchBufferManager, cmdInfo?.Arity ?? 0); | ||
|
||
// Create the session-specific instance and add it to the cache | ||
var customTranProc = entry.proc(); | ||
customTranProc.txnManager = txnManager; | ||
customTranProc.scratchBufferManager = scratchBufferManager; | ||
customTranProc.respServerSession = respServerSession; | ||
|
||
arity = entry.arity; | ||
tranToArity = new ValueTuple<CustomTransactionProcedure, int>(customTranProc, arity); | ||
sessionTransactionProcMap.TrySetValue(id, ref tranToArity); | ||
|
||
return customTranProc; | ||
} | ||
|
||
/// <summary> | ||
/// Get a custom raw-string command by name | ||
/// </summary> | ||
/// <param name="command">The command name to match</param> | ||
/// <param name="cmd">The matching command</param> | ||
/// <returns>True if command name matched an existing command</returns> | ||
public bool Match(ReadOnlySpan<byte> command, out CustomRawStringCommand cmd) | ||
=> customCommandManager.Match(command, out cmd); | ||
|
||
/// <summary> | ||
/// Get a custom transaction by name | ||
/// </summary> | ||
/// <param name="command">The transaction name to match</param> | ||
/// <param name="cmd">The matching transaction</param> | ||
/// <returns>True if transaction name matched an existing transaction</returns> | ||
public bool Match(ReadOnlySpan<byte> command, out CustomTransaction cmd) | ||
=> customCommandManager.Match(command, out cmd); | ||
|
||
/// <summary> | ||
/// Get a custom object command by name | ||
/// </summary> | ||
/// <param name="command">The command name to match</param> | ||
/// <param name="cmd">The matching command</param> | ||
/// <returns>True if command name matched an existing command</returns> | ||
public bool Match(ReadOnlySpan<byte> command, out CustomObjectCommand cmd) | ||
=> customCommandManager.Match(command, out cmd); | ||
|
||
/// <summary> | ||
/// Get a custom procedure by name | ||
/// </summary> | ||
/// <param name="command">The procedure name to match</param> | ||
/// <param name="cmd">The matching procedure</param> | ||
/// <returns>True if procedure name matched an existing procedure</returns> | ||
public bool Match(ReadOnlySpan<byte> command, out CustomProcedureWrapper cmd) | ||
=> customCommandManager.Match(command, out cmd); | ||
|
||
/// <summary> | ||
/// Get custom command info by name | ||
/// </summary> | ||
/// <param name="cmdName">The command name</param> | ||
/// <param name="respCommandsInfo">The matching command info</param> | ||
/// <returns>True if command info was found</returns> | ||
public bool TryGetCustomCommandInfo(string cmdName, out RespCommandsInfo respCommandsInfo) | ||
=> customCommandManager.TryGetCustomCommandInfo(cmdName, out respCommandsInfo); | ||
|
||
/// <summary> | ||
/// Get custom command docs by name | ||
/// </summary> | ||
/// <param name="cmdName">The command name</param> | ||
/// <param name="respCommandsDocs">The matching command docs</param> | ||
/// <returns>True if command docs was found</returns> | ||
public bool TryGetCustomCommandDocs(string cmdName, out RespCommandDocs respCommandsDocs) | ||
=> customCommandManager.TryGetCustomCommandDocs(cmdName, out respCommandsDocs); | ||
|
||
/// <summary> | ||
/// Get all custom command infos | ||
/// </summary> | ||
/// <returns>Map between custom command name and custom command info</returns> | ||
internal IReadOnlyDictionary<string, RespCommandsInfo> GetAllCustomCommandsInfos() | ||
=> customCommandManager.GetAllCustomCommandsInfos(); | ||
|
||
/// <summary> | ||
/// Get all custom command docs | ||
/// </summary> | ||
/// <returns>Map between custom command name and custom command docs</returns> | ||
internal IReadOnlyDictionary<string, RespCommandDocs> GetAllCustomCommandsDocs() | ||
=> customCommandManager.GetAllCustomCommandsDocs(); | ||
|
||
/// <summary> | ||
/// Get count of all custom command infos | ||
/// </summary> | ||
/// <returns>Count</returns> | ||
internal int GetCustomCommandInfoCount() => customCommandManager.GetCustomCommandInfoCount(); | ||
|
||
/// <summary> | ||
/// Get RespCommand enum by command ID | ||
/// </summary> | ||
/// <param name="id">Command ID</param> | ||
/// <returns>Matching RespCommand</returns> | ||
public RespCommand GetCustomRespCommand(int id) | ||
=> customCommandManager.GetCustomRespCommand(id); | ||
|
||
/// <summary> | ||
/// Get GarnetObjectType enum by object type ID | ||
/// </summary> | ||
/// <param name="id">Object type ID</param> | ||
/// <returns>Matching GarnetObjectType</returns> | ||
public GarnetObjectType GetCustomGarnetObjectType(int id) | ||
=> customCommandManager.GetCustomGarnetObjectType(id); | ||
|
||
private CustomTransactionProcedure GetCustomTransactionProcedureAndSetArity(CustomTransaction entry, RespServerSession respServerSession, TransactionManager txnManager, ScratchBufferManager scratchBufferManager, int arity) | ||
{ | ||
int id = entry.id; | ||
|
||
var customTranProc = new CustomTransactionProcedureWithArity(entry.proc(), arity) | ||
{ | ||
Procedure = | ||
{ | ||
txnManager = txnManager, | ||
scratchBufferManager = scratchBufferManager, | ||
respServerSession = respServerSession | ||
} | ||
}; | ||
var setSuccessful = sessionTransactionProcMap.TrySetValue(id, ref customTranProc); | ||
Debug.Assert(setSuccessful); | ||
|
||
return customTranProc.Procedure; | ||
} | ||
|
||
private struct CustomTransactionProcedureWithArity | ||
{ | ||
public CustomTransactionProcedure Procedure { get; } | ||
|
||
public int Arity { get; } | ||
|
||
public CustomTransactionProcedureWithArity(CustomTransactionProcedure procedure, int arity) | ||
{ | ||
this.Procedure = procedure; | ||
this.Arity = arity; | ||
} | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not start the expandable map at 0 so that we dont pay the cost of memory in the common case that there are no custom commands? then then grow as 1,2,4, etc. In particular for CCMS, we really need to limit the default memory allocated when sessions start, as there could be thousands of sessions over time.