Repository navigation
perf: B-spline curve evaluation into a caller buffer (outputs bit-identical) - #1089
Merged
Merged
Conversation
…ntical) BSplineCurve::evaluate returned a fresh Vec<Vec3> from crv_derivs on every call. On the tangency-labs#948 loft about 180 profile samples are that allocation, almost all from FindCurveDistanceExtrema::evaluate (two evaluations of 3 entries per step of every curve-cache distance solve). - nurbs_crv::crv_derivs_into writes C^(k)(u) into the caller's slice; crv_derivs allocates and calls it. - BSplineCurve::evaluate_into holds the old evaluate body (clamping, zero-tangent repair) over a slice; evaluate allocates and calls it, and keeps NLib's order error for num_derivs >= GW_MAX_DERIV without allocating an output of that size. - Curve::evaluate_into dispatches the B-spline kinds (B-spline, hyperbola, ellipse, parabola) to it; other kinds fall back to evaluate and fill missing entries with Vec3::UNDEF, as at() reads them. - FindCurveDistanceExtrema::evaluate uses two [Vec3; 3] buffers. A new test compares evaluate_into with evaluate bit for bit, errors included, for 0 to GW_MAX_DERIV + 1 derivatives, with and without the zero-tangent repair (one curve has a zero first derivative). Loft driver, 3 interleaved runs, P-cores, medians: user instructions -6.1 % and cycles -4.8 % / -4.6 % (20 / 40 sections); all non-timing output identical. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
bherbruck
approved these changes
Oct 26, 2026
bherbruck
left a comment
Collaborator
There was a problem hiding this comment.
Approved by the automated first-pass review: CI green, mergeable, no CI/build/dependency file changes, no red-flag patterns in added code (process/network/env/unsafe outside FFI/blobs/hidden unicode), 0 known-mismatch entries added. Logic is gated by CI and the difftest merge queue.
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.
In short: every curve evaluation allocated a small list for its result, and the curve tessellation runs millions of them. The hot caller now passes a fixed buffer instead. Results do not change. On the loft boolean of #948, user instructions fall by about 5 % and cycles by 3 to 4 %.
What is wrong
BSplineCurve::evaluatereturns a freshVec<Vec3>thatnurbs_crv::crv_derivsallocates (Vec::with_capacity(der + 1)) on every call. On the #948 loft (20 sections, maind4940b63e) about 180 profile samples are that allocation. Almost all calls come fromFindCurveDistanceExtrema::evaluate(SmFindCurveDistanceExtremaEFO::Evaluate): two evaluations of 3 entries per step of every distance solve in the curve-cache constants. The C++ evaluates into the caller's stack array.Vec<Vec3>per curve evaluation in the distance solve[Vec3; 3]on the stackChange
nurbs_crv::crv_derivs_into(cur, u, side, out)writesC^(k)(u)into the caller's slice;crv_derivsallocates and calls it.BSplineCurve::evaluate_intoholds the oldevaluatebody (clamping, zero-tangent repair) over a slice;evaluateallocates and calls it. Fornum_derivs >= GW_MAX_DERIVit returns NLib's order error as before, without allocating an output of that size.Curve::evaluate_intosends the B-spline kinds (B-spline, hyperbola, ellipse, parabola) to it. Other kinds fall back toevaluateand fill missing entries withVec3::UNDEF, which is whatat()read for them.FindCurveDistanceExtrema::evaluate(incurve_cache.rs) uses two[Vec3; 3]buffers. This is a two-line change incurve_cache.rs. The curve-cache memo (perf: memoise curve caches in the curve value (outputs bit-identical) #992) is already in main and this branch is rebased on it; perf: keep each surface's boundary curves for CoincidenceCheck (outputs bit-identical) #1031 does not touchcurve_cache.rs, andgit merge-treeis clean against it.Test plan
toebox_loft <sections> 1 full(the #948 driver, a local example, not in this PR), release build, mainb206d3f5aagainst this branch, 3 interleaved runs each,taskset -c 0-3,perf stat -e instructions:u,cycles:u,task-clock, medians (load 1 to 3):Profile samples (frame-pointer builds,
perf record -F 1000, 20 sections, 2 runs each):malloc/freeleaves underFindCurveDistanceExtrema::evaluatefall from 12 and 17 to 0 and 0. All non-timing output (counts, volumes and bounding boxes to 17 digits) is identical across all runs of main and the branch, for both inputs.Checks
cargo fmt --all --check: passcargo clippy --workspace --all-targets -- -D warnings: passcargo test -p solidcore: 2275 passed, 0 failed. New test:evaluate_intoequalsevaluatebit for bit, errors included, for 0 toGW_MAX_DERIV + 1derivatives, both sides, with and without the zero-tangent repair, on two curves (one with a zero first derivative, so both repair branches run), and an empty output is an error.cargo check -p solidcore --target wasm32-unknown-unknown: pass--unit, allfailed=0 deferred=0 max_rel_err=0e0:Not run
difftest --green(the CI gate) did not run locally; CI runs it.What follows
b206d3f5a;git merge-treeis clean against the current main4fa6661e2.perf/continuity-evaluate-into(evaluate_continuityinto stack buffers; separate draft).Refs #948.