Set perf flags all at once #272
Merged
Conversation
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
Signed-off-by: Yilin Chen <sticnarf@gmail.com>
tabokie
reviewed
Apr 1, 2022
| // & 0b111 means find the flag location is a alternative way to do mod | ||
| // operation | ||
| GET_FLAG(flag) ^= (uint64_t)0b1 << ((uint64_t)flag & (uint64_t)0b111); | ||
| PerfFlags NewPerfFlags(std::initializer_list<PerfFlag> l) { |
There was a problem hiding this comment.
Could you & them? And implement + for PerfFlags at rust side.
There was a problem hiding this comment.
In which use case do you think we need & perf flags? Typically, we create some PerfFlags presets to use, so I don't know when we need to combine flags.
There was a problem hiding this comment.
(I misread the PerfFlag as a bitmap itself.) After implementing Add<PerfFlag, Output = PerfFlags> for PerfFlag and PerfFlags, you can composing a flags at rust side instead of creating an array of flags and let RocksDB handle it.
There was a problem hiding this comment.
I don't modify this rocksdb PR because the C wrappers are only needed to be implemented in rust-rocksdb.
Here, the NewPerfFlags is only for convenience in rocksdb tests.
The overloaded operators are implemented in tikv/rust-rocksdb#682. Then, we can create a PerfFlags by writing PerfFlag::A | PerfFlag::B | PerfFlag::C.
PTAL
tabokie
approved these changes
Apr 1, 2022
youjiali1995
approved these changes
Apr 2, 2022
tabokie
pushed a commit
to tabokie/rocksdb
that referenced
this pull request
May 11, 2022
This PR changes some bits of tikv#237. Considering our use cases in TiKV, _although not really necessary_, we frequently set perf levels around RocksDB operations. So, if we switch to perf flags, it requires us to set perf flags efficiently. And we have quite a few flags to set, setting flags one by one in tikv#237 is not that efficient. All flags can be saved in two words. So, we **can** achieve similar efficiency by setting all flags all at once. And meanwhile, we can flexibly specify which metrics we care about at runtime. This PR also changes to use `std::bitset` and a named enum instead of handmade int array and anonymous enum. Hopefully they will make the code cleaner. See tikv/rust-rocksdb#682 for the companion Rust wrapper Signed-off-by: Yilin Chen <sticnarf@gmail.com> Signed-off-by: tabokie <xy.tao@outlook.com>
tabokie
pushed a commit
that referenced
this pull request
May 11, 2022
This PR changes some bits of #237. Considering our use cases in TiKV, _although not really necessary_, we frequently set perf levels around RocksDB operations. So, if we switch to perf flags, it requires us to set perf flags efficiently. And we have quite a few flags to set, setting flags one by one in #237 is not that efficient. All flags can be saved in two words. So, we **can** achieve similar efficiency by setting all flags all at once. And meanwhile, we can flexibly specify which metrics we care about at runtime. This PR also changes to use `std::bitset` and a named enum instead of handmade int array and anonymous enum. Hopefully they will make the code cleaner. See tikv/rust-rocksdb#682 for the companion Rust wrapper Signed-off-by: Yilin Chen <sticnarf@gmail.com> Signed-off-by: tabokie <xy.tao@outlook.com>
v01dstar
pushed a commit
to v01dstar/rocksdb
that referenced
this pull request
Feb 8, 2024
This PR changes some bits of tikv#237. Considering our use cases in TiKV, _although not really necessary_, we frequently set perf levels around RocksDB operations. So, if we switch to perf flags, it requires us to set perf flags efficiently. And we have quite a few flags to set, setting flags one by one in tikv#237 is not that efficient. All flags can be saved in two words. So, we **can** achieve similar efficiency by setting all flags all at once. And meanwhile, we can flexibly specify which metrics we care about at runtime. This PR also changes to use `std::bitset` and a named enum instead of handmade int array and anonymous enum. Hopefully they will make the code cleaner. See tikv/rust-rocksdb#682 for the companion Rust wrapper Signed-off-by: Yilin Chen <sticnarf@gmail.com> Signed-off-by: tabokie <xy.tao@outlook.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR changes some bits of #237.
Considering our use cases in TiKV, although not really necessary, we frequently set perf levels around RocksDB operations. So, if we switch to perf flags, it requires us to set perf flags efficiently. And we have quite a few flags to set, setting flags one by one in #237 is not that efficient.
All flags can be saved in two words. So, we can achieve similar efficiency by setting all flags all at once. And meanwhile, we can flexibly specify which metrics we care about at runtime.
This PR also changes to use
std::bitsetand a named enum instead of handmade int array and anonymous enum. Hopefully they will make the code cleaner.See tikv/rust-rocksdb#682 for the companion Rust wrapper