GitHub actions #497
GitHub actions #497
Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds automated benchmarking support and reporting via GitHub Actions, including a new workflow, result serialization, comparison tooling, and summary utilities.
- Updates RAT exclusions for new workflows
- Introduces
BenchmarkSummarizer, unit tests, and a fat‐jar assembly for AutoBenchYAML - Adds
AutoBenchYAMLrunner,compare_benchmarks.pycomparator, and CI workflow
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| rat-excludes.txt | Ignore new workflow YAMLs in license checks |
| jvector-examples/src/test/java/io/github/jbellis/jvector/...Test.java | Unit tests for BenchmarkSummarizer |
| jvector-examples/src/main/resources/logback.xml | Console and file logging configuration |
| jvector-examples/src/main/java/io/github/jbellis/jvector/...SummarizerTest.java | Main‐method summarizer tests located in production code |
| jvector-examples/src/main/java/io/github/jbellis/jvector/...BenchmarkSummarizer.java | Summarizer implementation for recall, QPS, and latency |
| jvector-examples/src/main/java/io/github/jbellis/jvector/...Grid.java | Extended with runAllAndCollectResults for automated runs |
| jvector-examples/src/main/java/io/github/jbellis/jvector/...BenchResult.java | Data class for benchmark entries |
| jvector-examples/src/main/java/io/github/jbellis/jvector/...AutoBenchYAML.java | Main runner for GitHub Actions–driven benchmarks |
| jvector-examples/pom.xml | Assembly plugin, logging deps, and bench main class adjustments |
| compare_benchmarks.py | Python script to diff current vs. previous benchmark results |
| .github/workflows/run-bench.yml | CI workflow to build, run benchmarks, and upload/comparison |
Comments suppressed due to low confidence (2)
jvector-examples/src/main/java/io/github/jbellis/jvector/example/util/SummarizerTest.java:1
- This test class lives under src/main, mixing production code and tests. Consider moving it to src/test to keep test code separate.
/*
.github/workflows/run-bench.yml:86
- The 'workflow' input to action-download-artifact should match the workflow's name ('Run Bench Main') rather than the filename. Otherwise previous artifacts may not be retrieved correctly.
workflow: run-bench.yml
|
|
||
| import io.github.jbellis.jvector.example.BenchResult; | ||
| import io.github.jbellis.jvector.example.util.BenchmarkSummarizer.SummaryStats; | ||
| import org.junit.Test; |
There was a problem hiding this comment.
Using JUnit4's @test annotation with JUnit Jupiter assertions may prevent these tests from being discovered or run. Consider switching to org.junit.jupiter.api.Test for consistency with the imported assertions.
| import org.junit.Test; | |
| import org.junit.jupiter.api.Test; |
| } | ||
| } | ||
|
|
||
| public static List<BenchResult> runAllAndCollectResults( |
There was a problem hiding this comment.
[nitpick] This method contains deeply nested loops, making it hard to read and maintain. Consider refactoring inner loops into helper methods or using a builder pattern to simplify.
| "efConstruction", ef, | ||
| "neighborOverflow", neighborOverflow, | ||
| "addHierarchy", addHierarchy, | ||
| "features", features.toString(), |
There was a problem hiding this comment.
[nitpick] Using Set.toString() for features can produce nondeterministic ordering. Consider converting the feature set into a sorted list before stringifying to ensure consistent keys.
| "features", features.toString(), | |
| "features", features.stream().sorted().toList().toString(), |
No description provided.