Reduce npm lockfile parsing allocations

internal/npm/npm.go

@@ -3,6 +3,7 @@ package npm
 import (
 	"bytes"
 	"encoding/json"
+	"iter"
 	"net/url"
 	"strings"
 
@@ -182,7 +183,22 @@ func (p *npmPackageLockParser) Parse(filename string, content []byte) (*core.Res
 }
 
 func parsePackageLockV1(deps map[string]packageLockDep) []core.Dependency {
-	var result []core.Dependency
+	count := countPackageLockV1(deps)
+	if count == 0 {
+		return nil
+	}
+	return appendPackageLockV1(make([]core.Dependency, 0, count), deps)
+}
+
+func countPackageLockV1(deps map[string]packageLockDep) int {
+	count := len(deps)
+	for _, dep := range deps {
+		count += countPackageLockV1(dep.Dependencies)
+	}
+	return count
+}
+
+func appendPackageLockV1(result []core.Dependency, deps map[string]packageLockDep) []core.Dependency {
 	for name, dep := range deps {
 		scope := core.Runtime
 		if dep.Dev {
@@ -200,10 +216,8 @@ func parsePackageLockV1(deps map[string]packageLockDep) []core.Dependency {
 			RegistryURL: dep.Resolved,
 		})
 
-		// Recursively add nested dependencies
 		if len(dep.Dependencies) > 0 {
-			nested := parsePackageLockV1(dep.Dependencies)
-			result = append(result, nested...)
+			result = appendPackageLockV1(result, dep.Dependencies)
 		}
 	}
 	return result
@@ -392,54 +406,68 @@ func isPackagePathLine(trimmed string) bool {
 
 // isPackagesSectionEnd detects the closing brace of the "packages" object.
 func isPackagesSectionEnd(line, trimmed string) bool {
+	line = strings.TrimSuffix(line, "\r")
 	return (line == "  }," || line == "  }") && strings.HasPrefix(trimmed, "}")
 }
 
 // parsePackageLockV3Lines parses v3 format using line-based parsing.
 // Format: "packages": { "node_modules/name": { "version": "x", ... } }
 func parsePackageLockV3Lines(content []byte) []core.Dependency {
+	text := string(content)
+	count := 0
+	for range packageLockV3Entries(text) {
+		count++
+	}
 	var deps []core.Dependency
-	lines := strings.Split(string(content), "\n")
+	if count > 0 {
+		deps = make([]core.Dependency, 0, count)
+	}
 	directDependencies := parsePackageLockDirectDependencies(content)
+	for entry := range packageLockV3Entries(text) {
+		if dep, ok := entry.toDependency(directDependencies); ok {
+			deps = append(deps, dep)
+		}
+	}
+	if len(deps) == 0 {
+		return nil
+	}
+	return deps
+}
 
-	inPackages := false
-	var entry v3PackageEntry
+func packageLockV3Entries(content string) iter.Seq[v3PackageEntry] {
+	return func(yield func(v3PackageEntry) bool) {
+		inPackages := false
+		var entry v3PackageEntry
 
-	for _, line := range lines {
-		trimmed := strings.TrimSpace(line)
+		for line := range strings.SplitSeq(content, "\n") {
+			trimmed := strings.TrimSpace(line)
 
-		if !inPackages {
-			if strings.HasPrefix(trimmed, `"packages"`) {
-				inPackages = true
+			if !inPackages {
+				if strings.HasPrefix(trimmed, `"packages"`) {
+					inPackages = true
+				}
+				continue
 			}
-			continue
-		}
 
-		if isPackagesSectionEnd(line, trimmed) {
-			break
-		}
+			if isPackagesSectionEnd(line, trimmed) {
+				break
+			}
 
-		if isPackagePathLine(trimmed) {
-			if entry.hasContent() {
-				if dep, ok := entry.toDependency(directDependencies); ok {
-					deps = append(deps, dep)
+			if isPackagePathLine(trimmed) {
+				if entry.hasContent() && !yield(entry) {
+					return
 				}
+				entry.reset(extractQuotedPath(trimmed))
+				continue
 			}
-			entry.reset(extractQuotedPath(trimmed))
-			continue
-		}
 
-		entry.updateFromLine(trimmed)
-	}
+			entry.updateFromLine(trimmed)
+		}
 
-	// Don't forget the last package
-	if entry.hasContent() {
-		if dep, ok := entry.toDependency(directDependencies); ok {
-			deps = append(deps, dep)
+		if entry.hasContent() {
+			yield(entry)
 		}
 	}
-
-	return deps
 }
 
 // extractJSONStringValue extracts the string value from a JSON line like: "key": "value"

npm_bench_test.go

@@ -0,0 +1,88 @@
+package manifests
+
+import (
+	"encoding/json"
+	"fmt"
+	"strings"
+	"testing"
+)
+
+func npmLockHistory(tb testing.TB, format, count int) []byte {
+	tb.Helper()
+	entries := make(map[string]any, count)
+	for i := range count {
+		name := fmt.Sprintf("@scope/package-%d", i)
+		key := name
+		if format == 3 {
+			key = "node_modules/" + name
+		}
+		entries[key] = map[string]any{
+			"version":   "1.2.3",
+			"resolved":  "https://registry.npmjs.org/" + name + "/-/package-1.2.3.tgz",
+			"integrity": "sha512-Zml4dHVyZS1wYWNrYWdlLWludGVncml0eQ==",
+			"license":   "MIT", "dev": i%2 == 0,
+		}
+	}
+	document := map[string]any{"name": "benchmark-project", "version": "1.0.0", "lockfileVersion": format}
+	if format == 3 {
+		entries[""] = map[string]any{"dependencies": map[string]string{"@scope/package-1": "^1.2.0"}}
+		document["packages"] = entries
+	} else {
+		document["dependencies"] = entries
+	}
+	data, err := json.MarshalIndent(document, "", "  ")
+	if err != nil {
+		tb.Fatal(err)
+	}
+	return data
+}
+
+func TestNPMLargeLockfile(t *testing.T) {
+	const count = 10000
+	for _, format := range []int{1, 3} {
+		t.Run(fmt.Sprintf("v%d", format), func(t *testing.T) {
+			result, err := Parse("package-lock.json", npmLockHistory(t, format, count))
+			if err != nil {
+				t.Fatal(err)
+			}
+			if len(result.Dependencies) != count {
+				t.Fatalf("got %d dependencies, want %d", len(result.Dependencies), count)
+			}
+			seen := make(map[string]bool, count)
+			for _, dep := range result.Dependencies {
+				if seen[dep.Name] {
+					t.Fatalf("duplicate dependency: %s", dep.Name)
+				}
+				seen[dep.Name] = true
+				if dep.Version != "1.2.3" || dep.PURL != "pkg:npm/%40"+strings.TrimPrefix(dep.Name, "@")+"@1.2.3" || dep.Integrity != "sha512-Zml4dHVyZS1wYWNrYWdlLWludGVncml0eQ==" {
+					t.Fatalf("unexpected dependency: %+v", dep)
+				}
+				if dep.Direct != (format == 3 && dep.Name == "@scope/package-1") {
+					t.Fatalf("unexpected direct flag: %+v", dep)
+				}
+			}
+		})
+	}
+}
+
+func BenchmarkNPMLockfile(b *testing.B) {
+	for _, format := range []int{1, 3} {
+		for _, count := range []int{10, 1000, 10000} {
+			b.Run(fmt.Sprintf("v%d/packages=%d", format, count), func(b *testing.B) {
+				data := npmLockHistory(b, format, count)
+				b.ReportAllocs()
+				b.SetBytes(int64(len(data)))
+				b.ResetTimer()
+				for range b.N {
+					result, err := Parse("package-lock.json", data)
+					if err != nil {
+						b.Fatal(err)
+					}
+					if len(result.Dependencies) != count {
+						b.Fatalf("got %d dependencies, want %d", len(result.Dependencies), count)
+					}
+				}
+			})
+		}
+	}
+}

npm_test.go

@@ -1,7 +1,12 @@
 package manifests
 
 import (
+	"bytes"
+	"fmt"
 	"os"
+	"reflect"
+	"sort"
+	"strings"
 	"testing"
 )
 
@@ -84,3 +89,127 @@ func TestPURLGeneration(t *testing.T) {
 		}
 	}
 }
+
+func TestNPMLockfileNestedDependencies(t *testing.T) {
+	const content = `{"lockfileVersion":1,"dependencies":{
+		"parent":{"version":"1.0.0","dev":true,"dependencies":{
+			"shared":{"version":"2.0.0","optional":true,"dependencies":{
+				"@scope/leaf":{"version":"3.0.0","dev":true,"optional":true,"integrity":"sha512-leaf"}
+			}}
+		}},
+		"shared":{"version":"4.0.0"}
+	}}`
+	result, err := Parse("package-lock.json", []byte(content))
+	if err != nil {
+		t.Fatal(err)
+	}
+	want := []Dependency{
+		{Name: "@scope/leaf", Version: "3.0.0", Scope: Development, Integrity: "sha512-leaf", PURL: "pkg:npm/%40scope/leaf@3.0.0"},
+		{Name: "parent", Version: "1.0.0", Scope: Development, PURL: "pkg:npm/parent@1.0.0"},
+		{Name: "shared", Version: "2.0.0", Scope: Optional, PURL: "pkg:npm/shared@2.0.0"},
+		{Name: "shared", Version: "4.0.0", Scope: Runtime, PURL: "pkg:npm/shared@4.0.0"},
+	}
+	sort.Slice(result.Dependencies, func(i, j int) bool {
+		a, b := result.Dependencies[i], result.Dependencies[j]
+		if a.Name != b.Name {
+			return a.Name < b.Name
+		}
+		return a.Version < b.Version
+	})
+	if !reflect.DeepEqual(result.Dependencies, want) {
+		t.Fatalf("got %+v, want %+v", result.Dependencies, want)
+	}
+}
+
+func TestNPMLockfileLineIteration(t *testing.T) {
+	const content = `{
+  "lockfileVersion": 3,
+  "packages": {
+    "": {
+      "dependencies": {"direct": "^1.0.0"}
+    },
+    "node_modules/direct": {
+      "version": "1.0.0",
+      "integrity": "LONG_INTEGRITY"
+    },
+    "node_modules/direct/node_modules/@scope/nested": {
+      "version": "2.0.0",
+      "devOptional": true
+    },
+    "node_modules/last": {
+      "version": "3.0.0",
+      "optional": true
+    }
+  }
+}`
+	integrity := strings.Repeat("x", 128<<10)
+	for _, version := range []int{2, 3} {
+		for _, newline := range []string{"\n", "\r\n"} {
+			for _, trailing := range []bool{false, true} {
+				t.Run(fmt.Sprintf("v%d/newline=%q/trailing=%t", version, newline, trailing), func(t *testing.T) {
+					input := strings.Replace(content, `"lockfileVersion": 3`, fmt.Sprintf(`"lockfileVersion": %d`, version), 1)
+					input = strings.Replace(input, "LONG_INTEGRITY", integrity, 1)
+					input = strings.ReplaceAll(input, "\n", newline)
+					if trailing {
+						input += newline
+					}
+					result, err := Parse("npm-shrinkwrap.json", []byte(input))
+					if err != nil {
+						t.Fatal(err)
+					}
+					want := []Dependency{
+						{Name: "direct", Version: "1.0.0", Scope: Runtime, Direct: true, Integrity: integrity, PURL: "pkg:npm/direct@1.0.0"},
+						{Name: "@scope/nested", Version: "2.0.0", Scope: Development, PURL: "pkg:npm/%40scope/nested@2.0.0"},
+						{Name: "last", Version: "3.0.0", Scope: Optional, PURL: "pkg:npm/last@3.0.0"},
+					}
+					if !reflect.DeepEqual(result.Dependencies, want) {
+						t.Fatal("dependency fields or order differ")
+					}
+				})
+			}
+		}
+	}
+}
+
+func TestNPMLockfileEmptyDependencies(t *testing.T) {
+	for _, content := range []string{
+		`{"lockfileVersion":1,"dependencies":{}}`,
+		"{\n  \"lockfileVersion\": 3,\n  \"packages\": {\n  }\n}",
+		"{\n  \"lockfileVersion\": 3,\n  \"packages\": {\n    \"node_modules/\": {\n      \"version\": \"1.0.0\"\n    }\n  }\n}",
+	} {
+		result, err := Parse("package-lock.json", []byte(content))
+		if err != nil {
+			t.Fatal(err)
+		}
+		if result.Dependencies != nil {
+			t.Fatalf("expected nil dependencies, got %+v", result.Dependencies)
+		}
+	}
+}
+
+func TestNPMV2LockfileCRLF(t *testing.T) {
+	content, err := os.ReadFile("testdata/npm/npm-lockfile-version-2/package-lock.json")
+	if err != nil {
+		t.Fatal(err)
+	}
+	lf := bytes.ReplaceAll(content, []byte("\r\n"), []byte("\n"))
+	crlf := bytes.ReplaceAll(lf, []byte("\n"), []byte("\r\n"))
+	for _, filename := range []string{"package-lock.json", "npm-shrinkwrap.json"} {
+		t.Run(filename, func(t *testing.T) {
+			want, err := Parse(filename, lf)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if len(want.Dependencies) != 3 {
+				t.Fatalf("LF fixture has %d dependencies, want 3", len(want.Dependencies))
+			}
+			got, err := Parse(filename, crlf)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if !reflect.DeepEqual(got, want) {
+				t.Fatalf("CRLF result differs from LF: got %d dependencies, want %d", len(got.Dependencies), len(want.Dependencies))
+			}
+		})
+	}
+}