Skip to content

perf(table): reuse field ID scratch in files metadata - #1919

Open
fallintoplace wants to merge 1 commit into
apache:mainfrom
fallintoplace:perf/reuse-inspect-file-field-ids
Open

perf(table): reuse field ID scratch in files metadata#1919
fallintoplace wants to merge 1 commit into
apache:mainfrom
fallintoplace:perf/reuse-inspect-file-field-ids

Conversation

@fallintoplace

Copy link
Copy Markdown
Contributor

What changed

  • Reuse one []int scratch slice while sorting field IDs for the six stats maps written by the files metadata table.
  • Keep deterministic sorted keys and the existing validation.

Benchmark

  • Machine: Apple M1 Pro, darwin/arm64
  • Go: go1.26.3
  • Command: go test ./table -run '^$' -bench '^BenchmarkInspectContentFileAppenderDataFileStats/files=(4096|16384)/stats=(32|128)/(borrowed|public)$' -benchmem -count=5 -benchtime=1s
  • Representative borrowed-data results (median of 5 runs):
case ns/op B/op allocs/op
4096 files / 32 stats 42,162,059 -> 40,548,478 49,853,388 -> 43,561,937 (12.6% less) 54,222 -> 29,646 (45.3% fewer)
16384 files / 128 stats 707,448,646 -> 727,017,084 773,116,304 -> 672,455,176 (13.0% less) 312,525 -> 214,226 (31.5% fewer)

Runtime varied between runs, while the allocation reduction was consistent.

@zeroshade zeroshade left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I found no code issues in the current change, but this head now conflicts with main after #1924. Please rebase before this is approved.

When resolving table/inspect_files.go, please preserve both #1924's borrowed collection access and this PR's shared scratch slice:

columnSizes, keyMetadata, splitOffsets, equalityFieldIDs := dataFileCollections(file)
if err := appendInspectInt64Map(b.columnSizes, columnSizes, &b.mapFieldIDs); err != nil {

I tested that combined resolution with the focused race tests repeated 100 times and the full table package; both passed. I'll re-review the resulting SHA after the rebase.


This review was drafted by an AI-assisted tool and confirmed by an Iceberg Go maintainer. After you've rebased and pushed an update, an Iceberg Go maintainer — a real person — will take the next look at the PR. The review applies the project's review criteria; if you think it is mis-applied, please reply on the PR and a maintainer will weigh in.

More on how Iceberg Go handles maintainer review:
CONTRIBUTING.md.

@fallintoplace
fallintoplace force-pushed the perf/reuse-inspect-file-field-ids branch from a5b7b1b to 3293535 Compare August 27, 2026 22:46
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