Code inlining - #192
Open
rnro wants to merge 6 commits into
Open
Code inlining#192rnro wants to merge 6 commits into
rnro wants to merge 6 commits into
Conversation
`Protector.processHeaderProtection` recomputed the encoded packet number length that writing the header had already stored, so `encode` ran twice per sent packet. Its two diagnostics also sat inline, which is many times the size of the arithmetic the function exists to do and kept it out of line. - Read the recorded packet number length in `processHeaderProtection` rather than recomputing it, halving the calls to `PacketNumber.encode` per sent packet - Added `outlinedLogError`, held out of line with `@inline(never)` and taking a `StaticString` so nothing is interpolated at the call site - Routed `encode`'s two reports through it, leaving the arithmetic small enough to inline into the header builders - Marked `encode` `@inline(always)` so it folds into those builders
Building a log message is many times the size of the arithmetic a small function performs, and that code counts against the inliner's budget whether or not the message is ever emitted. Such a function stops being inlinable, and its callers pay for a diagnostic they never see. - Added `outlinedLogError` and `outlinedLogFault`, taking a `StaticString` and integer values and held out of line with `@inline(never)` - Routed the reports in `variableLengthSize`, `Frame.claim`, `Frame.unclaim`, `Frame.dscpValue` and `Packet.overrideSentNumberSize` through them - Took a `StaticString` rather than an autoclosure so no interpolation is left at the call site - Measured at `-Osize`: `Frame.unclaim` 430 to 65 instructions, `Frame.claim` 338 to 60, `variableLengthSize` 316 to 38, and `overrideSentNumberSize` inlined away - Kept the existing `DisableErrorLogging` guards, so behaviour under that trait is unchanged
The serializer and deserializer each carry a fragmented path for a value that straddles a span boundary. Both were `@inline(always)`, so a copy landed in every caller, and the callers are `writeFixedSize` and `readFixedSize` -- which implement every integer field and, through `decodeVariableLength`, every variable-length read. With a single-span factory that loop cannot run at all. - Held `writeFragmented` and `readFragmented` out of line - Moved the fragmented loops out of the three `span` overloads into `@inline(never)` helpers, leaving the single-span fast path inlinable - Added no call on the single-span path - Measured at `-Osize`: binary `__text` down 62,892 bytes, the inbound STREAM frame parse from 1667 to 648 instructions, `decodeVariableLength` from 353 to 132
`AckBitstring.validate` runs on every received ack, and both it and `validateInitial` reported their guard failures inline, so each carried the logging metadata and once-initialisation in its own instruction footprint. - Routed the six reports in `AckBitstring` through the outlined helpers - Added three-value `outlinedLogFault` and `outlinedLogInfo` overloads to cover them - Corrected a message that described a stop-word failure as a start-word one - Measured at `-Osize`: `validateInitial` folds away entirely and `validate` drops from 720 to 47 instructions
rnro
requested review from
agnosticdev,
ekinnear,
kkuk24,
rpaulo and
tfpauly
as code owners
October 1, 2026 16:07
tfpauly
reviewed
Oct 1, 2026
| @available(macOS 11, iOS 14, tvOS 14, watchOS 7, *) | ||
| @inline(never) | ||
| func outlinedLogError(_ message: StaticString) { | ||
| Logger.proto.error("\(message)") |
Collaborator
There was a problem hiding this comment.
Are these top-level functions? If so, having them all default to being in the "proto" category seems surprising and quite possibly incorrect for some uses.
Contributor
Author
There was a problem hiding this comment.
All of the converted call-sites used Logger.proto.* - I'll rename the outlined methods to include Proto in the name.
agnosticdev
reviewed
Oct 2, 2026
agnosticdev
left a comment
agnosticdev
left a comment
Collaborator
There was a problem hiding this comment.
Can you do a CPU measurement too with QUICTransfer and make sure you are not seeing an increase in CPU usage on Deserializer / Serializer functions?
| packetNumberLength = length | ||
| } else { | ||
| // The packet number would not have been written if it doesn't encode | ||
| packetNumberLength = try! packet.number.encode(lastAcked: packet.lastAcked).size |
| /// Call this method when `hasRoom` fails but `internalResult` is still valid. | ||
| @inlinable | ||
| @inline(always) | ||
| @inline(never) |
|
|
||
| /// Compares across span boundaries, refilling from the factory as each span is exhausted. | ||
| @usableFromInline | ||
| @inline(never) |
Collaborator
There was a problem hiding this comment.
Why inlined never here if the calling function is inlined?
| @inline(never) | ||
| func outlinedProtoLogError<Value: FixedWidthInteger>(_ message: StaticString, _ value: Value) { | ||
| Logger.proto.error("\(message): \(value)") | ||
| } |
Collaborator
There was a problem hiding this comment.
We'll need a way to call DisableErrorLogging on these too. Same with DisableDebugLogging
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
3 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.
Several hot-path functions were too large to inline because a cold diagnostic or error path sat inline in the body. Outlining that code explicitly lets them inline again, and unlocks further optimization.
This PR:
PacketNumber.encodecall per sent packet, which writing the header had already performedoutlinedLogErrorandoutlinedLogFault, and routes the hot-path diagnostics through themResult:
__text5,011,508 → 4,947,960 - −63,548 bytes (−1.27%)PacketNumber.encode,Packet.overrideSentNumberSize,AckBitstring.validateInitialare now inlined away