feat: Add performance benchmarks for various modules and expose funda… by tekawade · Pull Request #23 · tekawade/algs4-rust

tekawade · 2025-11-19T01:46:08Z

…mental submodules.

gemini-code-assist · 2025-11-19T01:46:29Z

Summary of Changes

Hello @tekawade, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the project's performance analysis capabilities by integrating criterion-based benchmarks across a wide range of algorithm and data structure modules. This allows for systematic measurement and tracking of performance characteristics. Additionally, it refactors the fundamentals module to expose its internal components, improving modularity and testability.

Highlights

Benchmarking Integration: Performance benchmarks have been added across various modules including advanced, fundamentals, geometry, searching, sorting, and strings, utilizing the criterion crate.
Module Exposure: Key submodules within algs4-fundamentals (specifically collections and union-find implementations) have been made public to facilitate broader usage and testing, including for the newly added benchmarks.
Documentation Update: The README.md now includes comprehensive instructions for running benchmarks for individual modules and the entire workspace, detailing the algorithms covered by each benchmark suite.
Dependency Management: Cargo.toml files for all benchmarked modules have been updated to include criterion and rand as development dependencies, and the top-level Cargo.toml now specifies a rust-version of "1.82.0".

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request adds a comprehensive set of performance benchmarks for various modules using the criterion framework. It also exposes some submodules in algs4-fundamentals to make them accessible to the benchmarks. The changes are a great addition for monitoring performance.

My review focuses on improving the accuracy and clarity of the new benchmark code. I've identified several places where criterion::black_box should be used to prevent the compiler from optimizing away the code being measured. I also found some unused code and opportunities to make the benchmark setup more efficient. In one case, the structure of a benchmark was measuring setup cost along with the operation, for which I've suggested a fix.

gemini-code-assist · 2025-11-19T01:48:50Z

+        group.bench_with_input(BenchmarkId::new("kmp_search", size), &text, |b, txt| {
+            b.iter(|| {
+                let kmp = KMP::new(black_box(pattern));
+                kmp.search(black_box(txt));
+            });
+        });


The KMP::new() call is inside the b.iter loop, which means you are benchmarking the DFA construction on every iteration, not just the search. For a more accurate benchmark of the search performance, the KMP instance should be created outside the loop. Additionally, the result of kmp.search should be wrapped in black_box to prevent the compiler from optimizing it away.

group.bench_with_input(BenchmarkId::new("kmp_search", size), &text, |b, txt| { let kmp = KMP::new(pattern); b.iter(|| { black_box(kmp.search(black_box(txt))); }); });

gemini-code-assist · 2025-11-19T01:48:51Z

+        let mut rng = thread_rng();
+        let mut data: Vec<i32> = (0..*size as i32).collect();
+        data.shuffle(&mut rng);


The rng and data variables are created but never used within the benchmark loop. This dead code should be removed to improve clarity.

gemini-code-assist · 2025-11-19T01:48:51Z

+use algs4_fundamentals::union_find::quick_find_uf::QuickFindUF;
+use algs4_fundamentals::union_find::quick_union_uf::QuickUnionUF;
+use algs4_fundamentals::union_find::weighted_quick_union_uf::WeightedQuickUnionUF;
+use algs4_fundamentals::union_find::weighted_quick_union_uf::WeightedQuickUnionUF as UF; // UF trait might not be public or exist as named


The alias UF is imported but never used. This line can be removed to avoid confusion.

gemini-code-assist · 2025-11-19T01:48:51Z

