Conversation
Codecov Report
@@ Coverage Diff @@
## master #91 +/- ##
==========================================
+ Coverage 72.69% 73.01% +0.32%
==========================================
Files 68 70 +2
Lines 3065 3202 +137
==========================================
+ Hits 2228 2338 +110
- Misses 837 864 +27
Continue to review full report at Codecov.
|
|
|
||
| for (const auto& key_count : access.keys()) { | ||
| Key key = key_count.key(); | ||
| hot_key_access_frequency[key] |
There was a problem hiding this comment.
Could this be extracted into a method with the cold_key_access_frequency or hoy_key_access_frequency be passed in?
|
|
||
| void reset() { set_values(); }; | ||
|
|
||
| // static void reset_threshold_percent(float new_threshold) { |
| map<Key, map<Address, unsigned>> cold_key_access_frequency; | ||
| // keep track of the keys' access summary | ||
| map<Key, unsigned> key_access_summary; | ||
| // keep track of the hot keys' access summary |
There was a problem hiding this comment.
These comments don't add any value - they state what the code does not why or how etc.
| unsigned long hash_key(Key key) { | ||
| unsigned long hash = 5381; | ||
| int char_int = 0; | ||
| for (unsigned i = 0; i < key.length(); i++) { |
There was a problem hiding this comment.
can you use the for (key: key type syntax rather than int index
|
|
||
| #include <stdbool.h> | ||
| #include <stdio.h> | ||
| #include <chrono> |
There was a problem hiding this comment.
What are these imports for? Some of them seem unrelated to what you're doing here.
| #include <utility> | ||
| #include "heavy_hitters.hpp" | ||
|
|
||
| #define alpha 0.2 |
There was a problem hiding this comment.
Why is this defined outside of the class and gamma and epsilon inside?
|
|
||
| #define alpha 0.2 | ||
|
|
||
| typedef std::string Key; |
There was a problem hiding this comment.
This should not be re-typedefed. It's in includes/types.hpp, I think.
| class AdaptiveThresholdHeavyHitters { | ||
| protected: | ||
| HeavyHittersSketch* hh_sketch; | ||
| float threshold_percent = 0.01; |
There was a problem hiding this comment.
Comments should explain the role of these variables.
| std::unordered_map<Key, int> reset_hot_map; | ||
| std::unordered_map<Key, int> reset_cold_map; | ||
|
|
||
| total_set = reset_total_set; |
There was a problem hiding this comment.
Is this meant to clear the collection? There are methods for doing that.
| // NOTE: No const here because we are calling erase | ||
| for (auto& key_access_pair : key_access_frequency) { | ||
| for (auto& key_access_pair : hot_key_access_frequency) { | ||
| for (unsigned i = 0; i < kMemoryThreadCount; i++) { |
There was a problem hiding this comment.
Why are we only iterating over the hot and cold keys here?
| map<Key, map<Address, unsigned>> cold_key_access_frequency; | ||
| // keep track of the keys' access summary | ||
| map<Key, unsigned> key_access_summary; | ||
| // keep track of the hot keys' access summary |
|
|
||
| // initialize replication factor for new keys | ||
| for (const auto &key_access_pair : key_access_summary) { | ||
| for (const auto &key_access_pair : hot_key_access_summary) { |
There was a problem hiding this comment.
Again, why are we only iterating over the hot and cold keys here?
|
|
||
| for (const auto& key_count : access.keys()) { | ||
| Key key = key_count.key(); | ||
| hot_key_access_frequency[key] |
|
|
||
| hot_key_access_summary[key] = access_count; | ||
|
|
||
| if (access_count > 0) { |
There was a problem hiding this comment.
Explain this code? Not clear what is happening.
4 participants
No description provided.