Skip to content

[9.4](backport #50020) perf: optimize FileMetaReader hot path and EncoderReader map sizing#50257

Merged
mauri870 merged 4 commits into
9.4from
mergify/bp/9.4/pr-50020
May 18, 2026
Merged

[9.4](backport #50020) perf: optimize FileMetaReader hot path and EncoderReader map sizing#50257
mauri870 merged 4 commits into
9.4from
mergify/bp/9.4/pr-50020

Conversation

@mergify
Copy link
Copy Markdown
Contributor

@mergify mergify Bot commented Apr 22, 2026

~20% faster filestream reader pipeline, 29% fewer allocations per file, benefits all inputs using EncoderReader.

BenchmarkFilestream exercises the full filestream reader chain: file I/O → line buffering → UTF-8 encoding → newline stripping → file metadata enrichment → parsers → line limiting → ToEvent() → cursor tracking → publish to pipeline. This measures the complete input-side cost of reading and enriching log lines (no output serialization or network I/O).

Commit 1: FileMetaReader — avoid DeepUpdate, cache metadata

Replace DeepUpdate (recursive map merge) and dotted-key Put with direct map assignment. Lazy-cache per-file metadata (path, device_id, inode, fingerprint) on first Next() call since these values are constant for a file's lifetime. The cached interface{} values are copied via maps.Copy without re-boxing, which is where the alloc savings come from.

                                              │    main        │       commit 1 only        │
  sec/op                                        11.840m ± 2%     9.655m ± 6%  -18.5% (p=0.002)
  allocs/op                                      170.5k ± 0%     120.5k ± 0%  -29.3% (p=0.002)
  B/op                                           15.84Mi ± 0%    15.16Mi ± 0%  -4.3% (p=0.002)

Commit 2: EncoderReader — pre-size Fields map

Change mapstr.M{} (capacity 0) to make(mapstr.M, 1) so the first downstream write doesn't trigger a map grow. Benefits all inputs using EncoderReader: filestream, legacy log, and S3.

                                              │    main        │      both commits          │
  sec/op                                        11.840m ± 2%     9.541m ± 5%  -19.4% (p=0.002)
  allocs/op                                      170.5k ± 0%     120.5k ± 0%  -29.3% (p=0.002)
  B/op                                           15.84Mi ± 0%    15.16Mi ± 0%  -4.3% (p=0.002)

Safety analysis

  • Thread safety: FileMetaReader runs in a single harvester goroutine — no concurrent access to cachedMeta.
  • No reference escapes: Each event gets its own fileMap via make + maps.Copy. Downstream code (LimitReader, parsers, ToEvent, publishers) can freely mutate the per-event map without corrupting the cache.
  • Immutable values: All cached values are Go strings (immutable). Copying the interface{} wrappers avoids re-boxing.
  • Values are truly constant: fi is captured at file-open time; GetOSState() returns a stored struct. path and fingerprint are constructor args.
  • Error path: cachedMeta is built into a local variable and only assigned on success. A failed setFileSystemMetadata leaves cachedMeta nil so the next call retries — identical behavior to the original per-call code.
  • Output schema unchanged: The nested structure log.file.device_id is identical — only the internal construction path changed from dotted-key traversal to direct map assignment.
  • make(mapstr.M, 1) is a drop-in replacement for mapstr.M{} — both produce an empty, non-nil map. The only difference is the capacity hint. len(), == nil, and all standard operations behave identically.
Raw benchmark data (benchstat, 6 runs each, Apple M2 Pro)

BenchmarkFilestream — full pipeline, 10k lines

                                           │     main       │       commit 1 only        │        both commits         │
                                           │     sec/op     │   sec/op     vs base       │   sec/op     vs base        │
Filestream/single_file/inode_throughput-10    11.840m ± 2%    9.655m ± 6%  -18.45%         9.541m ± 5%  -19.42%
                                           │     B/op       │                             │                             │
                                             15.84Mi ± 0%   15.16Mi ± 0%  -4.34%         15.16Mi ± 0%  -4.34%
                                           │   allocs/op    │                             │                             │
                                              170.5k ± 0%    120.5k ± 0%  -29.32%         120.5k ± 0%  -29.32%

All comparisons p=0.002 n=6.

BenchmarkFileMetaReaderNext — isolated FileMetaReader

                                       │   main         │          this PR              │
FileMetaReaderNext/no-fingerprint-10      587.0n ± 1%     370.5n ± 21%  -36.9% (p=0.002)
FileMetaReaderNext/with-fingerprint-10    652.6n ± 1%     384.0n ±  8%  -41.2% (p=0.002)
geomean                                   619.0n          377.2n        -39.1%

allocs/op: 11 → 7 (-36%). B/op: 1072 → 1016 (-5%).

BenchmarkEncoderReader — commit 2 improvement (benefits all inputs)

                                    │   main     │   make(mapstr.M, 1)        │
EncoderReader/long_lines-10           867.8µ ± 8%  815.7µ ± 3%  -6.0% (p=0.002)
EncoderReader/skip_lines-10           101.3µ ± 2%   93.7µ ± 2%  -7.5% (p=0.002)
EncoderReader/short_lines-10          10.91µ ± 4%  10.89µ ± 4%       ~ (p=0.589)
EncoderReader/buffer-sized_lines-10   57.10µ ± 3%  56.19µ ± 2%       ~ (p=0.394)
geomean                               86.01µ       82.69µ       -3.9%

Test plan

  • Existing TestMetaFields / TestMetaFieldsOwnerAndGroup pass (updated for new map structure)
  • New BenchmarkFileMetaReaderNext added for regression detection
  • Windows cross-compilation verified (GOOS=windows GOARCH=amd64 go build)
  • CI: Linux + Windows unit tests
  • CI: integration tests

🤖 Generated with Claude Code


This is an automatic backport of pull request #50020 done by Mergify.

…50020)

FileMetaReader.Next() is called for every line harvested by filebeat.

Replace DeepUpdate (recursive map merge) and dotted-key Put
("log.file.device_id") with direct map assignment. Lazy-cache the
per-file metadata (path, device_id, inode, fingerprint) on first call
since these values are constant for the file's lifetime.

The cached interface{} values are copied via maps.Copy without
re-boxing, which is where the alloc savings come from. The cache is
built into a local variable and only assigned on success, so a failed
setFileSystemMetadata retries on the next call.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* perf: pre-size Fields map in EncoderReader to avoid grow on first write

EncoderReader.Next() creates a Fields map for every line. Changing
from mapstr.M{} (capacity 0) to make(mapstr.M, 1) avoids a runtime
map grow when the first downstream consumer writes into it. Benefits
all inputs using EncoderReader: filestream, legacy log, and S3.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: add changelog fragment and resolve lint issues

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore: remove changelog fragment, using skip-changelog label

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test: use require.IsType and consolidate benchmark into metafields_test.go

Replace bare type assertion + require.True(ok) patterns with require.IsType
in metafields_other_test.go and metafields_windows_test.go. Move benchmark
functions from metafields_bench_test.go into metafields_test.go and delete
the separate bench file, following the one-test-file-per-processor principle.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: use comma-ok type assertions to satisfy errcheck linter

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* perf: exact cachedMeta pre-sizing via platformFileFields constant

Add a platformFileFields constant to each platform file (2 on non-Windows,
3 on Windows) so metafields.go can compute the exact map capacity without
over-allocating. Add TestCachedMetaSizing on both platforms to verify the
pre-allocation matches the actual entry count. Also document that the direct
log key assignment is intentional, and fix TestMetaFieldsOwnerAndGroup to
use the platform-correct group name (wheel on macOS, root on Linux).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix testifylint: use require.Len instead of len() comparison

The linter requires require.Len(t, x, n) instead of
require.Equal(t, n, len(x)).

Assisted-By: Cursor
Made-with: Cursor

* fix testifylint: use require.Len in windows test file

Same fix as previous commit, but for the Windows-specific test.

Assisted-By: Cursor
Made-with: Cursor

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
(cherry picked from commit 5f5774b)
@mergify mergify Bot added the backport label Apr 22, 2026
@mergify mergify Bot requested a review from a team as a code owner April 22, 2026 12:39
@mergify mergify Bot requested review from mauri870 and rdner and removed request for a team April 22, 2026 12:39
@botelastic botelastic Bot added the needs_team Indicates that the issue/PR needs a Team:* label label Apr 22, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🤖 GitHub comments

Just comment with:

  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)
  • /test : Run the Buildkite pipeline.

@github-actions github-actions Bot added Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team skip-changelog labels Apr 22, 2026
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

@botelastic botelastic Bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Apr 22, 2026
@mauri870 mauri870 enabled auto-merge (squash) May 15, 2026 12:12
@github-actions

This comment has been minimized.

@mauri870
Copy link
Copy Markdown
Member

/test

@github-actions
Copy link
Copy Markdown
Contributor

TL;DR

Libbeat: Run check/update fails due to a compile-time type reference error in libbeat/publisher/pipeline/client_test.go:220: the test closure uses queue.Entry, but that type does not exist in libbeat/publisher/queue/queue.go. Update that argument type to publisher.Event.

Remediation

  • Change libbeat/publisher/pipeline/client_test.go:220 from:
    publish: func(_ bool, event queue.Entry) (queue.EntryID, bool) {
    to:
    publish: func(_ bool, event publisher.Event) (queue.EntryID, bool) {
  • Re-run the failing Buildkite step (make -C libbeat check update && make check-no-changes) and confirm go vet ./... passes.
Investigation details

Root Cause

go vet fails in libbeat/publisher/pipeline because queue.Entry is referenced in a test, but queue defines EntryID and generic interfaces (Batch[T].Entry(i int) T) and has no Entry type.

  • Failing symbol: libbeat/publisher/pipeline/client_test.go:220
  • Missing type definition: libbeat/publisher/queue/queue.go (no type Entry ...)

This is a code bug (type mismatch/invalid symbol), not infrastructure.

Evidence

# github.com/elastic/beats/v7/libbeat/publisher/pipeline
vet: publisher/pipeline/client_test.go:220:38: undefined: queue.Entry
Error: failed running go vet, please fix the issues reported: running "go vet ./..." failed with exit code 1
make: *** [scripts/Makefile:141: check] Error 1

Verification

  • Not run locally in this workflow; diagnosis is based on the Buildkite failure log plus source inspection of the PR head revision.

Follow-up

If there are other backported test hunks around pipeline client race fixes, quickly grep for additional queue.Entry references and convert them to publisher.Event for consistency.


What is this? | From workflow: PR Buildkite Detective

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants