perf(explore): emit the index status on change, not every three seconds
`IndexStatusChanged` was pushed on a 3 s ticker for the life of the process, byte-identical once the index was ready, and `config-page` assigns it to a @state field — so a user who had once opened Settings paid a full re-render of a 2 000-line template every 3 s, forever, for no news. Measured sitting on Settings: 5 events and 5 re-renders per 15 s, against 0 and 0. `emitStatus` drops a status equal to the last one it sent, which is the rule stated once instead of at twenty call sites. The corollary is load-bearing: every mutation of something the status derives must now call `emitStatus` itself. Two were relying on the ticker — `si.ready` when an existing index is adopted, and `si.cancel` when a build ends — and without them the header badge said "Building search index" over an index the settings page called ready. A polling loop is a hidden dependency for every state transition that forgot to announce itself.
This commit is contained in:
@@ -0,0 +1,184 @@
|
||||
package explore
|
||||
|
||||
import (
|
||||
"context"
|
||||
"log/slog"
|
||||
"testing"
|
||||
|
||||
"yellowjacket/backend/database"
|
||||
"yellowjacket/backend/events"
|
||||
)
|
||||
|
||||
// The index status used to be pushed on a 3 s ticker for the life of
|
||||
// the process, with a byte-identical payload once the index was ready.
|
||||
// The frontend's handler assigns it to a @state field, so every tick
|
||||
// re-rendered the whole settings page — a cached view that never
|
||||
// unmounts — saying nothing (`perf.M6` / `H-14`).
|
||||
//
|
||||
// The ticker is gone and emitStatus suppresses an unchanged payload, so
|
||||
// what these cover is the pair of properties that replaced it: a change
|
||||
// still gets through, and a non-change does not.
|
||||
|
||||
// setupRecordedIndex builds a SearchIndex with an event sink installed,
|
||||
// so what the frontend would receive is assertable in-process.
|
||||
func setupRecordedIndex(t *testing.T) (*SearchIndex, *events.Recorder) {
|
||||
t.Helper()
|
||||
|
||||
db := database.NewTestDB(t)
|
||||
si := NewSearchIndex(db, nil, nil, slog.Default())
|
||||
|
||||
rec := events.NewRecorder()
|
||||
si.SetContext(events.WithSink(context.Background(), rec))
|
||||
|
||||
return si, rec
|
||||
}
|
||||
|
||||
func TestEmitStatus_SuppressesAnUnchangedPayload(t *testing.T) {
|
||||
si, rec := setupRecordedIndex(t)
|
||||
|
||||
// SetContext emits once via refreshStatusCounts; everything after
|
||||
// this describes the same state.
|
||||
rec.Reset()
|
||||
|
||||
for range 5 {
|
||||
si.emitStatus()
|
||||
}
|
||||
|
||||
if got := rec.Count(events.IndexStatusChanged); got != 0 {
|
||||
t.Fatalf(
|
||||
"emitted %d IndexStatusChanged for an unchanged status, want 0",
|
||||
got,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEmitStatus_EmitsOnChange(t *testing.T) {
|
||||
si, rec := setupRecordedIndex(t)
|
||||
rec.Reset()
|
||||
|
||||
si.setTierStatus("artists", "running", 100, 10)
|
||||
|
||||
if got := rec.Count(events.IndexStatusChanged); got != 1 {
|
||||
t.Fatalf("emitted %d IndexStatusChanged for a new tier, want 1", got)
|
||||
}
|
||||
|
||||
// Progress within the tier is a change too — this is what a build
|
||||
// reports, and dropping it would freeze the progress bar.
|
||||
si.setTierStatus("artists", "running", 100, 20)
|
||||
|
||||
if got := rec.Count(events.IndexStatusChanged); got != 2 {
|
||||
t.Fatalf("emitted %d after progress moved, want 2", got)
|
||||
}
|
||||
|
||||
// The same call again is not.
|
||||
si.setTierStatus("artists", "running", 100, 20)
|
||||
|
||||
if got := rec.Count(events.IndexStatusChanged); got != 2 {
|
||||
t.Fatalf("emitted %d after a repeated status, want 2", got)
|
||||
}
|
||||
}
|
||||
|
||||
// The status carries a slice, so a snapshot that aliases it would
|
||||
// compare equal to a later mutation of the same backing array and the
|
||||
// change would never be emitted.
|
||||
func TestEmitStatus_SnapshotDoesNotAliasTiers(t *testing.T) {
|
||||
si, rec := setupRecordedIndex(t)
|
||||
|
||||
si.setTierStatus("artists", "running", 100, 10)
|
||||
rec.Reset()
|
||||
|
||||
si.setTierStatus("artists", "complete", 100, 100)
|
||||
|
||||
if got := rec.Count(events.IndexStatusChanged); got != 1 {
|
||||
t.Fatalf(
|
||||
"emitted %d when a tier completed in place, want 1", got,
|
||||
)
|
||||
}
|
||||
|
||||
status, ok := rec.Last(events.IndexStatusChanged)
|
||||
if !ok {
|
||||
t.Fatal("no IndexStatusChanged recorded")
|
||||
}
|
||||
|
||||
payload, ok := status.Payload().(IndexStatus)
|
||||
if !ok {
|
||||
t.Fatalf("payload is %T, want explore.IndexStatus", status.Payload())
|
||||
}
|
||||
|
||||
if len(payload.Tiers) != 1 || payload.Tiers[0].State != "complete" {
|
||||
t.Fatalf("payload tiers = %+v, want one complete tier", payload.Tiers)
|
||||
}
|
||||
}
|
||||
|
||||
// `Building` and `Ready` are derived inside emitStatus from fields the
|
||||
// dedupe never sees directly, so a transition in either has to survive
|
||||
// it. A build ending is the case that matters: syncIndexJob only
|
||||
// resolves the job in the registry on a sync reporting Building false,
|
||||
// and suppressing that left the header badge saying "Building search
|
||||
// index" over an index the settings page called ready.
|
||||
func TestEmitStatus_BuildingTransitionsAreNotSuppressed(t *testing.T) {
|
||||
si, rec := setupRecordedIndex(t)
|
||||
|
||||
si.mu.Lock()
|
||||
_, si.cancel = context.WithCancel(context.Background())
|
||||
si.mu.Unlock()
|
||||
|
||||
si.emitStatus()
|
||||
rec.Reset()
|
||||
|
||||
// Nothing else changed, so this one is noise.
|
||||
si.emitStatus()
|
||||
|
||||
if got := rec.Count(events.IndexStatusChanged); got != 0 {
|
||||
t.Fatalf("emitted %d while still building unchanged, want 0", got)
|
||||
}
|
||||
|
||||
si.mu.Lock()
|
||||
si.cancel = nil
|
||||
si.mu.Unlock()
|
||||
|
||||
si.emitStatus()
|
||||
|
||||
ev, ok := rec.Last(events.IndexStatusChanged)
|
||||
if !ok {
|
||||
t.Fatalf("a build ending emitted nothing; got %v", rec.Names())
|
||||
}
|
||||
|
||||
payload, ok := ev.Payload().(IndexStatus)
|
||||
if !ok {
|
||||
t.Fatalf("payload is %T, want explore.IndexStatus", ev.Payload())
|
||||
}
|
||||
|
||||
if payload.Building {
|
||||
t.Fatal("payload still reports building after the build ended")
|
||||
}
|
||||
}
|
||||
|
||||
// Becoming ready used to be the one status mutation with no emit behind
|
||||
// it; the ticker carried it, so removing the ticker without an explicit
|
||||
// emit would have left the settings page reading "not ready" forever
|
||||
// over a fully built index.
|
||||
func TestEmitStatus_ReadyIsAChange(t *testing.T) {
|
||||
si, rec := setupRecordedIndex(t)
|
||||
rec.Reset()
|
||||
|
||||
si.mu.Lock()
|
||||
si.ready = true
|
||||
si.mu.Unlock()
|
||||
|
||||
si.emitStatus()
|
||||
|
||||
ev, ok := rec.Last(events.IndexStatusChanged)
|
||||
if !ok {
|
||||
t.Fatalf("becoming ready emitted nothing; got %v", rec.Names())
|
||||
}
|
||||
|
||||
payload, ok := ev.Payload().(IndexStatus)
|
||||
if !ok {
|
||||
t.Fatalf("payload is %T, want explore.IndexStatus", ev.Payload())
|
||||
}
|
||||
|
||||
if !payload.Ready {
|
||||
t.Fatal("payload reports not ready after si.ready was set")
|
||||
}
|
||||
}
|
||||
@@ -199,6 +199,13 @@ type SearchIndex struct {
|
||||
// Build status tracking — read by GetIndexStatus for the UI.
|
||||
buildStatus IndexStatus
|
||||
|
||||
// lastEmitted is the status most recently pushed to the frontend, so
|
||||
// an unchanged one can be dropped rather than re-rendering the whole
|
||||
// settings page for nothing. It has its own mutex: emitStatus is
|
||||
// called from paths that already hold mu for reading.
|
||||
emitMu sync.Mutex
|
||||
lastEmitted *IndexStatus
|
||||
|
||||
// jobs is the background job registry; buildPaused records that the
|
||||
// user paused the build, distinguishing a deliberate stop from a
|
||||
// build that merely finished. Both are protected by mu.
|
||||
@@ -267,24 +274,16 @@ func (si *SearchIndex) SetContext(ctx context.Context) {
|
||||
si.mu.Unlock()
|
||||
|
||||
// Load current row counts + last-built timestamp from DB.
|
||||
//
|
||||
// There is deliberately no ticker here. This used to emit the status
|
||||
// every 3 seconds for the life of the process, with a byte-identical
|
||||
// payload once the index was ready — which re-rendered the whole of
|
||||
// `config-page` on every tick, since it is a cached view that never
|
||||
// unmounts. Every path that mutates the status already calls
|
||||
// emitStatus, that call now suppresses an unchanged payload, and the
|
||||
// frontend seeds itself with GetIndexStatus() on connect rather than
|
||||
// waiting for the next tick.
|
||||
si.refreshStatusCounts()
|
||||
|
||||
// Start a background ticker that emits status every 3 seconds.
|
||||
// This replaces frontend polling — the Wails binding dispatcher
|
||||
// can be blocked by other calls, but EventsEmit bypasses it.
|
||||
go func() {
|
||||
ticker := time.NewTicker(3 * time.Second)
|
||||
defer ticker.Stop()
|
||||
|
||||
for {
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
return
|
||||
case <-ticker.C:
|
||||
si.emitStatus()
|
||||
}
|
||||
}
|
||||
}()
|
||||
}
|
||||
|
||||
// EnsureArtistDiscography lazily fetches an artist's top release groups
|
||||
@@ -522,6 +521,14 @@ func (si *SearchIndex) StartBuild(ctx context.Context) {
|
||||
si.cancel = nil
|
||||
si.mu.Unlock()
|
||||
|
||||
// `Building` is derived from si.cancel, so clearing it is a
|
||||
// status change and has to say so. This is what resolves the
|
||||
// job in the registry — syncIndexJob only finishes a job on a
|
||||
// sync that reports Building false, and without this line the
|
||||
// header badge reads "Building search index" over an index the
|
||||
// settings page calls ready.
|
||||
si.emitStatus()
|
||||
|
||||
close(si.done)
|
||||
|
||||
// The index rows (and their popularities) may have changed, so
|
||||
@@ -677,7 +684,38 @@ func (si *SearchIndex) setTierDetail(name, state string, total, completed int, d
|
||||
si.emitStatus()
|
||||
}
|
||||
|
||||
// emitStatus pushes the current index status to the frontend via Wails event.
|
||||
// sameStatusAs reports whether two statuses would render identically.
|
||||
// IndexStatus holds a slice, so it is not comparable with ==.
|
||||
func (s IndexStatus) sameStatusAs(o IndexStatus) bool {
|
||||
if s.Building != o.Building ||
|
||||
s.Ready != o.Ready ||
|
||||
s.LastBuilt != o.LastBuilt ||
|
||||
s.Artists != o.Artists ||
|
||||
s.Recordings != o.Recordings ||
|
||||
s.ReleaseGroups != o.ReleaseGroups ||
|
||||
s.TotalRows != o.TotalRows ||
|
||||
len(s.Tiers) != len(o.Tiers) {
|
||||
return false
|
||||
}
|
||||
|
||||
for i := range s.Tiers {
|
||||
if s.Tiers[i] != o.Tiers[i] {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
return true
|
||||
}
|
||||
|
||||
// emitStatus pushes the current index status to the frontend via Wails
|
||||
// event — but only when it differs from the last one pushed.
|
||||
//
|
||||
// The status is emitted from every path that touches it, several of
|
||||
// which report progress in a tight loop, and the frontend's handler
|
||||
// assigns to a @state field: an identical payload is therefore a full
|
||||
// re-render of a 2 000-line template saying nothing. Deduplicating
|
||||
// here rather than at the call sites means no future emitter has to
|
||||
// remember (`perf.M6` / `H-14`).
|
||||
func (si *SearchIndex) emitStatus() {
|
||||
if si.runtimeCtx == nil {
|
||||
return
|
||||
@@ -689,6 +727,23 @@ func (si *SearchIndex) emitStatus() {
|
||||
status.Building = si.cancel != nil
|
||||
si.mu.RUnlock()
|
||||
|
||||
si.emitMu.Lock()
|
||||
unchanged := si.lastEmitted != nil &&
|
||||
si.lastEmitted.sameStatusAs(status)
|
||||
|
||||
if !unchanged {
|
||||
snapshot := status
|
||||
snapshot.Tiers = append(
|
||||
[]TierStatus(nil), status.Tiers...,
|
||||
)
|
||||
si.lastEmitted = &snapshot
|
||||
}
|
||||
si.emitMu.Unlock()
|
||||
|
||||
if unchanged {
|
||||
return
|
||||
}
|
||||
|
||||
events.Emit(si.runtimeCtx, events.IndexStatusChanged, status)
|
||||
|
||||
// Mirror into the shared job registry. Every status mutation goes
|
||||
@@ -2644,6 +2699,13 @@ func (si *SearchIndex) MarkReadyIfPopulated() {
|
||||
|
||||
si.logger.Info("search index: using existing index", "entries", count)
|
||||
|
||||
// Becoming ready is a change the UI has to see, and this was the one
|
||||
// path that mutated the status without saying so — the 3 s ticker
|
||||
// carried it, invisibly, which is why removing the ticker without
|
||||
// this line would have left the settings page reading "not ready"
|
||||
// over a fully built index.
|
||||
si.emitStatus()
|
||||
|
||||
if !championBuilt {
|
||||
si.scheduleChampionRebuild()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user