Sped up the Normalization by removing an unnecessary O(n^2) loop

[ni-fgenois]

This should speedup the normalization when there are a lot of branches.
The .Any(x => x.Name.Equals(referenceName) was causing an unnecessary O(n^2) loop, as Refs is already an indexed structure (Dict).

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.

[arturcic]

I think this line is needed - we need to parse the localCanonicalName

[ni-fgenois]

Are you sure? Is the ReferenceName.Parse doing any validation? Because we are not using referenceName anywhere

[arturcic]

Yes it does

GitVersion/src/GitVersion.Core/Git/ReferenceName.cs

Line 29 in 3626ce5

public static ReferenceName Parse(string canonicalName)

[ni-fgenois]

The following validation snippet from ReferenceName.Parse : IsPrefixedBy(canonicalName, LocalBranchPrefix)
will match the constructed string "refs/heads/" + branchName;

What do you think about changing the construction of the string to ReferenceName.LocalBranchPrefix + branchName; instead?

[ni-fgenois]

Or adding a factory method ReferenceName.LocalBranch(branchName)?

[asbjornu]

The point is to get an instance of the class ReferenceName, which implements IComparabale and thus takes care of all reference comparison logic, such as disregarding refs/heads. Why do we only want to compare against only the local canonical name here?

[ni-fgenois]

@asbjornu
Well, the problem here is actually using the IComparable interface for searching a matching IReference. Doing such a linear lookup leads to O(n) algorithmic complexity, and as we are already within a loop, this leads to a total O(n^2) algorithm complexity.

We should instead use an data structure with logarithmic lookups directly in order to take this first algorithmic complexity down to a O(1), and the whole tjomg to an O(n) algorithmic complexity.

Lucky for us, the IReferenceCollection class does provide such an access (IReference? this[string name] { get; }), but the key of that dictionnary seems to be a CanonicalName.

[ni-fgenois]

@arturcic I've modified my code so we use a ReferenceName to query the IReferenceCollection structure. And I've added a factory method to ReferenceName

[asbjornu]

I think this now looks like a decent improvement. Thoughts @arturcic?

[arturcic]

I think this now looks like a decent improvement. Thoughts @arturcic?

Agree. It will need a rebase on top of main and it should be good to go

[ni-fgenois]

Thanks to you two! I've just rebased my changes :)

[mergify]

Thank you @ni-fgenois for your contribution!

[ni-fgenois]

@arturcic @asbjornu what is the process to get a new release?

[arturcic]

We don't have an exact list of features we want to include in a release, but we try to release every 2 to 3 months if there are enough items fixed/added

[ni-fgenois]

Ok! Is there one planned soon? (as the last one was Nov 15th)

[ni-fgenois]

Also, is there anything I can do to help?

[arturcic]

I think we can have a release now.

[github-actions]

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

• GitHub Release
• Chocolatey Package
• .Net Global Tool

Your GitReleaseManager bot 📦🚀