Repository navigation
Line scanner reuses its buffer across files - #70
Merged
Merged
Conversation
Every line scan allocated a fresh 64 KiB scanner buffer, even for small config files that touch a fraction of it. Line scans (scanFileLines, scanReaderLines and the shell history probe) now borrow the buffer from a sync.Pool and return it when the scan is done. Lines are copied out with scanner.Text before the buffer goes back, and the scanner still grows past 64 KiB up to the same max line size, so long-line behaviour is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SUSTAPLE117
added this pull request to stack #71
October 2, 2026 20:52
SUSTAPLE117
marked this pull request as ready for review
October 2, 2026 20:54
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
Deferred buffer release preserves scan ownership and existing limits, with regression coverage and no unresolved findings.
Review effort: Balanced
Findings: None
What changed in this PR
Reduces allocation overhead in Bagel’s line-based probes by reusing scanner buffers, complementing the detector allocation improvements in #69.
Changes:
- Adds pooled 64 KiB buffers while preserving line-size limits and per-scan ownership.
- Adds allocation, line-limit, and concurrency tests plus a small-file benchmark.
| File | Description |
|---|---|
pkg/probe/shell_history.go |
Uses the pooled scanner for history files. |
pkg/probe/race_on_test.go |
Enables skipping allocation checks under race detection. |
pkg/probe/race_off_test.go |
Enables allocation checks in ordinary test runs. |
pkg/probe/probe.go |
Adds the buffer pool and shared scanner helper. |
pkg/probe/probe_test.go |
Adds regression tests and an allocation benchmark. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
fproulx-boostsecurity
approved these changes
Oct 5, 2026
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.
Stacked on #69.
Summary
Every line scan (
scanFileLines,scanReaderLines, and the shell history probe) allocated a fresh 64 KiBbufio.Scannerbuffer. Most scanned files are small configs and dotfiles that use only a fraction of it.Line scans now take that buffer from a
sync.PoolthroughnewLineScannerand give it back when the scan finishes:maxLineSize. A line over the limit still stops the scan withErrTooLong, which is logged as before.releaseruns, which is deferred to the end of the scan. Every line is copied out withscanner.Text(), so nothing still points into the buffer once it returns to the pool.Tests
TestScanFileLines_SmallFilesDoNotEachAllocateALineBuffer: scanning 200 small files allocates under 16 KiB per file (before: about 67 KiB per file). It is skipped under-race.TestScanReaderLines_MaxLineSize: a line just under the limit is read whole and lines after it are still scanned. A line over the limit stops the scan, keeping the findings before it. This test passed before the change and still passes, which pins the existing behaviour.TestScanFileLines_ConcurrentScansKeepTheirOwnLines: 128 concurrent scans over 32 files. Each scan must return only its own finding, with the right path and line number. Run under-race.Benchmark
BenchmarkScanFileLines_SmallFiles(200 three-line config files per op, default registry, Apple M1 Pro, n=6):Checks
go test ./... -cover,go test -race -count=1 ./...andgolangci-lint run(0 issues) all pass.🤖 Generated with Claude Code