try to compare #20
Open
try to compare #20
Conversation
This file contains hidden or 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
update hotness estimate
add basic filter unit support
restore old version filter
add compile command
Conflicts: .gitignore
fix warning
filter cache client
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a full adaptive filter-cache subsystem, including sampling heat via HeatBuckets, allocating filter units with a greedy algorithm and two-heap adjustment, and integrating with a LightGBM-based model and YCSB workloads.
- Add
HeatBucketsandSamplesPoolto track and update key-range hotness - Implement
GreedyAlgo,FilterCacheHeap/FilterCacheHeapManager, andFilterCacheManagerfor dynamic unit allocation - Wire up client APIs (
FilterCacheClient) and model I/O (ClfModel), plus minor YCSB DB changes
Reviewed Changes
Copilot reviewed 80 out of 86 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| db/art/heat_buckets.h/.cc | Sampling and hotness‐tracking buckets |
| db/art/greedy_algo.h/.cc | Greedy OPT unit‐allocation algorithm |
| db/art/filter_cache_item.h/.cc | Skeleton for per‐segment filter units |
| db/art/filter_cache_heap.h | Heap structures for cost/benefit adjustment |
| db/art/filter_cache_client.h/.cc | Async client integration with thread pool |
| db/art/filter_cache.h | Core FilterCache and FilterCacheManager logic |
| db/art/clf_model.h/.cc | CSV I/O & LightGBM server interface |
| YCSB/rocksdb/rocksdb_db.cc | Log workload & switch update to insert |
| YCSB/leveldb/leveldb_db.cc | Remove unused SerializeRow |
Files not reviewed (6)
- CMakeLists.txt: Language not supported
- TARGETS: Language not supported
- YCSB/Makefile: Language not supported
- YCSB/rocksdb/rocksdb.properties: Language not supported
- YCSB/workloads/workloadt: Language not supported
- build_tools/build_detect_platform: Language not supported
Comments suppressed due to low confidence (1)
db/art/clf_model.h:45
LessorComparoris misspelled; considerLessOrComparatoror simplyLessComparator.
inline bool RangeRatePairLessorComparor(const RangeRatePair& pair_1, const RangeRatePair& pair_2);
| class HeatBuckets { | ||
| private: | ||
| // TODO: mutex can be optimized | ||
| static std::vector<std::string> seperators_; |
There was a problem hiding this comment.
The variable name seperators_ is misspelled; it should be separators_.
Suggested change
| static std::vector<std::string> seperators_; | |
| static std::vector<std::string> separators_; |
Comment on lines
+41
to
+50
| double enable_benifit; | ||
| SegmentAlgoHelper(const uint32_t& id, const uint32_t& cnt, const uint32_t& size, const uint16_t& units) { | ||
| segment_id = id; visit_cnt = cnt; size_per_unit = size; units_num = units; | ||
| enable_benifit = StandardBenefit(visit_cnt, units_num); | ||
| // assert(units_num <= MAX_UNITS_NUM); | ||
| } | ||
| SegmentAlgoHelper(const uint32_t& id, SegmentAlgoInfo& segment_algo_info) { | ||
| segment_id = id; visit_cnt = segment_algo_info.visit_cnt; | ||
| size_per_unit = segment_algo_info.size_per_unit; units_num = 0; | ||
| enable_benifit = StandardBenefit(visit_cnt, units_num); |
There was a problem hiding this comment.
The field enable_benifit is misspelled; it should be enable_benefit.
Suggested change
| double enable_benifit; | |
| SegmentAlgoHelper(const uint32_t& id, const uint32_t& cnt, const uint32_t& size, const uint16_t& units) { | |
| segment_id = id; visit_cnt = cnt; size_per_unit = size; units_num = units; | |
| enable_benifit = StandardBenefit(visit_cnt, units_num); | |
| // assert(units_num <= MAX_UNITS_NUM); | |
| } | |
| SegmentAlgoHelper(const uint32_t& id, SegmentAlgoInfo& segment_algo_info) { | |
| segment_id = id; visit_cnt = segment_algo_info.visit_cnt; | |
| size_per_unit = segment_algo_info.size_per_unit; units_num = 0; | |
| enable_benifit = StandardBenefit(visit_cnt, units_num); | |
| double enable_benefit; | |
| SegmentAlgoHelper(const uint32_t& id, const uint32_t& cnt, const uint32_t& size, const uint16_t& units) { | |
| segment_id = id; visit_cnt = cnt; size_per_unit = size; units_num = units; | |
| enable_benefit = StandardBenefit(visit_cnt, units_num); | |
| // assert(units_num <= MAX_UNITS_NUM); | |
| } | |
| SegmentAlgoHelper(const uint32_t& id, SegmentAlgoInfo& segment_algo_info) { | |
| segment_id = id; visit_cnt = segment_algo_info.visit_cnt; | |
| size_per_unit = segment_algo_info.size_per_unit; units_num = 0; | |
| enable_benefit = StandardBenefit(visit_cnt, units_num); |
Comment on lines
+25
to
+26
| inline bool FilterCacheHeapNodeLessComparor(const FilterCacheHeapNode& node_1, const FilterCacheHeapNode& node_2); | ||
| inline bool FilterCacheHeapNodeGreaterComparor(const FilterCacheHeapNode& node_1, const FilterCacheHeapNode& node_2); |
There was a problem hiding this comment.
The word Comparor is misspelled; it should be Comparator.
Suggested change
| inline bool FilterCacheHeapNodeLessComparor(const FilterCacheHeapNode& node_1, const FilterCacheHeapNode& node_2); | |
| inline bool FilterCacheHeapNodeGreaterComparor(const FilterCacheHeapNode& node_1, const FilterCacheHeapNode& node_2); | |
| inline bool FilterCacheHeapNodeLessComparator(const FilterCacheHeapNode& node_1, const FilterCacheHeapNode& node_2); | |
| inline bool FilterCacheHeapNodeGreaterComparator(const FilterCacheHeapNode& node_1, const FilterCacheHeapNode& node_2); |
| // need to generate random integer in [0, old samples_cnt_] (equal to [0, old samples_cnt_ + 1)) | ||
| // new samples_cnt_ = old samples_cnt_ + 1 | ||
| // if you want random integer in [a, b], use (rand() % (b-a+1))+a; | ||
| srand((unsigned)time(NULL)); |
There was a problem hiding this comment.
Seeding with srand on each call can lead to predictable random sequences; move srand to a one‐time initialization outside this function.
Suggested change
| srand((unsigned)time(NULL)); |
Comment on lines
+62
to
+63
| std::vector<std::string>& range_seperators() { | ||
| return filter_cache_manager_.range_seperators(); |
There was a problem hiding this comment.
Method name range_seperators is misspelled; it should be range_separators.
Suggested change
| std::vector<std::string>& range_seperators() { | |
| return filter_cache_manager_.range_seperators(); | |
| std::vector<std::string>& range_separators() { | |
| return filter_cache_manager_.range_separators(); |
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.
No description provided.