perf: Improve height calculation performance #99
Conversation
Changed algorithm to compare commit with previous one, instead of always comparing with HEAD. This way complexity is dropping from O(n^2) to O(n), as each change is processed only once.
|
The build is currently failing due to this issue in nuget: NuGet/Home#8692 I'll try to work around... |
It takes time and we don't actually need it.
|
@Kieranties Ok. Just out of curiosity profiled app and found that actually the reason is rename detection. It hits previous code more because change set is larger all the time. Once I disable rename detection, both code variances perform at equivalent speed. I pushed change to make code even faster (2 secs vs 4 secs). If you want to get back to old code when you always use tip - it's also fine, as it will not degrade performance noticeably. But I still somehow feel that we will do extra work in that case... 🤔 |
|
@Kieranties Added one more improvement, |
|
@zvirja I've fixed up the build specifically targeting netcore 3.1 locally and things work - however the azure VMs don't have LTS 3.1 installed yet. It seems they've identified the fix for the issue over at NuGet/Home#8692 - hopefully we'll be able to pick that up soon. Once it's ready I can confirm with #100 (as it has some housekeeping anyway) then bring in this. |
|
Thanks for the update. Even if they make the fix soon, it will take time to get it merged to .NET Core SDK, published, pushed to VMs. Am worried that it could take at least a month 😐 Do you see a way it might be speeded up? Is the issue you hit intermittent or stable? Could it just be worked around by running it again? 🤨 |
|
Agreed, I'll look into bringing in the sdk via their install script first -
https://docs.microsoft.com/en-us/dotnet/core/tools/dotnet-install-script
Failing that, I'll need to remove the use of centralpackageversions in the
short term
…On Tue, 17 Dec 2019 at 22:34, Alex Povar ***@***.***> wrote:
Thanks for the update.
Even if they make the fix soon, it will take time to get it merged to .NET
Core SDK, published, pushed to VMs. Am worried that it could take at least
a month 😐
Do you have a way it might be speeded up? Is the issue you hit
intermittent or stable? Could it just be worked around by running it again?
🤨
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#99?email_source=notifications&email_token=AACVZP4FZULUXLVZRWC32VLQZFHYJA5CNFSM4J3MT5MKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHEGALY#issuecomment-566779951>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACVZP23YS52LFG7LNWDU5TQZFHYJANCNFSM4J3MT5MA>
.
|
Kieranties
changed the title
|
And we're good! I've updated the build to enforce the install of LTS (3.1.100 at this point in time). |
|
Thank you for all the time you spent on this and for quick response time - it was a pleasure to contribute See you around! |
|
Shipped - https://github.com/Kieranties/SimpleVersion/releases/tag/v0.2.0-alpha1.32 (or via nuget) |
|
Perfect! 🤝 Do you know whether some actions are required to consume that in Brew/Cake? Does it happen automatically when having a proper Brew import? |
|
I believe the brew builds are bringing in the latest pre-release version by default - If not, you should be able to specify in your packages.config for cake to overrride. |
Description
Changed algorithm to compare commit with previous one,
instead of always comparing with HEAD. This way complexity is dropping from O(n^2) to O(n), as each change is processed only once.
I've also changed
while()loop withforeach, as:foreachdoes the same and looks more naturalLet me know if you want me to revert that change.
Change Type
Bug Fix: Fixes an issue in implementation
Quality