[WIP] Specialize the two hot container shapes + add build-time benchmark#140
Closed
acostarelli wants to merge 2 commits into
Closed
[WIP] Specialize the two hot container shapes + add build-time benchmark#140acostarelli wants to merge 2 commits into
acostarelli wants to merge 2 commits into
Conversation
Add concretely-typed container_spec / sparse_container_spec methods for the two
dominant axis shapes — dense (names::Vector{String}, time_steps::UnitRange{Int})
and sparse (names, segments, time_steps) — alongside the generic Vararg
fallbacks. Every add_*_container! reaches them automatically through the
@generated _add_container! splat, so no call sites change. The sparse
specializations hardcode the key tuple type, eliding the runtime
eltype(Iterators.product(axs...)) computation; the dense ones pin an inferable
return type. Other shapes fall through to the unchanged generic methods.
Add test/performance/container_build.jl, a dependency-free (@elapsed/@allocated)
benchmark of container build-time for these shapes, and wire it into the
existing main-vs-branch performance_test.yml so a PR comment compares the
specialized methods (branch) against the unaware Vararg baseline (main).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds targeted Julia method specializations to reduce build-time overhead for the two most common optimization-container shapes, and introduces a CI-run performance script to compare container allocation costs between main and this branch.
Changes:
- Add specialized
container_specmethods fornames::Vector{String} × time_steps::UnitRange{Int}(includingFloat64NaN-fill path). - Add specialized
sparse_container_specmethods fornames × timeandnames × segments × timeby hardcoding hot key tuple types. - Add a new build-time benchmark script and extend the performance workflow to run it on both
mainand the PR branch, posting outputs to the PR.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
test/performance/container_build.jl |
New container build-time benchmark for dense/sparse “hot shapes” and wrapper paths. |
src/utils/jump_utils.jl |
Adds concrete specializations for dense and sparse container construction to reduce overhead vs generic Vararg methods. |
.github/workflows/performance_test.yml |
Runs the new container benchmark for main and branch and includes results in the PR comment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+94
to
+99
| ), | ||
| ] | ||
| for (label, f) in cases | ||
| ns, bytes = bench(f) | ||
| @printf("%-28s %8d %14.1f %14d\n", label, n, ns, bytes) | ||
| end |
Comment on lines
+158
to
+162
| for (label, f) in | ||
| [("add_variable (dense)", dense), ("add_constraints (sparse)", sparse)] | ||
| ns, bytes = bench(f) | ||
| @printf("%-28s %8d %14.1f %14d\n", label, n, ns, bytes) | ||
| end |
Comment on lines
+145
to
+147
| () -> begin | ||
| empty!(container.constraints) | ||
| IOM.add_constraints_container!( |
Contributor
|
Performance Results This branch Container Build (Main) Container Build (This branch) |
…o we get a comparison
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.
Claude generated, I will review later.