-
Notifications
You must be signed in to change notification settings - Fork 876
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
chore: Introduce LockKey for LockTable #2463
Conversation
d0f286e
to
459f913
Compare
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.
Not against it, but if you want I can take care of the simpler solution
489dcc2
to
701d04e
Compare
@romange PTAL. I can't request a review because it's your PR |
@dranikpg and I can not approve because it's my PR 👯♂️ . Once it passes tests, please approve and I will submit 😆 |
This should reduce allocations in a common case (not multi). In addition, rename Transaction::args_ to kv_args_. Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
|
||
auto begin() const { | ||
return locks_.cbegin(); | ||
} | ||
|
||
auto end() const { | ||
return locks_.cbegin(); | ||
} |
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.
Actually interesting how this leaks a private type but the compiler doesn't complain 🤷🏻♂️
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.
apparently it's a language feature
auto s = KeyLockArgs::GetLockKey(lock_args.args[i]); | ||
auto it = lt.find(s); | ||
if (it != lt.end() && !it->second.Check(mode)) { | ||
string_view s = KeyLockArgs::GetLockKey(lock_args.args[i]); |
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.
CheckLock is used now only for debug/testing code.
This should reduce allocations in a common case (not multi). In addition, rename Transaction::args_ to kv_args_.