perf(match_order): bound MatchResult pre-alloc by fill count (0.8.3)#107
Merged
Conversation
…el depth match_order pre-sized the MatchResult trades / filled_order_ids buffers to the whole level depth (order_count), so a qty-1 taker against a deep level reserved ~176 B x depth (multi-MB on a 100k-deep level) and freed it at the end of the match -- pure transient allocator pressure, not a leak. Bound the pre-size by min(incoming_quantity, order_count): each emitted trade consumes >= 1 unit of the taker, so the number of fills is at most incoming_quantity, and at most order_count. Capacity stays advisory (both Vecs still grow if a concurrent add_order lands mid-sweep). Matching semantics, FIFO/price-time priority, and trade output are unchanged. Tests assert the trade-buffer capacity is bounded by incoming quantity (qty-1 vs a 200-deep level) and by order count (huge taker vs a 3-maker level).
Bump 0.8.2 -> 0.8.3 and document the match_order pre-allocation fix so OrderBook-rs can bump the dependency (its alloc_count bytes_alloc/op drops back to the low-KB range).
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
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.
Closes #106.
Problem
PriceLevel::match_orderpre-sized theMatchResultbuffers (trades,filled_order_ids) to the whole level depth (self.order_count()), then freed the unused capacity at the end of the match. On a deep level a qty-1 taker still reserved ~176 B × depth — a multi-MB transient allocation per match (17.6 MB for a qty-1 match against a 100k-deep level). It is not a leak (allocs ≈ deallocs,allocs/opunchanged), which is why the downstream OrderBook-rs alloc-budget guard (asserts onallocs/op, not bytes) didn't catch it — itsbytes_alloc/opballooned to ~790 KB.Fix (
src/price_level/level.rs)Tight upper bound on the number of fills: every emitted trade consumes ≥1 unit of the taker (a
consumed == 0maker is set aside without a trade), so#trades ≤ incoming_quantity, and trivially≤ order_count. Capacity stays advisory — bothVecs still grow if a concurrentadd_orderlands mid-sweep. Computed once before the loop, so no new hot-loop branch; strictly fewer/smaller allocations.min(1, depth) = 1(KB, not MB).Also updates the adjoining comment and the now-stale sentence in
MatchResult::with_capacity's doc.No behavior change
Capacity is a hint only. Trade contents/order, FIFO/price-time priority, counters, and
MatchResultfield agreement are all unchanged. The full matching / FOK / iceberg-reserve correctness suite stays green (regression guard).Tests (
src/price_level/tests/level.rs)test_match_order_capacity_bounded_by_incoming_quantity— qty-1 match against a 200-deep level:result.trades().as_vec().capacity() <= 1(pre-fix: 200), exactly one fill.test_match_order_capacity_bounded_by_order_count— taker of qty 10 000 against a 3-maker level: capacity<= 3(theminpicks depth), all three filled.Verification
make pre-pushclean: 417 lib tests (415 + 2 new), 9 proptests, 1 integration, 9 doctests; clippy + fmt + rustdoc-D warningsall green.make run-examples); benches compile and run (cargo bench).README.mddiff; no public API / wire-format change → nolib.rs/prelude.rs/ migration-guide change.Ships as 0.8.3 (CHANGELOG entry added) so OrderBook-rs can bump and its
bytes_alloc/opreturns to the low-KB range.