From 575fd960617b20805c43ec5e9319dae2d72522c4 Mon Sep 17 00:00:00 2001 From: Omar Date: Mon, 19 Oct 2026 18:29:07 +0530 Subject: [PATCH 1/2] Don't hold the window for a game database rebuild Since device settings (#84) the daemon reads the game database while it starts, to know which files in each save folder are settings. When the database is due a rebuild it waited for it: a parse of the whole manifest, about a minute, with the window on "Starting OpenSave..." the whole time. The rebuild is due at the first start after an update that changes the index format (every user moving to this release) and after each weekly manifest download. At start it now takes the index on disk as it is, stale or not, or the embedded one when there is none, and rebuilds in the background. The new index is read at the next start, which is already when a database update takes effect for device settings. Until then a settings file the old index does not name is synced, the safe direction. Scans and the other lookups still wait for a fresh index, as before. --- internal/daemon/devicesettings.go | 8 +- internal/presets/devicesettings.go | 16 +++- internal/presets/index_nowait_test.go | 105 ++++++++++++++++++++++++++ internal/presets/ludusavi.go | 71 +++++++++++++++-- 4 files changed, 190 insertions(+), 10 deletions(-) create mode 100644 internal/presets/index_nowait_test.go diff --git a/internal/daemon/devicesettings.go b/internal/daemon/devicesettings.go index 308f3608..a50b12e7 100644 --- a/internal/daemon/devicesettings.go +++ b/internal/daemon/devicesettings.go @@ -15,6 +15,12 @@ import ( // and a game's answer only changes when its name, App ID or folder does — // which makes it a different key here. A database update takes effect at the // next start, where adoptExclusionView carries every recorded hash over. +// +// First needed while the daemon starts, before the window can open, so it does +// not wait for the database to be rebuilt: that parses the whole manifest, +// about a minute, and is due after an update changes the index format and +// after every weekly manifest download. The rebuild runs in the background and +// is what the next start reads. type devSettingsCache struct { loaded bool @@ -30,7 +36,7 @@ func (d *Daemon) deviceSettingsFor(game store.Game) []string { d.devMu.Lock() defer d.devMu.Unlock() if !d.dev.loaded { - d.dev.db = d.Scanner.DeviceSettings() + d.dev.db = d.Scanner.DeviceSettingsNoWait() d.dev.patterns = map[string][]string{} d.dev.loaded = true } diff --git a/internal/presets/devicesettings.go b/internal/presets/devicesettings.go index 6b5513c9..08a5c2df 100644 --- a/internal/presets/devicesettings.go +++ b/internal/presets/devicesettings.go @@ -29,7 +29,21 @@ func (sc *Scanner) DeviceSettings() *DeviceSettings { if sc == nil || sc.CacheFile == "" { return nil } - idx := sc.loadManifestIndex() + return sc.deviceSettings(sc.loadManifestIndex()) +} + +// DeviceSettingsNoWait is DeviceSettings for the daemon while it starts, which +// must not wait for an index that is due a rebuild (loadManifestIndexNoWait): +// until the rebuild is done, settings files are left out as the index on disk +// names them, and the next start goes by the new one. +func (sc *Scanner) DeviceSettingsNoWait() *DeviceSettings { + if sc == nil || sc.CacheFile == "" { + return nil + } + return sc.deviceSettings(sc.loadManifestIndexNoWait()) +} + +func (sc *Scanner) deviceSettings(idx []indexedGame) *DeviceSettings { if len(idx) == 0 { return nil } diff --git a/internal/presets/index_nowait_test.go b/internal/presets/index_nowait_test.go new file mode 100644 index 00000000..5d7b5dee --- /dev/null +++ b/internal/presets/index_nowait_test.go @@ -0,0 +1,105 @@ +package presets + +import ( + "encoding/json" + "os" + "testing" + "time" +) + +// The daemon reads the index while it starts, before the window opens. An +// index due a rebuild is used as it is, and rebuilt behind it: the window +// waited a minute for the rebuild at the first start after the index format +// changed. +func TestIndexNoWait_UsesTheIndexOnDiskAndRebuildsBehindIt(t *testing.T) { + sc := manifestScanner(t, ` +New Game: + files: + "/New/save": + tags: [save] +`) + yamlPath, indexPath := sc.manifestPaths() + waitForRebuild(t) + + // An index from before the manifest's last download. + old := []indexedGame{{Name: "Old Game", Paths: []string{"/Old"}}} + raw, _ := json.Marshal(old) + if err := os.WriteFile(indexPath, raw, 0o666); err != nil { + t.Fatal(err) + } + past := time.Now().Add(-time.Hour) + if err := os.Chtimes(indexPath, past, past); err != nil { + t.Fatal(err) + } + + got := sc.loadManifestIndexNoWait() + if len(got) != 1 || got[0].Name != "Old Game" { + t.Fatalf("got %+v, want the index on disk as it is", got) + } + + // Rebuilt in the background; what the next start reads. + deadline := time.Now().Add(10 * time.Second) + for { + yamlInfo, err := os.Stat(yamlPath) + if err != nil { + t.Fatal(err) + } + if games, fresh := readManifestIndex(indexPath, yamlInfo); fresh { + if len(games) != 1 || games[0].Name != "New Game" { + t.Fatalf("rebuilt index: %+v", games) + } + break + } + if time.Now().After(deadline) { + t.Fatal("the index was not rebuilt") + } + time.Sleep(20 * time.Millisecond) + } + if got := sc.loadManifestIndexNoWait(); len(got) != 1 || got[0].Name != "New Game" { + t.Errorf("after the rebuild: %+v", got) + } + waitForRebuild(t) +} + +// waitForRebuild waits for a background index build to be over, so the next +// test can start one of its own. +func waitForRebuild(t *testing.T) { + t.Helper() + deadline := time.Now().Add(10 * time.Second) + for rebuildInFlight.Load() { + if time.Now().After(deadline) { + t.Fatal("the background rebuild did not finish") + } + time.Sleep(10 * time.Millisecond) + } +} + +// With no index at all, nothing is waited for either: the embedded one stands +// in (none in a hermetic test), and the index is built behind it. +func TestIndexNoWait_NoIndexYet(t *testing.T) { + sc := manifestScanner(t, ` +New Game: + files: + "/New/save": + tags: [save] +`) + _, indexPath := sc.manifestPaths() + waitForRebuild(t) + if got := sc.loadManifestIndexNoWait(); len(got) != 0 { + t.Fatalf("got %+v, want nothing until the index is built", got) + } + deadline := time.Now().Add(10 * time.Second) + for { + if _, err := os.Stat(indexPath); err == nil { + break + } + if time.Now().After(deadline) { + t.Fatal("the index was not built") + } + time.Sleep(20 * time.Millisecond) + } + if got := sc.loadManifestIndexNoWait(); len(got) != 1 { + t.Errorf("after the build: %+v", got) + } + waitForRebuild(t) +} diff --git a/internal/presets/ludusavi.go b/internal/presets/ludusavi.go index c4411c47..b6119845 100644 --- a/internal/presets/ludusavi.go +++ b/internal/presets/ludusavi.go @@ -644,23 +644,78 @@ func (sc *Scanner) loadManifestIndex() []indexedGame { } // Reuse the index when it's newer than the YAML it was built from. - if idxInfo, err := os.Stat(indexPath); err == nil && idxInfo.ModTime().After(yamlInfo.ModTime()) { - if raw, err := os.ReadFile(indexPath); err == nil { - var games []indexedGame - if json.Unmarshal(raw, &games) == nil && len(games) > 0 { - return games - } + if games, fresh := readManifestIndex(indexPath, yamlInfo); fresh { + return games + } + + if games := rebuildManifestIndex(yamlPath, indexPath); len(games) > 0 { + return games + } + return embedded() +} + +// loadManifestIndexNoWait is loadManifestIndex for a caller that must not wait +// on a rebuild: the daemon while it starts, before the window can open. +// Building the index parses the whole manifest, about a minute on an ordinary +// PC, and it is due at the first start after the index format changes and +// after every weekly manifest download. This takes the index on disk as it is, +// stale or not, or the embedded one when there is none, and rebuilds it in the +// background; the next start reads the new one. +func (sc *Scanner) loadManifestIndexNoWait() []indexedGame { + yamlPath, indexPath := sc.manifestPaths() + + sc.refreshManifestAsync(yamlPath) + + yamlInfo, err := os.Stat(yamlPath) + if err != nil { + if sc.ManifestURL == "" { + return nil } + return loadEmbeddedIndex() } + games, fresh := readManifestIndex(indexPath, yamlInfo) + if !fresh && rebuildInFlight.CompareAndSwap(false, true) { + go func() { + defer rebuildInFlight.Store(false) + rebuildManifestIndex(yamlPath, indexPath) + }() + } + if len(games) > 0 { + return games + } + if sc.ManifestURL == "" { + return nil + } + return loadEmbeddedIndex() +} +// rebuildInFlight guards against overlapping background index builds. +var rebuildInFlight atomic.Bool + +// readManifestIndex reads the index on disk, and says whether it is newer +// than the manifest it was built from. Nil when there is none or it does not +// parse. +func readManifestIndex(indexPath string, yamlInfo os.FileInfo) (games []indexedGame, fresh bool) { + idxInfo, err := os.Stat(indexPath) + if err != nil { + return nil, false + } + raw, err := os.ReadFile(indexPath) + if err != nil || json.Unmarshal(raw, &games) != nil || len(games) == 0 { + return nil, false + } + return games, idxInfo.ModTime().After(yamlInfo.ModTime()) +} + +// rebuildManifestIndex builds the index from the manifest and saves it. +func rebuildManifestIndex(yamlPath, indexPath string) []indexedGame { games := buildManifestIndex(yamlPath) if len(games) > 0 { if raw, err := json.Marshal(games); err == nil { _ = writeFileAtomic(indexPath, raw) } - return games } - return embedded() + return games } // refreshInFlight guards against overlapping background downloads. From 125d5562c16f9c7730ba80d73e8084e3fe2496ae Mon Sep 17 00:00:00 2001 From: Omar Date: Mon, 19 Oct 2026 20:34:59 +0530 Subject: [PATCH 2/2] Index no-wait test: date the manifest in the past Linux CI's coarse file timestamps gave the rebuilt index the same time as the manifest written moments before it, so it never counted as newer. --- internal/presets/index_nowait_test.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/internal/presets/index_nowait_test.go b/internal/presets/index_nowait_test.go index 5d7b5dee..6d5d7ea9 100644 --- a/internal/presets/index_nowait_test.go +++ b/internal/presets/index_nowait_test.go @@ -27,10 +27,18 @@ New Game: if err := os.WriteFile(indexPath, raw, 0o666); err != nil { t.Fatal(err) } + // The manifest downloaded half an hour ago, after the index was built. + // Both in the past: a filesystem with coarse timestamps can give the + // rebuilt index the same time as a manifest written moments before it, + // and it would never count as newer. past := time.Now().Add(-time.Hour) if err := os.Chtimes(indexPath, past, past); err != nil { t.Fatal(err) } + downloaded := time.Now().Add(-30 * time.Minute) + if err := os.Chtimes(yamlPath, downloaded, downloaded); err != nil { + t.Fatal(err) + } got := sc.loadManifestIndexNoWait() if len(got) != 1 || got[0].Name != "Old Game" { @@ -83,8 +91,12 @@ New Game: "/New/save": tags: [save] `) - _, indexPath := sc.manifestPaths() + yamlPath, indexPath := sc.manifestPaths() waitForRebuild(t) + downloaded := time.Now().Add(-30 * time.Minute) + if err := os.Chtimes(yamlPath, downloaded, downloaded); err != nil { + t.Fatal(err) + } if got := sc.loadManifestIndexNoWait(); len(got) != 0 { t.Fatalf("got %+v, want nothing until the index is built", got) }