Skip to content

Fix algorithmic bug causing exponential blowup in dependency analysis #2218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

josh-arnold-1
Copy link
Contributor

@josh-arnold-1 josh-arnold-1 commented Jul 24, 2025

Fix exponential time complexity in build target depth computation

Problem

The targetDepthsAndDependents function in BuildSystemManager was taking ~24 seconds to process 636 build targets, causing significant delays during build system initialization and target change notifications.

This should complete in well under a second for this number of targets.

Root Cause

The issue was an algorithmic bug in the dependency traversal logic. The function was checking the wrong variable when deciding whether to add dependencies to the work queue:

// ❌ BEFORE: Checking current target instead of dependency
if depths[target, default: 0] < depth + 1 {
  worksList.append((dependency, depth + 1))
}

This caused dependencies to be added to the work list even when they had already been processed with adequate depth, leading to exponential time complexity as the same nodes were visited repeatedly.

Solution

Fixed the condition to check the dependency's depth instead of the current target's depth:

// ✅ AFTER: Correctly checking dependency depth
if depths[dependency, default: 0] < depth + 1 {
  worksList.append((dependency, depth + 1))
}

This ensures proper memoization and restores the intended O(V+E) linear time complexity.

Impact

  • Performance: Reduces target depth computation from ~24 seconds to well under 1 second
  • User Experience: Significantly faster build system initialization and target change handling
  • Algorithmic Complexity: Fixes exponential blowup, restoring linear time complexity

Testing

The fix maintains the same algorithmic correctness while dramatically improving performance. The function still correctly computes:

  • Target depths (distance from root targets)
  • Dependent relationships (reverse dependency mapping)

But now does so efficiently without redundant work.


Files Changed:

  • Sources/BuildSystemIntegration/BuildSystemManager.swift - Fixed variable name in depth checking condition

@bnbarham
Copy link
Contributor

@swift-ci please test

Copy link
Contributor

@bnbarham bnbarham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof, nice find. Thanks @josh-arnold-1!

@bnbarham bnbarham merged commit 5df8f3d into swiftlang:main Jul 24, 2025
3 checks passed
@ahoppen
Copy link
Member

ahoppen commented Jul 25, 2025

Great catch, thanks for debugging this and finding the fix! 🤩

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants