This is an automated email from the ASF dual-hosted git repository.
laskoviymishka pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-go.git
The following commit(s) were added to refs/heads/main by this push:
new d7ced4a7b perf(table): avoid cloning cached snapshot manifests (#2112)
d7ced4a7b is described below
commit d7ced4a7b27bc456066d38236af97c6b2e4e3ee0
Author: Minh Vu <[email protected]>
AuthorDate: Tue Oct 6 10:27:10 2026 +0200
perf(table): avoid cloning cached snapshot manifests (#2112)
* perf(table): avoid cloning cached snapshot manifests
* test(table): benchmark cached manifest access
* perf(table): document borrowed manifest slice contract
---
table/incremental_append_scan.go | 2 +-
table/inspect.go | 2 +-
table/inspect_all_manifests.go | 2 +-
table/inspect_files.go | 4 ++--
table/inspect_partitions.go | 2 +-
table/inspect_position_deletes.go | 2 +-
table/scanner.go | 2 +-
table/snapshot_manifest_cache.go | 12 ++++++++++
table/snapshot_manifest_cache_bench_test.go | 31 ++++++++++++++++++++++++++
table/snapshot_manifest_cache_internal_test.go | 4 ++++
table/table.go | 2 +-
11 files changed, 56 insertions(+), 9 deletions(-)
diff --git a/table/incremental_append_scan.go b/table/incremental_append_scan.go
index d2e71edb6..cdb0dc565 100644
--- a/table/incremental_append_scan.go
+++ b/table/incremental_append_scan.go
@@ -173,7 +173,7 @@ func (s *IncrementalAppendScan) PlanFiles(ctx
context.Context) ([]FileScanTask,
if err != nil {
return nil, err
}
- for _, manifest := range manifestSet.dataManifests() {
+ for _, manifest := range manifestSet.borrowDataManifests() {
if _, ok := appendSnapshots[manifest.SnapshotID()]; !ok
{
continue
}
diff --git a/table/inspect.go b/table/inspect.go
index 67ff62033..af1ea364e 100644
--- a/table/inspect.go
+++ b/table/inspect.go
@@ -456,7 +456,7 @@ func (i InspectTable) currentSnapshotManifests(ctx
context.Context) ([]iceberg.M
return nil, err
}
- return manifestSet.allManifests(), nil
+ return manifestSet.borrowAllManifests(), nil
}
func appendManifestBound(builder *array.StringBuilder, typ iceberg.Type,
transform iceberg.Transform, bound *[]byte) error {
diff --git a/table/inspect_all_manifests.go b/table/inspect_all_manifests.go
index 15ad1bb9d..144f3fbd9 100644
--- a/table/inspect_all_manifests.go
+++ b/table/inspect_all_manifests.go
@@ -50,7 +50,7 @@ func (i InspectTable) AllManifests(ctx context.Context)
(array.RecordReader, err
return nil, err
}
- return manifestSet.allManifests(), nil
+ return manifestSet.borrowAllManifests(), nil
}
}
diff --git a/table/inspect_files.go b/table/inspect_files.go
index 4705f785f..f326c0a46 100644
--- a/table/inspect_files.go
+++ b/table/inspect_files.go
@@ -140,7 +140,7 @@ func (i InspectTable) manifestEntryReader(
if err != nil {
return nil, err
}
- manifests := manifestSet.allManifests()
+ manifests := manifestSet.borrowAllManifests()
return i.manifestEntryReaderFromManifestSource(
ctx, arrowSchema, fs, discardDeleted, includeManifest,
appendEntry,
@@ -193,7 +193,7 @@ func (i InspectTable) allManifestEntryReader(
return
}
- for _, manifest := range
manifestSet.allManifests() {
+ for _, manifest := range
manifestSet.borrowAllManifests() {
if _, ok := seen[manifest.FilePath()];
ok {
continue
}
diff --git a/table/inspect_partitions.go b/table/inspect_partitions.go
index 65cc753a4..2ae944db9 100644
--- a/table/inspect_partitions.go
+++ b/table/inspect_partitions.go
@@ -154,7 +154,7 @@ func (i InspectTable) partitionAggregates(ctx
context.Context, partitionType *ic
if err != nil {
return nil, err
}
- manifests := manifestSet.allManifests()
+ manifests := manifestSet.borrowAllManifests()
snapshotTimes := make(map[int64]int64, len(i.tbl.metadata.Snapshots()))
for _, snapshot := range i.tbl.metadata.Snapshots() {
diff --git a/table/inspect_position_deletes.go
b/table/inspect_position_deletes.go
index 72ad08c51..e3bbc3977 100644
--- a/table/inspect_position_deletes.go
+++ b/table/inspect_position_deletes.go
@@ -89,7 +89,7 @@ func (i InspectTable) currentPositionDeleteManifests(
return nil, nil, err
}
- return fs, manifestSet.allManifests(), nil
+ return fs, manifestSet.borrowAllManifests(), nil
}
type positionDeleteRecordAppender struct {
diff --git a/table/scanner.go b/table/scanner.go
index 9592212b0..60507598d 100644
--- a/table/scanner.go
+++ b/table/scanner.go
@@ -1062,7 +1062,7 @@ func (scan *Scan)
fetchPartitionSpecFilteredManifestsWithSchema(
return nil, err
}
- return scan.filterManifestsWithSchema(manifestSet.allManifests(),
schema, acc, partitionFilters)
+ return scan.filterManifestsWithSchema(manifestSet.borrowAllManifests(),
schema, acc, partitionFilters)
}
func (scan *Scan) manifestSet(
diff --git a/table/snapshot_manifest_cache.go b/table/snapshot_manifest_cache.go
index b4bfedd24..867c99d53 100644
--- a/table/snapshot_manifest_cache.go
+++ b/table/snapshot_manifest_cache.go
@@ -77,6 +77,18 @@ func (s snapshotManifestSet) dataManifests()
[]iceberg.ManifestFile {
return slices.Clone(s.data)
}
+// borrowAllManifests returns the cache-owned manifest slice without copying.
+// Callers must treat the slice as read-only and must not reorder or replace
entries.
+func (s snapshotManifestSet) borrowAllManifests() []iceberg.ManifestFile {
+ return s.all
+}
+
+// borrowDataManifests returns the cache-owned data-manifest slice without
copying.
+// Callers must treat the slice as read-only and must not reorder or replace
entries.
+func (s snapshotManifestSet) borrowDataManifests() []iceberg.ManifestFile {
+ return s.data
+}
+
func snapshotManifestSetSize(set snapshotManifestSet) int {
return len(set.all)
}
diff --git a/table/snapshot_manifest_cache_bench_test.go
b/table/snapshot_manifest_cache_bench_test.go
index 1624d8c4c..9a92a1c87 100644
--- a/table/snapshot_manifest_cache_bench_test.go
+++ b/table/snapshot_manifest_cache_bench_test.go
@@ -23,6 +23,7 @@ import (
"sync"
"testing"
+ "github.com/apache/iceberg-go"
iceio "github.com/apache/iceberg-go/io"
)
@@ -96,3 +97,33 @@ func (fs *snapshotManifestCacheBenchmarkIO) Open(name
string) (iceio.File, error
return fs.IO.Open(name)
}
+
+var snapshotManifestSliceBenchmarkSink []iceberg.ManifestFile
+
+func BenchmarkInspectSnapshotManifestCacheHit(b *testing.B) {
+ for _, manifestCount := range []int{1, 1000, 10000} {
+ b.Run(fmt.Sprintf("manifests=%d", manifestCount), func(b
*testing.B) {
+ tbl := newRowLimitPlanningBenchmarkTable(b,
manifestCount)
+ inspector := tbl.Inspect()
+ ctx := b.Context()
+ manifests, err :=
inspector.currentSnapshotManifests(ctx)
+ if err != nil {
+ b.Fatal(err)
+ }
+ if len(manifests) != manifestCount {
+ b.Fatalf("got %d manifests, want %d",
len(manifests), manifestCount)
+ }
+ b.ReportAllocs()
+ for b.Loop() {
+ manifests, err :=
inspector.currentSnapshotManifests(ctx)
+ if err != nil {
+ b.Fatal(err)
+ }
+ if len(manifests) != manifestCount {
+ b.Fatalf("got %d manifests, want %d",
len(manifests), manifestCount)
+ }
+ snapshotManifestSliceBenchmarkSink = manifests
+ }
+ })
+ }
+}
diff --git a/table/snapshot_manifest_cache_internal_test.go
b/table/snapshot_manifest_cache_internal_test.go
index 662646fe6..30a47056b 100644
--- a/table/snapshot_manifest_cache_internal_test.go
+++ b/table/snapshot_manifest_cache_internal_test.go
@@ -97,6 +97,10 @@ func
TestSnapshotManifestCacheSeparatesContentAndProtectsSlices(t *testing.T) {
require.NoError(t, err)
require.Len(t, set.allManifests(), 2)
require.Len(t, set.dataManifests(), 1)
+ borrowedAll := set.borrowAllManifests()
+ borrowedData := set.borrowDataManifests()
+ assert.Same(t, &set.all[0], &borrowedAll[0])
+ assert.Same(t, &set.data[0], &borrowedData[0])
all := set.allManifests()
all[0] = nil
diff --git a/table/table.go b/table/table.go
index c77d66517..9bb3df320 100644
--- a/table/table.go
+++ b/table/table.go
@@ -417,7 +417,7 @@ func (t Table) AllManifests(ctx context.Context)
iter.Seq2[iceberg.ManifestFile,
if err != nil {
return err
}
- manifests := manifestSet.allManifests()
+ manifests :=
manifestSet.borrowAllManifests()
select {
case ch <- list{Index: i, Value:
manifests, Last: i == n-1}:
