diff --git a/internal/catalog/live.go b/internal/catalog/live.go index aa5b4211..2779d1d8 100644 --- a/internal/catalog/live.go +++ b/internal/catalog/live.go @@ -127,8 +127,15 @@ func SaveLive(provider, base string, models []Model) error { // of their own up to date. var Changed func() -// Touched tells Changed, if set. +// Forget, when set, is told so first: the provider package drops the +// catalog a request holds (provider.Hold). +var Forget func() + +// Touched tells Forget and Changed, if set. func Touched() { + if Forget != nil { + Forget() + } if Changed != nil { Changed() } diff --git a/internal/gui/api.go b/internal/gui/api.go index 05ceff9b..41db1184 100644 --- a/internal/gui/api.go +++ b/internal/gui/api.go @@ -988,7 +988,21 @@ func Handler(w Windows, gw *gateway.Server) http.Handler { }) agentModelsAPI(mux) devListen(mux) - return mux + return held(mux) +} + +// held has a page's reads share one build of the catalog, which every row +// resolving its model rebuilt (provider.Hold): /api/state took 4s with a +// few hundred models. Anything else may have written, and drops it. +func held(h http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/api/") { + defer provider.Hold()() + } else { + defer provider.Changed() + } + h.ServeHTTP(rw, r) + }) } func state() stateJSON { diff --git a/internal/provider/hold.go b/internal/provider/hold.go new file mode 100644 index 00000000..2806330b --- /dev/null +++ b/internal/provider/hold.go @@ -0,0 +1,98 @@ +package provider + +import ( + "slices" + "sync" + "time" + + "github.com/yetone/magpie/internal/catalog" + "github.com/yetone/magpie/internal/plugin" +) + +// The catalog is rebuilt from every provider's files on each Resolve (tens +// of milliseconds with a few hundred models), and a page of the GUI +// resolves a model for each row: /api/state and /api/groups took 4s and +// 7s with 24 providers. A request that only reads holds the catalog it +// built first for all its look-ups. +var held struct { + sync.Mutex + holds int + gen uint64 + // what was built, and when, by what it is + built map[string]heldValue +} + +type heldValue struct { + v any + at time.Time +} + +func init() { + catalog.Forget = Changed + // a plugin's sign-in saved or renewed, a plugin added + plugin.OnChange(Changed) +} + +// heldFor is how long a held catalog is used before it is built again, +// while one request after another keeps it held. +const heldFor = time.Second + +// Hold keeps the catalog built once until release is called, for a +// request that only reads; a write meanwhile (Changed) drops it. +func Hold() (release func()) { + held.Lock() + held.holds++ + held.Unlock() + return sync.OnceFunc(func() { + held.Lock() + if held.holds--; held.holds == 0 { + held.built = nil + } + held.Unlock() + }) +} + +// Changed drops what is held: a provider, an account, a plugin or a +// setting was written. +func Changed() { + held.Lock() + held.gen++ + held.built = nil + held.Unlock() +} + +// heldOf is what build gives, built once while a request holds it. +func heldOf[T any](key string, build func() T) T { + held.Lock() + if held.holds == 0 { + held.Unlock() + return build() + } + if h, ok := held.built[key]; ok && time.Since(h.at) < heldFor { + held.Unlock() + return h.v.(T) + } + gen := held.gen + held.Unlock() + v := build() + held.Lock() + if held.holds > 0 && held.gen == gen { + if held.built == nil { + held.built = map[string]heldValue{} + } + held.built[key] = heldValue{v, time.Now()} + } + held.Unlock() + return v +} + +func heldEntries(build func() []Entry) []Entry { + // clipped: a caller's append copies + return heldOf("entries", func() []Entry { return slices.Clip(build()) }) +} + +// heldPlugins is plugin.Cached, which reads the plugins' list and +// sign-ins each time, and PluginOf asks for every model. +func heldPlugins() []plugin.Provider { + return heldOf("plugins", func() []plugin.Provider { return slices.Clip(plugin.Cached()) }) +} diff --git a/internal/provider/hold_test.go b/internal/provider/hold_test.go new file mode 100644 index 00000000..6c98f497 --- /dev/null +++ b/internal/provider/hold_test.go @@ -0,0 +1,64 @@ +package provider + +import ( + "slices" + "testing" +) + +// A request that only reads builds the catalog once for all its look-ups +// (/api/state took 4s, resolving each agent's models anew); outside one, +// and after a write, it is built again. +func TestHoldBuildsOnce(t *testing.T) { + n := 0 + build := func() int { n++; return n } + heldOf("t", build) + heldOf("t", build) + if n != 2 { + t.Fatalf("not held: built %d times, want 2", n) + } + release := Hold() + heldOf("t", build) + heldOf("t", build) + if n != 3 { + t.Fatalf("held: built %d times, want 3", n) + } + Changed() + if heldOf("t", build); n != 4 { + t.Fatalf("after a write: built %d times, want 4", n) + } + release() + release() // a second release is no second one + if heldOf("t", build); n != 5 { + t.Fatalf("released: built %d times, want 5", n) + } + if held.holds != 0 { + t.Fatalf("holds left: %d", held.holds) + } +} + +// A provider saved while a request holds the catalog is in it at once. +func TestHoldSeesWrites(t *testing.T) { + if err := Save(Provider{ID: "held", Name: "Held", Chat: "https://held.example/v1", Key: "k", Models: []string{"a"}}); err != nil { + t.Fatal(err) + } + defer Hold()() + ids := func() []string { + var out []string + for _, e := range Catalog() { + out = append(out, e.ID) + } + return out + } + if !slices.Contains(ids(), "held/a") || slices.Contains(ids(), "held/b") { + t.Fatalf("before: %v", ids()) + } + if err := Save(Provider{ID: "held", Name: "Held", Chat: "https://held.example/v1", Key: "k", Models: []string{"a", "b"}}); err != nil { + t.Fatal(err) + } + if !slices.Contains(ids(), "held/b") { + t.Fatalf("a model picked meanwhile is missing: %v", ids()) + } + if p, m, ok := Resolve("held/b"); !ok || p.ID != "held" || m != "b" { + t.Fatalf("Resolve: %v %q %v", p.ID, m, ok) + } +} diff --git a/internal/provider/migrate.go b/internal/provider/migrate.go index 210a0eb9..28d2a090 100644 --- a/internal/provider/migrate.go +++ b/internal/provider/migrate.go @@ -354,6 +354,7 @@ func setMigration(id string, f func(m *Migration)) error { if err != nil { return err } + defer Changed() return writePrivate(migrationsPath(), append(b, '\n')) } diff --git a/internal/provider/models.go b/internal/provider/models.go index f9145ba3..8375edff 100644 --- a/internal/provider/models.go +++ b/internal/provider/models.go @@ -910,6 +910,10 @@ func Unlisted() []Entry { // providerEntries is the catalog without its groups. func providerEntries() []Entry { + return heldEntries(buildEntries) +} + +func buildEntries() []Entry { var out []Entry s := settings.Load() for _, p := range All() { @@ -1043,7 +1047,9 @@ func resolveIn(entries []Entry, id string) (Provider, string, bool) { } } if pid, model, ok := strings.Cut(id, "/"); ok { - if p, err := Find(pid); err == nil && p.On() { + // the providers a request holds: an agent's value of a model not + // listed lands here, and All is read anew each time + if p, err := findIn(heldOf("all", All), pid); err == nil && p.On() { return *p, model, true } } @@ -1058,15 +1064,9 @@ func resolveIn(entries []Entry, id string) (Provider, string, bool) { } // not exposed, but some provider lists it var found []Provider - for _, p := range All() { - if !p.On() || p.DecidesModel(id) { - continue - } - for _, m := range p.Available() { - if m.ID == id { - found = append(found, p) - break - } + for _, p := range heldOf("listed", listedBy)[id] { + if !p.DecidesModel(id) { + found = append(found, p) } } if len(found) == 1 { @@ -1075,6 +1075,24 @@ func resolveIn(entries []Entry, id string) (Provider, string, bool) { return Provider{}, "", false } +// listedBy is the providers on that list each model, in order, each once. +func listedBy() map[string][]Provider { + out := map[string][]Provider{} + for _, p := range All() { + if !p.On() { + continue + } + seen := map[string]bool{} + for _, m := range p.Available() { + if !seen[m.ID] { + seen[m.ID] = true + out[m.ID] = append(out[m.ID], p) + } + } + } + return out +} + // IDs lists the catalog ids, for error messages. func IDs() []string { var out []string diff --git a/internal/provider/plugins.go b/internal/provider/plugins.go index 751b59f4..52e87d0b 100644 --- a/internal/provider/plugins.go +++ b/internal/provider/plugins.go @@ -70,7 +70,7 @@ func (p Provider) PluginProvider() string { // PluginOf is the plugin provider magpie's provider id is, when it is one. func PluginOf(id string) (plugin.Provider, bool) { - for _, pp := range plugin.Cached() { + for _, pp := range heldPlugins() { if PluginID(pp.ID) == id { return pp, true } @@ -82,7 +82,7 @@ func PluginOf(id string) (plugin.Provider, bool) { // first account. func pluginAccounts() []Provider { var out []Provider - for _, pp := range plugin.Cached() { + for _, pp := range heldPlugins() { if movingNow(pp.ID) { continue // shown once the move is through (migrate.go) } diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 57a0d858..f23c31a4 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -351,8 +351,11 @@ func find(ps []Provider, id string) (Provider, bool) { // Find looks a provider up by id (or name, case-insensitively). func Find(id string) (*Provider, error) { + return findIn(All(), id) +} + +func findIn(all []Provider, id string) (*Provider, error) { q := strings.ToLower(strings.TrimSpace(id)) - all := All() // an id before a name: a provider of the user's called WorkBuddy isn't // the workbuddy subscription if i := slices.IndexFunc(all, func(p Provider) bool { return p.ID == q }); i >= 0 {