+    let mut group = c.benchmark_group("union_find");
+
+    for size in [1000, 5000].iter() {
+        let ops = size * 2;


The ops variable is assigned but never used. It should be removed.

gemini-code-assist · 2025-11-19T01:48:51Z

+                for i in 0..size - 1 {
+                    uf.union(i, i + 1);
+                }
+                uf.connected(0, size - 1)


The result of uf.connected is not used, so the call might be optimized away by the compiler. Wrap it in black_box to ensure the benchmark is accurate. This applies to the other union_find benchmarks as well.

Suggested change

uf.connected(0, size - 1)

black_box(uf.connected(0, size - 1))

gemini-code-assist · 2025-11-19T01:48:51Z

+                let mut p = pts.clone();
+                GrahamScan::new(black_box(&mut p));


The p vector does not need to be mutable. More importantly, the result of GrahamScan::new is not used and could be optimized away by the compiler. You should wrap the function call in black_box to ensure the benchmark is accurate.

Suggested change

let mut p = pts.clone();

GrahamScan::new(black_box(&mut p));

let p = pts.clone();

black_box(GrahamScan::new(&p));

gemini-code-assist · 2025-11-19T01:48:51Z

+            b.iter(|| {
+                let mut st = BST::new();
+                for &key in &keys {
+                    st.put(key, key);
+                }
+                for &key in &keys {
+                    st.get(&key);
+                }
+            });


The benchmark may not be accurate because the compiler could optimize away the put and get calls since their results are not used. To prevent this and ensure you're measuring the real performance, you should use criterion::black_box on the inputs to put and the output of get. This advice applies to all other benchmarks in this file as well.

Suggested change

b.iter(|| {

let mut st = BST::new();

for &key in &keys {

st.put(key, key);

}

for &key in &keys {

st.get(&key);

}

});

b.iter(|| {

let mut st = BST::new();

for &key in &keys {

st.put(black_box(key), black_box(key));

}

for &key in &keys {

black_box(st.get(&key));

}

});

gemini-code-assist · 2025-11-19T01:48:51Z

+        group.bench_with_input(BenchmarkId::new("lsd", size), &data, |b, d| {
+            b.iter(|| {
+                let mut arr = d.clone();
+                lsd::sort(&mut arr, 10);


The sort operation might be optimized away by the compiler because the result (the modified arr) is not used. To ensure the benchmark is accurate, you should wrap the mutable slice in black_box, just as you did for the integer sorts. This also applies to the msd and quick3string benchmarks in this file.

Suggested change

lsd::sort(&mut arr, 10);

lsd::sort(black_box(&mut arr), 10);

gemini-code-assist · 2025-11-19T01:48:51Z

+        let text: String = (0..*size)
+            .map(|_| {
+                thread_rng()
+                    .sample_iter(&Alphanumeric)
+                    .take(1)
+                    .map(char::from)
+                    .collect::<String>()
+            })
+            .collect();


The text generation is inefficient. It creates a new String for each character and then collects them. This can be done more efficiently by collecting directly from the iterator, which avoids intermediate String allocations.

let text: String = thread_rng() .sample_iter(&Alphanumeric) .take(*size) .map(char::from) .collect();

Copilot

Pull Request Overview

This PR adds comprehensive performance benchmarks across multiple algorithm modules and exposes fundamental submodules for direct access. The benchmarks enable performance testing of various data structures and algorithms using the Criterion framework.

Key Changes:

Adds benchmark suites for 6 modules: fundamentals, sorting, searching, strings, geometry, and advanced
Exposes union_find and collections submodules in the fundamentals module for granular access
Updates README with comprehensive benchmark documentation and usage examples

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
modules/fundamentals/benches/fundamentals_benchmarks.rs	Implements benchmarks for collections (bags, queues, stacks) and union-find algorithms
modules/fundamentals/Cargo.toml	Adds criterion and rand dev dependencies with benchmark harness configuration
modules/fundamentals/src/union_find/mod.rs	Changes module visibility from private to public for direct submodule access
modules/fundamentals/src/collections/mod.rs	Changes module visibility from private to public for direct submodule access
modules/sorting/benches/sorting_benchmarks.rs	Implements benchmarks for comparison-based and string sorting algorithms
modules/sorting/Cargo.toml	Adds criterion and rand dev dependencies with benchmark harness configuration
modules/searching/benches/searching_benchmarks.rs	Implements benchmarks for symbol table implementations (BST, hash tables, tries)
modules/searching/Cargo.toml	Adds criterion and rand dev dependencies with benchmark harness configuration
modules/strings/benches/strings_benchmarks.rs	Implements benchmarks for KMP pattern matching algorithm
modules/strings/Cargo.toml	Adds criterion and rand dev dependencies with benchmark harness configuration
modules/geometry/benches/geometry_benchmarks.rs	Implements benchmarks for Graham scan convex hull algorithm
modules/geometry/Cargo.toml	Adds criterion and rand dev dependencies with benchmark harness configuration
modules/advanced/benches/advanced_benchmarks.rs	Implements benchmarks for Fenwick tree operations
modules/advanced/Cargo.toml	Adds criterion and rand dev dependencies with benchmark harness configuration
README.md	Updates benchmarking section with commands for all modules and description of benchmarks
Cargo.toml	Sets minimum Rust version requirement to 1.82.0

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-19T01:49:20Z

+use algs4_fundamentals::union_find::quick_find_uf::QuickFindUF;
+use algs4_fundamentals::union_find::quick_union_uf::QuickUnionUF;
+use algs4_fundamentals::union_find::weighted_quick_union_uf::WeightedQuickUnionUF;
+use algs4_fundamentals::union_find::weighted_quick_union_uf::WeightedQuickUnionUF as UF; // UF trait might not be public or exist as named


The import alias UF is defined but never used in the code. Remove this unused import to clean up the code.

Suggested change

use algs4_fundamentals::union_find::weighted_quick_union_uf::WeightedQuickUnionUF as UF; // UF trait might not be public or exist as named

Copilot · 2025-11-19T01:49:21Z

+    let mut group = c.benchmark_group("union_find");
+
+    for size in [1000, 5000].iter() {
+        let ops = size * 2;


The variable ops is defined but never used. Remove this unused variable.

Suggested change

let ops = size * 2;

Copilot · 2025-11-19T01:49:21Z

+        let mut rng = thread_rng();
+        let mut data: Vec<i32> = (0..*size as i32).collect();
+        data.shuffle(&mut rng);


The variables rng and data are created outside the benchmark closure but never used. These should be removed as they don't contribute to the benchmark and may confuse readers about what is being measured.

Suggested change

let mut rng = thread_rng();

let mut data: Vec<i32> = (0..*size as i32).collect();

data.shuffle(&mut rng);

Copilot · 2025-11-19T01:49:21Z

+        let text: String = (0..*size)
+            .map(|_| {
+                thread_rng()
+                    .sample_iter(&Alphanumeric)
+                    .take(1)
+                    .map(char::from)
+                    .collect::<String>()
+            })


The random character generation is inefficient. Creating a new RNG for each character and collecting into an intermediate String is unnecessary. Consider using: thread_rng().sample_iter(&Alphanumeric).take(*size).map(char::from).collect::<String>()

Suggested change

let text: String = (0..*size)

.map(|_| {

thread_rng()

.sample_iter(&Alphanumeric)

.take(1)

.map(char::from)

.collect::<String>()

})

let text: String = thread_rng()

.sample_iter(&Alphanumeric)

.take(*size)

.map(char::from)

Copilot · 2025-11-19T01:49:21Z

+        let mut keys: Vec<i32> = (0..*size as i32).collect();
+        keys.shuffle(&mut rng);
+
+        group.bench_with_input(BenchmarkId::new("bst_put_get", size), size, |b, &size| {


The size parameter is passed to the closure but never used. The keys vector from the outer scope is used instead. Either remove the unused parameter by using |b, _| or restructure to pass &keys as the input.

Copilot · 2025-11-19T01:49:22Z

+            BenchmarkId::new("red_black_bst_put_get", size),
+            size,
+            |b, &size| {


The size parameter is passed to the closure but never used. The keys vector from the outer scope is used instead. Either remove the unused parameter by using |b, _| or restructure to pass &keys as the input.

Copilot · 2025-11-19T01:49:22Z

+            BenchmarkId::new("linear_probing_hash_st_put_get", size),
+            size,
+            |b, &size| {


The size parameter is passed to the closure but never used. The keys vector from the outer scope is used instead. Either remove the unused parameter by using |b, _| or restructure to pass &keys as the input.

Copilot · 2025-11-19T01:49:22Z

+        group.bench_with_input(
+            BenchmarkId::new("separate_chaining_hash_st_put_get", size),
+            size,
+            |b, &size| {


The size parameter is passed to the closure but never used. The keys vector from the outer scope is used instead. Either remove the unused parameter by using |b, _| or restructure to pass &keys as the input.

Suggested change

|b, &size| {

|b, _| {

Copilot · 2025-11-19T01:49:22Z

+            size,
+            |b, &_size| {
+                b.iter(|| {
+                    let mut st = TrieST::new();
+                    for (i, key) in keys.iter().enumerate() {
+                        st.put(key, i);
+                    }
+                    for key in &keys {


[nitpick] The _size parameter is intentionally ignored with underscore prefix, but size is still passed as the input parameter. Since the keys vector from the outer scope is used, consider passing &keys as the input or simply passing size and using it consistently.

Suggested change

size,

|b, &_size| {

b.iter(|| {

let mut st = TrieST::new();

for (i, key) in keys.iter().enumerate() {

st.put(key, i);

}

for key in &keys {

&keys,

|b, keys| {

b.iter(|| {

let mut st = TrieST::new();

for (i, key) in keys.iter().enumerate() {

st.put(key, i);

}

for key in keys {

feat: Add performance benchmarks for various modules and expose funda…

a8be45d

…mental submodules.

tekawade self-assigned this Nov 19, 2025

Copilot AI review requested due to automatic review settings November 19, 2025 01:46

Copilot started reviewing on behalf of tekawade November 19, 2025 01:46 View session

tekawade merged commit dc6eed1 into master Nov 19, 2025
8 of 9 checks passed

tekawade deleted the feature/benchmarks-for-algos branch November 19, 2025 01:46

Copilot finished reviewing on behalf of tekawade November 19, 2025 01:47

gemini-code-assist Bot reviewed Nov 19, 2025

View reviewed changes

Copilot AI reviewed Nov 19, 2025

View reviewed changes

	uf.connected(0, size - 1)
	black_box(uf.connected(0, size - 1))

	let mut rng = thread_rng();
	let mut data: Vec<i32> = (0..*size as i32).collect();
	data.shuffle(&mut rng);

Conversation

tekawade commented Nov 19, 2025

Uh oh!

gemini-code-assist Bot commented Nov 19, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants