Skip to content

perf(codec): precompute DataFile Avro field indexes - #1962

Open
fallintoplace wants to merge 1 commit into
apache:mainfrom
fallintoplace:perf/precompute-datafile-avro-indexes
Open

perf(codec): precompute DataFile Avro field indexes#1962
fallintoplace wants to merge 1 commit into
apache:mainfrom
fallintoplace:perf/precompute-datafile-avro-indexes

Conversation

@fallintoplace

Copy link
Copy Markdown
Contributor

What changed

  • Precompute the indexes of dataFile fields carrying an avro tag once during package initialization.
  • Reuse those indexes when cloning a DataFile for MarshalAvroEntry.
  • Keep tag discovery reflection-based so dataFile remains the single source of truth for the wire shape.
  • Add a coverage test and focused benchmarks.

Why

MarshalAvroEntry needs a shallow copy because dataFile contains synchronization state. The old clone helper scanned every struct field and looked up its Avro tag for every encoded manifest entry. The new helper keeps the same copy semantics and removes that repeated metadata lookup.

Benchmark

Apple M1 Pro, Go 1.26.3, -cpu=1, median of 8 runs.

Benchmark Before After Change
CloneDataFileAvroFields 2.00 us 0.46 us -77%
MarshalAvroEntry/v1 18.02 us 15.88 us -12%
MarshalAvroEntry/v2 17.53 us 15.92 us -9%
MarshalAvroEntry/v3 18.51 us 15.77 us -15%

Allocation counts and bytes per operation are unchanged:

  • Clone: 320 B/op, 1 alloc/op
  • Marshal v1: 13,376 B/op, 144 allocs/op
  • Marshal v2/v3: 13,360 B/op, 144 allocs/op

Command:

go test . -run '^$' -bench '^(BenchmarkCloneDataFileAvroFields|BenchmarkMarshalAvroEntry)$' -benchmem -benchtime=500ms -count=8 -cpu=1

Testing

  • go test .
  • go test -race .
  • go vet .
  • golangci-lint run --timeout=10m
  • All packages other than the metadata-endpoint-dependent io/gocloud package pass with go test.

Signed-off-by: Minh Vu <vuhoangminh97@gmail.com>
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.

1 participant