Perf T2-5: Avoid unnecessary Jacobian copy#141
Open
hmzh-khn wants to merge 4 commits into
Open
Conversation
Add two new tests to test_jacobian_buffer_safety.jl: 1. Verify similar() and copy() buffers produce identical Jacobian values after jacobian_z! evaluation (same sparse structure, same nonzeros) 2. End-to-end test that solve_raw produces identical results regardless of buffer allocation strategy These tests establish the correctness invariant for replacing copy() with similar() in solve_qp_linear's fallback buffer allocation. Co-Authored-By: Claude Opus 4.6 <[email protected]>
In solve_qp_linear (solve.jl:624), the fallback Jacobian buffer was allocated via copy(result_buffer), which copies both sparse structure and values. Since jacobian_z! fully overwrites all nonzero values on the next line, the value copy is wasted work. Replace with similar(), which preserves the sparse structure (colptr, rowval, dimensions) but leaves nzval uninitialized. This avoids a memcpy of the nonzero values array. Impact: minor per-solve savings on the allocation-only path (when no pre-allocated J_buffer is provided). The hot path with pre-allocated buffers is unaffected. Co-Authored-By: Claude Opus 4.6 <[email protected]>
…zation Full test suite: 1244/1246 pass (2 pre-existing failures in test_nonlinear_solver_options.jl, unrelated to this change). All 26 Jacobian Buffer Safety tests pass. Benchmark: similar() saves 10,800 bytes (73% reduction) in nzval allocation per fallback buffer creation vs copy(). Timing similar at ~140-150ns for this small (64x64, 116 nnz) matrix. Co-Authored-By: Claude Opus 4.6 <[email protected]>
The test created J_copy and J_sim but called solve_raw() without passing them, so both calls used identical internal buffer allocation. Now calls solve_qp_linear() directly with J_buffer kwarg to test the actual copy() vs similar() difference. Co-Authored-By: Claude Opus 4.6 <[email protected]>
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
2 participants
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.
Summary
Replace
copy(result_buffer)withsimilar(result_buffer)when allocating the Jacobian buffer in the fallback path. Sincejacobian_z!fully overwrites all nonzero values before reading, the initial values are irrelevant —similar()avoids copying thenzvalarray.Changes
src/solve.jl: Replacecopy()withsimilar()for Jacobian buffer allocationtest/: Add tests verifyingsimilar()buffer produces identical Jacobian, and thatjacobian_z!fully overwrites sparse buffer nonzerosBenchmark Results
copy(result_buffer): 14,688 bytessimilar(result_buffer): 3,888 bytesNote: This only affects the fallback path when no pre-allocated buffer is provided. The hot path with pre-allocated buffers is unaffected.
Testing
similar()produces identical results and full overwrite behaviorsolve_qp_linear with similar() buffer) creates local buffers but does not pass them intosolve_raw, so it tests two identical code paths rather than the actual change. The core safety guarantee comes from the unit-level overwrite verification test.Changelog
🤖 Generated with Claude Code