Repository navigation
Conversation
This was referenced Oct 29, 2026
igitur
force-pushed
the
callable-bond-grid-time-lookup
branch
from
October 29, 2026 15:48
0ec69b9 to
b1968b9
Compare
Owner
|
Thanks. I was also thinking about the |
preAdjustValuesImpl() and postAdjustValuesImpl() test every coupon and every callability against the current grid time at each step of the rollback, and each test runs TimeGrid::index() twice (once through isOnTime()). The grid does not change while a method rolls back, so the grid time of an event can be resolved on the first adjustment and reused. The cache is dropped when the lattice changes, or when the same lattice object is rebuilt on a different grid: the prices and thresholds are unaffected by that, but the node values are, so the memoised times cannot be trusted across a rebuild. Instruction counts on the benchmark workload (callgrind, deterministic): 35,513,641,371 -> 33,361,076,814, 6.1% fewer. The wall-clock difference is inside this machine's run-to-run spread at these timings. Values are bit-identical.
igitur
force-pushed
the
callable-bond-grid-time-lookup
branch
from
October 29, 2026 16:47
b1968b9 to
4686586
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.
Follow-up to #2962, split out as asked.
DiscretizedCallableFixedRateBond::preAdjustValuesImpl()andpostAdjustValuesImpl()run at every step of the rollback over every coupon and every callability, and each test goes throughisOnTime(), which runsTimeGrid::index()— a binary search — and then compares. Every event therefore costs a search at every step, for a grid time that cannot change while the method rolls back.Change
Resolve the grid time of each event once and reuse it. The cache is dropped when the lattice object changes, and also when the same lattice object is rebuilt on a different grid: prices and thresholds are unaffected by a rebuild, but the node values are, so the memoised times cannot be trusted across one.
What it is worth
Instruction counts (callgrind, deterministic) on the benchmark workload: 35,513,641,371 → 33,361,076,814, 6.1% fewer. That comparison is against #2972 alone, so it isolates this change. For scale, master is 61,328,202,039 on the same run, so #2972 plus this is 45.6% below master.
Wall clock on this machine (2 vCPU, medians of three rounds) does not separate it from noise:
Run-to-run spread here is ±10–15% at these timings — one round of the combined build came in at 993 ms — so read the wall clock as "no measurable change either way" and the instruction count as the signal. An earlier single round suggested ~10% for this change; with three-round medians that does not survive.
Values are bit-identical on the same 12-configuration harness as #2972.
For discussion
This is the half of #2962 I was least sure of, and the measurement agrees: it buys 6% of instructions at the cost of an invalidation rule, and every step still walks all coupons and callabilities — about 480 events in the 30y monthly case — testing each against the current time. Those times are sorted, so a cursor per event list (advance while
time <= t) would remove the scans rather than make them cheap, and would not need the memo bookkeeping at all.If you would rather have that shape, or would rather drop this and keep only the fitting-parameter change, say which and I will rework it.