Skip to content

Performance fix #696

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

Merged
merged 3 commits into from
Oct 19, 2015
Merged

Performance fix #696

merged 3 commits into from
Oct 19, 2015

Conversation

JakeGinnivan
Copy link
Contributor

// @asbjornu @orjan can you guys test this with commit-message-incrementing: Enabled. Hopefully it should fix the perf issues we talked about in #677

@JakeGinnivan
Copy link
Contributor Author

Crap, there are not tests around the commit message stuff. I broke it. Need to fix.

@JakeGinnivan
Copy link
Contributor Author

Ok, should be fixed. If either of you can test with one of your repos which caused the perf issues that would be awesome.

@orjan
Copy link
Contributor

orjan commented Oct 17, 2015

This weekend is pretty stuffed for me but will give it a go at Monday the latest!

On 17 Oct 2015, at 10:29, Jake Ginnivan [email protected] wrote:

Ok, should be fixed. If either of you can test with one of your repos which caused the perf issues that would be awesome.


Reply to this email directly or view it on GitHub.

@orjan
Copy link
Contributor

orjan commented Oct 18, 2015

It's looking good! From 13s to 10s running latest master against this pull request, Release mode, see snapshot diff below:
image

Until = baseCommit,
SortBy = CommitSortStrategies.Topological | CommitSortStrategies.Reverse
};
if (commit.Sha == baseCommit.Sha)
Copy link
Contributor

Choose a reason for hiding this comment

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

@JakeGinnivan why are we comparing the Sha instead of the commit? We're using Sha for comparison over the whole code base so I'll guess it's the way to go, but I'm just curious about why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No reason, I assume libgit2sharp does implement equal and equality operators properly but I haven't checked.

JakeGinnivan added a commit that referenced this pull request Oct 19, 2015
@JakeGinnivan JakeGinnivan merged commit 9c24953 into GitTools:master Oct 19, 2015
@JakeGinnivan JakeGinnivan deleted the PerformanceFix branch October 19, 2015 00:54
@asbjornu
Copy link
Member

@JakeGinnivan Sorry, I haven't had time to test this until now. But yea, this is a major improvement.

Before:

2015-10-19 08:14:13 INFO [10/19/15 8:14:13:18] End: Calculating base versions (Took: 24,395.41ms)

After:

2015-10-19 08:15:24 INFO [10/19/15 8:15:24:99] End: Calculating base versions (Took: 5,154.25ms)

@JakeGinnivan
Copy link
Contributor Author

Still would be good to make better, but that is a pretty good start.

From: Asbjørn Ulsberg [mailto:[email protected]]
Sent: Monday, 19 October 2015 2:17 PM
To: GitTools/GitVersion [email protected]
Cc: Jake Ginnivan [email protected]
Subject: Re: [GitVersion] Performance fix (#696)

@JakeGinnivanhttps://github.com/JakeGinnivan Sorry, I haven't had time to test this until now. But yea, this is a major improvement.

Before:

2015-10-19 08:14:13 INFO [10/19/15 8:14:13:18] End: Calculating base versions (Took: 24,395.41ms)

After:

2015-10-19 08:15:24 INFO [10/19/15 8:15:24:99] End: Calculating base versions (Took: 5,154.25ms)


Reply to this email directly or view it on GitHubhttps://github.com//pull/696#issuecomment-149112234.

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.

3 participants