Reserve textClusterColumns vector for performance #5645
Conversation
|
@msftbot make sure @miniksa signs off on this |
ghost
added
the
AutoMerge
|
Hello @zadjii-msft! Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:
If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you". |
zadjii-msft
added
Area-Performance
|
About the "There is a circular dependency in the target dependency graph involving target "ComputeGetResolvedWinMD". I've actually seen this in my local PC. Rebuilding fixes this. I don't really get it why it happens this often on the CI environment. |
Yea, I forwarded you a mail we've got open with the MsBuild team on this. They're still trying to figure it out. |
|
HOLD THE FLIPPING PHONE @skyline75489. How did you get the perf numbers per line to show up like that inside Visual Studio?! |
There was a problem hiding this comment.
Works for me.
If you're looking for one more follow on, I think we might be a little too aggressive at shrinking the vectors back down when the run lengths get smaller. I see that a lot with things like cacafire where it seems we spend more time expanding/shrinking the vector than the memory savings is worth.
ghost
merged commit
|
🎉 Handy links: |
|
🎉 Once again, thanks for the contribution! This pull request was included in a set of conhost changes that was just |
Summary of the Pull Request
In tonight's episode of "Can we be even faster?", we will... you know what, just take a look at the code.
References
PR Checklist
Detailed Description of the Pull Request / Additional comments
It is actually a quite common technique seen inside the codebase to first reserve the spaces before pushing something into vectors. I don't know why it is not used here.
Before:
After:
Validation Steps Performed