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

Speed improvement for large projects & path fixup for sln/vxproj inputs #4

Merged
merged 3 commits into from
Nov 15, 2016
Merged

Speed improvement for large projects & path fixup for sln/vxproj inputs #4

merged 3 commits into from
Nov 15, 2016

Conversation

peitschie
Copy link
Contributor

Was playing with this last week (before your email even! How coincidental!) and noticed these few small tweaks were required to get my current project going with your new and very cool vxproj integration.

A minor suggestion if you're interested, there's a lot of trailing whitespace in these files. If you're using Visual Studio, it might be worth installing https://visualstudiogallery.msdn.microsoft.com/a204e29b-1778-4dae-affd-209bea658a59 which can automatically clean this up for you on save.

@g-h-c
Copy link
Owner

g-h-c commented Nov 15, 2016

Thanks for your pull request. Why to use a set rather than vector? We do not perform any searches on sysincludedirs, why does using a set make a difference?

@g-h-c g-h-c closed this Nov 15, 2016
@g-h-c g-h-c reopened this Nov 15, 2016
@g-h-c g-h-c closed this Nov 15, 2016
@g-h-c g-h-c reopened this Nov 15, 2016
@g-h-c g-h-c merged commit 5753900 into g-h-c:master Nov 15, 2016
@peitschie
Copy link
Contributor Author

We do not perform any searches on sysincludedirs, why does using a set make a difference?

For some reason (perhaps I should dig more into why), this vector was growing very large with a lot of duplicate entries (e.g., 32,000+ system include directories!). Changing it to a set kept the performance high, purely because it avoided dupes.

So, the optimization wasn't for searching, but avoiding unnecessary copies.

@peitschie peitschie deleted the minor-fixes branch November 15, 2016 22:57
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