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 cubic runtime complexity for collecting source/import files. #1079

Merged
merged 2 commits into from
Feb 23, 2017

Conversation

s-ludwig
Copy link
Member

See vibe-d/vibe.d#1690.

This is still not very efficient, doing several temporary allocations and copies, but replaces cubic runtime by almost linear (except for allocation and AA creation times).

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 61.621% when pulling cbe25ea on issue1690_slow_file_collection into 41b245c on master.

Copy link
Member

@wilzbach wilzbach left a comment

Choose a reason for hiding this comment

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

This looks quite reasonable to me (though I am not that familiar with the DUB internals yet).

@@ -149,14 +149,14 @@ struct BuildSettings {
// add string import files (avoids file name duplicates in addition to path duplicates)
private void addSI(ref string[] arr, in string[] vals)
{
outer:
bool[string] existing;
Copy link
Member

Choose a reason for hiding this comment

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

A next iteration could cache this as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

True, I was a bit afraid to forget a place where it could get out of sync, because the arrays are public fields, so I wanted to leave that to a more in-depth optimization attempt.

@s-ludwig
Copy link
Member Author

Okay, since this is just a local change, I'm also pretty confident that it should be without mistakes. I'll rebase and merge after Travis passes, unless any issues come up.

This is still not very efficient, but now has a finite runtime for large numbers of files.
@s-ludwig s-ludwig force-pushed the issue1690_slow_file_collection branch from cbe25ea to a2e8923 Compare February 23, 2017 13:18
@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 61.635% when pulling a2e8923 on issue1690_slow_file_collection into 53f415c on master.

@s-ludwig s-ludwig merged commit 08469ba into master Feb 23, 2017
@s-ludwig s-ludwig deleted the issue1690_slow_file_collection branch March 10, 2017 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants