Skip to content

Commit d73e690

Browse files
committed
core/toposort: Do not calculate cycles within an SCC
Previously, the toposort algorithm would calculate all the cycles within a multi-node SCC, and then rank the cycles in order to figure out which cycle should be broken and entered first. It turns out that the number of cycles in an SCC can be a factorial of the number of nodes. Consequently, it is felt that this approach could lead to problems. Instead, when we reach an SCC with several nodes, we simply sort those nodes lexically. This is easier to explain and understand, and still allows the user to be aware of where there are cycles, and that they can be addressed. By not calculating cycles, performance is greatly improved. The changes to our own tests, in terms of ordering of output, are surprisingly small. Analysis of unity suggests there will likely be impact on users: maybe around 10% of the lines that changed when toposort was first introduced will change again. Fixes #3786. Signed-off-by: Matthew Sackman <[email protected]> Change-Id: I28edde9b3e8fb33a9fa5e97c93f1c7db1248c55d Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1209497 Reviewed-by: Marcel van Lohuizen <[email protected]> TryBot-Result: CUEcueckoo <[email protected]> Unity-Result: CUE porcuepine <[email protected]> Reviewed-by: Daniel Martí <[email protected]>
1 parent 18367a6 commit d73e690

File tree

9 files changed

+79
-468
lines changed

9 files changed

+79
-468
lines changed

.github/workflows/trybot.yaml

+6-6
Original file line numberDiff line numberDiff line change
@@ -128,27 +128,27 @@ jobs:
128128
- name: Test with -tags=cuewasm
129129
run: go test -tags cuewasm ./cmd/cue/cmd ./cue/interpreter/wasm
130130
- id: auth
131-
uses: google-github-actions/auth@v2
132-
with:
133-
credentials_json: ${{ secrets.E2E_GCLOUD_KEY }}
134131
if: |-
135132
github.repository == 'cue-lang/cue' && (((github.ref == 'refs/heads/master' || startsWith(github.ref, 'refs/heads/release-branch.')) && (! (contains(github.event.head_commit.message, '
136133
Dispatch-Trailer: {"type":"')))) || (github.ref == 'refs/heads/ci/test')) && (matrix.go-version == '1.24.x' && matrix.runner == 'ubuntu-24.04')
137134
name: gcloud auth for end-to-end tests
135+
uses: google-github-actions/auth@v2
136+
with:
137+
credentials_json: ${{ secrets.E2E_GCLOUD_KEY }}
138138
- if: |-
139139
github.repository == 'cue-lang/cue' && (((github.ref == 'refs/heads/master' || startsWith(github.ref, 'refs/heads/release-branch.')) && (! (contains(github.event.head_commit.message, '
140140
Dispatch-Trailer: {"type":"')))) || (github.ref == 'refs/heads/ci/test')) && (matrix.go-version == '1.24.x' && matrix.runner == 'ubuntu-24.04')
141141
name: gcloud setup for end-to-end tests
142142
uses: google-github-actions/setup-gcloud@v2
143-
- if: |-
143+
- env:
144+
CUE_TEST_TOKEN: ${{ secrets.E2E_PORCUEPINE_CUE_TOKEN }}
145+
if: |-
144146
github.repository == 'cue-lang/cue' && (((github.ref == 'refs/heads/master' || startsWith(github.ref, 'refs/heads/release-branch.')) && (! (contains(github.event.head_commit.message, '
145147
Dispatch-Trailer: {"type":"')))) || (github.ref == 'refs/heads/ci/test')) && (matrix.go-version == '1.24.x' && matrix.runner == 'ubuntu-24.04')
146148
name: End-to-end test
147149
run: |-
148150
cd internal/_e2e
149151
go test -race
150-
env:
151-
CUE_TEST_TOKEN: ${{ secrets.E2E_PORCUEPINE_CUE_TOKEN }}
152152
- if: (matrix.go-version == '1.24.x' && matrix.runner == 'ubuntu-24.04')
153153
name: Go checks
154154
run: |-

cmd/cue/cmd/testdata/script/issue986.txtar

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,6 @@ cmp stdout expect-stdout
3131

3232
-- expect-stdout --
3333
a: aaa
34-
b: bbb
3534
ab: aaabbb
35+
b: bbb
3636
cd: cccddd

internal/core/toposort/cycles.go

-153
This file was deleted.

internal/core/toposort/cycles_test.go

-115
This file was deleted.

0 commit comments

Comments
 (0)