Code refactoring#5351
Merged
crocodile-dentist merged 9 commits intoApr 13, 2026
Merged
Conversation
coot
force-pushed
the
coot/mw/mux-single-peer-performance
branch
from
5cb25f2 to
b4fd323
Compare
There was a problem hiding this comment.
Pull request overview
Refactors the mux egress burst batching logic to improve readability/maintainability by introducing clearer intermediate types and extracting token/SDU sizing logic into named helpers, while preserving the overall scheduling approach (weighted queues, batching, optional bursting).
Changes:
- Introduces
TokenSize,NextSDUSize,CanBurst, andSDUWithWantonStateto make burst state and sizing decisions explicit. - Reworks
buildBatchinto a structured loop (burstLoop) with extracted helpers (computeSDUSize,consumedTokens,nextSDUSizeToSDUSize). - Updates
processSingleWantonto returnSDUWithWantonStateinstead ofEither, and updates call sites accordingly.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case goAgain of | ||
| ContinueBurst | weight > 1 -> do | ||
| let egressQueues' = (pred weight, queue) : rest | ||
| go batch' egressQueues' StopBurst |
There was a problem hiding this comment.
This refactor materially changes the burst batching logic (token sizing, loop termination, and requeueing), but the existing test suite appears not to exercise burst mode (MiniProtocolLimits.burst = Nothing in network-mux/test/Test/Mux.hs). Adding at least one property/unit test that enables burst = Just ProtocolBurst{..} and asserts fairness/no starvation and token-bound sizing would help prevent regressions.
Suggested change
| go batch' egressQueues' StopBurst | |
| go batch' egressQueues' StopBurst | |
| ContinueBurst -> do | |
| let egressQueues' = rest <> [(1, queue)] | |
| go batch' egressQueues' StopBurst |
Contributor
Author
There was a problem hiding this comment.
@crocodile-dentist I'll keep it open, as it's relevant in the original #5322.
coot
force-pushed
the
coot/mw/mux-single-peer-performance
branch
3 times, most recently
from
59ee2a7 to
90c92ec
Compare
* added type signatures * change argument order of `processSingleWanton` * added `SDUWithWantonState` * added bangs to all `go` definitions
Added dedicated API: * `NextSDUSize`, `nextSDUSizeToSDUSize` * `TokenSize` (type alias), `consumedTokens`
An explicit loop is easier to follow. We also avoid calling `error "impossible"`.
Replaced a custom `allM` with `foldMap All . traverse`. In the future we can fuse `foldMap` with `traverse`.
Move constants to top level bindings: * `maxSDUsPerBatch` * `burstMinSdu`
Replaced `Bool` with: * `CanBatch` - can we batch more SDUs from different mini-protocols. * `CanBurst` - can we burst more SDUs from a single mini-protocol.
Use an explicit loop in `muxer`, as a result `transformers` dependency is gone.
Refactor `muxerLoop` It's easier to read `[(EgressQueue m, [(Word8, Egress m)])]`, than the other way around, since then it's the `EgressQueue` we read and its tail much like `x : xs`. This also avoids using `traverse` over tuple which is a bit surprising.
Use DuplicateRecordFields for: * `Wanton` * `TranslocationServiceRequest` * `ProtocolBurst` These are all internal data types, so we won't force it on `network-mux` user.
coot
force-pushed
the
coot/mw/mux-single-peer-performance
branch
from
90c92ec to
20e116a
Compare
crocodile-dentist
merged commit 9714b64
into
mw/mux-single-peer-performance
8 of 18 checks passed
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.
Description
A series of refactoring patches in order to make the burst engine more readable
/ maintainable.
Related to #5322.
Checklist
Quality
Maintenance
ouroboros-networkproject.