From b66513fc9f9a1803305f9e4ceec8a1076001e89c Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 11 Nov 2026 08:34:04 +0000 Subject: [PATCH 1/2] fix(view): share typed expansions in layout drawn-set queries Co-Authored-By: jason.han --- .../unreleased/layout-drawn-query.fixed.md | 1 + .../metadata-identity-positions.fixed.md | 1 + .../passes/diagram/layout_shared_test.go | 23 ++ internal/ir/view/place.go | 2 +- internal/ir/view/tree.go | 62 +++- internal/ir/view/tree_edges.go | 27 +- internal/ir/view/tree_query.go | 299 ++++++++++++++++++ internal/ir/view/tree_query_test.go | 189 +++++++++++ internal/ir/view/view.go | 11 +- internal/semantic/identity/identity.go | 129 +++++--- tests/identity/metadata_positions_test.go | 38 +++ 11 files changed, 721 insertions(+), 61 deletions(-) create mode 100644 changes/unreleased/layout-drawn-query.fixed.md create mode 100644 changes/unreleased/metadata-identity-positions.fixed.md create mode 100644 internal/check/passes/diagram/layout_shared_test.go create mode 100644 internal/ir/view/tree_query.go create mode 100644 internal/ir/view/tree_query_test.go create mode 100644 tests/identity/metadata_positions_test.go diff --git a/changes/unreleased/layout-drawn-query.fixed.md b/changes/unreleased/layout-drawn-query.fixed.md new file mode 100644 index 0000000000..44ab5a6f5e --- /dev/null +++ b/changes/unreleased/layout-drawn-query.fixed.md @@ -0,0 +1 @@ +- Layout validation reuses shared definition expansions instead of materializing every inherited occurrence, avoiding exponential tree walks while preserving typed features, placement diagnostics and rendered diagrams. diff --git a/changes/unreleased/metadata-identity-positions.fixed.md b/changes/unreleased/metadata-identity-positions.fixed.md new file mode 100644 index 0000000000..75b4273433 --- /dev/null +++ b/changes/unreleased/metadata-identity-positions.fixed.md @@ -0,0 +1 @@ +- Computing the identities of metadata body features reuses their owning membership positions, avoiding repeated scans of metadata-heavy scopes without changing their identities. diff --git a/internal/check/passes/diagram/layout_shared_test.go b/internal/check/passes/diagram/layout_shared_test.go new file mode 100644 index 0000000000..3723243a80 --- /dev/null +++ b/internal/check/passes/diagram/layout_shared_test.go @@ -0,0 +1,23 @@ +package diagram_test + +import ( + "fmt" + "strings" + "testing" +) + +func TestLayoutWithDeeplySharedDefinitions(t *testing.T) { + var src strings.Builder + src.WriteString("package Shared { private import Views::*; private import DiagramLayout::*; part def D0 { part leaf; }\n") + for level := 1; level <= 12; level++ { + fmt.Fprintf(&src, "part def D%d {\n", level) + for branch := range 8 { + fmt.Fprintf(&src, "part p%d : D%d;\n", branch, level-1) + } + src.WriteString("}\n") + } + src.WriteString("part root : D12; view v { expose root; render asTreeDiagram; metadata Layout about root { x = 0; y = 0; } } }") + if diags := w8dDiags(t, src.String()); len(diags) != 0 { + t.Fatalf("unexpected diagnostics: %v", diags) + } +} diff --git a/internal/ir/view/place.go b/internal/ir/view/place.go index ad2efaf49c..9cd90554b5 100644 --- a/internal/ir/view/place.go +++ b/internal/ir/view/place.go @@ -145,7 +145,7 @@ func (d *Drawn) noteMember(owner, member *symbols.Symbol) { // not render is the error Render gives. func (r *Renderer) DrawnIn(view *symbols.Symbol) (*Drawn, error) { drawn := newDrawn() - if _, err := r.render(view, drawn); err != nil { + if _, err := r.render(view, drawn, true); err != nil { return nil, err } return drawn, nil diff --git a/internal/ir/view/tree.go b/internal/ir/view/tree.go index d82f0b06e2..9ab77c1742 100644 --- a/internal/ir/view/tree.go +++ b/internal/ir/view/tree.go @@ -31,10 +31,17 @@ func (r *Renderer) treeDepth() int { // typing — are drawn as edges once every node is (treeEdges). func (r *Renderer) renderTree(view *symbols.Symbol, exposed []*symbols.Symbol, out *Rendering) { ids := &nodeIDs{} + query := r.treeQuery(view, out, exposed) out.sites = map[*Node]treeSite{} - descendants := r.exposedDescendants(view, exposed, func(sym *symbols.Symbol) []*symbols.Symbol { + members := func(sym *symbols.Symbol) []*symbols.Symbol { return append(r.walkMembers(view, sym), r.annotatingMembers(sym)...) - }) + } + var descendants map[symbols.ElementKey]bool + if query != nil { + descendants = query.exposedDescendants(exposed, true) + } else { + descendants = r.exposedDescendants(view, exposed, members) + } for _, elem := range exposed { if descendants[symbols.KeyOf(elem)] { continue @@ -47,7 +54,7 @@ func (r *Renderer) renderTree(view *symbols.Symbol, exposed []*symbols.Symbol, o } continue } - walk := treeWalk{view: view, ids: ids, path: expansion{}, out: out} + walk := treeWalk{view: view, ids: ids, path: expansion{}, out: out, query: query} out.Roots = append(out.Roots, r.treeNode(walk, elem, nil, 0, true)) } out.Roots = append(out.Roots, r.nestedViewNodes(view, ids, map[*symbols.Symbol]bool{view: true}, out)...) @@ -151,12 +158,18 @@ func (r *Renderer) nestedViewNodes(view *symbols.Symbol, ids *nodeIDs, rendered r.dress(view, sub, node, out) exposed, err := r.model.ExposedElements(sub) if err == nil { - descendants := r.exposedDescendants(sub, exposed, func(sym *symbols.Symbol) []*symbols.Symbol { return r.walkMembers(sub, sym) }) + query := r.treeQuery(sub, out, exposed) + var descendants map[symbols.ElementKey]bool + if query != nil { + descendants = query.exposedDescendants(exposed, false) + } else { + descendants = r.exposedDescendants(sub, exposed, func(sym *symbols.Symbol) []*symbols.Symbol { return r.walkMembers(sub, sym) }) + } for _, elem := range exposed { if descendants[symbols.KeyOf(elem)] { continue } - walk := treeWalk{view: sub, ids: ids, path: expansion{}, out: out} + walk := treeWalk{view: sub, ids: ids, path: expansion{}, out: out, query: query} node.Children = append(node.Children, r.treeNode(walk, elem, nil, 0, true)) } } @@ -172,18 +185,29 @@ func (r *Renderer) nestedViewNodes(view *symbols.Symbol, ids *nodeIDs, rendered // treeWalk is what one walk down a tree shares between its nodes: the view // (nil outside any view), the node ids, the expansion path and the rendering. type treeWalk struct { - view *symbols.Symbol - ids *nodeIDs - path expansion - out *Rendering + view *symbols.Symbol + ids *nodeIDs + path expansion + out *Rendering + query *treeQuery } // treeNode renders one element and what it declares. qualified names the node by // qualified name, which is what an exposed element is reported by; a member // nested in it is named as it was declared. func (r *Renderer) treeNode(walk treeWalk, sym, owner *symbols.Symbol, depth int, qualified bool) *Node { + var key treeQueryKey + if walk.query != nil { + key = walk.query.key(sym, owner, walk.path, qualified) + if previous := walk.query.node(key, depth); previous != nil { + return previous + } + } view, out := walk.view, walk.out node := r.treeNodeShallow(walk, sym, owner, qualified) + if walk.query != nil { + defer walk.query.remember(key, depth, node) + } if walk.path[sym] { return node } @@ -195,9 +219,20 @@ func (r *Renderer) treeNode(walk treeWalk, sym, owner *symbols.Symbol, depth int // A parameter is a pin of its owner's node, drawn at every depth. At the depth // bound, or collapsed, an element keeps its ports and pins — they count at // their owner's depth — and nothing else nested is drawn. - bound, cut := r.walkBound(view), 0 + bound, cut := 0, 0 + if walk.query != nil { + bound = walk.query.bound + } else { + bound = r.walkBound(view) + } folded := depth >= bound || collapsed(node) - for _, member := range r.walkMembers(view, sym) { + var members []*symbols.Symbol + if walk.query != nil { + members = walk.query.members(sym) + } else { + members = r.walkMembers(view, sym) + } + for _, member := range members { if recursive != nil && member.Owner() != sym { continue } @@ -246,6 +281,11 @@ func (r *Renderer) annotationChildren(ids *nodeIDs, view, sym *symbols.Symbol, o // the path being expanded above it is noted as already shown. func (r *Renderer) treeNodeShallow(walk treeWalk, sym, owner *symbols.Symbol, qualified bool) *Node { view, out := walk.view, walk.out + if walk.query != nil { + node := &Node{ID: walk.ids.take(), Origin: symbolOrigin(sym), Geometry: walk.query.geometryOf(sym, owner, out)} + out.noteTreeSite(node, view, sym) + return node + } name := r.notationName(sym) if !qualified { name = localName(sym) diff --git a/internal/ir/view/tree_edges.go b/internal/ir/view/tree_edges.go index 07d2176eb5..425c179b76 100644 --- a/internal/ir/view/tree_edges.go +++ b/internal/ir/view/tree_edges.go @@ -61,8 +61,9 @@ func (r *Renderer) treeEdges(out *Rendering) map[symbols.ElementKey]*Node { // its owner draws it as inherited: a relationship arrow joins declarations, // not one usage's copy to another's. var index func(node, parent *Node) + indexed := map[*Node]bool{} index = func(node, parent *Node) { - if parent != nil { + if parent != nil && (!out.query || !indexed[node]) { g.parent[node.ID] = parent.ID } parentSite, inElement := out.sites[parent] @@ -72,6 +73,12 @@ func (r *Renderer) treeEdges(out *Rendering) map[symbols.ElementKey]*Node { g.nodes[key] = node } } + if out.query && indexed[node] { + return + } + if out.query { + indexed[node] = true + } for _, child := range node.Children { index(child, node) } @@ -81,7 +88,14 @@ func (r *Renderer) treeEdges(out *Rendering) map[symbols.ElementKey]*Node { } g.associationLines() var walk func(node *Node) + walked := map[*Node]bool{} walk = func(node *Node) { + if out.query && walked[node] { + return + } + if out.query { + walked[node] = true + } if site, ok := out.sites[node]; ok && g.nodes[symbols.KeyOf(site.sym)] == node { g.edgesOf(node, site) } @@ -155,14 +169,25 @@ func declaredUnder(parent, sym *symbols.Symbol) bool { // the name up. func (g *treeGraph) associationLines() { var prune func(nodes []*Node) []*Node + visited, removed := map[*Node]bool{}, map[*Node]bool{} prune = func(nodes []*Node) []*Node { kept := nodes[:0] for _, node := range nodes { + if g.out.query && visited[node] { + if !removed[node] { + kept = append(kept, node) + } + continue + } + if g.out.query { + visited[node] = true + } site := g.out.sites[node] if line, ok := g.associationLine(node, site); ok && nary(len(line.ends)) { g.drawAssociationDot(node, site, line) } else if ok { g.drawAssociation(node, site, line) + removed[node] = true continue } node.Children = prune(node.Children) diff --git a/internal/ir/view/tree_query.go b/internal/ir/view/tree_query.go new file mode 100644 index 0000000000..77903a484b --- /dev/null +++ b/internal/ir/view/tree_query.go @@ -0,0 +1,299 @@ +package view + +import ( + "math/bits" + "slices" + "strconv" + "strings" + + "github.com/Open-MBEE/OpenSysML/internal/semantic/symbols" +) + +// treeQuery shares equivalent occurrence subtrees for a drawn-set query, never for emitted output. +type treeQuery struct { + *treeQueryGraph + r *Renderer + view *symbols.Symbol + bound int + nodes map[treeQueryKey]treeQueryNode + memberCache map[*symbols.Symbol][]*symbols.Symbol + geometry map[treeGeometryKey]*Geometry +} + +type treeQueryGraph struct { + component map[*symbols.Symbol]int + reachable [][]uint64 + ids map[*symbols.Symbol]int + keys map[symbols.ElementKey][]int +} + +func (g *treeQueryGraph) covers(roots []*symbols.Symbol) bool { + if g == nil { + return false + } + for _, root := range roots { + if _, ok := g.component[root]; !ok { + return false + } + } + return true +} + +type treeQueryKey struct { + sym, owner *symbols.Symbol + qualified bool + path string +} + +type treeQueryNode struct { + node *Node + depth int +} + +type treeGeometryKey struct{ sym, owner *symbols.Symbol } + +func (q *treeQuery) geometryOf(sym, owner *symbols.Symbol, out *Rendering) *Geometry { + key := treeGeometryKey{sym, owner} + if geometry, ok := q.geometry[key]; ok { + return geometry + } + geometry := q.r.inheritedGeometryOf(q.view, owner, sym, out) + q.geometry[key] = geometry + return geometry +} + +func (r *Renderer) treeQuery(view *symbols.Symbol, out *Rendering, roots []*symbols.Symbol) *treeQuery { + if !out.query { + return nil + } + q := &treeQuery{r: r, view: view, bound: r.walkBound(view), nodes: map[treeQueryKey]treeQueryNode{}, memberCache: map[*symbols.Symbol][]*symbols.Symbol{}, geometry: map[treeGeometryKey]*Geometry{}} + unfiltered := len(r.walkFilters(view)) == 0 + if unfiltered && r.queryGraph.covers(roots) { + q.treeQueryGraph = r.queryGraph + return q + } + q.treeQueryGraph = &treeQueryGraph{component: map[*symbols.Symbol]int{}, ids: map[*symbols.Symbol]int{}, keys: map[symbols.ElementKey][]int{}} + q.prepare(roots) + if unfiltered && (r.queryGraph == nil || len(q.component) > len(r.queryGraph.component)) { + r.queryGraph = q.treeQueryGraph + } + return q +} + +func (q *treeQuery) members(sym *symbols.Symbol) []*symbols.Symbol { + if members, ok := q.memberCache[sym]; ok { + return members + } + if q.r.queryMembers == nil { + q.r.queryMembers = map[*symbols.Symbol][]*symbols.Symbol{} + } + members, ok := q.r.queryMembers[sym] + if !ok { + members = q.r.effectiveMembers(sym) + q.r.queryMembers[sym] = members + } + if filters := q.r.walkFilters(q.view); len(filters) > 0 { + var kept []*symbols.Symbol + for _, member := range members { + if q.r.admits(filters, member) { + kept = append(kept, member) + } + } + members = kept + } + q.memberCache[sym] = members + return members +} + +// prepare condenses member cycles and computes reachable expansion tokens as +// bit sets. Types are tokens enter tests, not additional member walks. +func (q *treeQuery) prepare(roots []*symbols.Symbol) { + order, low := map[*symbols.Symbol]int{}, map[*symbols.Symbol]int{} + active := map[*symbols.Symbol]bool{} + var stack []*symbols.Symbol + var components [][]*symbols.Symbol + var visit func(*symbols.Symbol) + visit = func(sym *symbols.Symbol) { + order[sym] = len(order) + 1 + low[sym] = order[sym] + stack = append(stack, sym) + active[sym] = true + for _, member := range q.members(sym) { + if order[member] == 0 { + visit(member) + low[sym] = min(low[sym], low[member]) + } else if active[member] { + low[sym] = min(low[sym], order[member]) + } + } + if low[sym] != order[sym] { + return + } + var component []*symbols.Symbol + for { + last := stack[len(stack)-1] + stack = stack[:len(stack)-1] + active[last] = false + q.component[last] = len(components) + component = append(component, last) + if last == sym { + break + } + } + components = append(components, component) + } + for _, root := range roots { + if order[root] == 0 { + visit(root) + } + } + types := map[*symbols.Symbol][]*symbols.Symbol{} + for sym := range order { + types[sym] = q.r.drawnTypes(sym) + for _, token := range q.tokens(sym, types[sym]) { + if _, ok := q.ids[token]; !ok { + q.ids[token] = len(q.ids) + key := symbols.KeyOf(token) + q.keys[key] = append(q.keys[key], q.ids[token]) + } + } + } + q.reachable = make([][]uint64, len(components)) + for component, syms := range components { + bits := make([]uint64, (len(q.ids)+63)/64) + for _, sym := range syms { + for _, token := range q.tokens(sym, types[sym]) { + id := q.ids[token] + bits[id/64] |= 1 << (id % 64) + } + for _, member := range q.members(sym) { + if child := q.component[member]; child != component { + for word, value := range q.reachable[child] { + bits[word] |= value + } + } + } + } + q.reachable[component] = bits + } +} + +func (q *treeQuery) tokens(sym *symbols.Symbol, types []*symbols.Symbol) []*symbols.Symbol { + return append(append([]*symbols.Symbol{sym}, types...), q.r.annotatingMembers(sym)...) +} + +// exposedDescendants uses reachability only to reject impossible containers; +// the original guarded walk and fixpoint still decide every suppression. +func (q *treeQuery) exposedDescendants(exposed []*symbols.Symbol, annotations bool) map[symbols.ElementKey]bool { + if len(exposed) < 2 { + return nil + } + targets := map[int][]int{} + for i, sym := range exposed { + for _, id := range q.keys[symbols.KeyOf(sym)] { + targets[id] = append(targets[id], i) + } + } + containers := make([][]int, len(exposed)) + for j, sym := range exposed { + seen := map[int]bool{} + add := func(id int) { + for _, i := range targets[id] { + if i != j && !seen[i] { + containers[i] = append(containers[i], j) + seen[i] = true + } + } + } + reached := q.reachable[q.component[sym]] + if len(targets) < len(reached) { + for id := range targets { + if reached[id/64]&(1<<(id%64)) != 0 { + add(id) + } + } + continue + } + for word, value := range reached { + for value != 0 { + id := word*64 + bits.TrailingZeros64(value) + value &= value - 1 + add(id) + } + } + } + by := make([]map[symbols.ElementKey]bool, len(exposed)) + members := func(sym *symbols.Symbol) []*symbols.Symbol { + if annotations { + return append(slices.Clone(q.members(sym)), q.r.annotatingMembers(sym)...) + } + return q.members(sym) + } + suppressed := map[symbols.ElementKey]bool{} + for range exposed { + changed := false + for i, elem := range exposed { + key := symbols.KeyOf(elem) + contained := false + for _, j := range containers[i] { + if suppressed[symbols.KeyOf(exposed[j])] { + continue + } + if by[j] == nil { + by[j] = q.r.treeDescendants(q.view, exposed[j], members) + } + if by[j][key] { + contained = true + break + } + } + if contained != suppressed[key] { + changed = true + if contained { + suppressed[key] = true + } else { + delete(suppressed, key) + } + } + } + if !changed { + break + } + } + return suppressed +} + +// key retains exactly the ancestors reachable below sym; unrelated siblings cannot affect its walk. +func (q *treeQuery) key(sym, owner *symbols.Symbol, path expansion, qualified bool) treeQueryKey { + var active []int + reached := q.reachable[q.component[sym]] + for ancestor := range path { + id, ok := q.ids[ancestor] + if !ok || reached[id/64]&(1<<(id%64)) == 0 { + continue + } + active = append(active, id) + } + slices.Sort(active) + var signature strings.Builder + for _, id := range active { + signature.WriteString(strconv.Itoa(id)) + signature.WriteByte(',') + } + return treeQueryKey{sym: sym, owner: owner, qualified: qualified, path: signature.String()} +} + +// node reuses a completed walk with the same cycle barriers and more depth: +// it has already recorded everything this occurrence can draw. +func (q *treeQuery) node(key treeQueryKey, depth int) *Node { + if previous, ok := q.nodes[key]; ok && previous.depth <= depth { + return previous.node + } + return nil +} + +func (q *treeQuery) remember(key treeQueryKey, depth int, node *Node) { + if previous, ok := q.nodes[key]; !ok || depth < previous.depth { + q.nodes[key] = treeQueryNode{node: node, depth: depth} + } +} diff --git a/internal/ir/view/tree_query_test.go b/internal/ir/view/tree_query_test.go new file mode 100644 index 0000000000..7c1f49be1e --- /dev/null +++ b/internal/ir/view/tree_query_test.go @@ -0,0 +1,189 @@ +package view + +import ( + "fmt" + "math/rand/v2" + "path/filepath" + "reflect" + "strings" + "testing" + + "github.com/Open-MBEE/OpenSysML/internal/semantic/semantics" + "github.com/Open-MBEE/OpenSysML/internal/semantic/symbols" +) + +func visitQuerySymbols(scope *symbols.Scope, visit func(*symbols.Symbol)) { + if scope == nil { + return + } + for _, list := range [][]*symbols.Symbol{scope.Members(), scope.AnonymousMembers()} { + for _, sym := range list { + visit(sym) + visitQuerySymbols(sym.Scope, visit) + } + } +} + +func TestDrawnQueryMatchesRender(t *testing.T) { + files, err := filepath.Glob("testdata/*.sysml") + if err != nil { + t.Fatal(err) + } + var checked int + for _, path := range files { + file := filepath.Base(path) + r, idx := loadFixture(t, file) + visitQuerySymbols(idx.DocumentRoot(file), func(sym *symbols.Symbol) { + if !semantics.IsView(sym) { + return + } + _, _, err := r.KindOf(sym) + if err != nil { + return + } + t.Run(file+"/"+idx.GetFQN(sym), func(t *testing.T) { + query, err := r.DrawnIn(sym) + if err != nil { + t.Fatal(err) + } + full, err := r.Render(sym) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(full.Drawn(), query) { + t.Errorf("drawn query differs from full render:\nfull: %#v\nquery: %#v", full.Drawn(), query) + } + checked++ + }) + }) + } + if checked < 50 { + t.Fatalf("only checked %d views", checked) + } +} + +func sharedQueryModel(depth, width int) string { + var src strings.Builder + src.WriteString("package P { private import Views::*; private import DiagramLayout::*; part def D0 { part leaf; }\n") + for level := 1; level <= depth; level++ { + fmt.Fprintf(&src, "part def D%d {\n", level) + for branch := range width { + fmt.Fprintf(&src, "part p%d : D%d;\n", branch, level-1) + } + src.WriteString("}\n") + } + fmt.Fprintf(&src, "part root : D%d; view v { expose root; render asTreeDiagram; metadata Layout about root { x = 0; y = 0; } } }", depth) + return src.String() +} + +func TestTreeDrawnQuerySharesDeepDefinitions(t *testing.T) { + r, idx := loadSources(t, []string{"shared.sysml"}, [][]byte{[]byte(sharedQueryModel(12, 8))}) + drawn, err := r.DrawnIn(lookup(t, idx, "P::v")) + if err != nil { + t.Fatal(err) + } + if !drawn.Node(lookup(t, idx, "P::D0::leaf")) { + t.Fatal("deep inherited leaf is not drawn") + } + for level := 1; level <= 12; level++ { + for branch := range 8 { + if !drawn.Node(lookup(t, idx, fmt.Sprintf("P::D%d::p%d", level, branch))) { + t.Fatalf("missing D%d::p%d", level, branch) + } + } + } +} + +func TestTreeDrawnQueryKeepsRecursionContexts(t *testing.T) { + const src = `package P { + private import Views::*; + private import DiagramLayout::*; + part def A { part b : B; part leafA; } + part def B { part a : A; part leafB; } + part def C { part a : A; part b : B; } + part def D { part c : C; } + part x : D; part y : C; part z : D; + view v { expose x; expose y; expose z; expose A; expose B; + metadata Layout about x::c { x = 0; y = 0; collapsed = true; } + render asTreeDiagram; + } +}` + r, idx := loadSources(t, []string{"cycles.sysml"}, [][]byte{[]byte(src)}) + for _, depth := range []int{0, 1, 2, 3, 4} { + r.SetDepth(depth) + full, err := r.Render(lookup(t, idx, "P::v")) + if err != nil { + t.Fatal(err) + } + query, err := r.DrawnIn(lookup(t, idx, "P::v")) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(full.Drawn(), query) { + t.Errorf("drawn sets differ at depth %d", depth) + } + } +} + +func TestTreeDrawnQueryMatchesSharedGraphs(t *testing.T) { + random := rand.New(rand.NewPCG(1, 2)) + for sample := range 64 { + var src strings.Builder + src.WriteString("package P { private import Views::*; private import DiagramLayout::*;\n") + for def := range 4 { + fmt.Fprintf(&src, "part def D%d { doc /* a definition */ part leaf;\n", def) + for member := range 3 { + fmt.Fprintf(&src, "part p%d : D%d;\n", member, random.IntN(4)) + } + src.WriteString("}\n") + } + for root := range 3 { + fmt.Fprintf(&src, "part r%d : D%d;\n", root, random.IntN(4)) + } + src.WriteString("view v { expose r0; expose r1; expose r2;\n") + for def := range 4 { + if random.IntN(2) == 0 { + fmt.Fprintf(&src, "expose D%d;\n", def) + } + } + src.WriteString("metadata Layout about D0::p0 { x = 0; y = 0; collapsed = true; } render asTreeDiagram; } }") + r, idx := loadSources(t, []string{"graph.sysml"}, [][]byte{[]byte(src.String())}) + r.SetDepth(1 + sample%5) + full, err := r.Render(lookup(t, idx, "P::v")) + if err != nil { + t.Fatal(err) + } + query, err := r.DrawnIn(lookup(t, idx, "P::v")) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(full.Drawn(), query) { + t.Fatalf("drawn sets differ for graph %d:\n%s", sample, src.String()) + } + } +} + +func TestTreeDrawnQueryKeepsMultipleTypeBarriers(t *testing.T) { + r, idx := loadSources(t, []string{"multiple.sysml"}, [][]byte{[]byte(` + package P { + private import Views::*; + part def A; + part def B { part leaf; } + part def H { part mixed : A, B { part c : B; } } + part def Wrap { part h : H; } + part plain : Wrap; + part a : A { part wrap : Wrap; } + view v { expose plain; expose a; render asTreeDiagram; } + }`)}) + full, err := r.Render(lookup(t, idx, "P::v")) + if err != nil { + t.Fatal(err) + } + query, err := r.DrawnIn(lookup(t, idx, "P::v")) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(full.Drawn(), query) { + t.Fatal("drawn query differs when a recursion barrier suppresses marking another declared type") + } +} diff --git a/internal/ir/view/view.go b/internal/ir/view/view.go index 28d10e5ff3..fda556f135 100644 --- a/internal/ir/view/view.go +++ b/internal/ir/view/view.go @@ -164,6 +164,9 @@ type Renderer struct { depth int // filters caches each view's filter conditions for its walks. filters map[*symbols.Symbol][]symbols.ElementFilter + // queryMembers reuses effective members across drawn-set queries of one model. + queryMembers map[*symbols.Symbol][]*symbols.Symbol + queryGraph *treeQueryGraph // verdicts overlays verification verdicts on a requirement rendering; nil // draws it structurally. verdicts Verdicts @@ -604,6 +607,8 @@ type Rendering struct { emptyReason string // drawn collects the elements drawn while rendering, nil when no one asked. drawn *Drawn + // query shares equivalent tree expansions when only the drawn set is requested. + query bool // folds are the behaviour roots whose collapsed nodes foldCollapsed folds once every edge is drawn. folds []*Node // narrowed marks a rendering showing() already narrowed to its Shown. @@ -666,14 +671,14 @@ func (r *Rendering) Positioned() bool { // recognized kind this package does not produce is an *UnsupportedKindError — // never another kind's rendering. func (r *Renderer) Render(view *symbols.Symbol) (*Rendering, error) { - return r.render(view, newDrawn()) + return r.render(view, newDrawn(), false) } // Drawn is what the rendering draws, for the edits that apply to it. func (r *Rendering) Drawn() *Drawn { return r.drawn } // render is Render, collecting what is drawn into drawn when it is not nil. -func (r *Renderer) render(view *symbols.Symbol, drawn *Drawn) (*Rendering, error) { +func (r *Renderer) render(view *symbols.Symbol, drawn *Drawn, query bool) (*Rendering, error) { kind, stated, err := r.KindOf(view) if err != nil { return nil, err @@ -687,7 +692,7 @@ func (r *Renderer) render(view *symbols.Symbol, drawn *Drawn) (*Rendering, error if err != nil { return nil, err } - out := &Rendering{View: r.notationName(view), Kind: kind, Stated: stated, Shown: r.shownOf(view), drawn: drawn} + out := &Rendering{View: r.notationName(view), Kind: kind, Stated: stated, Shown: r.shownOf(view), drawn: drawn, query: query} exposed = r.withIncluded(exposed, out) switch kind { case KindTree: diff --git a/internal/semantic/identity/identity.go b/internal/semantic/identity/identity.go index 0d7dd6c36c..b994fa58e0 100644 --- a/internal/semantic/identity/identity.go +++ b/internal/semantic/identity/identity.go @@ -231,11 +231,13 @@ func Root(sym *symbols.Symbol) *symbols.Symbol { } type builder struct { - model *semantics.Model - res *resolve.Resolver - idx *symbols.Index - scopes map[*symbols.Symbol]*Scope - known map[*symbols.Symbol]bool // scope memo: sym has an entry in scopes + model *semantics.Model + res *resolve.Resolver + idx *symbols.Index + scopes map[*symbols.Symbol]*Scope + known map[*symbols.Symbol]bool // scope memo: sym has an entry in scopes + positions map[*symbols.Scope]*memberPositions + identitySites map[*symbols.Symbol]map[ast.Node]bool // norm decides which library symbols the norm fixes an id for. norm *qualifier // catalog places an unnamed library member under its catalogued owner; built on first need. @@ -244,12 +246,14 @@ type builder struct { func newBuilder(model *semantics.Model, res *resolve.Resolver) *builder { return &builder{ - model: model, - res: res, - idx: res.Index(), - scopes: make(map[*symbols.Symbol]*Scope), - known: make(map[*symbols.Symbol]bool), - norm: newQualifier(res.Index(), model), + model: model, + res: res, + idx: res.Index(), + scopes: make(map[*symbols.Symbol]*Scope), + known: make(map[*symbols.Symbol]bool), + positions: make(map[*symbols.Scope]*memberPositions), + identitySites: make(map[*symbols.Symbol]map[ast.Node]bool), + norm: newQualifier(res.Index(), model), } } @@ -473,46 +477,71 @@ func (b *builder) memberIndex(usage *symbols.Symbol) (int, bool) { if usage.Decl == nil || usage.OwnerScope == nil { return 0, false } - prefix, _ := usage.Decl.(*ast.PrefixMetadata) - var members []ast.Node - var prefixes []*ast.PrefixMetadata - switch n := usage.OwnerScope.Node().(type) { - case *ast.RootNamespace: - members = n.Members - case *ast.Package: - members, prefixes = n.Members, n.Prefixes - case *ast.Namespace: - members, prefixes = n.Members, n.Prefixes - case *ast.Definition: - members, prefixes = n.Members, n.Prefixes - case *ast.Usage: - members, prefixes = n.Members, n.Prefixes - default: - return 0, false - } - index := 0 - for _, member := range members { - node := member - if m, ok := member.(*ast.Membership); ok { - node = m.Member + positions, ok := b.positions[usage.OwnerScope] + if !ok { + positions = newMemberPositions(usage.OwnerScope) + b.positions[usage.OwnerScope] = positions + } + if index, ok := positions.indices[usage.Decl]; ok { + return index, true + } + for positions.cursor < len(positions.members)+len(positions.prefixes) { + var node ast.Node + if positions.cursor < len(positions.members) { + node = positions.members[positions.cursor] + if m, ok := node.(*ast.Membership); ok { + node = m.Member + } + positions.cursor++ + if node == nil || b.identityAnnotation(usage.OwnerScope, node) { + continue + } + } else { + p := positions.prefixes[positions.cursor-len(positions.members)] + positions.cursor++ + if p != nil { + node = p + } } - if node == nil || b.identityAnnotation(usage.OwnerScope, node) { + index := positions.index + positions.index++ + if node == nil { continue } - if node == usage.Decl { - return index, true + if _, exists := positions.indices[node]; !exists { + positions.indices[node] = index } - index++ - } - for _, p := range prefixes { - if prefix != nil && p == prefix { - return index, true + if node == usage.Decl { + return positions.indices[node], true } - index++ } return 0, false } +type memberPositions struct { + members []ast.Node + prefixes []*ast.PrefixMetadata + indices map[ast.Node]int + cursor, index int +} + +func newMemberPositions(scope *symbols.Scope) *memberPositions { + positions := &memberPositions{indices: map[ast.Node]int{}} + switch n := scope.Node().(type) { + case *ast.RootNamespace: + positions.members = n.Members + case *ast.Package: + positions.members, positions.prefixes = n.Members, n.Prefixes + case *ast.Namespace: + positions.members, positions.prefixes = n.Members, n.Prefixes + case *ast.Definition: + positions.members, positions.prefixes = n.Members, n.Prefixes + case *ast.Usage: + positions.members, positions.prefixes = n.Members, n.Prefixes + } + return positions +} + // identityAnnotation reports a member the conversion leaves out of its positions: an ElementId or // ProjectRef annotation, by the type it resolves to, of the element it is about or the owner it is in. func (b *builder) identityAnnotation(scope *symbols.Scope, node ast.Node) bool { @@ -529,10 +558,20 @@ func (b *builder) identityAnnotation(scope *symbols.Scope, node ast.Node) bool { targets = []*symbols.Symbol{owner} } for _, target := range targets { - for _, site := range b.model.AnnotationSitesOf(target) { - if site.Node == node && (site.TypeFQN == ElementIdFQN || site.TypeFQN == ProjectRefFQN) { - return true + nodes, ok := b.identitySites[target] + if !ok { + for _, site := range b.model.AnnotationSitesOf(target) { + if site.TypeFQN == ElementIdFQN || site.TypeFQN == ProjectRefFQN { + if nodes == nil { + nodes = map[ast.Node]bool{} + } + nodes[site.Node] = true + } } + b.identitySites[target] = nodes + } + if nodes[node] { + return true } } return false diff --git a/tests/identity/metadata_positions_test.go b/tests/identity/metadata_positions_test.go new file mode 100644 index 0000000000..cf0cdce16d --- /dev/null +++ b/tests/identity/metadata_positions_test.go @@ -0,0 +1,38 @@ +package identity_test + +import ( + "fmt" + "strings" + "testing" + + "github.com/Open-MBEE/OpenSysML/internal/translate/rdf" +) + +func TestSharedTargetMetadataBodyIdentities(t *testing.T) { + var src strings.Builder + src.WriteString(`package P { + metadata def M { attribute a; attribute b; } + part target; + @IdentityMetadata::ElementId about target { id = "target-id"; } + `) + const count = 200 + for i := range count { + fmt.Fprintf(&src, "@M about target { a = %d; b = %d; }\n", i, i) + } + src.WriteString("}") + table, _ := buildTable(t, src.String()) + got := map[string]bool{} + for _, sym := range table.Symbols() { + if info, ok := table.Info(sym); ok { + got[info.EffectiveID] = true + } + } + for i := range count { + for _, feature := range []string{"a", "b"} { + fqn := fmt.Sprintf("P::@%d::%s", i+2, feature) + if !got[rdf.EncodeElementID(fqn)] { + t.Errorf("missing identity for %s", fqn) + } + } + } +} From cc9006aa9b6786c631c02dc5bec0a028bfe107f5 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 11 Nov 2026 20:32:16 +0000 Subject: [PATCH 2/2] fix(view): compute drawn-query reachability on demand Co-Authored-By: jason.han --- .../unreleased/layout-drawn-query.fixed.md | 2 +- internal/ir/view/tree_query.go | 88 +++++++++++++------ internal/ir/view/tree_query_memory_test.go | 65 ++++++++++++++ 3 files changed, 127 insertions(+), 28 deletions(-) create mode 100644 internal/ir/view/tree_query_memory_test.go diff --git a/changes/unreleased/layout-drawn-query.fixed.md b/changes/unreleased/layout-drawn-query.fixed.md index 44ab5a6f5e..16f0061bf8 100644 --- a/changes/unreleased/layout-drawn-query.fixed.md +++ b/changes/unreleased/layout-drawn-query.fixed.md @@ -1 +1 @@ -- Layout validation reuses shared definition expansions instead of materializing every inherited occurrence, avoiding exponential tree walks while preserving typed features, placement diagnostics and rendered diagrams. +- Layout validation reuses shared definition expansions and computes reachability on demand instead of materializing every inherited occurrence or allocating quadratic reachability tables, while preserving typed features, placement diagnostics and rendered diagrams. diff --git a/internal/ir/view/tree_query.go b/internal/ir/view/tree_query.go index 77903a484b..bfd0d1b005 100644 --- a/internal/ir/view/tree_query.go +++ b/internal/ir/view/tree_query.go @@ -1,7 +1,6 @@ package view import ( - "math/bits" "slices" "strconv" "strings" @@ -22,11 +21,43 @@ type treeQuery struct { type treeQueryGraph struct { component map[*symbols.Symbol]int - reachable [][]uint64 + children [][]int + local [][]int + // sources lists each token's components in increasing condensation order. + sources [][]int + reachable map[treeReachKey]bool ids map[*symbols.Symbol]int keys map[symbols.ElementKey][]int } +type treeReachKey struct{ component, token int } + +// reaches answers only requested component/token pairs; SCC edges point to lower components. +func (g *treeQueryGraph) reaches(component, token int) bool { + sources := g.sources[token] + if component < sources[0] { + return false + } + if _, ok := slices.BinarySearch(sources, component); ok { + return true + } + if len(g.children[component]) == 0 { + return false + } + key := treeReachKey{component, token} + if reached, ok := g.reachable[key]; ok { + return reached + } + for _, child := range g.children[component] { + if g.reaches(child, token) { + g.reachable[key] = true + return true + } + } + g.reachable[key] = false + return false +} + func (g *treeQueryGraph) covers(roots []*symbols.Symbol) bool { if g == nil { return false @@ -105,8 +136,8 @@ func (q *treeQuery) members(sym *symbols.Symbol) []*symbols.Symbol { return members } -// prepare condenses member cycles and computes reachable expansion tokens as -// bit sets. Types are tokens enter tests, not additional member walks. +// prepare condenses member cycles and indexes local expansion tokens and edges. +// Types are tokens enter tests, not additional member walks. func (q *treeQuery) prepare(roots []*symbols.Symbol) { order, low := map[*symbols.Symbol]int{}, map[*symbols.Symbol]int{} active := map[*symbols.Symbol]bool{} @@ -158,23 +189,28 @@ func (q *treeQuery) prepare(roots []*symbols.Symbol) { } } } - q.reachable = make([][]uint64, len(components)) + q.children = make([][]int, len(components)) + q.local = make([][]int, len(components)) + q.sources = make([][]int, len(q.ids)) + q.reachable = map[treeReachKey]bool{} for component, syms := range components { - bits := make([]uint64, (len(q.ids)+63)/64) for _, sym := range syms { for _, token := range q.tokens(sym, types[sym]) { - id := q.ids[token] - bits[id/64] |= 1 << (id % 64) + q.local[component] = append(q.local[component], q.ids[token]) } for _, member := range q.members(sym) { if child := q.component[member]; child != component { - for word, value := range q.reachable[child] { - bits[word] |= value - } + q.children[component] = append(q.children[component], child) } } } - q.reachable[component] = bits + slices.Sort(q.children[component]) + q.children[component] = slices.Compact(q.children[component]) + slices.Sort(q.local[component]) + q.local[component] = slices.Compact(q.local[component]) + for _, token := range q.local[component] { + q.sources[token] = append(q.sources[token], component) + } } } @@ -195,6 +231,7 @@ func (q *treeQuery) exposedDescendants(exposed []*symbols.Symbol, annotations bo } } containers := make([][]int, len(exposed)) + visited := make([]int, len(q.children)) for j, sym := range exposed { seen := map[int]bool{} add := func(id int) { @@ -205,22 +242,20 @@ func (q *treeQuery) exposedDescendants(exposed []*symbols.Symbol, annotations bo } } } - reached := q.reachable[q.component[sym]] - if len(targets) < len(reached) { - for id := range targets { - if reached[id/64]&(1<<(id%64)) != 0 { - add(id) - } + var visit func(int) + visit = func(component int) { + if visited[component] == j+1 { + return } - continue - } - for word, value := range reached { - for value != 0 { - id := word*64 + bits.TrailingZeros64(value) - value &= value - 1 - add(id) + visited[component] = j + 1 + for _, token := range q.local[component] { + add(token) + } + for _, child := range q.children[component] { + visit(child) } } + visit(q.component[sym]) } by := make([]map[symbols.ElementKey]bool, len(exposed)) members := func(sym *symbols.Symbol) []*symbols.Symbol { @@ -266,10 +301,9 @@ func (q *treeQuery) exposedDescendants(exposed []*symbols.Symbol, annotations bo // key retains exactly the ancestors reachable below sym; unrelated siblings cannot affect its walk. func (q *treeQuery) key(sym, owner *symbols.Symbol, path expansion, qualified bool) treeQueryKey { var active []int - reached := q.reachable[q.component[sym]] for ancestor := range path { id, ok := q.ids[ancestor] - if !ok || reached[id/64]&(1<<(id%64)) == 0 { + if !ok || !q.reaches(q.component[sym], id) { continue } active = append(active, id) diff --git a/internal/ir/view/tree_query_memory_test.go b/internal/ir/view/tree_query_memory_test.go new file mode 100644 index 0000000000..e0c421c299 --- /dev/null +++ b/internal/ir/view/tree_query_memory_test.go @@ -0,0 +1,65 @@ +package view + +import ( + "fmt" + "reflect" + "strings" + "testing" + + "github.com/Open-MBEE/OpenSysML/internal/semantic/symbols" +) + +func wideQueryModel(width int) string { + var src strings.Builder + src.WriteString("package P { private import Views::*; part root {\n") + for member := range width { + fmt.Fprintf(&src, "part p%d;\n", member) + } + src.WriteString("} view v { expose root; render asTreeDiagram; } }") + return src.String() +} + +func TestTreeDrawnQueryShallowAllocations(t *testing.T) { + allocated := map[int]int64{} + for _, width := range []int{10000, 20000, 40000} { + t.Run(fmt.Sprint(width), func(t *testing.T) { + r, idx := loadSources(t, []string{"wide.sysml"}, [][]byte{[]byte(wideQueryModel(width))}) + r.SetDepth(1) + view := lookup(t, idx, "P::v") + drawn, err := r.DrawnIn(view) + if err != nil { + t.Fatal(err) + } + full, err := r.Render(view) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(full.Drawn(), drawn) { + t.Fatal("depth-one query differs from full render") + } + if !drawn.Node(lookup(t, idx, fmt.Sprintf("P::root::p%d", width-1))) { + t.Fatal("last member is not drawn") + } + roots := []*symbols.Symbol{lookup(t, idx, "P::root")} + // Isolate graph preparation from the renderer's linear work; never reuse a prepared graph. + result := testing.Benchmark(func(b *testing.B) { + for range b.N { + fresh := NewRenderer(r.model, r.resolver, r.text) + fresh.queryMembers = r.queryMembers + fresh.SetDepth(1) + fresh.treeQuery(view, &Rendering{query: true}, roots) + } + }) + allocated[width] = result.AllocedBytesPerOp() + t.Logf("depth-one graph preparation: %d B/op", allocated[width]) + }) + } + // About twice the 40k preparation budget, below the old dense table alone. + const ceiling = 112 << 20 + if allocated[40000] > ceiling { + t.Errorf("graph preparation allocated %d bytes, exceeding %d", allocated[40000], ceiling) + } + if allocated[40000]*2 > allocated[20000]*5 { + t.Errorf("graph allocations grew by more than 2.5x: %d -> %d", allocated[20000], allocated[40000]) + } +}