From 3936e1ac19e203a7200dcac99b1a4ff43f4ea209 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 9 Oct 2026 04:02:26 +0000 Subject: [PATCH 01/10] perf(grpc): convert a cached model's diagnostics to protobuf once Co-Authored-By: jason.han (cherry picked from commit 70242d87299675fcdc725ef62bc40aebad3a41d2) --- ...rpc-cache-diagnostics-proto.performance.md | 1 + internal/frontend/grpc/cache.go | 32 ++++ .../grpc/diagnostics_converted_once_test.go | 175 ++++++++++++++++++ internal/frontend/grpc/lineage.go | 44 ++++- internal/frontend/grpc/service.go | 46 +++-- 5 files changed, 272 insertions(+), 26 deletions(-) create mode 100644 changes/unreleased/grpc-cache-diagnostics-proto.performance.md create mode 100644 internal/frontend/grpc/diagnostics_converted_once_test.go diff --git a/changes/unreleased/grpc-cache-diagnostics-proto.performance.md b/changes/unreleased/grpc-cache-diagnostics-proto.performance.md new file mode 100644 index 0000000000..3db5349dd2 --- /dev/null +++ b/changes/unreleased/grpc-cache-diagnostics-proto.performance.md @@ -0,0 +1 @@ +- **The gRPC service converts a cached model's diagnostics to protobuf once.** `ParseFile`, `ParseSources` and `GetDiagnostics` on a model parsed before answered the same diagnostics converted again on every call, two allocations per warning; each document's are now converted on first use and shared by every later response, and a model answered from a lineage shares those of every document the edit did not reach. The responses are unchanged. diff --git a/internal/frontend/grpc/cache.go b/internal/frontend/grpc/cache.go index de86a341ef..db18fe4f0e 100644 --- a/internal/frontend/grpc/cache.go +++ b/internal/frontend/grpc/cache.go @@ -8,6 +8,7 @@ import ( "connectrpc.com/connect" + pb "github.com/Open-MBEE/OpenSysML/api/proto" "github.com/Open-MBEE/OpenSysML/internal/check/passes" "github.com/Open-MBEE/OpenSysML/internal/exec/analysis" "github.com/Open-MBEE/OpenSysML/internal/exec/runtime" @@ -34,6 +35,37 @@ type CachedDocument struct { // and the passes' own findings when the model parsed clean and was analyzed. Diagnostics []diag.Diagnostic Warnings []string + // proto is Diagnostics and Warnings converted to protobuf, once per document; + // a lineage hands the same cell to the next model when neither changed. + proto *protoDiagnostics +} + +// protoDiagnostics is one document's diagnostics as protobuf, built on first use +// and shared by every response that reports them, so nothing may mutate them +// once built; the service's capability filter is applied as they are built. +type protoDiagnostics struct { + once sync.Once + diags []*pb.Diagnostic +} + +// newCachedDocument is a document whose diagnostics are converted at most once. +func newCachedDocument(root *ast.RootNamespace, sf *source.SourceFile, parseDiags []parser.Diagnostic, + diagnostics []diag.Diagnostic, warnings []string) *CachedDocument { + return &CachedDocument{ + Root: root, Source: sf, ParseDiags: parseDiags, Diagnostics: diagnostics, + Warnings: append([]string(nil), warnings...), proto: &protoDiagnostics{}, + } +} + +// protoDiagnostics is the document's diagnostics as build converts them, built +// the first time any response asks for them. A document built without a cell +// converts on every call. +func (d *CachedDocument) protoDiagnostics(build func(*CachedDocument) []*pb.Diagnostic) []*pb.Diagnostic { + if d.proto == nil { + return build(d) + } + d.proto.once.Do(func() { d.proto.diags = build(d) }) + return d.proto.diags } // CachedModel holds parsed model data with semantic analysis results diff --git a/internal/frontend/grpc/diagnostics_converted_once_test.go b/internal/frontend/grpc/diagnostics_converted_once_test.go new file mode 100644 index 0000000000..8d686206e6 --- /dev/null +++ b/internal/frontend/grpc/diagnostics_converted_once_test.go @@ -0,0 +1,175 @@ +package grpc + +import ( + "context" + "testing" + + pb "github.com/Open-MBEE/OpenSysML/api/proto" + "google.golang.org/protobuf/proto" +) + +// warningLibrary, warningTop and warningOther are a model whose top document +// parses clean with a warning, so a parse of it reports diagnostics. +const ( + warningLibrary = "package Lib {\n\tpart def Engine;\n}\n" + warningTop = "package Top {\n\tprivate import Lib::*;\n\tcalc def F { 1 /* c */ }\n\tpart def Car {\n\t\tpart motor : Engine;\n\t}\n}\n" + warningOther = "package Other {\n\tpart def Wheel;\n}\n" +) + +// requireWarnings fails unless diags hold a warning and nothing more severe. +func requireWarnings(t *testing.T, diags []*pb.Diagnostic) { + t.Helper() + if len(diags) == 0 { + t.Fatal("no diagnostics to compare") + } + for _, d := range diags { + if d.Severity != "warning" { + t.Fatalf("diagnostic %v is not a warning", d) + } + } +} + +// requireSameDiagnostics fails unless got repeats want message for message. +func requireSameDiagnostics(t *testing.T, got, want []*pb.Diagnostic) { + t.Helper() + if len(got) != len(want) { + t.Fatalf("%d diagnostics, want %d:\n%v\n%v", len(got), len(want), got, want) + } + for i := range got { + if !proto.Equal(got[i], want[i]) { + t.Errorf("diagnostic %d is %v, want %v", i, got[i], want[i]) + } + } +} + +// A parse of a model parsed before answers the diagnostics the first parse +// did, converted once: the messages are the same, so a service seeing the model +// for the first time, a repeated parse and GetDiagnostics all agree, and a +// service that withholds diagnostic codes withholds them on every call. +func TestParseFileConvertsACachedModelsDiagnosticsOnce(t *testing.T) { + ctx := context.Background() + req := &pb.ParseFileRequest{Source: &pb.ParseFileRequest_Content{Content: misplacedComment}} + fresh := mustNewService(t, 10) + defer fresh.Close() + want, err := fresh.ParseFile(ctx, req) + if err != nil { + t.Fatalf("ParseFile: %v", err) + } + requireWarnings(t, want.Diagnostics) + requireEveryCoded(t, want.Diagnostics) + + srv := mustNewService(t, 10) + defer srv.Close() + first, err := srv.ParseFile(ctx, req) + if err != nil { + t.Fatalf("ParseFile: %v", err) + } + second, err := srv.ParseFile(ctx, req) + if err != nil { + t.Fatalf("ParseFile again: %v", err) + } + if !proto.Equal(first, want) { + t.Errorf("first parse is\n%v\nfresh parse is\n%v", first, want) + } + if !proto.Equal(second, want) { + t.Errorf("repeated parse is\n%v\nfresh parse is\n%v", second, want) + } + for i := range first.Diagnostics { + if first.Diagnostics[i] != second.Diagnostics[i] { + t.Errorf("diagnostic %d was converted again for the repeated parse", i) + } + } + // The list is the response's own: growing one leaves the other as it was. + second.Diagnostics = append(second.Diagnostics, &pb.Diagnostic{Severity: "error"}) + diags, err := srv.GetDiagnostics(ctx, &pb.DiagnosticsRequest{ModelHash: first.ModelHash}) + if err != nil { + t.Fatalf("GetDiagnostics: %v", err) + } + requireSameDiagnostics(t, diags.Diagnostics, want.Diagnostics) + requireSameDiagnostics(t, first.Diagnostics, want.Diagnostics) + + withheld, err := NewServiceWithUnavailableCapabilitiesForTesting(10, "test", []string{CapabilityDiagnosticCodes}) + if err != nil { + t.Fatal(err) + } + defer withheld.Close() + for call := 0; call < 2; call++ { + resp, err := withheld.ParseFile(ctx, req) + if err != nil { + t.Fatalf("ParseFile: %v", err) + } + if len(resp.Diagnostics) != len(want.Diagnostics) { + t.Fatalf("withheld service reported %d diagnostics, want %d", len(resp.Diagnostics), len(want.Diagnostics)) + } + for i, d := range resp.Diagnostics { + uncoded := proto.Clone(want.Diagnostics[i]).(*pb.Diagnostic) + uncoded.Code = "" + if !proto.Equal(d, uncoded) { + t.Errorf("call %d: withheld diagnostic = %v, want %v", call, d, uncoded) + } + } + } +} + +// documentDiagnosticsOf are the diagnostics of resp located in the document named. +func documentDiagnosticsOf(resp *pb.ParseSourcesResponse, name string) []*pb.Diagnostic { + var out []*pb.Diagnostic + for _, d := range resp.Diagnostics { + if d.GetSpan().GetFile() == name { + out = append(out, d) + } + } + return out +} + +// A model answered from its lineage shares the converted diagnostics of every +// document the edit before it did not reach, and converts afresh those of the +// documents it did; either way it reports what a fresh parse of the same +// documents reports. +func TestParseSourcesSharesConvertedDiagnosticsOfDocumentsAnEditDidNotReach(t *testing.T) { + ctx := context.Background() + srv := mustNewService(t, 32) + defer srv.Close() + parse := func(srv *Service, documents []*pb.SourceDocument) *pb.ParseSourcesResponse { + t.Helper() + resp, err := srv.ParseSources(ctx, &pb.ParseSourcesRequest{Documents: documents}) + if err != nil { + t.Fatalf("ParseSources: %v", err) + } + for _, d := range resp.Diagnostics { + if d.Severity != "warning" { + t.Fatalf("diagnostic %v is not a warning", d) + } + } + return resp + } + documents := func(other string) []*pb.SourceDocument { + return inlineDocuments("lib.sysml", warningLibrary, "top.sysml", warningTop, "other.sysml", other) + } + // Parsed twice so the edits are answered from the set's lineage. + parse(srv, documents(warningOther+"// first\n")) + base := parse(srv, documents(warningOther)) + requireWarnings(t, documentDiagnosticsOf(base, "top.sysml")) + + untouched := parse(srv, documents(warningOther+"// a comment\n")) + requireSameDiagnostics(t, untouched.Diagnostics, base.Diagnostics) + before, after := documentDiagnosticsOf(base, "top.sysml"), documentDiagnosticsOf(untouched, "top.sysml") + for i := range before { + if before[i] != after[i] { + t.Errorf("top.sysml diagnostic %d was converted again though the edit did not reach the document", i) + } + } + + reached := inlineDocuments("lib.sysml", warningLibrary, "top.sysml", warningTop+"// edited\n", "other.sysml", warningOther) + edited := parse(srv, reached) + fresh := mustNewService(t, 10) + defer fresh.Close() + requireSameDiagnostics(t, edited.Diagnostics, parse(fresh, reached).Diagnostics) + for _, d := range documentDiagnosticsOf(edited, "top.sysml") { + for _, was := range before { + if d == was { + t.Errorf("top.sysml diagnostic %v is the base model's though the edit reached the document", d) + } + } + } +} diff --git a/internal/frontend/grpc/lineage.go b/internal/frontend/grpc/lineage.go index ca7caf48e6..8be828d6d5 100644 --- a/internal/frontend/grpc/lineage.go +++ b/internal/frontend/grpc/lineage.go @@ -2,6 +2,7 @@ package grpc import ( "container/list" + "slices" "sort" "strings" "sync" @@ -42,6 +43,16 @@ type lineage struct { library libs.Source // the files the workspace's library was built from held map[string]string // document name to the content the workspace holds versions int // the version the next update is given + // answered is each document as the last model was answered with it, and the + // stamp of the analysis it carried: the next model shares its converted + // diagnostics when the analysis, the source and the warnings are the same. + answered map[string]answeredDocument +} + +// answeredDocument is one document of the model a lineage last answered. +type answeredDocument struct { + doc *CachedDocument + stamp uint64 } // lineages holds the most recently used lineages, a few at most: a lineage @@ -171,7 +182,7 @@ func (s *Service) parseFromLineage(inputs []sourceInput, mode diag.ConformanceMo } } if len(standIns) > 0 { - l.fresh, l.standIns, l.ws, l.held = true, standIns, nil, nil + l.fresh, l.standIns, l.ws, l.held, l.answered = true, standIns, nil, nil, nil return nil, false } names := make([]string, len(inputs)) @@ -199,24 +210,41 @@ func (s *Service) parseFromLineage(inputs []sourceInput, mode diag.ConformanceMo documents := make([]*CachedDocument, len(inputs)) for i, input := range inputs { doc := l.ws.Document(input.name) - documents[i] = &CachedDocument{ - Root: doc.AST, - Source: detached.Source(input.name), - ParseDiags: doc.ParseDiagnostics, - Diagnostics: diagnostics[i], - Warnings: append([]string(nil), input.warnings...), - } + documents[i] = newCachedDocument(doc.AST, detached.Source(input.name), doc.ParseDiagnostics, + diagnostics[i], input.warnings) } stamps := make(map[string]uint64, len(names)) for _, name := range names { stamps[name] = l.ws.AnalysisStamp(name) } + l.shareConvertedDiagnostics(documents, stamps) return &CachedModel{ Documents: documents, Index: detached.Index(), Library: l.library, Mode: mode, analysis: &analysisSnapshot{workspace: l.ws, stamps: stamps, scoped: declaresIdentityScope(inputs)}, }, true } +// shareConvertedDiagnostics hands a document the converted diagnostics of the +// document answered under its name before, when both carry one analysis (one +// non-zero stamp) of one source with the same warnings: the diagnostics are +// then the same, and converting them again would only repeat the work. An edit +// changes the stamp of every document whose analysis it reached, so a document +// the edit did not reach is the only kind that shares. +func (l *lineage) shareConvertedDiagnostics(documents []*CachedDocument, stamps map[string]uint64) { + if l.answered == nil { + l.answered = make(map[string]answeredDocument, len(documents)) + } + for _, doc := range documents { + name := doc.Source.Name() + stamp := stamps[name] + if last, ok := l.answered[name]; ok && stamp != 0 && last.stamp == stamp && + last.doc.Source == doc.Source && slices.Equal(last.doc.Warnings, doc.Warnings) { + doc.proto = last.doc.proto + } + l.answered[name] = answeredDocument{doc: doc, stamp: stamp} + } +} + // analysisSnapshot is which analysis of each document a model was answered // with, as the lineage's workspace numbers them (model.Workspace.AnalysisStamp). // A stamp changes exactly when the document is analyzed afresh, which happens diff --git a/internal/frontend/grpc/service.go b/internal/frontend/grpc/service.go index 2dda0747ce..8214ad86e3 100644 --- a/internal/frontend/grpc/service.go +++ b/internal/frontend/grpc/service.go @@ -873,13 +873,8 @@ func (s *Service) parseModel(inputs []sourceInput, mode diag.ConformanceMode, in if len(p.Diagnostics) > 0 { parsedClean = false } - documents = append(documents, &CachedDocument{ - Root: root, - Source: srcFile, - ParseDiags: p.Diagnostics, - Diagnostics: parser.AsDiagnostics(p.Diagnostics, p.Warnings), - Warnings: append([]string(nil), input.warnings...), - }) + documents = append(documents, newCachedDocument(root, srcFile, p.Diagnostics, + parser.AsDiagnostics(p.Diagnostics, p.Warnings), input.warnings)) } // Registers what a wildcard import re-exports, so a qualified name reaches a @@ -946,20 +941,35 @@ func (s *Service) GetDiagnostics(ctx context.Context, req *pb.DiagnosticsRequest // modelDiagnostics are every document's diagnostics, document by document, each // located in the source it came from. The parse's are among them once: the -// passes report them, escalated where a pass judged the notation. +// passes report them, escalated where a pass judged the notation. Each +// document's are converted once and shared by every response that reports +// them; the list itself is the response's own. func (s *Service) modelDiagnostics(model *CachedModel) []*pb.Diagnostic { var pbDiags []*pb.Diagnostic for _, doc := range model.Documents { - for _, diag := range doc.Diagnostics { - pbDiags = append(pbDiags, DiagnosticToProto(diag, doc.Source)) - } - for _, warning := range doc.Warnings { - pbDiags = append(pbDiags, &pb.Diagnostic{ - Severity: diag.SeverityWarning.String(), - Message: warning, - Span: &pb.Span{File: doc.Source.Name()}, - }) - } + pbDiags = append(pbDiags, doc.protoDiagnostics(s.documentDiagnostics)...) + } + return pbDiags +} + +// documentDiagnostics converts one document's diagnostics, then its warnings, +// to protobuf, filtered to what the service's capabilities report. The +// capability set is fixed when the service is built, so the result serves +// every later call on the same model. +func (s *Service) documentDiagnostics(doc *CachedDocument) []*pb.Diagnostic { + if len(doc.Diagnostics)+len(doc.Warnings) == 0 { + return nil + } + pbDiags := make([]*pb.Diagnostic, 0, len(doc.Diagnostics)+len(doc.Warnings)) + for _, diag := range doc.Diagnostics { + pbDiags = append(pbDiags, DiagnosticToProto(diag, doc.Source)) + } + for _, warning := range doc.Warnings { + pbDiags = append(pbDiags, &pb.Diagnostic{ + Severity: diag.SeverityWarning.String(), + Message: warning, + Span: &pb.Span{File: doc.Source.Name()}, + }) } return s.filterDiagnosticCapabilities(pbDiags) } From a1d90b1b95f9053b8b6adfa2bd6d9867e9f82250 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 9 Oct 2026 04:35:13 +0000 Subject: [PATCH 02/10] perf(grpc): keep diagnostics cache comments to two lines Co-Authored-By: jason.han (cherry picked from commit e4fde5359516bd142ee152a6171f73234b078585) --- internal/frontend/grpc/cache.go | 10 ++++------ internal/frontend/grpc/lineage.go | 13 ++++--------- internal/frontend/grpc/service.go | 11 ++++------- 3 files changed, 12 insertions(+), 22 deletions(-) diff --git a/internal/frontend/grpc/cache.go b/internal/frontend/grpc/cache.go index db18fe4f0e..4c666bded7 100644 --- a/internal/frontend/grpc/cache.go +++ b/internal/frontend/grpc/cache.go @@ -40,9 +40,8 @@ type CachedDocument struct { proto *protoDiagnostics } -// protoDiagnostics is one document's diagnostics as protobuf, built on first use -// and shared by every response that reports them, so nothing may mutate them -// once built; the service's capability filter is applied as they are built. +// protoDiagnostics is one document's diagnostics as protobuf, built once (capability +// filter applied) and shared read-only by every response that reports them. type protoDiagnostics struct { once sync.Once diags []*pb.Diagnostic @@ -57,9 +56,8 @@ func newCachedDocument(root *ast.RootNamespace, sf *source.SourceFile, parseDiag } } -// protoDiagnostics is the document's diagnostics as build converts them, built -// the first time any response asks for them. A document built without a cell -// converts on every call. +// protoDiagnostics is the document's diagnostics as build converts them, built on +// first use; a document built without a cell converts on every call. func (d *CachedDocument) protoDiagnostics(build func(*CachedDocument) []*pb.Diagnostic) []*pb.Diagnostic { if d.proto == nil { return build(d) diff --git a/internal/frontend/grpc/lineage.go b/internal/frontend/grpc/lineage.go index 8be828d6d5..ed98edefe7 100644 --- a/internal/frontend/grpc/lineage.go +++ b/internal/frontend/grpc/lineage.go @@ -43,9 +43,8 @@ type lineage struct { library libs.Source // the files the workspace's library was built from held map[string]string // document name to the content the workspace holds versions int // the version the next update is given - // answered is each document as the last model was answered with it, and the - // stamp of the analysis it carried: the next model shares its converted - // diagnostics when the analysis, the source and the warnings are the same. + // answered is each document of the model last answered, with the stamp of its + // analysis; the next model shares its converted diagnostics when both are unchanged. answered map[string]answeredDocument } @@ -224,12 +223,8 @@ func (s *Service) parseFromLineage(inputs []sourceInput, mode diag.ConformanceMo }, true } -// shareConvertedDiagnostics hands a document the converted diagnostics of the -// document answered under its name before, when both carry one analysis (one -// non-zero stamp) of one source with the same warnings: the diagnostics are -// then the same, and converting them again would only repeat the work. An edit -// changes the stamp of every document whose analysis it reached, so a document -// the edit did not reach is the only kind that shares. +// shareConvertedDiagnostics hands a document the converted diagnostics answered under +// its name before when stamp, source and warnings are unchanged, i.e. no edit reached it. func (l *lineage) shareConvertedDiagnostics(documents []*CachedDocument, stamps map[string]uint64) { if l.answered == nil { l.answered = make(map[string]answeredDocument, len(documents)) diff --git a/internal/frontend/grpc/service.go b/internal/frontend/grpc/service.go index 8214ad86e3..168872624a 100644 --- a/internal/frontend/grpc/service.go +++ b/internal/frontend/grpc/service.go @@ -941,9 +941,8 @@ func (s *Service) GetDiagnostics(ctx context.Context, req *pb.DiagnosticsRequest // modelDiagnostics are every document's diagnostics, document by document, each // located in the source it came from. The parse's are among them once: the -// passes report them, escalated where a pass judged the notation. Each -// document's are converted once and shared by every response that reports -// them; the list itself is the response's own. +// passes report them, escalated where a pass judged the notation. Each document's +// are converted once and shared by every response; the list is the response's own. func (s *Service) modelDiagnostics(model *CachedModel) []*pb.Diagnostic { var pbDiags []*pb.Diagnostic for _, doc := range model.Documents { @@ -952,10 +951,8 @@ func (s *Service) modelDiagnostics(model *CachedModel) []*pb.Diagnostic { return pbDiags } -// documentDiagnostics converts one document's diagnostics, then its warnings, -// to protobuf, filtered to what the service's capabilities report. The -// capability set is fixed when the service is built, so the result serves -// every later call on the same model. +// documentDiagnostics converts one document's diagnostics, then its warnings, to +// protobuf, filtered by the service's capabilities, which are fixed when it is built. func (s *Service) documentDiagnostics(doc *CachedDocument) []*pb.Diagnostic { if len(doc.Diagnostics)+len(doc.Warnings) == 0 { return nil From 92613fc784d35e2ee5559f3e9a3526fc17578c93 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 9 Oct 2026 05:37:38 +0000 Subject: [PATCH 03/10] test(grpc): pin cached diagnostics against a fresh conversion, state the read-only contract Co-Authored-By: jason.han (cherry picked from commit 24aaa3981ae7bd8a0f53d46e144b0acde44ffbd8) --- internal/frontend/grpc/cache.go | 2 +- .../grpc/diagnostics_converted_once_test.go | 48 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/internal/frontend/grpc/cache.go b/internal/frontend/grpc/cache.go index 4c666bded7..411a22ee71 100644 --- a/internal/frontend/grpc/cache.go +++ b/internal/frontend/grpc/cache.go @@ -57,7 +57,7 @@ func newCachedDocument(root *ast.RootNamespace, sf *source.SourceFile, parseDiag } // protoDiagnostics is the document's diagnostics as build converts them, built on -// first use; a document built without a cell converts on every call. +// first use and shared read-only across responses: never mutate a returned message. func (d *CachedDocument) protoDiagnostics(build func(*CachedDocument) []*pb.Diagnostic) []*pb.Diagnostic { if d.proto == nil { return build(d) diff --git a/internal/frontend/grpc/diagnostics_converted_once_test.go b/internal/frontend/grpc/diagnostics_converted_once_test.go index 8d686206e6..3d117e6b32 100644 --- a/internal/frontend/grpc/diagnostics_converted_once_test.go +++ b/internal/frontend/grpc/diagnostics_converted_once_test.go @@ -173,3 +173,51 @@ func TestParseSourcesSharesConvertedDiagnosticsOfDocumentsAnEditDidNotReach(t *t } } } + +// freshlyConverted converts the cached model's diagnostics again, past the cache. +func freshlyConverted(t *testing.T, srv *Service, hash string) []*pb.Diagnostic { + t.Helper() + cached, ok := srv.cache.Get(hash) + if !ok { + t.Fatalf("model %s is not cached", hash) + } + var fresh []*pb.Diagnostic + for _, doc := range cached.Documents { + fresh = append(fresh, srv.documentDiagnostics(doc)...) + } + return fresh +} + +// The shared messages are never written after they are built: every later +// response still equals a conversion made afresh from the cached model. +func TestCachedDiagnosticsStayEqualToAFreshConversion(t *testing.T) { + ctx := context.Background() + req := &pb.ParseFileRequest{Source: &pb.ParseFileRequest_Content{Content: misplacedComment}} + withheld, err := NewServiceWithUnavailableCapabilitiesForTesting(10, "test", []string{CapabilityDiagnosticCodes}) + if err != nil { + t.Fatal(err) + } + defer withheld.Close() + for name, srv := range map[string]*Service{"coded": mustNewService(t, 10), "codes withheld": withheld} { + t.Run(name, func(t *testing.T) { + defer srv.Close() + first, err := srv.ParseFile(ctx, req) + if err != nil { + t.Fatalf("ParseFile: %v", err) + } + requireWarnings(t, first.Diagnostics) + second, err := srv.ParseFile(ctx, req) + if err != nil { + t.Fatalf("ParseFile again: %v", err) + } + diags, err := srv.GetDiagnostics(ctx, &pb.DiagnosticsRequest{ModelHash: first.ModelHash}) + if err != nil { + t.Fatalf("GetDiagnostics: %v", err) + } + fresh := freshlyConverted(t, srv, first.ModelHash) + requireSameDiagnostics(t, first.Diagnostics, fresh) + requireSameDiagnostics(t, second.Diagnostics, fresh) + requireSameDiagnostics(t, diags.Diagnostics, fresh) + }) + } +} From 4b74bb9f21b7b0b5f678d17768ada1e60d385d9d Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 9 Oct 2026 06:23:45 +0000 Subject: [PATCH 04/10] perf(grpc): keep the modelDiagnostics comment to two lines Co-Authored-By: jason.han (cherry picked from commit c6bdb49bbb58bd99fb4b2f8388682d2173c17ebb) --- internal/frontend/grpc/service.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/internal/frontend/grpc/service.go b/internal/frontend/grpc/service.go index 168872624a..d9d8ac2ed3 100644 --- a/internal/frontend/grpc/service.go +++ b/internal/frontend/grpc/service.go @@ -939,10 +939,8 @@ func (s *Service) GetDiagnostics(ctx context.Context, req *pb.DiagnosticsRequest }, nil } -// modelDiagnostics are every document's diagnostics, document by document, each -// located in the source it came from. The parse's are among them once: the -// passes report them, escalated where a pass judged the notation. Each document's -// are converted once and shared by every response; the list is the response's own. +// modelDiagnostics are every document's diagnostics in document order, each in its own +// source, the parse's among them once as the passes report them; the list is the response's own. func (s *Service) modelDiagnostics(model *CachedModel) []*pb.Diagnostic { var pbDiags []*pb.Diagnostic for _, doc := range model.Documents { From fba0279f4baef9a45e80fb56b4cd4e961ed33514 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 9 Oct 2026 04:48:21 +0000 Subject: [PATCH 05/10] perf(symbols): stop copying re-export claims for change checks Before a name's registration changes, the index notes what a lookup of it reads so that one registered again as it was is not a change. The note held a deep copy of every document's re-export claims and routes on the name, taken once per name a replaced document re-registers. A claims map and the claims in it are now never written once installed: a write installs a new map, so the note shares the index's own tables. What counts as a change is unchanged; tests pin the comparison and the resolver invalidations an edit of a re-exporting document reports. Co-Authored-By: jason.han (cherry picked from commit 4b4b35121e7daa9760e1c73f19c040105886741f) --- ...egistration-snapshot-claims.performance.md | 1 + internal/semantic/resolve/frames_test.go | 34 +++++ internal/semantic/symbols/changes.go | 17 +-- internal/semantic/symbols/changes_test.go | 65 +++++++++ internal/semantic/symbols/index.go | 125 +++++++++--------- 5 files changed, 173 insertions(+), 69 deletions(-) create mode 100644 changes/unreleased/registration-snapshot-claims.performance.md diff --git a/changes/unreleased/registration-snapshot-claims.performance.md b/changes/unreleased/registration-snapshot-claims.performance.md new file mode 100644 index 0000000000..03e0ac8617 --- /dev/null +++ b/changes/unreleased/registration-snapshot-claims.performance.md @@ -0,0 +1 @@ +- **Replacing a document no longer copies the re-export claims of every name it re-registers.** Before a name's registration changes, the index notes what a lookup of it reads so that one registered again exactly as it was does not count as a change; the note held a copy of every document's claims and routes on the name, taken once per name touched. It now shares the index's own claim tables, which a write replaces rather than changes, so an edit beside a large model allocates a fraction of what it did. What counts as a change is as before. diff --git a/internal/semantic/resolve/frames_test.go b/internal/semantic/resolve/frames_test.go index f9b6a6b171..6acc0d34ca 100644 --- a/internal/semantic/resolve/frames_test.go +++ b/internal/semantic/resolve/frames_test.go @@ -224,3 +224,37 @@ func TestFramesReadingTheWholeIndexSurviveAJudgmentChange(t *testing.T) { t.Fatalf("the refreshed table still declares X as %v", got) } } + +// A document replaced by one stating the same wildcard import registers the +// names the import surfaces again, as they were; a document that resolved +// through one keeps what it memoized. Stated otherwise, the import moves them. +func TestFramesSurviveAReexportRegisteredAgainAsItWas(t *testing.T) { + w := newTrackedIndex(t, map[string]string{ + "lib.sysml": "package Lib { part def Base; }", + "gate.sysml": "package P { public import Lib::*; }", + "user.sysml": "package P { part def Derived :> P::Base; }", + }) + w.idx.ExpandWildcardImports() + w.idx.TakeChanges() + w.analyzeAll() + // As the workspace does: the imports are expanded again before the changes + // are read. + edit := func(src string) []string { + w.roots["gate.sysml"] = parsedRoot(t, "gate.sysml", src) + w.idx.AddDocument("gate.sysml", w.roots["gate.sysml"]) + w.idx.ExpandWildcardImports() + ch := w.idx.TakeChanges() + ch.Docs = map[string]bool{"gate.sysml": true} + return w.r.Invalidate(ch) + } + if dropped := edit("package P { public import Lib::*; } // edited"); !reflect.DeepEqual(dropped, []string{"gate.sysml"}) { + t.Fatalf("dropped %v, want [gate.sysml]", dropped) + } + if !w.owned("user.sysml") { + t.Fatal("user.sysml, resolving through a re-export registered again as it was, lost what it memoized") + } + w.analyzeAll() + if dropped := edit("package P { private import Lib::*; }"); !reflect.DeepEqual(dropped, []string{"gate.sysml", "user.sysml"}) { + t.Fatalf("dropped %v, want [gate.sysml user.sysml]", dropped) + } +} diff --git a/internal/semantic/symbols/changes.go b/internal/semantic/symbols/changes.go index 0acecfa54a..f725d3157f 100644 --- a/internal/semantic/symbols/changes.go +++ b/internal/semantic/symbols/changes.go @@ -80,13 +80,15 @@ func (idx *Index) noteBefore(fqn string) { // tables: the symbols registered under it, in order, each with its re-export // and hidden marks and the claims and routes that surfaced it. It holds the // symbols themselves, so one dropped since cannot have its address reused by -// a new symbol that would then compare equal to it. +// a new symbol that would then compare equal to it. The claims are the index's +// own maps, not copies: a write replaces a claims map rather than changing it +// (see Index.reexportDocs), so the one noted stays as it was read. type registration []registeredSymbol type registeredSymbol struct { sym *Symbol reexported, hidden bool - claims map[string]reexportClaim + claims map[string]*reexportClaim } func registrationOf(idx *Index, fqn string) registration { @@ -98,12 +100,11 @@ func registrationOf(idx *Index, fqn string) registration { hidden, _ := idx.hidden.get(fqn) out := make(registration, len(syms)) for i, sym := range syms { - out[i] = registeredSymbol{sym: sym, reexported: reexported.has(sym), hidden: hidden.has(sym)} - if claims := idx.reexportDocs.at(reexportKey{fqn: fqn, sym: sym}); len(claims) > 0 { - out[i].claims = make(map[string]reexportClaim, len(claims)) - for doc, claim := range claims { - out[i].claims[doc] = reexportClaim{public: claim.public, routes: append([]gateRoute(nil), claim.routes...)} - } + out[i] = registeredSymbol{ + sym: sym, + reexported: reexported.has(sym), + hidden: hidden.has(sym), + claims: idx.reexportDocs.at(reexportKey{fqn: fqn, sym: sym}), } } return out diff --git a/internal/semantic/symbols/changes_test.go b/internal/semantic/symbols/changes_test.go index 2f3795fe75..6dd02204dc 100644 --- a/internal/semantic/symbols/changes_test.go +++ b/internal/semantic/symbols/changes_test.go @@ -145,3 +145,68 @@ func TestTakeChangesLeavesOutWhatIsRegisteredAgainAsItWas(t *testing.T) { t.Errorf("a library parsed again leaves its declarations and their re-exports unchanged: %v", keys(ch.Names)) } } + +// A re-export two documents surface outlives the replacement of one of them +// under the other's claim. The replacement's claim is compared with the one it +// held before: the same import is no change, a differently stated one is. +func TestTakeChangesComparesAReplacedClaimBesideTheOnesThatStayed(t *testing.T) { + idx := buildIndex(t, map[string]string{ + "base.sysml": "package L { metadata def M; part def A; }", + "a.sysml": "package P { public import L::*; }", + "b.sysml": "package P { public import L::*; }", + }) + idx.TrackChanges() + replace := func(src string) Changes { + addDoc(t, idx, "b.sysml", src) + idx.ExpandWildcardImports() + return idx.TakeChanges() + } + if ch := replace("package P { public import L::*; } // edited"); ch.Names["P::A"] { + t.Errorf("a re-export claimed again as it was is a change: %v", keys(ch.Names)) + } + // Claimed privately beside a public claim, the name stays exported; the + // claims alone read differently. + if ch := replace("package P { private import L::*; }"); !ch.Names["P::A"] { + t.Errorf("a re-export claimed privately where it was public is not a change: %v", keys(ch.Names)) + } + if ch := replace("package P { private import L::*; } // edited"); ch.Names["P::A"] { + t.Errorf("a re-export claimed privately again is a change: %v", keys(ch.Names)) + } + // A second import widens the claim's routes after the first recorded them. + if ch := replace("package P { private import L::*; public import L::*; }"); !ch.Names["P::A"] { + t.Errorf("a re-export whose routes widened is not a change: %v", keys(ch.Names)) + } + if ch := replace("package P { private import L::*; public import L::*; } // edited"); ch.Names["P::A"] { + t.Errorf("a re-export whose routes were recorded again as they were is a change: %v", keys(ch.Names)) + } + // A filter's condition is the expression declaring it, so one parsed again + // is another condition and the route it gates another route. + if ch := replace("package P { private import L::*[@M]; }"); !ch.Names["P::A"] { + t.Errorf("a re-export gated where it was not is not a change: %v", keys(ch.Names)) + } + if ch := replace("package P { private import L::*[@M]; } // edited"); !ch.Names["P::A"] { + t.Errorf("a re-export gated by a condition parsed again is not a change: %v", keys(ch.Names)) + } +} + +// What a registration noted before a write reads is what the index held then, +// whatever the writes since did to the claims under the same key. +func TestNotedRegistrationIsUnmovedByLaterClaims(t *testing.T) { + idx := buildIndex(t, map[string]string{ + "base.sysml": "package L { part def A; }", + "a.sysml": "package P { public import L::*; }", + "b.sysml": "package P { private import L::*; }", + }) + idx.TrackChanges() + before := registrationOf(idx, "P::A") + addDoc(t, idx, "b.sysml", "package P { private import L::*; public import L::*; }") + idx.ExpandWildcardImports() + if before.same(registrationOf(idx, "P::A")) { + t.Fatal("the registration noted before b.sysml widened its claim reads as the one after") + } + addDoc(t, idx, "b.sysml", "package P { private import L::*; }") + idx.ExpandWildcardImports() + if !before.same(registrationOf(idx, "P::A")) { + t.Fatal("b.sysml claiming as it first did does not read as the registration noted then") + } +} diff --git a/internal/semantic/symbols/index.go b/internal/semantic/symbols/index.go index b855d9a4c7..0c3539dc41 100644 --- a/internal/semantic/symbols/index.go +++ b/internal/semantic/symbols/index.go @@ -81,6 +81,10 @@ type Index struct { // This is deliberately not folded into contributions: that is an append-only // slice per document, and a re-export has to be found by (FQN, symbol) to // drop one document's claim, or purged across all documents at once. + // + // A claims map and the claims in it are never written once installed: a + // write installs a new map, so a registration noted before it (noteBefore) + // keeps reading what it read without copying. reexportDocs *layer[reexportKey, map[string]*reexportClaim] docReexports *layer[string, map[reexportKey]bool] @@ -1221,14 +1225,10 @@ func filtersSubsume(a, b []ElementFilter) bool { // a name is a member of the importing namespace when any one of them admits it, // so an unfiltered import re-exports it whatever another route filters out. func (idx *Index) reexportGated(fqn string, sym *Symbol, doc string, private bool, gates [][]ElementFilter) { - claim := idx.reexport(fqn, sym, doc, private) - if claim == nil { + if !idx.reexport(fqn, sym) { return // the namespace declares it; nothing was borrowed } - widened := false - for _, gate := range gates { - widened = claim.record(gateRoute{private: private, filters: gate}) || widened - } + widened := idx.claimReexport(reexportKey{fqn: fqn, sym: sym}, doc, !private, gates) if widened { idx.changedName(fqn) } @@ -1244,14 +1244,15 @@ func (idx *Index) reexportGated(fqn string, sym *Symbol, doc string, private boo // beside it redundant, and re-expanding an importer records nothing new. Keeping // only the routes no other subsumes also bounds the set, which is what lets a // cycle of filtered imports settle. It reports whether the claim now admits more -// than it did. +// than it did. The routes are replaced, not written in place: a claim installed +// in the index may share them (see claimReexport). func (c *reexportClaim) record(route gateRoute) bool { for _, have := range c.routes { if have.private == route.private && filtersSubsume(have.filters, route.filters) { return false } } - kept := c.routes[:0] + kept := make([]gateRoute, 0, len(c.routes)+1) for _, have := range c.routes { if have.private != route.private || !filtersSubsume(route.filters, have.filters) { kept = append(kept, have) @@ -1333,58 +1334,80 @@ func nonZeroFilters(filters []ElementFilter) []ElementFilter { return out } -// reexport registers sym under fqn on doc's behalf and returns doc's writable -// claim on it, or nil when the namespace declares sym there itself. -func (idx *Index) reexport(fqn string, sym *Symbol, doc string, private bool) *reexportClaim { +// reexport registers sym under fqn as a re-export and reports whether it is +// one, which it is not when the namespace declares sym there itself. +func (idx *Index) reexport(fqn string, sym *Symbol) bool { if idx.hasFQN(fqn, sym) { - if !idx.reexported.at(fqn).has(sym) { - return nil // declared here, not borrowed - } - } else { - idx.link(fqn, sym) // claimReexport notes the gain + return idx.reexported.at(fqn).has(sym) // else declared here, not borrowed } - return idx.claimReexport(reexportKey{fqn: fqn, sym: sym}, doc, !private) + idx.link(fqn, sym) // claimReexport notes the gain + return true } // claimReexport records that doc's wildcard import surfaces the re-export key, -// publicly or not, and updates the marks a lookup reads. A name is exported when -// any import that surfaced it was public, so a public claim clears the hidden -// mark a private one left. It returns doc's writable claim. -func (idx *Index) claimReexport(key reexportKey, doc string, public bool) *reexportClaim { - docs := idx.writableClaims(key) - claim, claimed := docs[doc] - if claimed && (claim.public || !public) { - return claim // nothing new - } - if !claimed { - claim = &reexportClaim{} - docs[doc] = claim +// publicly or not and by the given routes, and updates the marks a lookup reads. +// A name is exported when any import that surfaced it was public, so a public +// claim clears the hidden mark a private one left. It reports whether the routes +// now admit more than they did. +// +// The claims on key are replaced rather than written: the map and the claims in +// it may be what a registration noted before this write reads (noteBefore). +func (idx *Index) claimReexport(key reexportKey, doc string, public bool, gates [][]ElementFilter) bool { + idx.noteBefore(key.fqn) + docs := idx.reexportDocs.at(key) + have, claimed := docs[doc] + var claim reexportClaim + if claimed { + claim = *have } + upgraded := !claimed || (public && !claim.public) claim.public = claim.public || public - idx.changedName(key.fqn) - writableMap(idx.docReexports, doc)[key] = true - idx.applyReexportMarks(key, docs) - parent, _ := splitFQN(key.fqn) - idx.markGained(parent) // a public claim can un-hide it, which exports it onward - return claim + widened := false + for _, gate := range gates { + widened = claim.record(gateRoute{private: !public, filters: gate}) || widened + } + if !upgraded && !widened { + return false // nothing new + } + next := make(map[string]*reexportClaim, len(docs)+1) + for other, c := range docs { + next[other] = c + } + next[doc] = &claim + idx.reexportDocs.set(key, next) + if upgraded { + idx.changedName(key.fqn) + writableMap(idx.docReexports, doc)[key] = true + idx.applyReexportMarks(key, next) + parent, _ := splitFQN(key.fqn) + idx.markGained(parent) // a public claim can un-hide it, which exports it onward + } + return widened } // dropClaim forgets doc's claim on a re-export, deregistering the name once no // document surfaces it any more and re-hiding it when only private imports -// remain. +// remain. The surviving claims go into a new map (see claimReexport). func (idx *Index) dropClaim(key reexportKey, doc string) { - if _, claimed := idx.reexportDocs.at(key)[doc]; !claimed { + docs := idx.reexportDocs.at(key) + if _, claimed := docs[doc]; !claimed { return } - docs := idx.writableClaims(key) - delete(docs, doc) // the routes this document recorded go with its claim + idx.noteBefore(key.fqn) idx.changedName(key.fqn) - if len(docs) == 0 { + if len(docs) == 1 { idx.reexportDocs.del(key) idx.deregister(key.fqn, key.sym) return } - idx.applyReexportMarks(key, docs) + kept := make(map[string]*reexportClaim, len(docs)-1) + for other, c := range docs { + if other != doc { // the routes this document recorded go with its claim + kept[other] = c + } + } + idx.reexportDocs.set(key, kept) + idx.applyReexportMarks(key, kept) parent, _ := splitFQN(key.fqn) idx.markLost(parent) // only private imports may remain, hiding it again } @@ -1404,26 +1427,6 @@ func (idx *Index) purgeReexport(key reexportKey) { idx.deregister(key.fqn, key.sym) } -// writableClaims returns the claims on key that this index may write to. A claim -// the frozen base recorded is copied with them: recording a route on it would -// otherwise change what every index over that base re-exports. -func (idx *Index) writableClaims(key reexportKey) map[string]*reexportClaim { - idx.noteBefore(key.fqn) - if docs, owned := idx.reexportDocs.own[key]; owned { - idx.reexportDocs.gen.bump() - return docs - } - shared, _ := idx.reexportDocs.below(key) - docs := make(map[string]*reexportClaim, len(shared)+1) - for doc, claim := range shared { - copied := *claim - copied.routes = append([]gateRoute(nil), claim.routes...) - docs[doc] = &copied - } - idx.reexportDocs.set(key, docs) - return docs -} - // applyReexportMarks brings the reexported and hidden marks in line with docs, // the claims on key: a claimed name is re-exported, and hidden while every // document that surfaced it did so with a private import (KerML 8.2.3.3). From afaa786e9950dade2fc67b0f2029b6fb279a6541 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 9 Oct 2026 04:01:01 +0000 Subject: [PATCH 06/10] perf(runtime): describe a body write's host only when the binding check refuses it The binding check of an assignment or declaration in an action, state or calculation body took the description of where it ran as a string built on every write, whether or not the check failed. checkAssignable, checkNamedWriteAs and checkBodyDeclarationAs now take a func() string and format it only in the refusal they report; the hot callers pass the host's describe method, and the action's own label and a node's description are built the same way. Co-Authored-By: jason.han (cherry picked from commit 37b4a17c9d626486246dcb52341df56a69c38ac8) --- ...y-binding-check-description.performance.md | 1 + .../exec/runtime/action_control_objects.go | 2 +- internal/exec/runtime/action_executor.go | 2 +- internal/exec/runtime/action_frame.go | 14 +++++------ internal/exec/runtime/held_image_behavior.go | 2 +- internal/exec/runtime/state_executor.go | 6 ++--- internal/exec/runtime/state_statements.go | 4 ++-- internal/exec/runtime/statements.go | 10 ++++---- internal/exec/runtime/write_conformance.go | 24 ++++++++++++++----- 9 files changed, 39 insertions(+), 26 deletions(-) create mode 100644 changes/unreleased/lazy-binding-check-description.performance.md diff --git a/changes/unreleased/lazy-binding-check-description.performance.md b/changes/unreleased/lazy-binding-check-description.performance.md new file mode 100644 index 0000000000..f0a622afd0 --- /dev/null +++ b/changes/unreleased/lazy-binding-check-description.performance.md @@ -0,0 +1 @@ +- **A body statement's write no longer pays for its diagnostic description when it conforms.** The binding check of an assignment or declaration in an action, state or calculation body took the description of where it ran (`action node step`, `state behavior s`, `action A`) as a string built on every write; it now takes a function and formats the description only in the refusal it reports. An assignment step of an action loop allocates about a thousand fewer objects per thousand iterations, and every diagnostic reads as before. diff --git a/internal/exec/runtime/action_control_objects.go b/internal/exec/runtime/action_control_objects.go index 1797694c8c..097949987b 100644 --- a/internal/exec/runtime/action_control_objects.go +++ b/internal/exec/runtime/action_control_objects.go @@ -134,7 +134,7 @@ func arrivingObject(frame *actionFrame, graph *lower.ActionGraph, node ast.Node, func (e *performances) queueControlObject(frame *actionFrame, graph *lower.ActionGraph, flow lower.ObjectFlow, value Value) error { for _, f := range graph.Features[flow.Target] { if f.Name == flow.TargetPin && f.Direction == ast.DirIn { - if err := e.ctx.checkNamedWrite(graph.Scopes[flow.Target], nodeDescription(flow.Target), f.Name, &value); err != nil { + if err := e.ctx.checkNamedWriteAs(graph.Scopes[flow.Target], func() string { return nodeDescription(flow.Target) }, f.Name, &value); err != nil { return fmt.Errorf("%s: %w", flowDescription(flow), err) } frame.hold(flow.Target, f.Name, nodeObject{value: value, flow: flow.Decl, carried: flow.Kind == lower.FlowSuccession}) diff --git a/internal/exec/runtime/action_executor.go b/internal/exec/runtime/action_executor.go index a6d92c2fb3..55dfcbd49c 100644 --- a/internal/exec/runtime/action_executor.go +++ b/internal/exec/runtime/action_executor.go @@ -1537,7 +1537,7 @@ func (e *ActionExecutor) setFeature(name string, value Value) error { if value, err = e.mirrorOccurrence(name, value); err != nil { return err } - } else if err := e.ctx.checkNamedWrite(e.graph.Scope, actionLabelPrefix+symbolText(e.action), name, &value); err != nil { + } else if err := e.ctx.checkNamedWriteAs(e.graph.Scope, func() string { return actionLabelPrefix + symbolText(e.action) }, name, &value); err != nil { // No occurrence holds this feature, so its declaration is checked here // rather than by the write to that occurrence. return err diff --git a/internal/exec/runtime/action_frame.go b/internal/exec/runtime/action_frame.go index d52fd2307d..42e4d9d7d8 100644 --- a/internal/exec/runtime/action_frame.go +++ b/internal/exec/runtime/action_frame.go @@ -422,7 +422,7 @@ func (e *performances) seedPerformance(parent *actionFrame, flow *lower.ActionGr continue } check := func(value *Value) error { - return e.ctx.checkBodyDeclaration(feature.Scope, perf.describe(), feature.Name, value) + return e.ctx.checkBodyDeclarationAs(feature.Scope, perf.describe, feature.Name, value) } context := func(scope *symbols.Scope) *EvalContext { return e.evalBindingContext(perf, scope, feature.Name, activation) @@ -497,7 +497,7 @@ func (e *performances) seedDeclaredValues(perf *actionFrame, features []lower.Fe if feature.Binding { cells := e.bodyCells(perf) check := func(value *Value) error { - return e.ctx.checkBodyDeclaration(feature.Scope, perf.describe(), feature.Name, value) + return e.ctx.checkBodyDeclarationAs(feature.Scope, perf.describe, feature.Name, value) } name := perf.key(feature.Name) cell := cells.existingCell(name) @@ -514,7 +514,7 @@ func (e *performances) seedDeclaredValues(perf *actionFrame, features []lower.Fe } else { value, err = ec.Eval(feature.Value) if err == nil { - err = e.ctx.checkBodyDeclaration(feature.Scope, perf.describe(), feature.Name, &value) + err = e.ctx.checkBodyDeclarationAs(feature.Scope, perf.describe, feature.Name, &value) } if err == nil { e.ctx.writeBodyValue(perf.cells, perf.data, perf.key(feature.Name), value) @@ -894,7 +894,7 @@ func (e *performances) deliver(f *actionFrame, flow *lower.ActionGraph, node ast if !pins.declares(pin) { return fmt.Errorf("%w: %s declares no %s", ErrNodePin, nodeDescription(node), pin) } - if err := e.ctx.checkNamedWrite(flow.Scopes[node], nodeDescription(node), pin, &value); err != nil { + if err := e.ctx.checkNamedWriteAs(flow.Scopes[node], func() string { return nodeDescription(node) }, pin, &value); err != nil { return err } f.queue(node, canonical(pins.aliases, pin), value) @@ -952,7 +952,7 @@ func (e *performances) checkNestedDelivery(flow *lower.ActionGraph, node ast.Nod if !pins.declares(pin) { return fmt.Errorf("%w: %s declares no %s", ErrNodePin, nodeDescription(node), pin) } - return e.ctx.checkNamedWrite(flow.Scopes[node], nodeDescription(node), pin, value) + return e.ctx.checkNamedWriteAs(flow.Scopes[node], func() string { return nodeDescription(node) }, pin, value) } // takeDeliveries moves the oldest delivery at each pin of node into perf, so that @@ -1035,7 +1035,7 @@ func (e *performances) setFrameFeature(f *actionFrame, name string, value Value) e.noteFrameWrite(f, name, value) return nil } - if err := e.ctx.checkNamedWrite(f.scope, f.describe(), name, &value); err != nil { + if err := e.ctx.checkNamedWriteAs(f.scope, f.describe, name, &value); err != nil { return err } e.ctx.writeBodyValue(f.cells, f.data, f.key(name), value) @@ -1101,7 +1101,7 @@ func (e *performances) streamFlow( if !pins.declares(flow.TargetPin) { return fmt.Errorf("%s: %w: %s declares no %s", flowDescription(flow), ErrNodePin, nodeDescription(flow.Target), flow.TargetPin) } - if err := e.ctx.checkNamedWrite(graph.Scopes[flow.Target], nodeDescription(flow.Target), flow.TargetPin, &value); err != nil { + if err := e.ctx.checkNamedWriteAs(graph.Scopes[flow.Target], func() string { return nodeDescription(flow.Target) }, flow.TargetPin, &value); err != nil { return fmt.Errorf("%s: %w", flowDescription(flow), err) } pin := canonical(pins.aliases, flow.TargetPin) diff --git a/internal/exec/runtime/held_image_behavior.go b/internal/exec/runtime/held_image_behavior.go index 6377ff78bc..51a159558a 100644 --- a/internal/exec/runtime/held_image_behavior.go +++ b/internal/exec/runtime/held_image_behavior.go @@ -833,7 +833,7 @@ func (m *materializing) frameCells( continue } check := func(value *Value) error { - return m.dst.checkBodyDeclaration(state.scope, perf.describe(), state.name, value) + return m.dst.checkBodyDeclarationAs(state.scope, perf.describe, state.name, value) } var onDerived func(*Value) error if perf.node == nil && perf.parent == nil { diff --git a/internal/exec/runtime/state_executor.go b/internal/exec/runtime/state_executor.go index 2f851fcc5b..db4effa1cc 100644 --- a/internal/exec/runtime/state_executor.go +++ b/internal/exec/runtime/state_executor.go @@ -639,11 +639,11 @@ func (e *StateExecutor) assignAttribute(name string, value Value) error { } else { // No occurrence holds this feature, so its declaration is checked here // rather than by the write to that occurrence. - where := "state machine " + symbolText(e.stateMachine) - if err := e.ctx.checkMutable(e.graph.Scope, func() string { return where + ": assignment to " + name }, name); err != nil { + where := func() string { return "state machine " + symbolText(e.stateMachine) } + if err := e.ctx.checkMutable(e.graph.Scope, func() string { return where() + ": assignment to " + name }, name); err != nil { return err } - if err := e.ctx.checkNamedWrite(e.graph.Scope, where, name, &value); err != nil { + if err := e.ctx.checkNamedWriteAs(e.graph.Scope, where, name, &value); err != nil { return err } } diff --git a/internal/exec/runtime/state_statements.go b/internal/exec/runtime/state_statements.go index 4ba9de1c6e..e4d4cdcedb 100644 --- a/internal/exec/runtime/state_statements.go +++ b/internal/exec/runtime/state_statements.go @@ -396,7 +396,7 @@ func (h *stateStmtHost) assignStateAttribute(name string, value Value) (bool, er if !ok { return false, nil } - if err := h.exec.ctx.checkNamedWrite(scope, h.describe(), name, &value); err != nil { + if err := h.exec.ctx.checkNamedWriteAs(scope, h.describe, name, &value); err != nil { return true, err } h.exec.ctx.writeBodyValue(cells, data, name, value) @@ -512,7 +512,7 @@ func (h *stateStmtHost) runFlow(block lower.Block) (stmtFlow, error) { // the body's flow, else what is around it. func (h *stateStmtHost) setFeature(name string, value Value) error { if root := h.perfs.root; root.holds(name) { - if err := h.exec.ctx.checkNamedWrite(root.scope, h.describe(), name, &value); err != nil { + if err := h.exec.ctx.checkNamedWriteAs(root.scope, h.describe, name, &value); err != nil { return err } h.exec.ctx.writeBodyValue(root.cells, root.data, root.key(name), value) diff --git a/internal/exec/runtime/statements.go b/internal/exec/runtime/statements.go index 78fff53a07..1f1d515993 100644 --- a/internal/exec/runtime/statements.go +++ b/internal/exec/runtime/statements.go @@ -833,7 +833,7 @@ func (e *stmtEngine) execute(stmt lower.Statement) (stmtFlow, error) { if s.Target == "" { return flowNext, fmt.Errorf("%s: unsupported assignment target", e.host.describe()) } - if err := e.ctx.checkAssignable(e.host.describe(), s); err != nil { + if err := e.ctx.checkAssignable(e.host.describe, s); err != nil { return flowNext, err } value, err := e.evalIn(s.Scope).Eval(s.Value) @@ -895,7 +895,7 @@ func (e *stmtEngine) execute(stmt lower.Statement) (stmtFlow, error) { cells = e.ensureRootCells() } check := func(value *Value) error { - return e.ctx.checkBodyDeclaration(s.Scope, e.host.describe(), name, value) + return e.ctx.checkBodyDeclarationAs(s.Scope, e.host.describe, name, value) } bindingContext := e.evalIn(s.Scope) visible := make([]map[string]bool, len(bindingContext.frames)) @@ -930,7 +930,7 @@ func (e *stmtEngine) execute(stmt lower.Statement) (stmtFlow, error) { if err != nil { return flowNext, fmt.Errorf("eval declaration %s: %w", s.Name, err) } - if err := e.ctx.checkBodyDeclaration(s.Scope, e.host.describe(), s.Name, &evaluated); err != nil { + if err := e.ctx.checkBodyDeclarationAs(s.Scope, e.host.describe, s.Name, &evaluated); err != nil { return flowNext, err } value = evaluated @@ -945,7 +945,7 @@ func (e *stmtEngine) execute(stmt lower.Statement) (stmtFlow, error) { if err != nil { return flowNext, fmt.Errorf("eval declaration %s: %w", s.Name, err) } - if err := e.ctx.checkBodyDeclaration(s.Scope, e.host.describe(), s.Name, &evaluated); err != nil { + if err := e.ctx.checkBodyDeclarationAs(s.Scope, e.host.describe, s.Name, &evaluated); err != nil { return flowNext, err } value = evaluated @@ -958,7 +958,7 @@ func (e *stmtEngine) execute(stmt lower.Statement) (stmtFlow, error) { if err != nil { return flowNext, fmt.Errorf("eval declaration %s: %w", s.Name, err) } - if err := e.ctx.checkBodyDeclaration(s.Scope, e.host.describe(), s.Name, &evaluated); err != nil { + if err := e.ctx.checkBodyDeclarationAs(s.Scope, e.host.describe, s.Name, &evaluated); err != nil { return flowNext, err } e.env.declare(e.ctx, s.Name, evaluated) diff --git a/internal/exec/runtime/write_conformance.go b/internal/exec/runtime/write_conformance.go index b05782d955..31d56b2703 100644 --- a/internal/exec/runtime/write_conformance.go +++ b/internal/exec/runtime/write_conformance.go @@ -102,14 +102,14 @@ func (ctx *Context) readOnlyRefusal(what func() string, name string, feature *sy // statement was written — before its value is evaluated. A chained target is // judged on the object its chain reaches (writeThroughChain), unless it ends in // a `featured by` feature no object holds a slot for (featuredChainTarget). -func (ctx *Context) checkAssignable(where string, s lower.Assign) error { +func (ctx *Context) checkAssignable(where func() string, s lower.Assign) error { if s.Chain != nil { if feature, ok := ctx.featuredChainTarget(s); ok { - return ctx.readOnlyRefusal(func() string { return fmt.Sprintf("%s: assignment to %s", where, s.Chain.Text) }, s.Chain.Text, feature) + return ctx.readOnlyRefusal(func() string { return fmt.Sprintf("%s: assignment to %s", where(), s.Chain.Text) }, s.Chain.Text, feature) } return nil } - what := func() string { return fmt.Sprintf("%s: assignment to %s", where, s.Target) } + what := func() string { return fmt.Sprintf("%s: assignment to %s", where(), s.Target) } if s.Qualified { if s.Feature == nil { return nil @@ -182,17 +182,23 @@ func (ctx *Context) writeCountRefusal(target *writeTarget, value *Value) string // block-local, a parameter, an output - against the declaration of the name it // writes, before that value is stored. func (ctx *Context) checkBodyWrite(host stmtHost, s lower.Assign, value *Value) error { - return ctx.checkNamedWrite(s.Scope, host.describe(), s.Target, value) + return ctx.checkNamedWriteAs(s.Scope, host.describe, s.Target, value) } // checkNamedWrite checks a write of a name resolved in scope, for a path that // stores the value itself rather than reaching Instance.SetFeatureValue. func (ctx *Context) checkNamedWrite(scope *symbols.Scope, where, name string, value *Value) error { + return ctx.checkNamedWriteAs(scope, func() string { return where }, name, value) +} + +// checkNamedWriteAs is checkNamedWrite naming where the write ran only when it is +// refused, so a conformant write on a hot path does not pay for the description. +func (ctx *Context) checkNamedWriteAs(scope *symbols.Scope, where func() string, name string, value *Value) error { target, ok := ctx.writeTargetIn(scope, name) if !ok { return nil } - what := func() string { return fmt.Sprintf("%s: assignment to %s", where, name) } + what := func() string { return fmt.Sprintf("%s: assignment to %s", where(), name) } return ctx.checkTargetAs(scope, what, target, value, admitWritten, true) } @@ -203,13 +209,19 @@ func (ctx *Context) checkNamedWrite(scope *symbols.Scope, where, name string, va // holds the count its initializer has and, as a namespace-level one, is judged for // uniqueness only where it declares more than one value. func (ctx *Context) checkBodyDeclaration(scope *symbols.Scope, where, name string, value *Value) error { + return ctx.checkBodyDeclarationAs(scope, func() string { return where }, name, value) +} + +// checkBodyDeclarationAs is checkBodyDeclaration naming where the declaration ran +// only when it is refused, so a conformant binding does not pay for the description. +func (ctx *Context) checkBodyDeclarationAs(scope *symbols.Scope, where func() string, name string, value *Value) error { target, ok := ctx.writeTargetIn(scope, name) if !ok { return nil } declared := *target declared.unique = target.unique && multiValued(target.mult) - what := func() string { return fmt.Sprintf("%s: declaration of %s", where, name) } + what := func() string { return fmt.Sprintf("%s: declaration of %s", where(), name) } return ctx.checkTargetAs(scope, what, &declared, value, admitDeclared, target.countJudged) } From caea13a3a2cb00d6b7536f2092477151eb12b227 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 9 Oct 2026 05:23:08 +0000 Subject: [PATCH 07/10] perf(runtime): memoize the succession-cycle check while no behavior is held The cycle walk over held behaviors ran on every quiescent scan, which a constraint check reaches once per check. Remember that no behavior is held under a count of the behaviors that ever entered held, moved by the deferred behavior constructor and by snapshot restore, and answer nil from the memo until that count moves. Co-Authored-By: jason.han (cherry picked from commit 8bd97fcf6f04a71a42fd2fa69459e3532996b6c6) --- .../succession-cycle-memo.performance.md | 1 + internal/exec/runtime/context.go | 4 + internal/exec/runtime/occurrence_terminate.go | 2 +- internal/exec/runtime/snapshot.go | 9 +- .../runtime/succession_cycle_memo_test.go | 86 +++++++++++++++++++ internal/exec/runtime/succession_order.go | 28 ++++++ 6 files changed, 126 insertions(+), 4 deletions(-) create mode 100644 changes/unreleased/succession-cycle-memo.performance.md create mode 100644 internal/exec/runtime/succession_cycle_memo_test.go diff --git a/changes/unreleased/succession-cycle-memo.performance.md b/changes/unreleased/succession-cycle-memo.performance.md new file mode 100644 index 0000000000..45ec811f34 --- /dev/null +++ b/changes/unreleased/succession-cycle-memo.performance.md @@ -0,0 +1 @@ +- Constraint checks and other runs no longer walk the succession graph for a cycle among held behaviors on every quiescent scan: the runtime remembers that no behavior is held until one enters held, by instantiation, image or snapshot restore, which cut `BenchmarkBatchConstraints` per-check time. diff --git a/internal/exec/runtime/context.go b/internal/exec/runtime/context.go index 909bb16ad6..b028d53019 100644 --- a/internal/exec/runtime/context.go +++ b/internal/exec/runtime/context.go @@ -303,6 +303,10 @@ type Context struct { // quiescent is the memo a full scan leaves when it finds them all idle. work uint64 quiescent quiescence + // held counts the behaviors that entered held on a succession; noneHeld is + // the memo the cycle check leaves when it finds none held at that count. + held uint64 + noneHeld noneHeldMemo // onStack lists the runs of the executors whose calls are under way, outermost first. onStack []*executorRun diff --git a/internal/exec/runtime/occurrence_terminate.go b/internal/exec/runtime/occurrence_terminate.go index ccfcfb9f23..dba75a6835 100644 --- a/internal/exec/runtime/occurrence_terminate.go +++ b/internal/exec/runtime/occurrence_terminate.go @@ -110,7 +110,7 @@ func (ctx *Context) noteEndingUndo(b *ObjectBehavior) { captured.captureState(b.State) captured.captureRunState(b.State.driven.state) } - ctx.noteProbeUndo(captured.restore) + ctx.noteProbeUndo(func() { captured.restore(ctx) }) } // endsWith reports whether the executor's performance ends with the objects ended: diff --git a/internal/exec/runtime/snapshot.go b/internal/exec/runtime/snapshot.go index 6f05ad23aa..433043d37c 100644 --- a/internal/exec/runtime/snapshot.go +++ b/internal/exec/runtime/snapshot.go @@ -219,7 +219,7 @@ func (s *Snapshot) Restore() { ctx.snapshots = ctx.snapshots[:at+1] ctx.rollbackJournal(s.journal) s.run.restore(ctx) - s.executorCaptures.restore() + s.executorCaptures.restore(ctx) } // captureAction captures an action executor once, with the executors the work of @@ -267,7 +267,7 @@ func (s *executorCaptures) captureBehavior(behavior *ObjectBehavior) { }) } -func (s *executorCaptures) restore() { +func (s *executorCaptures) restore(ctx *Context) { for _, capture := range s.runStates { capture.restore() } @@ -285,6 +285,9 @@ func (s *executorCaptures) restore() { capture.behavior.State = capture.state capture.behavior.Err = capture.err capture.behavior.deferred = capture.deferred + if capture.deferred != nil { + ctx.behaviorHeld() + } } } @@ -1033,5 +1036,5 @@ func (m *moveMark) undo() { m.run.steps, m.run.elements, m.run.notes = m.steps, m.elements, m.notes e.ctx.choices, e.ctx.draws = e.ctx.choices[:m.choices], e.ctx.draws[:m.draws] m.trace.restore(e.ctx.trace) - m.state.restore() + m.state.restore(e.ctx) } diff --git a/internal/exec/runtime/succession_cycle_memo_test.go b/internal/exec/runtime/succession_cycle_memo_test.go new file mode 100644 index 0000000000..81c2f7bb5f --- /dev/null +++ b/internal/exec/runtime/succession_cycle_memo_test.go @@ -0,0 +1,86 @@ +package runtime + +import ( + "errors" + "testing" +) + +// TestSuccessionCycleMemo: the none-held memo answers the cycle check only while +// no behavior has entered held; one entering, by instantiation or by a snapshot +// restore, sends the check back to the walk, whose verdict it must then match. +func TestSuccessionCycleMemo(t *testing.T) { + src := `package test { + private import ScalarValues::*; + part def Plain { + attribute n : Integer = 0; + perform action step { assign n := 1; } + } + part def Ring { + perform action a { action stepA; } + perform action b { action stepB; } + first a then b; + first b then a; + } + }` + idx, _, ctx := buildRuntimeWithLibraries(t, "", parseAndBuild(t, src)) + if _, err := ctx.Instantiate(oneSymbol(t, idx, "test::Plain")); err != nil { + t.Fatalf("instantiate Plain: %v", err) + } + requireSameCycleVerdict(t, ctx) + if !ctx.noneHeld.answers(ctx) { + t.Fatal("cycle check over no held behavior left no memo") + } + + _, cached := ctx.Instantiate(oneSymbol(t, idx, "test::Ring")) + requireSuccessionError(t, cached, ErrSuccessionOrderCycle, successionOrderCycleCode) + if ctx.noneHeld.answers(ctx) { + t.Fatal("memo still answers after behaviors entered held") + } + requireSameCycleVerdict(t, ctx) + + // The same objects in a context whose check never answered from the memo. + freshIdx, _, fresh := buildRuntimeWithLibraries(t, "", parseAndBuild(t, src)) + if _, err := fresh.Instantiate(oneSymbol(t, freshIdx, "test::Plain")); err != nil { + t.Fatalf("instantiate Plain afresh: %v", err) + } + fresh.noneHeld = noneHeldMemo{} + _, uncached := fresh.Instantiate(oneSymbol(t, freshIdx, "test::Ring")) + if uncached == nil || uncached.Error() != cached.Error() { + t.Fatalf("cycle after memo = %v, want the uncached %v", cached, uncached) + } + + timed, inst := instantiateTimedSuccession(t) + snapshot, err := timed.Snapshot() + if err != nil { + t.Fatalf("snapshot: %v", err) + } + if _, err := timed.Advance(1); err != nil { + t.Fatalf("advance clock: %v", err) + } + if behaviorNamed(t, inst, "later").deferred != nil { + t.Fatal("Advance did not release later") + } + requireSameCycleVerdict(t, timed) + if !timed.noneHeld.answers(timed) { + t.Fatal("cycle check over released behaviors left no memo") + } + snapshot.Restore() + if behaviorNamed(t, inst, "later").deferred == nil { + t.Fatal("restore did not return later to held") + } + if timed.noneHeld.answers(timed) { + t.Fatal("memo still answers after a restore returned a behavior to held") + } + requireSameCycleVerdict(t, timed) +} + +func requireSameCycleVerdict(t *testing.T, ctx *Context) { + t.Helper() + got, want := ctx.successionCycle(), ctx.walkSuccessionCycle() + if (got == nil) != (want == nil) || (got != nil && got.Error() != want.Error()) { + t.Fatalf("memoized cycle check = %v, want the walk's %v", got, want) + } + if !errors.Is(got, ErrSuccessionOrderCycle) && want != nil { + t.Fatalf("memoized cycle check lost the sentinel: %v", got) + } +} diff --git a/internal/exec/runtime/succession_order.go b/internal/exec/runtime/succession_order.go index 9da593cd91..a92df0571f 100644 --- a/internal/exec/runtime/succession_order.go +++ b/internal/exec/runtime/succession_order.go @@ -317,6 +317,7 @@ func (ctx *Context) deferredBehaviorFor(inst *Instance, decl classifierBehaviorD return nil, err } deferred := decl + ctx.behaviorHeld() return &ObjectBehavior{ Name: decl.behavior.Name, Kind: decl.behavior.Kind, @@ -584,7 +585,34 @@ func (ctx *Context) releaseDeferredBehavior(behavior *ObjectBehavior) error { return nil } +// noneHeldMemo remembers a cycle check that found no behavior held; it answers +// until a behavior enters held, the one move that can put a cycle in the graph. +type noneHeldMemo struct { + taken bool + held uint64 +} + +func (m noneHeldMemo) answers(ctx *Context) bool { return m.taken && m.held == ctx.held } + +// behaviorHeld records a behavior entering held, retiring the none-held memo. +func (ctx *Context) behaviorHeld() { ctx.held++ } + +// successionCycle is walkSuccessionCycle, answered from the memo while no behavior +// is held: the walk visits held behaviors only, so with none it finds no cycle. func (ctx *Context) successionCycle() error { + if ctx.noneHeld.answers(ctx) { + return nil + } + for _, behavior := range ctx.objectBehaviors { + if behavior.deferred != nil { + return ctx.walkSuccessionCycle() + } + } + ctx.noneHeld = noneHeldMemo{taken: true, held: ctx.held} + return nil +} + +func (ctx *Context) walkSuccessionCycle() error { var visit func(*ObjectBehavior) bool state := make(map[*ObjectBehavior]uint8) stack := make([]*ObjectBehavior, 0) From 3ba38510a92b6cd6f7c5c7fdf2fa4580fec70a4a Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 9 Oct 2026 05:57:04 +0000 Subject: [PATCH 08/10] perf(runtime): pack compiled calc scalars into three words The compiled calc tier held each scalar in 56 bytes and, for every parameter, local and result, boxed it back into the evaluator's constant to decide whether the declaration holds it, building the diagnostic a refusal would name whether or not one followed. A scalar is now the constant packed into 24 bytes (semantics.Packed), unpacked without allocating; which kinds a declaration holds whatever the value is decided once at compile time; the diagnostic is built only on refusal; and the step charge inlines into every node. Results, promotions, refusals, error messages and step counts are unchanged. Co-Authored-By: jason.han (cherry picked from commit 4a1b937f29c1ba8373f854b54f4199efd036bdaf) --- .../compiled-calc-scalar-path.performance.md | 9 ++ internal/exec/runtime/compile.go | 89 +++++++++---- internal/exec/runtime/compiled_ops.go | 123 +++++++----------- internal/exec/runtime/compiled_stmts.go | 11 +- internal/semantic/semantics/eval.go | 39 ++++++ internal/semantic/semantics/packed_test.go | 48 +++++++ 6 files changed, 218 insertions(+), 101 deletions(-) create mode 100644 changes/unreleased/compiled-calc-scalar-path.performance.md create mode 100644 internal/semantic/semantics/packed_test.go diff --git a/changes/unreleased/compiled-calc-scalar-path.performance.md b/changes/unreleased/compiled-calc-scalar-path.performance.md new file mode 100644 index 0000000000..98fee9e4d6 --- /dev/null +++ b/changes/unreleased/compiled-calc-scalar-path.performance.md @@ -0,0 +1,9 @@ +- **Compiled calcs pass their Integers, Rationals, Reals and Booleans in three words.** The + compiled calc tier held each scalar in 56 bytes and, for every parameter, local and result, + boxed it back into the evaluator's constant to decide whether the declaration holds it, and + built the diagnostic a refusal would name whether or not one followed. A scalar is now the + constant packed into 24 bytes (`semantics.Packed`), unpacked without allocating; which kinds + a declaration holds whatever the value is decided once when the calc is compiled; the + diagnostic is built only on refusal; and the step charge inlines into every node. + `Fib(25)` interpreted runs in a fifth of the time with a quarter of the allocations; every + result, promotion to a big value, refusal, error message and step count is unchanged. diff --git a/internal/exec/runtime/compile.go b/internal/exec/runtime/compile.go index 4a98ac4a80..864cfa6fbc 100644 --- a/internal/exec/runtime/compile.go +++ b/internal/exec/runtime/compile.go @@ -107,21 +107,46 @@ type scalarCheck struct { holdsReal bool // least is the smallest integer a Natural-holding declaration takes: 1 for Positive. least int64 + // plain has bit 1< 0) + switch v.Kind { + case semantics.ValInt: + if i, ok := v.smallInt(); ok { + return c.integerOK || (c.naturalOK && i >= c.least) } - return c.integerOK || (c.naturalOK && v.int() >= c.least) - case scalarBool: + return c.integerOK || (c.naturalOK && v.Unpack().BigIntView().Sign() > 0) + case semantics.ValBool: return c.boolOK } return c.acceptsReal(v) @@ -138,15 +163,21 @@ func (c *scalarCheck) acceptsReal(v scalar) bool { return false } +// holds reports whether the declaration takes v as it is, so held has nothing to do. +func (c *scalarCheck) holds(v scalar) bool { + return !c.holdsReal || v.Kind != semantics.ValRational +} + // held is v as the declaration holds it: a Rational a Real declaration takes as its // nearest binary64, refused when no finite Real is; what, if any, names the declaration. func (c *scalarCheck) held(v scalar, what func() string) (scalar, error) { - if !c.holdsReal || v.kind != scalarRational { + if c.holds(v) { return v, nil } - real, err := semantics.RealOf(v.exact) + exact := v.Unpack() + real, err := semantics.RealOf(exact) if err != nil { - err = fmt.Errorf("%s as a Real: %w", v.exact.FormatRational(), err) + err = fmt.Errorf("%s as a Real: %w", exact.FormatRational(), err) if what != nil { err = fmt.Errorf("%s: %w", what(), err) } @@ -388,11 +419,13 @@ func (c *calcCompiler) scalarCheckFor(decl *calcMemberDecl) (scalarCheck, bool) check := scalarCheck{decl: decl, countOK: true, boolOK: true, naturalOK: true, integerOK: true, rationalOK: true, realOK: true} if decl.Target == nil { + check.settle() return check, true } one := Value{Kind: ValConst} check.countOK = c.ctx.writeCountRefusal(decl.Target, &one) == "" if decl.Target.typ == nil { + check.settle() return check, true } if decl.Target.typ.Kind == symbols.SymbolEnumerationDef { @@ -412,6 +445,7 @@ func (c *calcCompiler) scalarCheckFor(decl *calcMemberDecl) (scalarCheck, bool) if c.ctx.positiveScalar(decl.Target.typ) { check.least = 1 } + check.settle() return check, true } @@ -810,26 +844,19 @@ func (c *compiledCalc) invoke(ctx *Context, base int, bound paramSet) (scalar, e for i := range c.params { p := &c.params[i] v := frame[i] - source := "argument" - if !bound.has(i) { + defaulted := !bound.has(i) + if defaulted { var err error if v, err = p.dflt(ctx, frame); err != nil { ctx.leaveCalc() return scalar{}, calcDefaultError(c.kind, c.name, p.name, err) } - source = "default" frame[i] = v } - what := func() string { - return fmt.Sprintf("%s %s: %s for parameter %q", c.kind, c.name, source, p.name) - } - if !p.check.accepts(v) { - if err := p.check.refuse(ctx, v, what); err != nil { - ctx.leaveCalc() - return scalar{}, err - } + if p.check.accepts(v) && p.check.holds(v) { + continue } - held, err := p.check.held(v, what) + held, err := c.holdParam(ctx, p, v, defaulted) if err != nil { ctx.leaveCalc() return scalar{}, err @@ -841,7 +868,7 @@ func (c *compiledCalc) invoke(ctx *Context, base int, bound paramSet) (scalar, e if err != nil { return scalar{}, calcFrame(c.kind, c.name, fmt.Errorf("%s%w", c.bodyErr, err)) } - if c.result != nil { + if c.result != nil && !(c.result.accepts(result) && c.result.holds(result)) { what := func() string { return c.resultWhat } if !c.result.accepts(result) { if err := c.result.refuse(ctx, result, what); err != nil { @@ -855,6 +882,24 @@ func (c *compiledCalc) invoke(ctx *Context, base int, bound paramSet) (scalar, e return result, nil } +// holdParam settles a parameter value its check did not take as it is: the +// reference's verdict on a value accepts declined, then the declaration's hold. +func (c *compiledCalc) holdParam(ctx *Context, p *compiledParam, v scalar, defaulted bool) (scalar, error) { + source := "argument" + if defaulted { + source = "default" + } + what := func() string { + return fmt.Sprintf("%s %s: %s for parameter %q", c.kind, c.name, source, p.name) + } + if !p.check.accepts(v) { + if err := p.check.refuse(ctx, v, what); err != nil { + return scalar{}, err + } + } + return p.check.held(v, what) +} + // frameAt extends the scalar stack to hold a frame of size slots at base; the // local slots are written by their declarations before any read. func (ctx *Context) frameAt(base, size int) []scalar { diff --git a/internal/exec/runtime/compiled_ops.go b/internal/exec/runtime/compiled_ops.go index 291b7f11f3..d0f417fbbd 100644 --- a/internal/exec/runtime/compiled_ops.go +++ b/internal/exec/runtime/compiled_ops.go @@ -3,93 +3,60 @@ package runtime import ( "fmt" "math" - "math/big" "slices" "github.com/Open-MBEE/OpenSysML/internal/semantic/semantics" "github.com/Open-MBEE/OpenSysML/internal/syntax/ast" ) -// scalar is an unboxed Integer, Rational, Real or Boolean, the values the -// compiled calc tier computes over; it is boxed into a Value only at an -// invocation boundary. An Integer beyond int64 is held in big, immutable, with -// bits zero; a Rational is held exactly in exact. -type scalar struct { - kind scalarKind - bits uint64 - big *big.Int - exact semantics.Value -} - -type scalarKind uint8 - -const ( - scalarInt scalarKind = iota - scalarReal - scalarBool - scalarRational -) +// scalar is an Integer, Rational, Real or Boolean constant packed into three +// words, the value the compiled calc tier passes between nodes and holds in frames. +type scalar struct{ semantics.Packed } // #nosec G115 -- the Integer's two's-complement bits are stored, not its magnitude. -func intScalar(i int64) scalar { return scalar{kind: scalarInt, bits: uint64(i)} } +func intScalar(i int64) scalar { + return scalar{semantics.Packed{Kind: semantics.ValInt, Bits: uint64(i)}} +} -func realScalar(f float64) scalar { return scalar{kind: scalarReal, bits: math.Float64bits(f)} } +func realScalar(f float64) scalar { + return scalar{semantics.Packed{Kind: semantics.ValReal, Bits: math.Float64bits(f)}} +} func boolScalar(b bool) scalar { if b { - return scalar{kind: scalarBool, bits: 1} + return scalar{semantics.Packed{Kind: semantics.ValBool, Bits: 1}} } - return scalar{kind: scalarBool, bits: 0} + return scalar{semantics.Packed{Kind: semantics.ValBool}} } // #nosec G115 -- the inverse of intScalar: the same bits read back as an Integer. -func (s scalar) int() int64 { return int64(s.bits) } +func (s scalar) int() int64 { return int64(s.Bits) } -func (s scalar) real() float64 { return math.Float64frombits(s.bits) } +func (s scalar) truth() bool { return s.Bits != 0 } -func (s scalar) truth() bool { return s.bits != 0 } +// smallInt answers s as an int64 when it is an Integer within int64. +func (s scalar) smallInt() (int64, bool) { + return s.int(), s.Kind == semantics.ValInt && s.Ext == nil +} -// smallInts reports whether l and r are both Integers within int64. -func smallInts(l, r scalar) bool { - return l.kind == scalarInt && r.kind == scalarInt && l.big == nil && r.big == nil +// smallInts answers l and r as int64 when both are Integers within int64. +func smallInts(l, r scalar) (a, b int64, ok bool) { + return l.int(), r.int(), l.Kind == semantics.ValInt && r.Kind == semantics.ValInt && l.Ext == nil && r.Ext == nil } // semantic is the constant the scalar stands for. -func (s scalar) semantic() semantics.Value { - switch s.kind { - case scalarInt: - if s.big != nil { - return semantics.BigIntValue(s.big) - } - return semantics.IntValue(s.int()) - case scalarReal: - return semantics.Value{Kind: semantics.ValReal, Real: s.real()} - case scalarRational: - return s.exact - default: - return semantics.Value{Kind: semantics.ValBool, Bool: s.truth()} - } -} +func (s scalar) semantic() semantics.Value { return s.Unpack() } // boxed is the scalar as the evaluator's Value. func (s scalar) boxed() Value { - return Value{Kind: ValConst, Const: s.semantic()} + return Value{Kind: ValConst, Const: s.Unpack()} } // scalarOfConst unboxes a scalar constant, declining any other constant kind. func scalarOfConst(c semantics.Value) (scalar, bool) { switch c.Kind { - case semantics.ValInt: - if i, ok := c.Int64(); ok { - return intScalar(i), true - } - return scalar{kind: scalarInt, big: c.BigIntView()}, true - case semantics.ValReal: - return realScalar(c.Real), true - case semantics.ValRational: - return scalar{kind: scalarRational, exact: c}, true - case semantics.ValBool: - return boolScalar(c.Bool), true + case semantics.ValInt, semantics.ValReal, semantics.ValRational, semantics.ValBool: + return scalar{c.Pack()}, true } return scalar{}, false } @@ -110,16 +77,24 @@ type compiledExpr func(ctx *Context, args []scalar) (scalar, error) // which can fail. It leaves the counter where the evaluator's would stop. func (ctx *Context) chargeSteps(n int64) error { if n > ctx.maxSteps-ctx.run.steps { - ctx.run.steps = ctx.maxSteps - if ctx.run.steps < math.MaxInt64 { - ctx.run.steps++ - } - return ctx.stepLimitExceeded() + return ctx.stepsExhausted() } ctx.run.steps += n return nil } +// stepsExhausted is the refusal of a charge the budget cannot meet, kept out +// of line so chargeSteps inlines into every compiled node. +// +//go:noinline +func (ctx *Context) stepsExhausted() error { + ctx.run.steps = ctx.maxSteps + if ctx.run.steps < math.MaxInt64 { + ctx.run.steps++ + } + return ctx.stepLimitExceeded() +} + // binaryOp combines two evaluated operands. type binaryOp func(ctx *Context, l, r scalar) (scalar, error) @@ -136,8 +111,7 @@ func arithmeticOp(op ast.OperatorKind) binaryOp { } func addScalars(ctx *Context, l, r scalar) (scalar, error) { - if smallInts(l, r) { - a, b := l.int(), r.int() + if a, b, ok := smallInts(l, r); ok { if res := a + b; (b <= 0 || res > a) && (b >= 0 || res < a) { return intScalar(res), nil } @@ -146,8 +120,7 @@ func addScalars(ctx *Context, l, r scalar) (scalar, error) { } func subScalars(ctx *Context, l, r scalar) (scalar, error) { - if smallInts(l, r) { - a, b := l.int(), r.int() + if a, b, ok := smallInts(l, r); ok { if res := a - b; (b >= 0 || res > a) && (b <= 0 || res < a) { return intScalar(res), nil } @@ -181,29 +154,29 @@ func comparisonOp(op ast.OperatorKind) binaryOp { switch op { case ast.OpLt: return func(_ *Context, l, r scalar) (scalar, error) { - if smallInts(l, r) { - return boolScalar(l.int() < r.int()), nil + if a, b, ok := smallInts(l, r); ok { + return boolScalar(a < b), nil } return compareScalars(op, l, r) } case ast.OpLe: return func(_ *Context, l, r scalar) (scalar, error) { - if smallInts(l, r) { - return boolScalar(l.int() <= r.int()), nil + if a, b, ok := smallInts(l, r); ok { + return boolScalar(a <= b), nil } return compareScalars(op, l, r) } case ast.OpGt: return func(_ *Context, l, r scalar) (scalar, error) { - if smallInts(l, r) { - return boolScalar(l.int() > r.int()), nil + if a, b, ok := smallInts(l, r); ok { + return boolScalar(a > b), nil } return compareScalars(op, l, r) } case ast.OpGe: return func(_ *Context, l, r scalar) (scalar, error) { - if smallInts(l, r) { - return boolScalar(l.int() >= r.int()), nil + if a, b, ok := smallInts(l, r); ok { + return boolScalar(a >= b), nil } return compareScalars(op, l, r) } @@ -242,7 +215,7 @@ func unaryScalar(op ast.OperatorKind, v scalar) (scalar, error) { // scalarTruth reads a Boolean operand, reporting any other scalar as the // evaluator's boolOperand does. func scalarTruth(what string, v scalar) (bool, error) { - if v.kind != scalarBool { + if v.Kind != semantics.ValBool { return boolOperand(what, v.boxed()) } return v.truth(), nil diff --git a/internal/exec/runtime/compiled_stmts.go b/internal/exec/runtime/compiled_stmts.go index 0ebf833bae..51ff74d420 100644 --- a/internal/exec/runtime/compiled_stmts.go +++ b/internal/exec/runtime/compiled_stmts.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/Open-MBEE/OpenSysML/internal/ir/lower" + "github.com/Open-MBEE/OpenSysML/internal/semantic/semantics" "github.com/Open-MBEE/OpenSysML/internal/semantic/symbols" "github.com/Open-MBEE/OpenSysML/internal/syntax/ast" ) @@ -228,8 +229,10 @@ func declareStmt(s lower.Declare, slot int, value compiledExpr, check *scalarChe return scalar{}, false, err } } - if v, err = check.held(v, nil); err != nil { - return scalar{}, false, err + if !check.holds(v) { + if v, err = check.held(v, nil); err != nil { + return scalar{}, false, err + } } } frame[slot] = v @@ -245,7 +248,7 @@ func returnStmt(value compiledExpr, result *scalarCheck) compiledStmt { if err != nil { return scalar{}, false, fmt.Errorf("evaluating the returned expression: %w", err) } - if result != nil { + if result != nil && !(result.accepts(v) && result.holds(v)) { what := func() string { return "result" } if !result.accepts(v) { if err := result.refuse(ctx, v, what); err != nil { @@ -268,7 +271,7 @@ func ifStmt(cond compiledExpr, then, els compiledStmt) compiledStmt { if err != nil { return scalar{}, false, fmt.Errorf("eval condition of 'if': %w", err) } - if cv.kind != scalarBool { + if cv.Kind != semantics.ValBool { return scalar{}, false, fmt.Errorf("%s: condition of 'if' must evaluate to a Boolean, got %s", calcBodyDescription, ValConst) } diff --git a/internal/semantic/semantics/eval.go b/internal/semantic/semantics/eval.go index 6331d0eee3..381a3f0ecc 100644 --- a/internal/semantic/semantics/eval.go +++ b/internal/semantic/semantics/eval.go @@ -172,6 +172,45 @@ type Value struct { ext *big.Rat } +// Packed is an Integer, Rational, Real or Boolean Value in three words, for a +// holder that keeps or passes many; Unpack restores the Value it was packed from. +type Packed struct { + Kind ValueKind + Den uint32 + Bits uint64 + Ext *big.Rat +} + +// Pack is v as a Packed. +func (v Value) Pack() Packed { + p := Packed{Kind: v.Kind, Den: v.den, Ext: v.ext} + switch v.Kind { + case ValReal: + p.Bits = math.Float64bits(v.Real) + case ValBool: + if v.Bool { + p.Bits = 1 + } + default: + p.Bits = uint64(v.Int) // #nosec G115 -- the Integer's two's-complement bits are stored, not its magnitude. + } + return p +} + +// Unpack is the Value p packs. +func (p Packed) Unpack() Value { + v := Value{Kind: p.Kind, den: p.Den, ext: p.Ext} + switch p.Kind { + case ValReal: + v.Real = math.Float64frombits(p.Bits) + case ValBool: + v.Bool = p.Bits != 0 + default: + v.Int = int64(p.Bits) // #nosec G115 -- the inverse of Pack: the same bits read back as an Integer. + } + return v +} + // IsNumeric reports whether the value is an Integer, a Rational or a Real. func (v Value) IsNumeric() bool { return v.Kind == ValInt || v.Kind == ValReal || v.Kind == ValRational diff --git a/internal/semantic/semantics/packed_test.go b/internal/semantic/semantics/packed_test.go new file mode 100644 index 0000000000..c275bf6b23 --- /dev/null +++ b/internal/semantic/semantics/packed_test.go @@ -0,0 +1,48 @@ +package semantics + +import ( + "math" + "math/big" + "testing" +) + +// Every Value a Packed can hold unpacks to the Value it was packed from, field +// for field, the big form sharing its immutable big.Rat. +func TestPackedRoundTrip(t *testing.T) { + bigRat, _ := new(big.Rat).SetString("12345678901234567890123/7") + values := []Value{ + {}, + {Kind: ValInfinity}, + IntValue(0), IntValue(1), IntValue(-1), IntValue(math.MaxInt64), IntValue(math.MinInt64), + bigVal("1180591620717411303424"), bigVal("-1180591620717411303424"), + mustFrac(t, 1, 3), mustFrac(t, -7, 2), mustFrac(t, 6, 3), mustFrac(t, math.MaxInt64, math.MaxUint32), + RatValue(bigRat), + {Kind: ValReal, Real: 0}, {Kind: ValReal, Real: math.Copysign(0, -1)}, {Kind: ValReal, Real: 1.5}, + {Kind: ValReal, Real: math.Inf(1)}, {Kind: ValReal, Real: math.Inf(-1)}, {Kind: ValReal, Real: math.NaN()}, + {Kind: ValBool, Bool: true}, {Kind: ValBool, Bool: false}, + } + for _, v := range values { + got := v.Pack().Unpack() + if v.Kind == ValReal && math.IsNaN(v.Real) { + if got.Kind != ValReal || !math.IsNaN(got.Real) || math.Float64bits(got.Real) != math.Float64bits(v.Real) { + t.Errorf("NaN round-trips to %+v", got) + } + continue + } + if got != v { + t.Errorf("%+v round-trips to %+v", v, got) + } + if got.ext != v.ext { + t.Errorf("%+v round-trips to another big.Rat", v) + } + } +} + +func mustFrac(t *testing.T, num, den int64) Value { + t.Helper() + v, ok := FracValue(num, den) + if !ok { + t.Fatalf("FracValue(%d, %d) declined", num, den) + } + return v +} From 2e95310ac14e27ed3eb5e933990cc88ec31de567 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 9 Oct 2026 05:52:58 +0000 Subject: [PATCH 09/10] perf(semantic): compare a symbol's qualified name without building it IsStateActivity, IsBaseThat and IsOccurrenceThis asked for a target's fully-qualified name on every feature-chain read to compare it against a known name; they now match the owner chain segment by segment with symbols.HasFQN, so a conformant body allocates nothing for the question. Co-Authored-By: jason.han (cherry picked from commit 367ca7e3a721835bca1832a44516e048548b904c) --- .../compare-fqn-without-building-it.performance.md | 1 + internal/semantic/resolve/document.go | 2 +- internal/semantic/resolve/this_context.go | 7 ++++++- internal/semantic/semantics/state_activity.go | 2 +- 4 files changed, 9 insertions(+), 3 deletions(-) create mode 100644 changes/unreleased/compare-fqn-without-building-it.performance.md diff --git a/changes/unreleased/compare-fqn-without-building-it.performance.md b/changes/unreleased/compare-fqn-without-building-it.performance.md new file mode 100644 index 0000000000..244c74142a --- /dev/null +++ b/changes/unreleased/compare-fqn-without-building-it.performance.md @@ -0,0 +1 @@ +- **Recognising `this`, `that` and `isActive` no longer builds a qualified name.** Each read of a feature chain in a calculation or action body asked whether its target was `Occurrences::Occurrence::this`, `Base::things::that` or `StateActivity::isActive` by assembling the target's fully-qualified name and comparing it; the owner chain is now compared against the known name segment by segment, so a conformant body allocates nothing for the question, and every answer is as before. diff --git a/internal/semantic/resolve/document.go b/internal/semantic/resolve/document.go index cc8db81ada..230350b665 100644 --- a/internal/semantic/resolve/document.go +++ b/internal/semantic/resolve/document.go @@ -1918,7 +1918,7 @@ func (r *Resolver) IsBaseThat(sym *symbols.Symbol) bool { if sym == nil { return false } - return sym.Name == baseThatFQN || r.registeredFQN(sym) == baseThatFQN + return sym.Name == baseThatFQN || (r.idx != nil && symbols.HasFQN(sym, baseThatFQN)) } // getUsageType returns the type symbol of a usage by resolving its typing relationship. diff --git a/internal/semantic/resolve/this_context.go b/internal/semantic/resolve/this_context.go index e77382a203..e04b387d6d 100644 --- a/internal/semantic/resolve/this_context.go +++ b/internal/semantic/resolve/this_context.go @@ -19,7 +19,12 @@ func (r *Resolver) IsOccurrenceThis(sym *symbols.Symbol) bool { if sym == nil { return false } - return sym.Name == thisFeatureName || strings.HasSuffix(sym.Name, thisFeatureSuffix) || + if sym.Name == thisFeatureName || strings.HasSuffix(sym.Name, thisFeatureSuffix) { + return true + } + // Only a name that is itself a tail of `::this` can qualify to that suffix, + // so the qualified name is built for those alone. + return strings.HasSuffix(thisFeatureSuffix, sym.Name) && strings.HasSuffix(r.registeredFQN(sym), thisFeatureSuffix) } diff --git a/internal/semantic/semantics/state_activity.go b/internal/semantic/semantics/state_activity.go index cdd15e631b..8507ec3126 100644 --- a/internal/semantic/semantics/state_activity.go +++ b/internal/semantic/semantics/state_activity.go @@ -14,7 +14,7 @@ const StateActivityFQN = StateActivityPackage + "::isActive" // IsStateActivity reports whether sym is the StateActivity::isActive feature. func IsStateActivity(sym *symbols.Symbol) bool { - return sym != nil && symbols.FQNOf(sym) == StateActivityFQN + return sym != nil && symbols.HasFQN(sym, StateActivityFQN) } // FeaturingTypes are the types sym declares itself `featured by` (KerML From 8a7eb521c39a7a127af673e999d2282ccc6919d1 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 9 Oct 2026 12:07:34 +0000 Subject: [PATCH 10/10] chore(changelog): fold the performance fixes into 0.10.0 Co-Authored-By: jason.han --- CHANGELOG.md | 20 +++++++++++++++++++ ...are-fqn-without-building-it.performance.md | 1 - .../compiled-calc-scalar-path.performance.md | 9 --------- ...rpc-cache-diagnostics-proto.performance.md | 1 - ...y-binding-check-description.performance.md | 1 - ...egistration-snapshot-claims.performance.md | 1 - .../succession-cycle-memo.performance.md | 1 - 7 files changed, 20 insertions(+), 14 deletions(-) delete mode 100644 changes/unreleased/compare-fqn-without-building-it.performance.md delete mode 100644 changes/unreleased/compiled-calc-scalar-path.performance.md delete mode 100644 changes/unreleased/grpc-cache-diagnostics-proto.performance.md delete mode 100644 changes/unreleased/lazy-binding-check-description.performance.md delete mode 100644 changes/unreleased/registration-snapshot-claims.performance.md delete mode 100644 changes/unreleased/succession-cycle-memo.performance.md diff --git a/CHANGELOG.md b/CHANGELOG.md index f057ad70bc..36e846ace2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -665,6 +665,26 @@ release is described in [docs/project/releasing.md](docs/project/releasing.md). - **`ParseSources` re-analyzes only what an edit reaches when a client parses the same documents again.** From the second parse of a document set, the service answers from an incremental workspace held for it: unchanged documents keep their trees and diagnostics, and only the changed documents and those whose analysis read them are analyzed again. The answer is the one a fresh parse gives. On a 186-document model a re-parse after a one-line edit takes about 0.9 s instead of 1.7 s. +- **Recognising `this`, `that` and `isActive` no longer builds a qualified name.** Each read of a feature chain in a calculation or action body asked whether its target was `Occurrences::Occurrence::this`, `Base::things::that` or `StateActivity::isActive` by assembling the target's fully-qualified name and comparing it; the owner chain is now compared against the known name segment by segment, so a conformant body allocates nothing for the question, and every answer is as before. + +- **Compiled calcs pass their Integers, Rationals, Reals and Booleans in three words.** The + compiled calc tier held each scalar in 56 bytes and, for every parameter, local and result, + boxed it back into the evaluator's constant to decide whether the declaration holds it, and + built the diagnostic a refusal would name whether or not one followed. A scalar is now the + constant packed into 24 bytes (`semantics.Packed`), unpacked without allocating; which kinds + a declaration holds whatever the value is decided once when the calc is compiled; the + diagnostic is built only on refusal; and the step charge inlines into every node. + `Fib(25)` interpreted runs in a fifth of the time with a quarter of the allocations; every + result, promotion to a big value, refusal, error message and step count is unchanged. + +- **The gRPC service converts a cached model's diagnostics to protobuf once.** `ParseFile`, `ParseSources` and `GetDiagnostics` on a model parsed before answered the same diagnostics converted again on every call, two allocations per warning; each document's are now converted on first use and shared by every later response, and a model answered from a lineage shares those of every document the edit did not reach. The responses are unchanged. + +- **A body statement's write no longer pays for its diagnostic description when it conforms.** The binding check of an assignment or declaration in an action, state or calculation body took the description of where it ran (`action node step`, `state behavior s`, `action A`) as a string built on every write; it now takes a function and formats the description only in the refusal it reports. An assignment step of an action loop allocates about a thousand fewer objects per thousand iterations, and every diagnostic reads as before. + +- **Replacing a document no longer copies the re-export claims of every name it re-registers.** Before a name's registration changes, the index notes what a lookup of it reads so that one registered again exactly as it was does not count as a change; the note held a copy of every document's claims and routes on the name, taken once per name touched. It now shares the index's own claim tables, which a write replaces rather than changes, so an edit beside a large model allocates a fraction of what it did. What counts as a change is as before. + +- Constraint checks and other runs no longer walk the succession graph for a cycle among held behaviors on every quiescent scan: the runtime remembers that no behavior is held until one enters held, by instantiation, image or snapshot restore, which cut `BenchmarkBatchConstraints` per-check time. + ## 0.9.2 — 2026-10-05 ### Added diff --git a/changes/unreleased/compare-fqn-without-building-it.performance.md b/changes/unreleased/compare-fqn-without-building-it.performance.md deleted file mode 100644 index 244c74142a..0000000000 --- a/changes/unreleased/compare-fqn-without-building-it.performance.md +++ /dev/null @@ -1 +0,0 @@ -- **Recognising `this`, `that` and `isActive` no longer builds a qualified name.** Each read of a feature chain in a calculation or action body asked whether its target was `Occurrences::Occurrence::this`, `Base::things::that` or `StateActivity::isActive` by assembling the target's fully-qualified name and comparing it; the owner chain is now compared against the known name segment by segment, so a conformant body allocates nothing for the question, and every answer is as before. diff --git a/changes/unreleased/compiled-calc-scalar-path.performance.md b/changes/unreleased/compiled-calc-scalar-path.performance.md deleted file mode 100644 index 98fee9e4d6..0000000000 --- a/changes/unreleased/compiled-calc-scalar-path.performance.md +++ /dev/null @@ -1,9 +0,0 @@ -- **Compiled calcs pass their Integers, Rationals, Reals and Booleans in three words.** The - compiled calc tier held each scalar in 56 bytes and, for every parameter, local and result, - boxed it back into the evaluator's constant to decide whether the declaration holds it, and - built the diagnostic a refusal would name whether or not one followed. A scalar is now the - constant packed into 24 bytes (`semantics.Packed`), unpacked without allocating; which kinds - a declaration holds whatever the value is decided once when the calc is compiled; the - diagnostic is built only on refusal; and the step charge inlines into every node. - `Fib(25)` interpreted runs in a fifth of the time with a quarter of the allocations; every - result, promotion to a big value, refusal, error message and step count is unchanged. diff --git a/changes/unreleased/grpc-cache-diagnostics-proto.performance.md b/changes/unreleased/grpc-cache-diagnostics-proto.performance.md deleted file mode 100644 index 3db5349dd2..0000000000 --- a/changes/unreleased/grpc-cache-diagnostics-proto.performance.md +++ /dev/null @@ -1 +0,0 @@ -- **The gRPC service converts a cached model's diagnostics to protobuf once.** `ParseFile`, `ParseSources` and `GetDiagnostics` on a model parsed before answered the same diagnostics converted again on every call, two allocations per warning; each document's are now converted on first use and shared by every later response, and a model answered from a lineage shares those of every document the edit did not reach. The responses are unchanged. diff --git a/changes/unreleased/lazy-binding-check-description.performance.md b/changes/unreleased/lazy-binding-check-description.performance.md deleted file mode 100644 index f0a622afd0..0000000000 --- a/changes/unreleased/lazy-binding-check-description.performance.md +++ /dev/null @@ -1 +0,0 @@ -- **A body statement's write no longer pays for its diagnostic description when it conforms.** The binding check of an assignment or declaration in an action, state or calculation body took the description of where it ran (`action node step`, `state behavior s`, `action A`) as a string built on every write; it now takes a function and formats the description only in the refusal it reports. An assignment step of an action loop allocates about a thousand fewer objects per thousand iterations, and every diagnostic reads as before. diff --git a/changes/unreleased/registration-snapshot-claims.performance.md b/changes/unreleased/registration-snapshot-claims.performance.md deleted file mode 100644 index 03e0ac8617..0000000000 --- a/changes/unreleased/registration-snapshot-claims.performance.md +++ /dev/null @@ -1 +0,0 @@ -- **Replacing a document no longer copies the re-export claims of every name it re-registers.** Before a name's registration changes, the index notes what a lookup of it reads so that one registered again exactly as it was does not count as a change; the note held a copy of every document's claims and routes on the name, taken once per name touched. It now shares the index's own claim tables, which a write replaces rather than changes, so an edit beside a large model allocates a fraction of what it did. What counts as a change is as before. diff --git a/changes/unreleased/succession-cycle-memo.performance.md b/changes/unreleased/succession-cycle-memo.performance.md deleted file mode 100644 index 45ec811f34..0000000000 --- a/changes/unreleased/succession-cycle-memo.performance.md +++ /dev/null @@ -1 +0,0 @@ -- Constraint checks and other runs no longer walk the succession graph for a cycle among held behaviors on every quiescent scan: the runtime remembers that no behavior is held until one enters held, by instantiation, image or snapshot restore, which cut `BenchmarkBatchConstraints` per-check time.