Repository navigation
perf(outbox): drop the sort and the copy from the in-memory outstanding count - #4583
Merged
iancooper merged 6 commits intoOct 11, 2026
Conversation
Pin what InMemoryOutbox.GetOutstandingMessageCount returns before changing how it is computed: only undispatched entries at least one interval old are counted, the count stops at maxCount, and a zero maxCount counts nothing. The tests pass on the current code. Each was seen to fail under a temporary mutation of GetOutstandingMessageCount, never committed: - counting dispatched entries too: the interval test fails - comparing WriteTime with < instead of <=: the interval test fails - comparing WriteTime with > or >= instead of <=: the interval and max count tests fail - dropping Take(maxCount): the max count and zero max count tests fail - ageing entries by Message.Header.TimeStamp instead of WriteTime: the interval and max count tests fail - sorting by time stamp and taking maxCount entries before the filter: the max count test fails, as the dispatched entries carry the oldest time stamps - a loop that only checks the cap after counting an entry: the zero max count test fails Part of issue BrighterCommand#4554
The number of matching entries, capped at maxCount, does not depend on the order they are visited in, so the sort by time stamp was wasted work. On every count the OrderBy buffered every entry again, with an array of their time stamps and an index map, and sorted them before the filter could run. Remove the OrderBy. Part of issue BrighterCommand#4554
Requests.Values copies every entry into a new list while it holds all of the dictionary locks, so each count allocated in proportion to the size of the outbox and held up writes to it while it copied. Enumerate the ConcurrentDictionary directly, as RemoveExpiredMessages and Compact already do, so nothing is copied and the walk takes no lock. When the count has to visit every entry, walking the dictionary takes longer than copying it and walking the copy did, but the count no longer allocates more as the outbox grows. The walk is not a point-in-time view, but the copy was not one either: MarkDispatched sets TimeFlushed on the entry in place, outside those locks. The cleanup worker only removes dispatched entries, which the count skips, and the mediator only compares the count with the limit later, so it is out of date by then anyway. Part of issue BrighterCommand#4554
The issue BrighterCommand#4554 entry covers how often the outstanding count runs, but not the count in the InMemoryOutbox, which no longer copies and sorts every entry. Add a paragraph for it and retitle the entry to match. Part of issue BrighterCommand#4554
guilhermeappel
requested review from
DevJonny,
holytshirt,
iancooper and
preardon
as code owners
October 11, 2026 00:31
iancooper
added
2 - In Progress
Bug
.NET
Member
|
Thanks @guilhermeappel |
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
2 participants
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.
Description
This is the last of the three PRs I described on issue #4554, after PR #4564 and PR #4569. Those two cut how often the outstanding count runs. This one makes the count itself cheaper for the
InMemoryOutbox.GetOutstandingMessageCountcopied every entry withRequests.Values, which holds all of the dictionary's locks while it copies, then sorted the copy by time stamp, and only then filtered it and stopped atmaxCount. The order doesn't change how many entries match, so I removed the sort. The count now walks theConcurrentDictionarydirectly, asRemoveExpiredMessagesandCompactalready do, so it copies nothing and the walk takes no lock. It returns the same number as before.Walking the dictionary isn't a point-in-time view, so an entry added, dispatched or deleted during the walk may or may not be counted. The copy wasn't one either, because
MarkDispatchedsetsTimeFlushedon the entry in place, and the count is already out of date by the time the mediator compares it with the limit. The cleanup worker only removes dispatched entries, which the count skips anyway.Related Issues
Fixes #4554. The static semaphore and the missing timeout and cancellation on the count aren't part of this PR. Now that the three PRs are up, I'll open the separate issue I mentioned on PR #4552 for them, and my question on issue #4554 about whether the semaphore is intentional can move there.
Type of Change
Checklist
Additional Notes
The tests come first and pass on master. They pin what the count returns, and the test commit lists the mutations of
GetOutstandingMessageCountI checked them against. Then the sort goes, then the copy, and the release note comes last, as one more paragraph in the issue Each Post sorts the whole in-memory outbox to count outstanding messages, even with no limit set #4554 entry, which I retitled to match.One call of
GetOutstandingMessageCountwith amaxCountof 101 (what the mediator passes with a limit of 100), Release build on net10.0 on my machine. The times move between runs, the allocations don't:At 20,000 entries the old count put arrays on the large object heap: 1,500 calls caused 35 gen2 collections before and none after. Most of the cost was the sort. Compared with the commit that only drops the sort, dropping the copy as well makes a count that has to visit every entry slower: in an earlier run, up to about twice as slow with 2,000 entries and about 5 to 40% slower with 20,000. In exchange the count no longer allocates more as the outbox grows (with only the sort gone, the copy was still 157 KB per call at 20,000 entries, with 5 gen2 collections per 1,500 calls), and writes to the outbox no longer wait for it.
Per
Postthe gain is smaller now that PR fix(outbox): stop the outstanding count checks from piling up #4569 runs the count far less often. WithAddProducers, a limit of 100000, the default interval and one poster, aPostallocated about 21 to 24 KB before and 17 to 18 KB after. With 16 posters at once, or with no limit, there is no real difference.The InMemory suite passes on net10.0 and net9.0 (163 passed on each, 167 once merged with the current master), and so does the Core suite (1707 passed, 7 skipped on each).
Please merge with a merge commit rather than squash, as CONTRIBUTING asks.