From cf49269972b5cd7c438e6a8f77ce77caf6a97dfb Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 22:20:18 +0000 Subject: [PATCH] perf(check): flow access and entity-access calls read a flow without decoding it Two whole-script checks run once per `check`, and each read every microflow of the project in full. CheckFlowAccess listed every microflow and nanoflow, decoding and converting each flow's body, for its name, Excluded flag and allowed module roles. CheckEntityAccessCalls unmarshalled every microflow and rule for the name, two flags and the call activities, including the flows that hold no call. A check of five statements cost 2 s on a 5,373-unit project whatever the script held, and had since v0.28.0 (mendixlabs/mxcli#1842). Backend.ListFlowAccess reads the three values from each stored flow's top level, over the units ListMicroflows and ListNanoflows cover; a backend without it is asked for the listings as before. storedEAFlow reads the name and flags the same way and decodes a document only when its bytes hold a call type's name, which a document with a call always does. Both are compared with what they replace: the listing on the fixture's 31 flows after a script gives one roles and excludes another, the reader on every stored microflow and rule and on bytes that are not a document. At 5,373 units, best of three, warm: check of 5 updates 1.98 s -> 0.91 s, of 200 updates 3.85 s -> 2.83 s. Output identical. Refs #2160 Co-Authored-By: Claude Fable 5.2 Claude-Session: https://claude.ai/code/session_011oK82u4HRCKfX2BBvhgsNm --- ...1-12-check-decodes-every-flow-per-run.json | 1 + CHANGELOG.md | 1 + mdl/backend/modelsdk/microflow.go | 45 ++++++++++ .../flow_access_listing_pedapp_test.go | 76 ++++++++++++++++ mdl/executor/validate_entity_access_calls.go | 85 +++++++++++++++--- .../validate_entity_access_calls_read_test.go | 87 +++++++++++++++++++ mdl/executor/validate_flow_access.go | 18 ++++ mdl/types/infrastructure.go | 11 +++ 8 files changed, 314 insertions(+), 10 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-11-12-check-decodes-every-flow-per-run.json create mode 100644 mdl/executor/flow_access_listing_pedapp_test.go create mode 100644 mdl/executor/validate_entity_access_calls_read_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-11-12-check-decodes-every-flow-per-run.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-12-check-decodes-every-flow-per-run.json new file mode 100644 index 0000000000..07e6d3d7ef --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-12-check-decodes-every-flow-per-run.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-11-12", "symptom": "`check` of even a five-statement script took about 2 s on a 5,373-unit project with a warm file cache (1 s at 1,373 units), unchanged from v0.28.0 through v0.31.0 to main (mendixlabs/mxcli#1842, ako/mxcli#2160)", "cause": "two whole-script checks run once per check and each read every microflow of the project in full. CheckFlowAccess listed every microflow and nanoflow through ListMicroflows/ListNanoflows, decoding and converting each flow's whole body, for its name, Excluded flag and allowed module roles. CheckEntityAccessCalls unmarshalled every microflow and rule into a bson.D to read the name, two flags and the call activities, including for the flows that hold no call", "file": "mdl/executor/validate_entity_access_calls.go (storedEAFlow, storedEAFlowDecoded), mdl/executor/validate_flow_access.go (flowAccessLister), mdl/backend/modelsdk/microflow.go (ListFlowAccess), mdl/types/infrastructure.go (FlowAccess)", "insight": "Every measurement under #2160 until this one was of exec, or of check with hundreds of statements, where a per-run cost of a second or two is noise; the reporter's batch-size table (check: 3.5 s for 10 statements, 4.5 s for 200) is what showed a cost per run, and rerunning their experiment on three versions showed nobody had ever moved it. Both checks need a few top-level values of every flow, and both got them by decoding the flow. The values are read with bson.Raw lookups instead; the calls, which are anywhere in the body, still need the document decoded, but only when its bytes contain a call type's name - a $Type is stored as its name, so a document with a call always contains it. The flow listing went into the backend rather than the executor's raw listing so that it covers exactly the units ListMicroflows covers, and so mock backends, which serve only the decoded listings, keep working through the fallback. The fixture has 16 stored flows and one with a call, too thin to carry the claim alone: the old and new readers were also compared on 5,632 flows of two generated projects, 301 with calls. Measured at 5,373 units, best of three, warm: check of 5 updates 1.98 s -> 0.91 s, 100 updates 2.91 s -> 1.75 s, 200 updates 3.85 s -> 2.83 s; output identical. Cold file reads, which made the first check after a copy take 4 s, are not changed."} diff --git a/CHANGELOG.md b/CHANGELOG.md index 321a3cdb8e..df137344fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **`check` no longer decodes every microflow of the project before it starts** (mendixlabs/mxcli#1842, ako/mxcli#2160) — two of its checks, flow access (who may run a flow) and entity-access calls (MDL-SEC22), each read every microflow in full once per run, so a `check` of five statements cost seconds on a large project whatever the script held. They now read the few values they need from each stored flow, and decode a flow's body only when it contains a call. What `check` reports is unchanged. On a project of 5,373 units, with files already read once: `check` of 5 `create or modify microflow` statements 2.0 s before, 0.9 s after; of 200, 3.9 s before, 2.8 s after. - **A pluggable widget's template is prepared once per widget type** (ako/mxcli#2160) — every data grid, filter and other pluggable widget a script builds loaded its template by deep-cloning the built-in one and merging the project's widget package into the clone, the same work each time. The merged template is now made once for each widget type and package and reused; each widget still gets IDs of its own. Creating 600 pages: 29.5 s before, 26.5 s after; re-running them settled: 83.1 s before, 73.9 s after. The pages written are the same: all 1,719 units of the two projects compare equal once IDs are set aside. - **`describe page` loads the page it describes** (ako/mxcli#2160) — it loaded every page with the same local name, in whatever module, before picking the one asked for, and read the page's document twice. `Overview`, `NewEdit` and `Home` repeat across the modules of most projects, and rewriting a page describes the stored one up to three times, so a script that re-runs its pages paid for it per statement. Output is unchanged. Re-running a settled script of 600 `create or modify page` statements: 87.8 s before, 77.2 s after. - **The decode cache compares a unit's bytes once per run** (ako/mxcli#2160) — the cache that spares a script decoding the same unit again recognises the unit by its bytes. It kept the buffer of an earlier pass, so in the next pass every listing compared every unit of the type byte for byte: all of a project's pages, twice per page statement. It now keeps the buffer it matched. Re-running a settled script of 600 `create or modify page` statements: 103.7 s before, 86.8 s after. diff --git a/mdl/backend/modelsdk/microflow.go b/mdl/backend/modelsdk/microflow.go index 2837572047..5d71042cae 100644 --- a/mdl/backend/modelsdk/microflow.go +++ b/mdl/backend/modelsdk/microflow.go @@ -3,6 +3,7 @@ package modelsdkbackend import ( + "go.mongodb.org/mongo-driver/v2/bson" "strconv" "strings" @@ -136,6 +137,50 @@ func (b *Backend) GetNanoflow(id model.ID) (*microflows.Nanoflow, error) { return nanoflowFromGen(u.Element, u.ContainerID), nil } +// ListFlowAccess lists, for every microflow and then every nanoflow, what +// deciding its access takes (types.FlowAccess), in the order ListMicroflows +// and ListNanoflows list them, and without decoding any flow: the name, the +// Excluded flag and the allowed module roles are top-level values of the +// stored document, read where they lie. +// +// `check` simulates flow access once per script and needs these three values +// of every flow in the project. Listing the flows for them decoded and +// converted every flow's whole body, which is what a check cost per project +// unit before it had looked at a statement (ako/mxcli#2160, +// mendixlabs/mxcli#1842). +func (b *Backend) ListFlowAccess() ([]types.FlowAccess, error) { + var out []types.FlowAccess + for _, kind := range []struct{ name, typ string }{ + {"microflow", "Microflows$Microflow"}, + {"nanoflow", "Microflows$Nanoflow"}, + } { + refs, err := b.reader.ListUnitsByTypeShared(kind.typ, b.unitScope) + if err != nil { + return nil, err + } + for _, ref := range refs { + if ref.Type != kind.typ { + continue + } + raw := bson.Raw(ref.Contents) + fa := types.FlowAccess{Kind: kind.name, ContainerID: model.ID(ref.ContainerID)} + fa.Name, _ = raw.Lookup("Name").StringValueOK() + fa.Excluded, _ = raw.Lookup("Excluded").BooleanOK() + if roles, ok := raw.Lookup("AllowedModuleRoles").ArrayOK(); ok { + values, _ := roles.Values() + for _, v := range values { + // The list's first value is its marker, a number. + if qn, ok := v.StringValueOK(); ok { + fa.AllowedModuleRoles = append(fa.AllowedModuleRoles, model.ID(qn)) + } + } + } + out = append(out, fa) + } + } + return out, nil +} + // ListNanoflowHeaders lists every nanoflow's ID, container, name and Excluded // flag without decoding any flow. func (b *Backend) ListNanoflowHeaders() ([]types.DocumentHeader, error) { diff --git a/mdl/executor/flow_access_listing_pedapp_test.go b/mdl/executor/flow_access_listing_pedapp_test.go new file mode 100644 index 0000000000..d27ebffecc --- /dev/null +++ b/mdl/executor/flow_access_listing_pedapp_test.go @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "reflect" + "testing" + + "github.com/mendixlabs/mxcli/mdl/types" +) + +// ListFlowAccess reads a flow's name, Excluded flag and allowed module roles +// from the stored document's top level, where the flow listings decode and +// convert the whole flow for them. It must list what they list, in their +// order: compared on every microflow and nanoflow of the fixture, after a +// script has given one flow roles and excluded another, so that neither value +// is compared only in its empty form. +func TestListFlowAccess_ListsWhatTheFlowListingsList(t *testing.T) { + exec, out, _ := openPedAppCopy(t) + const setup = `mdl 1; +create module role MyFirstModule.FlowAccessProbe; +create microflow MyFirstModule.FlowAccessGranted () returns boolean begin return true; end; +grant execute on microflow MyFirstModule.FlowAccessGranted to MyFirstModule.FlowAccessProbe; +@excluded +create microflow MyFirstModule.FlowAccessExcluded () returns boolean begin return true; end; +` + if err := runScript(t, exec, setup); err != nil { + t.Fatalf("setup: %v\n%s", err, out.String()) + } + lister, ok := exec.Backend().(flowAccessLister) + if !ok { + t.Fatalf("backend %T does not list flow access", exec.Backend()) + } + got, err := lister.ListFlowAccess() + if err != nil { + t.Fatalf("ListFlowAccess: %v", err) + } + + var want []types.FlowAccess + mfs, err := exec.Backend().ListMicroflows() + if err != nil { + t.Fatalf("ListMicroflows: %v", err) + } + for _, mf := range mfs { + want = append(want, types.FlowAccess{Kind: "microflow", ContainerID: mf.ContainerID, Name: mf.Name, Excluded: mf.Excluded, AllowedModuleRoles: mf.AllowedModuleRoles}) + } + nfs, err := exec.Backend().ListNanoflows() + if err != nil { + t.Fatalf("ListNanoflows: %v", err) + } + for _, nf := range nfs { + want = append(want, types.FlowAccess{Kind: "nanoflow", ContainerID: nf.ContainerID, Name: nf.Name, Excluded: nf.Excluded, AllowedModuleRoles: nf.AllowedModuleRoles}) + } + + withRoles, excluded := 0, 0 + for _, f := range want { + if len(f.AllowedModuleRoles) > 0 { + withRoles++ + } + if f.Excluded { + excluded++ + } + } + t.Logf("%d microflows, %d nanoflows; %d with roles, %d excluded", len(mfs), len(nfs), withRoles, excluded) + if len(mfs) == 0 || len(nfs) == 0 || withRoles == 0 || excluded == 0 { + t.Fatalf("control: the fixture must hold microflows, nanoflows, a flow with roles and an excluded one") + } + if len(got) != len(want) { + t.Fatalf("ListFlowAccess lists %d flows, the flow listings %d", len(got), len(want)) + } + for i := range want { + if !reflect.DeepEqual(got[i], want[i]) { + t.Errorf("flow %d: ListFlowAccess gives %+v, the flow listing %+v", i, got[i], want[i]) + } + } +} diff --git a/mdl/executor/validate_entity_access_calls.go b/mdl/executor/validate_entity_access_calls.go index faf58abd99..845ab97c9e 100644 --- a/mdl/executor/validate_entity_access_calls.go +++ b/mdl/executor/validate_entity_access_calls.go @@ -3,10 +3,12 @@ package executor import ( + "bytes" "context" "fmt" "sort" "strings" + "sync/atomic" "go.mongodb.org/mongo-driver/v2/bson" @@ -93,21 +95,15 @@ func CheckEntityAccessCalls(ctx *ExecContext, prog *ast.Program) []linter.Violat if u.Type != typ || len(u.Contents) == 0 { continue } - var doc bson.D - if err := bson.Unmarshal(u.Contents, &doc); err != nil { + st, ok := storedEAFlow(u.Contents, kind == "microflow") + if !ok { continue } - name, _ := bsonField(doc, "Name").(string) - qn := h.GetQualifiedName(u.ContainerID, name) - excluded, _ := bsonField(doc, "Excluded").(bool) + qn := h.GetQualifiedName(u.ContainerID, st.name) if prev, ok := sim.flows[qn]; ok && !prev.excluded { continue // a live namesake wins over excluded ones (#914) } - ea, _ := bsonField(doc, "ApplyEntityAccess").(bool) - f := &eaFlow{kind: kind, name: qn, exists: true, excluded: excluded, ea: ea} - if kind == "microflow" { - f.calls = collectRawEntityAccessCalls(doc) - } + f := &eaFlow{kind: kind, name: qn, exists: true, excluded: st.excluded, ea: st.ea, calls: st.calls} sim.flows[qn] = f } } @@ -173,6 +169,75 @@ func scriptTouchesEntityAccessCalls(prog *ast.Program) bool { return false } +// storedEAState is what the simulation reads of one stored microflow or rule. +type storedEAState struct { + name string + excluded, ea bool + calls []eaCall +} + +// eaCallMarkers are the $Type values collectRawEntityAccessCalls looks for. A +// document that holds neither string holds no call. +var eaCallMarkers = [][]byte{[]byte("Microflows$MicroflowCall"), []byte("Microflows$RuleCall")} + +// eaDocumentsDecoded counts the stored documents decoded in full to read +// their calls. For tests. +var eaDocumentsDecoded atomic.Int64 + +// storedEAFlow reads one stored microflow or rule for the simulation. ok is +// false for contents that are not a document. +// +// The check runs once per script and reads every microflow of the project, so +// what it costs per document is what `check` costs per project unit before it +// has looked at a statement (ako/mxcli#2160, mendixlabs/mxcli#1842). The name +// and the two flags are top-level values, read where they lie. The calls are +// anywhere in the body, and finding them takes the whole document decoded: +// that is done only for a document whose bytes hold one of the call types' +// names, which a document with a call always does, since a $Type is stored as +// its name. +func storedEAFlow(contents []byte, withCalls bool) (storedEAState, bool) { + raw := bson.Raw(contents) + if raw.Validate() != nil { + return storedEAState{}, false + } + var st storedEAState + st.name, _ = raw.Lookup("Name").StringValueOK() + st.excluded, _ = raw.Lookup("Excluded").BooleanOK() + st.ea, _ = raw.Lookup("ApplyEntityAccess").BooleanOK() + if !withCalls { + return st, true + } + for _, marker := range eaCallMarkers { + if bytes.Contains(contents, marker) { + var doc bson.D + if err := bson.Unmarshal(contents, &doc); err != nil { + return storedEAState{}, false + } + eaDocumentsDecoded.Add(1) + st.calls = collectRawEntityAccessCalls(doc) + break + } + } + return st, true +} + +// storedEAFlowDecoded is storedEAFlow as it was: the whole document decoded, +// whatever it holds. It is what storedEAFlow is compared with. +func storedEAFlowDecoded(contents []byte, withCalls bool) (storedEAState, bool) { + var doc bson.D + if err := bson.Unmarshal(contents, &doc); err != nil { + return storedEAState{}, false + } + var st storedEAState + st.name, _ = bsonField(doc, "Name").(string) + st.excluded, _ = bsonField(doc, "Excluded").(bool) + st.ea, _ = bsonField(doc, "ApplyEntityAccess").(bool) + if withCalls { + st.calls = collectRawEntityAccessCalls(doc) + } + return st, true +} + // collectRawEntityAccessCalls reads the calls out of a stored microflow: the // qualified name under `Microflow` in a Microflows$MicroflowCall (call // activities, queued or not) or a Microflows$RuleCall (a rule split). diff --git a/mdl/executor/validate_entity_access_calls_read_test.go b/mdl/executor/validate_entity_access_calls_read_test.go new file mode 100644 index 0000000000..59e4111665 --- /dev/null +++ b/mdl/executor/validate_entity_access_calls_read_test.go @@ -0,0 +1,87 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "io/fs" + "os" + "path/filepath" + "reflect" + "testing" + + "go.mongodb.org/mongo-driver/v2/bson" +) + +// pedAppFlowUnits is the stored bytes of every microflow and rule of the +// PedApp fixture. +func pedAppFlowUnits(t *testing.T) [][]byte { + t.Helper() + root := filepath.Join("..", "..", "testdata", "pedapp", "mprcontents") + var out [][]byte + err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil || d.IsDir() || filepath.Ext(path) != ".mxunit" { + return err + } + raw, err := os.ReadFile(path) + if err != nil { + return err + } + switch typ, _ := bson.Raw(raw).Lookup("$Type").StringValueOK(); typ { + case "Microflows$Microflow", "Microflows$Rule": + out = append(out, raw) + } + return nil + }) + if err != nil { + t.Fatalf("read the PedApp units: %v", err) + } + if len(out) < 10 { + t.Fatalf("found %d microflows and rules in the fixture, expected its whole set", len(out)) + } + return out +} + +// storedEAFlow reads the name and flags where they lie and decodes a document +// only when its bytes name a call type. It must read what decoding every +// document read: compared on every microflow and rule of the fixture, with +// and without the calls, and on bytes that are not a document. +func TestStoredEAFlow_ReadsWhatDecodingTheDocumentRead(t *testing.T) { + units := pedAppFlowUnits(t) + withCalls, without := 0, 0 + before := eaDocumentsDecoded.Load() + for i, raw := range units { + for _, calls := range []bool{true, false} { + want, wantOK := storedEAFlowDecoded(raw, calls) + got, gotOK := storedEAFlow(raw, calls) + if gotOK != wantOK || !reflect.DeepEqual(got, want) { + t.Fatalf("unit %d (calls %v): read %+v (ok %v), decoding the document reads %+v (ok %v)", i, calls, got, gotOK, want, wantOK) + } + if want.name == "" { + t.Fatalf("control: unit %d has no name, so the comparison is of empty values", i) + } + if calls { + if len(want.calls) > 0 { + withCalls++ + } else { + without++ + } + } + } + } + decoded := eaDocumentsDecoded.Load() - before + t.Logf("%d documents: %d with calls, %d without; %d decoded in full", len(units), withCalls, without, decoded) + // Controls: both kinds must be in the fixture, and the ones without a + // call must not have been decoded. + if withCalls == 0 || without == 0 { + t.Fatalf("the fixture does not hold both kinds: %d with calls, %d without", withCalls, without) + } + if decoded > int64(withCalls)+int64(len(units))/10 { + t.Errorf("%d documents decoded in full for %d that hold a call", decoded, withCalls) + } + for _, junk := range [][]byte{nil, {}, []byte("not a document"), units[0][:len(units[0])/2]} { + _, wantOK := storedEAFlowDecoded(junk, true) + if _, gotOK := storedEAFlow(junk, true); gotOK != wantOK { + t.Errorf("%d bytes that are not a document: ok %v, decoding gives ok %v", len(junk), gotOK, wantOK) + } + } +} diff --git a/mdl/executor/validate_flow_access.go b/mdl/executor/validate_flow_access.go index f26f293a59..b875bd8a2a 100644 --- a/mdl/executor/validate_flow_access.go +++ b/mdl/executor/validate_flow_access.go @@ -5,6 +5,7 @@ package executor import ( "context" "fmt" + "github.com/mendixlabs/mxcli/mdl/types" "sort" "strings" @@ -210,6 +211,17 @@ func (s *flowAccessSim) loadStoredFlows(h *ContainerHierarchy) { } s.flows[key] = st } + // A backend that can say what a flow's access takes without decoding the + // flow is asked that; the listings below decode every flow of the project + // for three top-level values of each. + if l, ok := s.ctx.Backend.(flowAccessLister); ok { + if flows, err := l.ListFlowAccess(); err == nil { + for _, f := range flows { + put(f.Kind, h.GetQualifiedName(f.ContainerID, f.Name), f.Excluded, f.AllowedModuleRoles) + } + return + } + } if mfs, err := projectMicroflows(s.ctx); err == nil { for _, mf := range mfs { put("microflow", h.GetQualifiedName(mf.ContainerID, mf.Name), mf.Excluded, mf.AllowedModuleRoles) @@ -222,6 +234,12 @@ func (s *flowAccessSim) loadStoredFlows(h *ContainerHierarchy) { } } +// flowAccessLister is a backend that lists every flow's access without +// decoding the flows (modelsdk.Backend.ListFlowAccess). +type flowAccessLister interface { + ListFlowAccess() ([]types.FlowAccess, error) +} + // loadStoredUses reads every flow reference out of the documents that make a // flow need a role. The walk is over raw BSON because the references live in // many shapes (button actions, data sources, menu items, call activities) that diff --git a/mdl/types/infrastructure.go b/mdl/types/infrastructure.go index b92f3f88e6..12aa079c86 100644 --- a/mdl/types/infrastructure.go +++ b/mdl/types/infrastructure.go @@ -75,6 +75,17 @@ type DocumentHeader struct { Excluded bool } +// FlowAccess is what deciding a stored flow's access takes: where it is, what +// it is called, whether it is excluded, and the module roles allowed to run it +// (qualified names). Kind is "microflow" or "nanoflow". +type FlowAccess struct { + Kind string + ContainerID model.ID + Name string + Excluded bool + AllowedModuleRoles []model.ID +} + // UnitInfo is a lightweight unit descriptor. type UnitInfo struct { ID model.ID