-
Notifications
You must be signed in to change notification settings - Fork 18
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
feat(sidecar): ScoreCache
for account_states
#399
Conversation
LowestScoreMap
for account_states
ScoreCache
for account_states
Open for a first review! |
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.
Love it! Great work
4d6e866
to
dd91eae
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.
Great!
bolt-sidecar/benches/score_cache.rs
Outdated
// Insert benchmark | ||
group.bench_function("ScoreCache Insert", |b| { | ||
b.iter(|| { | ||
for i in 0..1000 { |
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.
Consider varying sizes of operations (1000, 10k, 100k) to understand how perf varies
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.
Since I want to measure the raw overhead of modifying the score when doing get, insert, update operations I don't think there is a big advantage in testing different sizes. Also it slows down the benchmarking by really a lot. Just 1000 iterations takes a couple of mins on a M3 pro cpu
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.
But don't wanna block this from merging
No worries! It was indeed great feedback. Pushing a commit soon to address it |
25b98ee
to
3726ac8
Compare
3726ac8
to
fbd709b
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.
Nice job, LGTM
Related to #393, this PR introduces a wrapper over
HashMap
calledScoreCache
that implements a scoring system on get, insert and update operations.The map size can be configured, with a default of 1 MiB that should hold around 15k account state entries considering also its key. For account states in particular a metrics
bolt_sidecar_account_states
has been added to track its usage.