Improve performance of showDivergenceFromBaseBranch#5536
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Duplication | 2 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
858b514 to
10c9daa
Compare
stefanhaller
left a comment
There was a problem hiding this comment.
Nice work, this is a welcome improvement (although on my machine the difference isn't nearly as big; 700ms before vs. 300ms after).
I pushed a whole bunch of fixup commits for minor style things; they all have commit messages, please have a look.
The assets branch is an interesting case: it has no common history with master, so it's a bit questionable to show a behind value that is the entire length of master. On the other hand, showing a behind of 0 like we did before makes it look like it was up to date with master, which is also misleading. So maybe the new behavior is a little bit better, but I think the scenario is uncommon enough that it probably doesn't matter much.
That's a good point, the only better indicator I can think of is putting something like NA when that happens. Though as you say it is a pretty specific edge case. My guess is the performance difference is due to Windows being slower and getting slower with each version 😂 |
|
Changes done! though I was not able to replicate the issue I had initially found, it was likely caused by manipulations I had done to the original code when prototyping. |
stefanhaller
left a comment
There was a problem hiding this comment.
like continue in a loop
I was actually torn on that one, and it's a bit of a matter of taste in this particular case. Many go devs follow the rule that the happy path should always have the least indentation, and my fixup violates that. (Shrug)
Anyway, looks great now, thanks.
| branchByRef := lo.KeyBy(branches, func(b *models.Branch) string { | ||
| return b.FullRefName() | ||
| }) | ||
| branchByRef := lo.KeyBy(branches, (*models.Branch).FullRefName) |
There was a problem hiding this comment.
Oops, thanks for fixing this up. 😊
| // Branches not in parse are default to 0 | ||
| for _, branch := range branchByRef { | ||
| branch.BehindBaseBranch.Store(0) | ||
| } |
There was a problem hiding this comment.
Nice, this turned out easier than I thought.
Greatly improves performance for showDivergenceFromBaseBranch config Fixes jesseduffield#5532
556309e to
3d14881
Compare
PR Description
Instead of manually figuring out how much ahead each branch is from it's upstream branch, use for-each-ref ahead-behind if supported by the git binary.
On my Windows machine, using the lazygit repo with all 61 branches created locally, it goes from ~8.5s to ~200ms.
Before:
After:
It was also able to find a base branch for
assetswhich the previous iteration did not.Fixes #5532
Please check if the PR fulfills these requirements
go generate ./...)