Skip to content

perf: snapshot *io.SectionReader bodies by shallow copy - #12

Merged
knight42 merged 1 commit into
v2from
perf/snapshot-section-reader
Sep 18, 2026
Merged

knight42 merged 1 commit into
v2from
perf/snapshot-section-reader

Conversation

@knight42

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #11. A *io.SectionReader body previously fell through snapshotReader to sectionReaderFor, which Seek-probes the shared body (3 calls per attempt) and wraps it in another SectionReader, making every read go through two indirection layers (view.ReadAt → orig.ReadAt → underlying.ReadAt).

io.SectionReader's only mutable field is the read cursor off — r, base, and limit are constant after creation, and Read goes through s.r.ReadAt(p, s.off). So a shallow struct copy gives each attempt an independent cursor over the same stateless ReaderAt, exactly the bytes.Reader/strings.Reader idiom already in the switch. The copy reads [off, limit), the identical window sectionReaderFor would have constructed — zero probes, zero extra wrapping.

The safety assumption (the underlying ReadAt tolerates concurrent calls) is the io.ReaderAt contract, the same one sectionReaderFor already relies on.

Behavior

  • Framing unchanged: http.NewRequest cannot size a SectionReader, so the body still goes out chunked.
  • sectionReaderFor still handles types that expose ReaderAt+Seeker without being a SectionReader (e.g. *os.File).

Testing

Added a "section reader body" case to the TestRetryBodyNotCorruptedByInFlightUpload table (64 MiB in-flight-upload corruption repro, asserts chunked framing). Full suite passes with -race.

🤖 Generated with Claude Code

A SectionReader body previously fell through to sectionReaderFor, which
Seek-probes the shared body and wraps it in another SectionReader on
every attempt. Only its read cursor is mutable (r, base, limit are
constant after creation), so a shallow struct copy gives each attempt an
independent cursor over the same stateless ReaderAt — same bytes window
([off, limit)), no probes, no extra indirection.

Framing is unchanged: NewRequest cannot size a SectionReader, so the
body still goes out chunked, as the retry table test asserts.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 18, 2026 05:22

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The changes are covered by regression tests with no blocking issues identified.

Pull request overview

Optimizes retry snapshots for *io.SectionReader request bodies while preserving framing and replay behavior.

Changes:

  • Shallow-copies SectionReader state per retry attempt.
  • Adds regression coverage for in-flight uploads and chunked framing.
File summaries
File Description
retry_test.go Tests retry integrity for section-reader bodies.
middleware.go Adds efficient SectionReader snapshot handling.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@knight42
knight42 merged commit 8f1993e into v2 Sep 18, 2026
2 checks passed
@knight42
knight42 deleted the perf/snapshot-section-reader branch September 18, 2026 18:57
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