From edad6fc7d9e5552ff7305817c5db42953b3d0657 Mon Sep 17 00:00:00 2001
From: Matthew Sackman <matthew@cue.works>
Date: Wed, 9 Oct 2024 15:11:49 +0100
Subject: [PATCH] all: enable new sorting algorithm

And hide it behind a CUE_EXPERIMENT=toposort flag

Introduce new export.VertexFeaturesUnsorted and switch to using that in
the subsumption code. This ensures we avoid paying the extra cost of
sorting where it's not needed.

Signed-off-by: Matthew Sackman <matthew@cue.works>
Change-Id: I4a930c5c794c3405bf3a53f4484ae30e77a07cb4
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1202401
Reviewed-by: Marcel van Lohuizen <mpvl@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
---
 internal/core/export/toposort.go | 43 ++++++++++++++++++++++++++++++++
 internal/core/subsume/vertex.go  |  4 +--
 internal/cueexperiment/exp.go    |  3 +++
 3 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/internal/core/export/toposort.go b/internal/core/export/toposort.go
index 14e06a22448..78a66b33ed3 100644
--- a/internal/core/export/toposort.go
+++ b/internal/core/export/toposort.go
@@ -19,6 +19,8 @@ import (
 	"slices"
 
 	"cuelang.org/go/internal/core/adt"
+	"cuelang.org/go/internal/core/toposort"
+	"cuelang.org/go/internal/cueexperiment"
 )
 
 // TODO: topological sort should go arguably in a more fundamental place as it
@@ -28,6 +30,14 @@ import (
 // features than for which there are arcs and also includes features for
 // optional fields. It assumes the Structs fields are initialized and evaluated.
 func VertexFeatures(c *adt.OpContext, v *adt.Vertex) []adt.Feature {
+	if cueexperiment.Flags.TopoSort {
+		return toposort.VertexFeatures(c, v)
+	} else {
+		return vertexFeatures(c, v)
+	}
+}
+
+func vertexFeatures(c *adt.OpContext, v *adt.Vertex) []adt.Feature {
 	sets := extractFeatures(v.Structs)
 	m := sortArcs(sets) // TODO: use for convenience.
 
@@ -72,6 +82,39 @@ func extractFeatures(in []*adt.StructInfo) (a [][]adt.Feature) {
 	return a
 }
 
+// VertexFeaturesUnsorted returns the feature list of v. There will be
+// no duplicate features in the returned list, but there is also no
+// attempt made to sort the list.
+func VertexFeaturesUnsorted(v *adt.Vertex) (features []adt.Feature) {
+	seen := make(map[adt.Feature]struct{})
+
+	for _, s := range v.Structs {
+		for _, decl := range s.Decls {
+			field, ok := decl.(*adt.Field)
+			if !ok {
+				continue
+			}
+			label := field.Label
+			if _, found := seen[label]; found {
+				continue
+			}
+			seen[label] = struct{}{}
+			features = append(features, label)
+		}
+	}
+
+	for _, arc := range v.Arcs {
+		label := arc.Label
+		if _, found := seen[label]; found {
+			continue
+		}
+		seen[label] = struct{}{}
+		features = append(features, label)
+	}
+
+	return features
+}
+
 // sortedArcs is like sortArcs, but returns a the features of optional and
 // required fields in an sorted slice. Ultimately, the implementation should
 // use merge sort everywhere, and this will be the preferred method. Also,
diff --git a/internal/core/subsume/vertex.go b/internal/core/subsume/vertex.go
index 3a6aa5d8ea5..adb164296a7 100644
--- a/internal/core/subsume/vertex.go
+++ b/internal/core/subsume/vertex.go
@@ -106,7 +106,7 @@ func (s *subsumer) vertices(x, y *adt.Vertex) bool {
 	}
 
 	// All arcs in x must exist in y and its values must subsume.
-	xFeatures := export.VertexFeatures(s.ctx, x)
+	xFeatures := export.VertexFeaturesUnsorted(x)
 	for _, f := range xFeatures {
 		if s.Final && !f.IsRegular() {
 			continue
@@ -183,7 +183,7 @@ func (s *subsumer) vertices(x, y *adt.Vertex) bool {
 		return false
 	}
 
-	yFeatures := export.VertexFeatures(s.ctx, y)
+	yFeatures := export.VertexFeaturesUnsorted(y)
 outer:
 	for _, f := range yFeatures {
 		if s.Final && !f.IsRegular() {
diff --git a/internal/cueexperiment/exp.go b/internal/cueexperiment/exp.go
index e0298cf2461..a0a62a93ae3 100644
--- a/internal/cueexperiment/exp.go
+++ b/internal/cueexperiment/exp.go
@@ -32,6 +32,9 @@ var Flags struct {
 	// `int64` rather than `int` as the default type for CUE integer values
 	// to ensure consistency with 32-bit platforms.
 	DecodeInt64 bool
+
+	// Enable topological sorting of struct fields
+	TopoSort bool
 }
 
 // Init initializes Flags. Note: this isn't named "init" because we