From dff14ff4651aaa777c352df14cfe8b4bf267ff59 Mon Sep 17 00:00:00 2001 From: yetone Date: Sat, 3 Oct 2026 12:36:15 +0800 Subject: [PATCH] =?UTF-8?q?gui:=20the=20panel's=20reads=20build=20the=20ca?= =?UTF-8?q?talog=20once=20per=20request=20(owner:=20=E8=BF=99=E4=B8=AA?= =?UTF-8?q?=E5=BA=94=E8=AF=A5=E6=98=AF=20magpie=20API=20=E6=85=A2,=20on=20?= =?UTF-8?q?a=20Discord=20report=20that=20the=20window=20renders=20slowly)?= =?UTF-8?q?=20=E2=80=94=20provider.Resolve=20rebuilt=20every=20provider's?= =?UTF-8?q?=20catalog=20(24=20providers,=20270=20models:=2035-90ms)=20for?= =?UTF-8?q?=20each=20model=20it=20was=20asked=20about,=20and=20/api/state?= =?UTF-8?q?=20and=20/api/groups=20resolve=20one=20for=20every=20agent=20an?= =?UTF-8?q?d=20row;=20plugin.Cached=20re-read=20the=20plugins'=20files=20f?= =?UTF-8?q?or=20each=20PluginOf,=20and=20resolveIn's=20fallbacks=20read=20?= =?UTF-8?q?All()=20and=20every=20provider's=20Available()=20again.=20A=20G?= =?UTF-8?q?ET=20under=20/api/=20now=20holds=20what=20it=20builds=20(provid?= =?UTF-8?q?er.Hold:=20the=20entries,=20the=20plugins,=20All,=20which=20pro?= =?UTF-8?q?viders=20list=20a=20model)=20for=20the=20request,=20at=20most?= =?UTF-8?q?=201s=20while=20requests=20overlap;=20a=20save=20drops=20it=20(?= =?UTF-8?q?catalog.Touched=20=E2=86=92=20catalog.Forget,=20plugin.OnChange?= =?UTF-8?q?,=20setMigration,=20and=20any=20non-GET=20request).=20Outside?= =?UTF-8?q?=20a=20request=20nothing=20is=20held,=20so=20the=20gateway=20an?= =?UTF-8?q?d=20tests=20behave=20as=20before.=20On=20the=20owner's=20config?= =?UTF-8?q?:=20state=204.2s=20=E2=86=92=200.42s,=20groups=206.7s=20?= =?UTF-8?q?=E2=86=92=200.26s,=20providers=201.4s=20=E2=86=92=200.33s.=20Sa?= =?UTF-8?q?ndbox=20(fixed=20build,=20GETs=20looping):=20a=20model=20saved?= =?UTF-8?q?=20through=20/api/provider/save=20is=20in=20providers,=20groups?= =?UTF-8?q?=20and=20state=20at=20once=20and=20gone=20after=20the=20restore?= =?UTF-8?q?.=20TestHoldBuildsOnce,=20TestHoldSeesWrites=20(fails=20with=20?= =?UTF-8?q?Forget=20unset).=20go=20vet,=20go=20build=20-tags=20nogui=20(da?= =?UTF-8?q?rwin,=20GOOS=3Dlinux),=20GOOS=3Dwindows=20go=20build,=20go=20te?= =?UTF-8?q?st=20-tags=20nogui=20-count=3D1=20./...=20under=20a=20temp=20HO?= =?UTF-8?q?ME=20pass.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/catalog/live.go | 9 +++- internal/gui/api.go | 16 +++++- internal/provider/hold.go | 98 ++++++++++++++++++++++++++++++++++ internal/provider/hold_test.go | 64 ++++++++++++++++++++++ internal/provider/migrate.go | 1 + internal/provider/models.go | 38 +++++++++---- internal/provider/plugins.go | 4 +- internal/provider/provider.go | 5 +- 8 files changed, 220 insertions(+), 15 deletions(-) create mode 100644 internal/provider/hold.go create mode 100644 internal/provider/hold_test.go 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 {