perf(spdx): make findRootPackages linear instead of packages x relationships (#5280)

Decoding an SPDX document into the syft model was quadratic in document size. findRootPackages (spdxhelpers/to_syft_model.go), which locates the packages the document DESCRIBES, scanned every relationship for every package. On a 119MB SPDX JSON document (70,701 packages, 153,301 relationships) that is 1.1e10 comparisons and 91% of all CPU samples; on a 238MB document it is 4.3e10 comparisons and 95%. A 1.3GB document did not finish in 91 minutes. Everything else on the decode path is map-indexed and linear, and spdx/tools-golang's own parse is linear (4.5s and 9.3s for the two documents above). The scan dates from 9480f10c (top-level SPDX package support, 2023). The function now indexes the DESCRIBES / DESCRIBED_BY relationships once, counting how many times each element is described by the DOCUMENT, then walks the packages once. Semantics are unchanged: results follow package order, an element described by more than one edge still appears once per edge (so the existing "exactly one root" check behaves as before), only edges touching DOCUMENT count, and nil entries are skipped. Measured on an Apple M1 Max, `syft convert <spdx> -o <format>=<file>`: input target before (main) after 119MB spdx-json@2.3 68.2s / 1.3GB 10.4s / 1.3GB 119MB spdx-json@2.2 70.2s / 1.3GB 12.6s / 1.3GB 238MB spdx-json@2.3 291.2s / 2.3GB 20.9s / 2.6GB 1.3GB spdx-json@2.3 killed at 91min 115.5s / 13.6GB 1.3GB spdx-json@2.2 not attempted 136.3s / 14.1GB 1.3GB syft-json not attempted 102.6s / 14.1GB 1.3GB cyclonedx-json not attempted 114.1s / 13.1GB Isolating the conversion step (ToSyftModel) from the tools-golang parse: 119MB 72.3s -> 1.5s, 238MB 265.8s -> 3.1s. The remaining decode time and all of the memory are in tools-golang, which parses the document several times over; that is a separate, upstream concern. Output equivalence was checked by converting 10 SPDX documents (the 8 SPDX fixtures in this repository plus the 119MB and 238MB documents) to syft-json, spdx-json@2.3, spdx-json@2.2, spdx-tag-value and cyclonedx-json@1.6 with both binaries and comparing the results with timestamps and generated ids removed: all 50 pairs are identical. Tests pin the original semantics (including the duplicate-edge corner case) and guard the complexity with a 50k x 100k document that must complete in under 10s; the old implementation takes about a minute on it. Signed-off-by: juanjsebgarcia <github@juangarcia.co.uk> Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
2026-10-11 21:57:22 +02:00 · 2026-09-11 21:11:21 +00:00
parent c4800b2393
commit fb59c4e65c
2 changed files with 204 additions and 13 deletions
@@ -0,0 +1,181 @@
+package spdxhelpers
+
+import (
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/spdx/tools-golang/spdx"
+	"github.com/spdx/tools-golang/spdx/v2/common"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// The tests in this file cover findRootPackages, which locates the packages an SPDX document declares it DESCRIBES
+// (or which declare themselves DESCRIBED_BY the document). Until it was rewritten, this function compared every
+// package against every relationship, so its cost grew with packages x relationships. On real documents that made
+// SPDX decoding quadratic in document size: a 119MB SPDX JSON document (70k packages, 153k relationships) spent
+// 72s here, a 238MB document 266s, and a 1.3GB document did not finish within 90 minutes. Encoding the same
+// documents, and decoding equivalently sized syft-json or CycloneDX documents, is linear and takes seconds.
+//
+// Test_findRootPackages pins the exact semantics of the original implementation, including the corner cases a
+// rewrite could easily change: result order follows package order, an element described twice appears twice, and
+// only edges that touch the DOCUMENT element count. Test_findRootPackages_scalesLinearly is the regression guard
+// for the performance fix itself.
+
+func Test_findRootPackages(t *testing.T) {
+	pkgA := &spdx.Package{PackageSPDXIdentifier: "Package-a"}
+	pkgB := &spdx.Package{PackageSPDXIdentifier: "Package-b"}
+	pkgC := &spdx.Package{PackageSPDXIdentifier: "Package-c"}
+
+	describes := func(id common.ElementID) *spdx.Relationship {
+		return &spdx.Relationship{
+			RefA:         common.DocElementID{ElementRefID: "DOCUMENT"},
+			RefB:         common.DocElementID{ElementRefID: id},
+			Relationship: spdx.RelationshipDescribes,
+		}
+	}
+	describedBy := func(id common.ElementID) *spdx.Relationship {
+		return &spdx.Relationship{
+			RefA:         common.DocElementID{ElementRefID: id},
+			RefB:         common.DocElementID{ElementRefID: "DOCUMENT"},
+			Relationship: spdx.RelationshipDescribedBy,
+		}
+	}
+
+	tests := []struct {
+		name          string
+		packages      []*spdx.Package
+		relationships []*spdx.Relationship
+		want          []*spdx.Package
+	}{
+		{
+			name:          "no relationships",
+			packages:      []*spdx.Package{pkgA, pkgB},
+			relationships: nil,
+			want:          nil,
+		},
+		{
+			name:     "document describes one package",
+			packages: []*spdx.Package{pkgA, pkgB, pkgC},
+			relationships: []*spdx.Relationship{
+				{RefA: common.DocElementID{ElementRefID: "Package-a"}, RefB: common.DocElementID{ElementRefID: "Package-b"}, Relationship: spdx.RelationshipContains},
+				describes("Package-b"),
+			},
+			want: []*spdx.Package{pkgB},
+		},
+		{
+			name:     "package described by the document",
+			packages: []*spdx.Package{pkgA, pkgB, pkgC},
+			relationships: []*spdx.Relationship{
+				describedBy("Package-c"),
+			},
+			want: []*spdx.Package{pkgC},
+		},
+		{
+			name:     "multiple roots are returned in package order",
+			packages: []*spdx.Package{pkgA, pkgB, pkgC},
+			relationships: []*spdx.Relationship{
+				describes("Package-c"),
+				describedBy("Package-a"),
+			},
+			want: []*spdx.Package{pkgA, pkgC},
+		},
+		{
+			name:     "duplicate edges yield duplicate roots (preserved behavior)",
+			packages: []*spdx.Package{pkgA, pkgB},
+			relationships: []*spdx.Relationship{
+				describes("Package-a"),
+				describedBy("Package-a"),
+			},
+			want: []*spdx.Package{pkgA, pkgA},
+		},
+		{
+			name:     "describes edges not from the document are ignored",
+			packages: []*spdx.Package{pkgA, pkgB},
+			relationships: []*spdx.Relationship{
+				{RefA: common.DocElementID{ElementRefID: "Package-a"}, RefB: common.DocElementID{ElementRefID: "Package-b"}, Relationship: spdx.RelationshipDescribes},
+				{RefA: common.DocElementID{ElementRefID: "Package-b"}, RefB: common.DocElementID{ElementRefID: "Package-a"}, Relationship: spdx.RelationshipDescribedBy},
+			},
+			want: nil,
+		},
+		{
+			name:     "described element that is not a package is ignored",
+			packages: []*spdx.Package{pkgA},
+			relationships: []*spdx.Relationship{
+				describes("File-1"),
+			},
+			want: nil,
+		},
+		{
+			name:     "nil packages and relationships are skipped",
+			packages: []*spdx.Package{nil, pkgA},
+			relationships: []*spdx.Relationship{
+				nil,
+				describes("Package-a"),
+			},
+			want: []*spdx.Package{pkgA},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := findRootPackages(&spdx.Document{
+				Packages:      tt.packages,
+				Relationships: tt.relationships,
+			})
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
+// Test_findRootPackages_scalesLinearly guards against reintroducing a packages x relationships scan. At 50k
+// packages and 100k relationships (5e9 comparisons) the previous implementation needs on the order of a minute on
+// a fast laptop and several minutes on a CI runner, whereas the indexed implementation completes in about a
+// millisecond. The 10s bound is therefore loose enough never to flake on slow hardware yet far below what any
+// quadratic implementation could achieve.
+func Test_findRootPackages_scalesLinearly(t *testing.T) {
+	doc := largeSPDXDocument(50_000, 100_000)
+
+	start := time.Now()
+	got := findRootPackages(doc)
+	elapsed := time.Since(start)
+
+	require.Len(t, got, 1)
+	assert.Equal(t, common.ElementID("Package-0"), got[0].PackageSPDXIdentifier)
+	assert.Less(t, elapsed, 10*time.Second, "findRootPackages took %s on %d packages and %d relationships", elapsed, len(doc.Packages), len(doc.Relationships))
+}
+
+// Benchmark_findRootPackages measures the same 50k x 100k document as the scaling test so the two can be compared
+// directly: run with `go test ./syft/format/common/spdxhelpers/ -run xxx -bench findRootPackages`.
+func Benchmark_findRootPackages(b *testing.B) {
+	doc := largeSPDXDocument(50_000, 100_000)
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		_ = findRootPackages(doc)
+	}
+}
+
+// largeSPDXDocument builds a document with the given number of packages and relationships where the document
+// describes exactly the first package and every other relationship links packages to one another.
+func largeSPDXDocument(packages int, relationships int) *spdx.Document {
+	doc := &spdx.Document{}
+	for i := 0; i < packages; i++ {
+		doc.Packages = append(doc.Packages, &spdx.Package{
+			PackageSPDXIdentifier: common.ElementID(fmt.Sprintf("Package-%d", i)),
+		})
+	}
+	doc.Relationships = append(doc.Relationships, &spdx.Relationship{
+		RefA:         common.DocElementID{ElementRefID: "DOCUMENT"},
+		RefB:         common.DocElementID{ElementRefID: "Package-0"},
+		Relationship: spdx.RelationshipDescribes,
+	})
+	for i := 1; i < relationships; i++ {
+		doc.Relationships = append(doc.Relationships, &spdx.Relationship{
+			RefA:         common.DocElementID{ElementRefID: common.ElementID(fmt.Sprintf("Package-%d", i%packages))},
+			RefB:         common.DocElementID{ElementRefID: common.ElementID(fmt.Sprintf("Package-%d", (i+1)%packages))},
+			Relationship: spdx.RelationshipDependsOn,
+		})
+	}
+	return doc
+}
@@ -83,20 +83,30 @@ func removeRelationships(relationships []*spdx.Relationship, spdxID spdx.Element
 }

 func findRootPackages(doc *spdx.Document) (out []*spdx.Package) {
+	// index the DESCRIBES / DESCRIBED_BY relationships once, counting how many
+	// times each element is described by the document, so that the result
+	// matches the previous packages x relationships scan exactly (one entry per
+	// matching relationship) without the quadratic cost.
+	describedCount := make(map[spdx.ElementID]int)
+	for _, r := range doc.Relationships {
+		if r == nil {
+			continue
+		}
+		switch {
+		case r.RefA.ElementRefID == "DOCUMENT" && r.Relationship == spdx.RelationshipDescribes:
+			describedCount[r.RefB.ElementRefID]++
+		case r.RefB.ElementRefID == "DOCUMENT" && r.Relationship == spdx.RelationshipDescribedBy:
+			describedCount[r.RefA.ElementRefID]++
+		}
+	}
+	if len(describedCount) == 0 {
+		return nil
+	}
 	for _, p := range doc.Packages {
-		for _, r := range doc.Relationships {
-			describes := r.RefA.ElementRefID == "DOCUMENT" &&
-				r.Relationship == spdx.RelationshipDescribes &&
-				r.RefB.ElementRefID == p.PackageSPDXIdentifier
-
-			describedBy := r.RefB.ElementRefID == "DOCUMENT" &&
-				r.Relationship == spdx.RelationshipDescribedBy &&
-				r.RefA.ElementRefID == p.PackageSPDXIdentifier
-
-			if !describes && !describedBy {
-				continue
-			}
-
+		if p == nil {
+			continue
+		}
+		for i := 0; i < describedCount[p.PackageSPDXIdentifier]; i++ {
 			out = append(out, p)
 		}
 	}