From 956d3277336c6f162e6b7012a13c18f567c2b62d Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Sun, 25 Oct 2026 16:54:05 -0700 Subject: [PATCH] state: Batch store-opening metadata reads Opening repository state reads its version and repository metadata separately, starting redundant Git processes on every command. Load both records together and consume completed partial results when the version is absent, retaining the legacy version-one default. A missing repository record still identifies an uninitialized store. Other storage failures end opening with their original cause and backend key context; only missing-key errors permit using partial data. On macOS arm64, paired immutable binaries compare this branch with its actual parent, main at 83b7695b3, using identical real-Git fixtures. Five samples of five openings alternate parent/candidate order. Values are median milliseconds per opening; legacy stores omit version. | Workload | Parent (ms/op) | This branch (ms/op) | | --- | ---: | ---: | | Current, 1 branch | 29.34 | 18.77 | | Current, 100 branches | 29.77 | 18.86 | | Legacy, 1 branch | 20.34 | 18.68 | | Legacy, 100 branches | 18.45 | 18.41 | [skip changelog]: negligible improvement, not worth changleog --- internal/spice/state/git_store_bench_test.go | 37 ++++++++++++++++ internal/spice/state/store.go | 23 ++++++---- internal/spice/state/store_open_bench_test.go | 27 ++++++++++++ internal/spice/state/store_test.go | 23 +++++++++- internal/spice/state/version.go | 23 +--------- internal/spice/state/version_test.go | 44 ------------------- 6 files changed, 100 insertions(+), 77 deletions(-) create mode 100644 internal/spice/state/git_store_bench_test.go create mode 100644 internal/spice/state/store_open_bench_test.go diff --git a/internal/spice/state/git_store_bench_test.go b/internal/spice/state/git_store_bench_test.go new file mode 100644 index 000000000..aa6c44587 --- /dev/null +++ b/internal/spice/state/git_store_bench_test.go @@ -0,0 +1,37 @@ +package state + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" + "go.abhg.dev/gs/internal/git" + "go.abhg.dev/gs/internal/silog/silogtest" + "go.abhg.dev/gs/internal/spice/state/storage" +) + +// benchmarkGitStore seeds sibling branches in one storage update. +// Transaction benchmarks create fresh transactions without committing, so every +// iteration starts with a cold transaction cache and the same persisted graph. +func benchmarkGitStore(b *testing.B, size int) (*Store, *storage.DB) { + b.Helper() + ctx := b.Context() + log := silogtest.New(b) + repo, _, err := git.Init(ctx, b.TempDir(), git.InitOptions{Log: log}) + require.NoError(b, err) + db := storage.NewDB(storage.NewGitBackend(storage.GitConfig{ + Repo: repo, Ref: "refs/spice/data", + AuthorName: "Benchmark", AuthorEmail: "benchmark@example.com", Log: log, + })) + store, err := InitStore(ctx, InitStoreRequest{DB: db, Trunk: "main", Log: log}) + require.NoError(b, err) + sets := make([]storage.SetRequest, size) + for i := range sets { + sets[i] = storage.SetRequest{ + Key: branchKey(fmt.Sprintf("branch-%03d", i)), + Value: &branchState{Base: branchStateBase{Name: "main", Hash: "initial"}}, + } + } + require.NoError(b, db.Update(ctx, storage.UpdateRequest{Sets: sets, Message: "seed branches"})) + return store, db +} diff --git a/internal/spice/state/store.go b/internal/spice/state/store.go index 5f21212f7..42cfe3419 100644 --- a/internal/spice/state/store.go +++ b/internal/spice/state/store.go @@ -5,6 +5,7 @@ import ( "context" "errors" "fmt" + "slices" "go.abhg.dev/gs/internal/silog" "go.abhg.dev/gs/internal/spice/state/storage" @@ -171,20 +172,24 @@ func OpenStore(ctx context.Context, db DB, logger *silog.Logger) (*Store, error) logger = silog.Nop() } - version, err := loadVersion(ctx, db) - if err != nil { - return nil, fmt.Errorf("load store version: %w", err) + version := VersionOne + var info repoInfo + err := db.GetMany(ctx, []storage.GetRequest{ + {Key: _versionFile, Value: &version}, + {Key: _repoJSON, Value: &info}, + }) + missing, ok := errors.AsType[*storage.MissingKeysError](err) + if err != nil && !ok { + return nil, fmt.Errorf("read store metadata: %w", err) } if err := checkVersion(version); err != nil { return nil, fmt.Errorf("check store layout: %w", err) } - var info repoInfo - if err := db.Get(ctx, _repoJSON, &info); err != nil { - if errors.Is(err, ErrNotExist) { - return nil, ErrUninitialized - } - return nil, fmt.Errorf("get repo state: %w", err) + // An absent version keeps the v1 default; a missing repository record + // means the store has not been initialized. + if ok && slices.Contains(missing.Indexes, 1) { + return nil, ErrUninitialized } if err := info.Validate(); err != nil { diff --git a/internal/spice/state/store_open_bench_test.go b/internal/spice/state/store_open_bench_test.go new file mode 100644 index 000000000..fc7b1be38 --- /dev/null +++ b/internal/spice/state/store_open_bench_test.go @@ -0,0 +1,27 @@ +package state + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" + "go.abhg.dev/gs/internal/silog/silogtest" +) + +func BenchmarkStoreOpen(b *testing.B) { + for _, size := range []int{1, 100} { + for _, legacy := range []bool{false, true} { + b.Run(fmt.Sprintf("Branches%d/Legacy%t", size, legacy), func(b *testing.B) { + _, db := benchmarkGitStore(b, size) + if legacy { + require.NoError(b, db.Delete(b.Context(), _versionFile, "legacy layout")) + } + for b.Loop() { + if _, err := OpenStore(b.Context(), db, silogtest.New(b)); err != nil { + b.Fatal(err) + } + } + }) + } + } +} diff --git a/internal/spice/state/store_test.go b/internal/spice/state/store_test.go index d2aeb787b..41cafaade 100644 --- a/internal/spice/state/store_test.go +++ b/internal/spice/state/store_test.go @@ -1,7 +1,9 @@ package state_test import ( + "context" "encoding/json/jsontext" + "io" "testing" "github.com/stretchr/testify/assert" @@ -262,7 +264,7 @@ func TestOpenStore_remoteMigrationMalformed(t *testing.T) { _, err := state.OpenStore(t.Context(), storage.NewDB(mem), nil) require.Error(t, err) - assert.ErrorContains(t, err, "get repo state:") + assert.ErrorContains(t, err, `decode JSON for "repo":`) } func TestInitStore_writesVersionOneForSameRemote(t *testing.T) { @@ -394,7 +396,7 @@ func TestOpenStore_errors(t *testing.T) { _, err := state.OpenStore(t.Context(), storage.NewDB(mem), nil) require.Error(t, err) - assert.ErrorContains(t, err, "get repo state:") + assert.ErrorContains(t, err, `decode JSON for "repo":`) }) t.Run("CorruptRepo/Incomplete", func(t *testing.T) { @@ -407,3 +409,20 @@ func TestOpenStore_errors(t *testing.T) { assert.ErrorContains(t, err, "corrupt state:") }) } + +func TestOpenStore_batchReadError(t *testing.T) { + db := &failedBatchDB{DB: storage.NewDB(storage.MapBackend{ + "version": []byte("1"), + "repo": []byte(`{"trunk":"main"}`), + })} + _, err := state.OpenStore(t.Context(), db, nil) + assert.ErrorIs(t, err, io.ErrUnexpectedEOF) +} + +type failedBatchDB struct { + *storage.DB +} + +func (*failedBatchDB) GetMany(context.Context, []storage.GetRequest) error { + return io.ErrUnexpectedEOF +} diff --git a/internal/spice/state/version.go b/internal/spice/state/version.go index 8a21cd744..d103b8151 100644 --- a/internal/spice/state/version.go +++ b/internal/spice/state/version.go @@ -1,10 +1,6 @@ package state -import ( - "context" - "errors" - "fmt" -) +import "fmt" const _versionFile = "version" @@ -41,23 +37,6 @@ func checkVersion(version Version) error { return nil } -// loadVersion loads the version of the storage layout used by the given [DB]. -func loadVersion(ctx context.Context, db DB) (Version, error) { - var version Version - - if err := db.Get(ctx, _versionFile, &version); err != nil { - if errors.Is(err, ErrNotExist) { - // Version file was added during storage version 1. - // If file does not exist, it's an old v1 store. - return VersionOne, nil - } - - return 0, err - } - - return version, nil -} - // VersionMismatchError indicates that the data store we attempted to open // is using a version older than this binary knows how to handle. type VersionMismatchError struct { diff --git a/internal/spice/state/version_test.go b/internal/spice/state/version_test.go index d2fe1ed9f..4b796653b 100644 --- a/internal/spice/state/version_test.go +++ b/internal/spice/state/version_test.go @@ -5,52 +5,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "go.abhg.dev/gs/internal/spice/state/storage" ) -func TestLoadVersion(t *testing.T) { - tests := []struct { - name string - files storage.MapBackend - want Version - }{ - { - name: "Empty", - want: VersionOne, - }, - { - name: "ExplicitV1", - files: storage.MapBackend{ - "version": []byte("1"), - }, - want: VersionOne, - }, - { - name: "ExplicitV2", - files: storage.MapBackend{ - "version": []byte("2"), - }, - want: VersionTwo, - }, - { - name: "FutureVersion", - files: storage.MapBackend{ - "version": []byte("42"), - }, - want: Version(42), - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - db := storage.NewDB(tt.files) - got, err := loadVersion(t.Context(), db) - require.NoError(t, err) - assert.Equal(t, tt.want, got) - }) - } -} - func TestCheckVersion(t *testing.T) { tests := []struct { name string