Add more benchmarks to algobench - #2655
Open
DanielLacina wants to merge 28 commits into
Open
Conversation
fabubaker
self-requested a review
July 16, 2026 20:20
fabubaker
approved these changes
Jul 17, 2026
fabubaker
requested changes
Jul 21, 2026
| 20, | ||
| 10, | ||
| large_random_attachment_filtered, | ||
| |graph, _| louvain::<ModularityUnDir, _>(graph, 1.0, None, None), |
ljeub-pometry
requested changes
Aug 5, 2026
ljeub-pometry
left a comment
Collaborator
There was a problem hiding this comment.
- I don't think you need the custom iter (unless some algorithm has extremely variable runtime, but then benchmarking it would be essentially useless anyway)
- don't shrink the warmup time too much (might contribute to bad timing estimates)
- Possible deadlocks in graph construction (see comments)
- Could simplify some code by getting rid of closures that get only called once and pass in the values directly (not really a problem that you need to change, more of a stylistic comment for the future)
| c: &mut Criterion, | ||
| name: &str, | ||
| sample_size: usize, | ||
| build_graph: BuildGraph, |
Collaborator
There was a problem hiding this comment.
You can just pass in graph: G?
| name: &str, | ||
| sample_size: usize, | ||
| build_graph: BuildGraph, | ||
| setup: Setup, |
Collaborator
There was a problem hiding this comment.
Same as above, no reason to pass in a closure, you are only calling it once, you can just pass in the result
This reverts commit 292972e.
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
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.
What changes were proposed in this pull request?
Added algo benchmarks that benchmark all algorithms with different views
Why are the changes needed?
To give insights on what algorithms need to be improved
Does this PR introduce any user-facing change? If yes is this documented?
no
How was this patch tested?
its a benchmark
Are there any further changes required?
no