perf(assert): compare assert_within_delta in fixed point, not via bc - #979
Merged
Conversation
assert_within_delta called bashunit::math::calculate twice, and each call is a subshell wrapping a `bc` (or `awk`) process. Four forks per assertion on a per-assertion path: 200 calls took 1092ms where the fork-free floor is ~108ms. They now take 165ms. The comparison is |expected - actual| <= delta, which needs no floating point at all. All three operands are padded to one decimal scale and compared as integers, in pure bash. The fast path is deliberately narrow and refuses what it cannot represent exactly -- exponent notation, a sign anywhere but the front, or enough digits to risk 64-bit overflow -- so those fall through to the existing bc/awk chain rather than getting a quietly wrong answer. Both paths were run against the full numeric suite: forcing the fixed-point path to always refuse leaves every test green, which is the check that they agree. This also fixes a bug rather than only moving it. _is_numeric accepts a leading `+`, but bc cannot parse one: `+5 - 5` returned an empty string, which compared unequal to "1", so `assert_within_delta +5 5 1` failed. The sign is now stripped once before either path, so the fallback is fixed too and not just bypassed. bashunit::math::is_le gets the same fast path; it had the identical bc > awk > strip-decimals chain for the same reason. The three helpers return through slots rather than echoing. That is not stylistic here: the caller needs the decimal count three times per assertion, and three `$( )` captures would cost more than the two bc forks the whole change exists to remove. Bash 3.0 safe: parameter expansion, `case` and integer arithmetic only. Fork budgets unchanged; compat gate green; 1661 sequential / 1620 parallel.
Member
Author
|
Reopening to re-trigger CI; no pull_request runs were created. |
Chemaclass
force-pushed
the
perf/fixed-point-numeric-comparison
branch
from
August 6, 2026 19:54
bc0e872 to
9608465
Compare
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.
🤔 Background
assert_within_deltacalledbashunit::math::calculatetwice, and each call is a subshell wrapping abc(orawk) process — four forks per assertion, on a per-assertion path.assert_within_delta×200💡 Changes
The comparison is
|expected - actual| <= delta, which needs no floating point at all. All three operands are padded to one decimal scale and compared as integers, in pure bash.The fast path is deliberately narrow and refuses what it cannot represent exactly — exponent notation, a sign anywhere but the front, or enough digits to risk 64-bit overflow — so those fall through to the existing
bc/awkchain rather than getting a quietly wrong answer.bashunit::math::is_legets the same fast path; it had the identicalbc > awk > strip-decimalschain for the same reason.🐛 Also fixes a bug, rather than moving it
_is_numericaccepts a leading+, butbccannot parse one:+5 - 5returned an empty string, which compared unequal to"1", soassert_within_delta +5 5 1failed. The sign is now stripped once before either path — so the fallback is fixed too, not just bypassed.✅ How agreement was verified
Forcing the fixed-point path to always refuse leaves the entire numeric suite green on the bc/awk path. That's the check that the two paths agree, rather than just that the new one passes.
Slots rather than echoes for the three helpers — not stylistic: the caller needs the decimal count three times per assertion, and three
$( )captures would cost more than the twobcforks this change exists to remove.🔒 Verification
Bash 3.0 safe — parameter expansion,
caseand integer arithmetic only; compat gate green (14/14). Fork budgets unchanged.make sa·make lint·bash build.sh bin -v→✅ Build verified ✅· 1661 sequential / 1620 parallel-simple-strict.