Faster in-place protobuf decode + codegen stale-cache fix#37
Merged
Conversation
FSharpGrpc_GenerateProto declared Inputs="@(Protobuf)" but not the codegen tool DLL. MSBuild skipped the target whenever the manifest output was newer than the proto inputs, so a rebuilt generator with unchanged .proto files never re-ran — downstream projects silently compiled stale obj/protos/*.generated.fs until obj/ was cleared. - Add $(_FSharpGrpc_CodegenDll) to the target Inputs so a rebuilt or upgraded generator invalidates the cached manifest automatically. - Add _FSharpGrpc_ForceCodegen pre-target that deletes the manifest when -p:FSharpGrpc_ForceCodegen=true, forcing unconditional regen (delete-the-output, since Inputs-without-Outputs is skipped by MSBuild, not run). This is the trap that made the #18/#35 benchmark un-measurable: the "AFTER" run compiled cached "BEFORE" code. Fixes #36. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The decoder read every nested message, packed field, and map entry by copying its bytes into a fresh ByteString and wrapping a second CodedInputStream (`input.ReadBytes().CreateCodedInput()`) — one byte[] + stream allocation per sub-message. For message-heavy payloads this was the dominant decode allocation (the UserProfile "Address sub-decode" hotspot from #18), independent of the list-vs-array surface type. Read sub-regions in place instead. `PushLimit`/`PopLimit` are internal in Google.Protobuf, and our records aren't `IMessage` (that would need a MessageDescriptor — runtime machinery this codegen avoids), so bound the field loop by the public `CodedInputStream.Position`: decodeFrom (input) (endPos: int64) = while input.Position < endPos do match GetTagFieldNumber(input.ReadTag()) with ... // nested: no copy, no second stream let endPos = int64 (input.ReadLength()) + input.Position Address.decodeFrom input endPos `int64 (ReadLength()) + Position` ordering is load-bearing: ReadLength advances past the prefix before Position is read. WKT/wrapper fields use the public `input.ReadMessage` (they are IMessage). `list`/`option` surface types are unchanged. decodeFrom gains an `endPos` parameter (top-level `decode` passes the buffer length). Hand-written generated code in the test projects and all snapshots updated to match. Cross-language byte-exact tests (F#<->C#) and gRPC interop tests stay green, proving wire compatibility. Re #18. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add HighCardinalityUserProfileBenchmarks (1000 repeated ints + 100 nested messages) — the case that exercises the sub-message decode path, where the in-place reader's win over the old buffer-copy is visible (F# decode 109.27 KB -> 82.3 KB, 15.8 us -> 12.7 us). Doubles as a regression guard against the decode allocation regressing. Refresh all README benchmark tables from a single confirmed-fresh Release run and add the high-cardinality row. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Three related commits, intended for rebase-merge so each lands on
mainindependently.1. Fix codegen stale-cache trap (#36)
FSharpGrpc_GenerateProtodeclaredInputs="@(Protobuf)"but not the codegen tool DLL, so a rebuilt generator with unchanged.protofiles never re-ran — downstream projects silently compiled staleobj/protos/*.generated.fs.$(_FSharpGrpc_CodegenDll)to the targetInputsso a rebuilt/upgraded generator invalidates the cached manifest._FSharpGrpc_ForceCodegenpre-target:-p:FSharpGrpc_ForceCodegen=truedeletes the manifest to force unconditional regeneration.This is the trap that made the original #18/#35 benchmark un-measurable — the "AFTER" run compiled cached "BEFORE" code.
2. Decode sub-messages in place (#18)
The decoder copied every nested message, packed field, and map entry into a fresh
ByteString+ secondCodedInputStream(input.ReadBytes().CreateCodedInput()) — onebyte[]+ stream allocation per sub-message, the dominant decode cost on message-heavy payloads.PushLimit/PopLimitareinternalin Google.Protobuf, and our records aren'tIMessage(that would need aMessageDescriptor— runtime machinery this codegen avoids). So bound the field loop by the publicCodedInputStream.Positioninstead:WKT/wrapper fields use the public
input.ReadMessage(they areIMessage).list/optionsurface types are unchanged.decodeFromgains anendPosparameter (top-leveldecodepasses the buffer length); hand-written generated code in the test projects and all snapshots updated to match.3. Benchmark + README
Add
HighCardinalityUserProfileBenchmarks(the case that stresses sub-message decode) and refresh README numbers from a confirmed-fresh Release run.Results (F# decode, before → after)
The win comes from the decoder implementation alone —
list/optionkept as-is. The residual F#-vs-C# gap at high cardinality is now almost entirely thelistcons cells +optionboxing (the immutability surface), which would only close with a breakinglist → arraychange — deliberately deferred.Tests
146/146 pass, including the 61 cross-language byte-exact (F#↔C#) and 10 gRPC interop tests that prove the new reader is wire-identical. F# formatting and C# formatting clean.
🤖 Generated with Claude Code