Skip to content

[Incremental] Try a cache for external dependency mod times #538

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

Merged
merged 1 commit into from
Mar 13, 2021

Conversation

davidungar
Copy link
Contributor

Playing a hunch to fix a performance issue.

rdar://75351845

@davidungar
Copy link
Contributor Author

@swift-ci please test

@davidungar davidungar requested a review from CodaFi March 12, 2021 19:48
@CodaFi
Copy link
Contributor

CodaFi commented Mar 12, 2021

Do we know if this is the root cause?

@davidungar
Copy link
Contributor Author

davidungar commented Mar 13, 2021

Do we know if this is the root cause?

It's suspicious, given the trace in the radar, but I'm waiting to hear from the person reporting the problem to see if it fixes it. I didn't see enough info the radar to reproduce the issue myself. Thus, the "DNM".

@davidungar davidungar changed the title [DNM, Incremental] Try a cache for external dependency mod times [Incremental] Try a cache for external dependency mod times Mar 13, 2021
@davidungar
Copy link
Contributor Author

Do we know if this is the root cause?

Fixed the issue, 40 secs -> 2 secs.

Copy link
Contributor

@CodaFi CodaFi left a comment

Choose a reason for hiding this comment

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

Approved. Please add a regression test that uses a mock file system instance to make sure we don’t see this again.

@CodaFi CodaFi merged commit 5b309e1 into swiftlang:main Mar 13, 2021
@davidungar davidungar deleted the try-mod-time-cache branch July 3, 2021 17:00
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.

2 participants