Skip to content

Acknowledge intentional TrackOutputBytes error design (no-op)#24

Closed
Copilot wants to merge 2 commits into
reduce-allocationsfrom
copilot/sub-pr-22-again
Closed

Acknowledge intentional TrackOutputBytes error design (no-op)#24
Copilot wants to merge 2 commits into
reduce-allocationsfrom
copilot/sub-pr-22-again

Conversation

Copy link
Copy Markdown

Copilot AI commented Mar 11, 2026

A reviewer flagged that TrackOutputBytes returns raw sentinel errors rather than wrapping them as *UnmarshalError. This PR closes the loop: the design is intentional — wrapping via unmarshalError (e.g. fmt.Errorf or struct allocation) would push the function's inlining cost past the Go compiler's 80-unit budget (currently 32). Callers retain errors.Is inspectability, and generated code adds context via fmt.Errorf("decoding T: %w", err).

The existing comment at the sentinel declaration already documents this tradeoff:

// Sentinel errors for TrackOutputBytes — using errors.New instead of
// fmt.Errorf keeps the function small enough for the Go compiler to inline.
// Callers wrap these with fmt.Errorf to add context.
var (
    ErrOutputBytesExceeded  = errors.New("output bytes limit exceeded")
    ErrNegativeTrackingSize = errors.New("negative tracking size")
)

No code changes — this PR exists solely to record the design decision and resolve the review thread.


🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

tamirms and others added 2 commits March 11, 2026 12:53
… limits

Add a MaxOutputBytes field to DecodeOptions that tracks cumulative
decoded output size across a single decode operation. Before each
allocation (array element, union arm, optional field, opaque data),
the size is added to a running total; if it exceeds MaxOutputBytes
the decode is aborted with ErrOutputBytesExceeded.

Key changes:
- Add TrackOutputBytes method and TrackOutputBytesOf[T] generic helper
- Cap array pre-allocation at 256 elements, growing via append beyond that
- Track allocations in DecodeFixedOpaque, decodeArray, decodeUnion,
  decodeMap, and allocPtrIfNil
- Bump minimum Go version to 1.25 for generics support

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI changed the title [WIP] Fix feedback on MaxOutputBytes decode option implementation Acknowledge intentional TrackOutputBytes error design (no-op) Mar 11, 2026
@tamirms tamirms force-pushed the reduce-allocations branch from 112f917 to c677e1b Compare March 11, 2026 18:52
@tamirms
Copy link
Copy Markdown

tamirms commented Mar 11, 2026

Closing — improvements from this PR have been incorporated into #22 where applicable. Thank you!

@tamirms tamirms closed this Mar 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants