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 TaggedCommitVersionStrategy performance problems #2800

Merged
merged 3 commits into from
Aug 10, 2021

Conversation

ron-macneil-youi
Copy link
Contributor

@ron-macneil-youi ron-macneil-youi commented Aug 7, 2021

The problem was a branch commits x version tags nested loop that quickly
grew into the millions of iterations as the number of commits and/or
tags in a repo increased, even modestly.

Each iteration performed Git tag-to-commit resolution and GitVersion
commit object-to-object comparison, both of which take a little bit of
time and aren't particularly slow in isolation but which add up fast
when performed millions of times.

This is solved by doing a single pass over the tags, resolving each to a
commit only once; then using that to build a commit-to-tag lookup
(hashtable) keyed by commit SHA, eliminating the object comparisons; and
finally doing a single pass over the branch commits, looking up
associated tags from the hashtable by SHA, eliminating the nested loop.

Testing on a repo with ~20,000 commits and ~5,000 tags showed an overall
GitVersion run time reduction from over 2 minutes down to around 5
seconds.

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

The problem was a branch commits x version tags nested loop that quickly
grew into the millions of iterations as the number of commits and/or
tags in a repo increased, even modestly.

Each iteration performed Git tag-to-commit resolution and GitVersion
commit object-to-object comparison, both of which take a little bit of
time and aren't particularly slow in isolation but which add up fast
when performed millions of times.

This is solved by doing a single pass over the tags, resolving each to a
commit only once; then using that to build a commit-to-tag lookup
(hashtable) keyed by commit SHA, eliminating the object comparisons; and
finally doing a single pass over the branch commits, looking up
associated tags from the hashtable by SHA, eliminating the nested loop.

Testing on a repo with ~20,000 commits and ~5,000 tags showed an overall
GitVersion run time reduction from over 2 minutes down to around 5
seconds.
Copy link
Member

@asbjornu asbjornu 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 great!

…TaggedCommitVersionStrategy.cs

Co-authored-by: Asbjørn Ulsberg <[email protected]>
@asbjornu asbjornu enabled auto-merge August 9, 2021 23:22
@asbjornu
Copy link
Member

asbjornu commented Aug 9, 2021

Can you please run dotnet format ./ --folder --exclude **/AddFormats/, commit the changes and push?

@ron-macneil-youi
Copy link
Contributor Author

ron-macneil-youi commented Aug 10, 2021

Hi @asbjornu I ran the dotnet format command you specified and it hasn't changed anything. Were you expecting a particular change?

Sorry, my mistake: I hadn't pulled the PR feedback change. dotnet format has corrected a line ending. Done.

auto-merge was automatically disabled August 10, 2021 03:21

Head branch was pushed to by a user without write access

@asbjornu asbjornu added this to the 5.x milestone Aug 10, 2021
@asbjornu asbjornu merged commit f9b510d into GitTools:main Aug 10, 2021
@mergify
Copy link
Contributor

mergify bot commented Aug 10, 2021

Thank you @ron-macneil-youi for your contribution!

@arturcic arturcic modified the milestones: 5.x, 5.6.12 Aug 14, 2021
@arturcic
Copy link
Member

🎉 This issue has been resolved in version 5.7.0 🎉
The release is available on:

Your GitReleaseManager bot 📦🚀

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