Skip to content
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

Fix high memory use and slowness with large number of targets/depende… #2354

Merged
merged 8 commits into from
Jul 15, 2019

Conversation

tenatus
Copy link

@tenatus tenatus commented Jul 15, 2019

…ncies

Description

I built a fake script from a large Visual Studio solution. It worked well but took a long time and used many GB of memory before it printed out the build order. I have fixed the issue by making determineBuildOrder more efficient (was repeating a lot of work)

I added a unit test that is my solution with the project names replaced with random words.

Copy link
Member

@matthid matthid left a comment

Choose a reason for hiding this comment

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

Looks like a reasonable change. Very much appreciated. Nice analysis fix and test. Not much left to ask for ;)


// These dependencies are taken from large .NET/COM VS sln. Only the names have been changed.
// Printing the running order was taking 3 minutes and ~ 12 GB or RAM using FAKE 5.15
Fake.ContextHelper.fakeContextTestCase "Running order resolves quickly when there are numerous targets with numerous dependencies" <| fun _ ->
Copy link
Member

Choose a reason for hiding this comment

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

Any reason not to use Fake.ContextHelper.fakeContextTestCaseAssertTime like in some tests above?

Copy link
Author

Choose a reason for hiding this comment

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

No reason, just didn't know about it :)

@matthid matthid changed the base branch from master to release/next July 15, 2019 22:54
@matthid matthid merged commit fe77589 into fsprojects:release/next Jul 15, 2019
@matthid matthid mentioned this pull request Jul 15, 2019
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