apd: avoid function calls on hot paths#112
Conversation
This commit adds a fast-path for the common case in `Context.goError` that avoids the function call to `Condition.GoError`. `Context.goError` is inlined in callers, so this avoids any function call in the common case. The commit also adds two addtional `gcassert:inline` directives.
yuzefovich
left a comment
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @nvanbenschoten)
-- commits, line 15 at r2:
nit: probably s/conditionally/conditional/.
round.go, line 70 at r2 (raw file):
if disableIfPrecisionZero && c.Precision == 0 { // Rounding has been disabled. res |= d.setExponent(c, nd, res, int64(d.Exponent))
nit: why do we perform logical OR and not just return d.setExponent(c, nd, 0, int64(d.Exponent))?
|
This change is |
This commit reworks the call to `Context.round` to allow for mid-stack function inlining. To do this, we remove the conditional call to one of two functions, which is considered too complex to inline.
23eba28 to
c932774
Compare
nvb
left a comment
There was a problem hiding this comment.
TFTR!
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @yuzefovich)
Previously, yuzefovich (Yahor Yuzefovich) wrote…
nit: probably
s/conditionally/conditional/.
Done.
round.go, line 70 at r2 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
nit: why do we perform logical OR and not just
return d.setExponent(c, nd, 0, int64(d.Exponent))?
The idea was to mirror the structure of the other logic in this function more closely and not assume that res is 0 at this point in the code. But I agree that it's unnecessary and not worth the confusion. Changed.
75710: vendor: bump cockroachdb/apd to v3.0.1 r=nvanbenschoten a=nvanbenschoten Picks up cockroachdb/apd#112. Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
This PR includes a pair of minor optimizations that avoid function calls on arithmetic hot paths.
short-circuit in Context.goError
This commit adds a fast-path for the common case in
Context.goErrorthat avoids the function call toCondition.GoError.Context.goErroris inlined in callers, so this avoids any function call in the common case.inline Context.round
This commit reworks the call to
Context.roundto allow for mid-stack function inlining. To do this, we remove the conditional call to one of two functions, which is considered too complex to inline.note: the GDA/add test suite is heavily skewed towards extreme values and edge cases. GDA/subtract is a more representative test suite for common addition and subtraction operations.