From 5313dcc9179b3043a3fd9b58365f76b4b3e1d5e9 Mon Sep 17 00:00:00 2001 From: Greg Villicana Date: Wed, 15 Jul 2026 00:55:56 -0700 Subject: [PATCH] Fix Yarn detector perf, too many component stream enumerations per workspace --- .../yarn/YarnLockComponentDetector.cs | 48 +++++-- .../Configs/CondaLockDetectorExperiment.cs | 2 +- .../YarnLockDetectorTests.cs | 127 ++++++++++++++++++ 3 files changed, 162 insertions(+), 15 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/yarn/YarnLockComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/yarn/YarnLockComponentDetector.cs index 4d7d8dd1b..c824e9456 100644 --- a/src/Microsoft.ComponentDetection.Detectors/yarn/YarnLockComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/yarn/YarnLockComponentDetector.cs @@ -263,29 +263,49 @@ private void GetWorkspaceDependencies(IList yarnWorkspaces, DirectoryInf ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal; + // Resolve all workspace package.json files in a SINGLE filesystem traversal to improve perf. + var unionMatcher = new Matcher(comparison); + foreach (var workspacePattern in yarnWorkspaces) + { + unionMatcher.AddInclude($"{workspacePattern}/package.json"); + } + + var componentStreams = this.ComponentStreamEnumerableFactory.GetComponentStreams( + root, + ["package.json"], + null, + recursivelyScanDirectories: true); + + var workspaceCandidates = new List<(string RelativePath, string Location, IDictionary> Dependencies)>(); + foreach (var stream in componentStreams) + { + var relativePath = Path.GetRelativePath(root.FullName, stream.Location).Replace('\\', '/'); + if (!unionMatcher.Match(relativePath).HasMatches) + { + continue; + } + + var combinedDependencies = NpmComponentUtilities.TryGetAllPackageJsonDependencies(stream.Stream, out _); + workspaceCandidates.Add((relativePath, stream.Location, combinedDependencies)); + } + foreach (var workspacePattern in yarnWorkspaces) { var matcher = new Matcher(comparison); matcher.AddInclude($"{workspacePattern}/package.json"); - var componentStreams = this.ComponentStreamEnumerableFactory.GetComponentStreams( - root, - (file) => + foreach (var (candidateRelativePath, candidateLocation, candidateDependencies) in workspaceCandidates) + { + if (!matcher.Match(candidateRelativePath).HasMatches) { - var relativePath = Path.GetRelativePath(root.FullName, file.FullName).Replace('\\', '/'); - return matcher.Match(relativePath).HasMatches; - }, - null, - true); + continue; + } - foreach (var stream in componentStreams) - { - this.Logger.LogInformation("{ComponentLocation} found for workspace {WorkspacePattern}", stream.Location, workspacePattern); - var combinedDependencies = NpmComponentUtilities.TryGetAllPackageJsonDependencies(stream.Stream, out _); + this.Logger.LogInformation("{ComponentLocation} found for workspace {WorkspacePattern}", candidateLocation, workspacePattern); - foreach (var dependency in combinedDependencies) + foreach (var dependency in candidateDependencies) { - this.ProcessWorkspaceDependency(dependencies, dependency, workspaceDependencyVsLocationMap, stream.Location); + this.ProcessWorkspaceDependency(dependencies, dependency, workspaceDependencyVsLocationMap, candidateLocation); } } } diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/CondaLockDetectorExperiment.cs b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/CondaLockDetectorExperiment.cs index d17f9b4f0..b55c034e6 100644 --- a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/CondaLockDetectorExperiment.cs +++ b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/CondaLockDetectorExperiment.cs @@ -1,8 +1,8 @@ namespace Microsoft.ComponentDetection.Orchestrator.Experiments.Configs; using Microsoft.ComponentDetection.Contracts; +using Microsoft.ComponentDetection.Detectors.CondaLock; using Microsoft.ComponentDetection.Detectors.Pip; -using Microsoft.ComponentDetection.Detectors.Poetry; /// /// Experiment to validate CondaLockComponentDetector against PipReportComponentDetector. diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/YarnLockDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/YarnLockDetectorTests.cs index 1c7804940..5ea762c45 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/YarnLockDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/YarnLockDetectorTests.cs @@ -427,6 +427,133 @@ public async Task WellFormedYarnLockV2WithWorkspaceAltForm_FindsComponentAsync() parentComponent => parentComponent.Name == componentA.Name && parentComponent.Version == version0); } + [TestMethod] + public async Task WellFormedYarnLockWithMultipleWorkspaces_FindsAllComponentsAsync() + { + // Regression test for the workspace resolution performance fix: a single + // filesystem traversal must still discover every workspace package.json that a + // glob workspace pattern expands to, and register the dependencies declared in + // each of them. + var version0 = NewRandomVersion(); + var version1 = NewRandomVersion(); + var version2 = NewRandomVersion(); + + var componentA = new YarnTestComponentDefinition { ActualVersion = version0, RequestedVersion = $"^{version0}", ResolvedVersion = "https://resolved0/a/resolved", Name = Guid.NewGuid().ToString("N") }; + var componentB = new YarnTestComponentDefinition { ActualVersion = version1, RequestedVersion = $"^{version1}", ResolvedVersion = "https://resolved1/b/resolved", Name = Guid.NewGuid().ToString("N") }; + var componentC = new YarnTestComponentDefinition { ActualVersion = version2, RequestedVersion = $"^{version2}", ResolvedVersion = "https://resolved2/c/resolved", Name = Guid.NewGuid().ToString("N") }; + + var yarnLockStream = YarnTestUtilities.GetMockedYarnLockStream("yarn.lock", this.CreateYarnLockV1FileContent([componentA, componentB, componentC])); + + var rootJson = JsonSerializer.Serialize(new + { + name = "testworkspace", + version = "1.0.0", + @private = true, + workspaces = new[] { "packages/*" }, + }); + + var app1Json = JsonSerializer.Serialize(new + { + name = "app1", + version = "1.0.0", + dependencies = new Dictionary { [componentA.Name] = componentA.RequestedVersion }, + }); + + var app2Json = JsonSerializer.Serialize(new + { + name = "app2", + version = "1.0.0", + dependencies = new Dictionary + { + [componentB.Name] = componentB.RequestedVersion, + [componentC.Name] = componentC.RequestedVersion, + }, + }); + + var app1Path = Path.Combine(Path.GetTempPath(), "packages", "app1", "package.json"); + var app2Path = Path.Combine(Path.GetTempPath(), "packages", "app2", "package.json"); + + var (scanResult, componentRecorder) = await this.detectorTestUtility + .WithFile("yarn.lock", yarnLockStream.Stream) + .WithFile("package.json", rootJson.ToStream(), ["package.json"], Path.Combine(Path.GetTempPath(), "package.json")) + .WithFile("package.json", app1Json.ToStream(), ["package.json"], app1Path) + .WithFile("package.json", app2Json.ToStream(), ["package.json"], app2Path) + .ExecuteDetectorAsync(); + + scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); + + var detectedComponents = componentRecorder.GetDetectedComponents().ToList(); + detectedComponents.Should().HaveCount(3); + + var detectedA = detectedComponents.Single(x => ((NpmComponent)x.Component).Name == componentA.Name); + var detectedB = detectedComponents.Single(x => ((NpmComponent)x.Component).Name == componentB.Name); + var detectedC = detectedComponents.Single(x => ((NpmComponent)x.Component).Name == componentC.Name); + + ((NpmComponent)detectedA.Component).Version.Should().Be(version0); + ((NpmComponent)detectedB.Component).Version.Should().Be(version1); + ((NpmComponent)detectedC.Component).Version.Should().Be(version2); + + // Each component is an explicitly referenced (root) dependency of its workspace. + componentRecorder.AssertAllExplicitlyReferencedComponents(detectedA.Component.Id, parent => parent.Name == componentA.Name); + componentRecorder.AssertAllExplicitlyReferencedComponents(detectedB.Component.Id, parent => parent.Name == componentB.Name); + componentRecorder.AssertAllExplicitlyReferencedComponents(detectedC.Component.Id, parent => parent.Name == componentC.Name); + + // The dependency's file path must point at the workspace package.json that declared it. + detectedA.FilePaths.Should().Contain(app1Path); + detectedB.FilePaths.Should().Contain(app2Path); + detectedC.FilePaths.Should().Contain(app2Path); + } + + [TestMethod] + public async Task WellFormedYarnLockWithWorkspaceSharedDependency_AttributesFirstWorkspaceAsync() + { + // Regression test guarding that the single-traversal workspace resolution preserves + // the original resolution order (workspace pattern order, then discovery order) so + // that first-wins location attribution stays deterministic when the same dependency + // is declared by more than one workspace. + var version0 = NewRandomVersion(); + var componentA = new YarnTestComponentDefinition { ActualVersion = version0, RequestedVersion = $"^{version0}", ResolvedVersion = "https://resolved0/a/resolved", Name = Guid.NewGuid().ToString("N") }; + + var yarnLockStream = YarnTestUtilities.GetMockedYarnLockStream("yarn.lock", this.CreateYarnLockV1FileContent([componentA])); + + var rootJson = JsonSerializer.Serialize(new + { + name = "testworkspace", + version = "1.0.0", + @private = true, + workspaces = new[] { "packages/app1", "packages/app2" }, + }); + + var memberJson = JsonSerializer.Serialize(new + { + name = "member", + version = "1.0.0", + dependencies = new Dictionary { [componentA.Name] = componentA.RequestedVersion }, + }); + + var app1Path = Path.Combine(Path.GetTempPath(), "packages", "app1", "package.json"); + var app2Path = Path.Combine(Path.GetTempPath(), "packages", "app2", "package.json"); + + var (scanResult, componentRecorder) = await this.detectorTestUtility + .WithFile("yarn.lock", yarnLockStream.Stream) + .WithFile("package.json", rootJson.ToStream(), ["package.json"], Path.Combine(Path.GetTempPath(), "package.json")) + .WithFile("package.json", memberJson.ToStream(), ["package.json"], app1Path) + .WithFile("package.json", memberJson.ToStream(), ["package.json"], app2Path) + .ExecuteDetectorAsync(); + + scanResult.ResultCode.Should().Be(ProcessingResultCode.Success); + + var detectedComponents = componentRecorder.GetDetectedComponents(); + detectedComponents.Should().ContainSingle(); + + var detectedA = detectedComponents.Single(); + ((NpmComponent)detectedA.Component).Name.Should().Be(componentA.Name); + + // The first workspace pattern (packages/app1) wins the location attribution. + detectedA.FilePaths.Should().ContainSingle(); + detectedA.FilePaths.Should().Contain(app1Path); + } + [TestMethod] public async Task WellFormedYarnLockV1WithMoreThanOneComponent_FindsComponentsAsync() {