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 sorting of commits in diff view #1747

Merged
merged 8 commits into from
Feb 12, 2024

Conversation

Joshix-1
Copy link
Contributor

@Joshix-1 Joshix-1 commented Jul 6, 2023

Commits were sorted by time in diff view. That's wrong as some people like to commit in the future or past.
To see the fix just compare my commit to one before by selecting both and pressing SHIFT+C.

This Pull Request fixes/closes #{issue_num}.

It changes the following:

  • diff view is correct now

I followed the checklist:

  • ( ) I added unittests
  • I ran make check without errors
  • I tested the overall application
  • I tested my changes
  • I added an appropriate item to the changelog

@Joshix-1 Joshix-1 marked this pull request as ready for review August 20, 2023 18:42
@Joshix-1
Copy link
Contributor Author

don't know how to write tests for my changes

@Joshix-1
Copy link
Contributor Author

@extrawurst this is ready for review

@kleenkanteen
Copy link

4 long months, he waits...

@extrawurst
Copy link
Owner

@Joshix-1 the problem with this change is that it is right in the hotpath for giant repos like linux. can you benchmark the differences with and without your change?

@Joshix-1
Copy link
Contributor Author

Joshix-1 commented Jan 8, 2024

How and what can I benchmark?

Shouldn't it improve performance as commits aren't unneccessarily sorted by date in most cases?

@extrawurst
Copy link
Owner

check the log for this line (after the commits are all collected in the log tab):

13:47:30 [TRACE] (4) scopetime: [scopetime/src/lib.rs:41] scopetime: 2360 ms [asyncgit::revlog::async::revlog] @asyncgit/src/revlog.rs:176 (after opening kubernetes repo)

13:57:42 [TRACE] (4) scopetime: [scopetime/src/lib.rs:41] scopetime: 26058 ms [asyncgit::revlog::async::revlog] @asyncgit/src/revlog.rs:176 (after opening linux repo)

@extrawurst
Copy link
Owner

and please update to current master to get ready to merge

@Joshix-1
Copy link
Contributor Author

my gitui is in ~/code/gitui i built it with cargo build --features=timing --release
I freshly cloned both repos at their current master, without --filter

Linux:

this pr:

~/code/linux master $ ~/code/gitui/target/release//gitui -l ; grep asyncgit::revlog::async::revlog ~/.cache/gitui/gitui.log
Logging enabled. log written to: "/home/josh/.cache/gitui/gitui.log"
17:39:51 [TRACE] (3) scopetime: [scopetime/src/lib.rs:41] scopetime: 35821 ms [asyncgit::revlog::async::revlog] @asyncgit/src/revlog.rs:176
~/code/linux master $ rm ~/.cache/gitui/gitui.log && ~/code/gitui/target/release//gitui -l && grep asyncgit::revlog::async::revlog ~/.cache/gitui/gitui.log
Logging enabled. log written to: "/home/josh/.cache/gitui/gitui.log"
17:41:54 [TRACE] (4) scopetime: [scopetime/src/lib.rs:41] scopetime: 37788 ms [asyncgit::revlog::async::revlog] @asyncgit/src/revlog.rs:176

current master

~/code/linux master $ rm ~/.cache/gitui/gitui.log && ~/code/gitui/target/release//gitui -l && grep asyncgit::revlog::async::revlog ~/.cache/gitui/gitui.log
Logging enabled. log written to: "/home/josh/.cache/gitui/gitui.log"
17:55:57 [TRACE] (4) scopetime: [scopetime/src/lib.rs:41] scopetime: 41685 ms [asyncgit::revlog::async::revlog] @asyncgit/src/revlog.rs:142
~/code/linux master $ rm ~/.cache/gitui/gitui.log && ~/code/gitui/target/release//gitui -l && grep asyncgit::revlog::async::revlog ~/.cache/gitui/gitui.log
Logging enabled. log written to: "/home/josh/.cache/gitui/gitui.log"
17:56:54 [TRACE] (4) scopetime: [scopetime/src/lib.rs:41] scopetime: 43782 ms [asyncgit::revlog::async::revlog] @asyncgit/src/revlog.rs:142

Kubernetes

this pr

~/code/kubernetes master $ rm ~/.cache/gitui/gitui.log && ~/code/gitui/target/release//gitui -l && grep asyncgit::revlog::async::revlog ~/.cache/gitui/gitui.log
Logging enabled. log written to: "/home/josh/.cache/gitui/gitui.log"
17:47:54 [TRACE] (4) scopetime: [scopetime/src/lib.rs:41] scopetime: 2382 ms [asyncgit::revlog::async::revlog] @asyncgit/src/revlog.rs:176
~/code/kubernetes master $ rm ~/.cache/gitui/gitui.log && ~/code/gitui/target/release//gitui -l && grep asyncgit::revlog::async::revlog ~/.cache/gitui/gitui.log
Logging enabled. log written to: "/home/josh/.cache/gitui/gitui.log"
17:48:38 [TRACE] (3) scopetime: [scopetime/src/lib.rs:41] scopetime: 2364 ms [asyncgit::revlog::async::revlog] @asyncgit/src/revlog.rs:176

current master

~/code/kubernetes master $ rm ~/.cache/gitui/gitui.log && ~/code/gitui/target/release//gitui -l && grep asyncgit::revlog::async::revlog ~/.cache/gitui/gitui.log
Logging enabled. log written to: "/home/josh/.cache/gitui/gitui.log"
17:53:30 [TRACE] (4) scopetime: [scopetime/src/lib.rs:41] scopetime: 2640 ms [asyncgit::revlog::async::revlog] @asyncgit/src/revlog.rs:142
~/code/kubernetes master $ rm ~/.cache/gitui/gitui.log && ~/code/gitui/target/release//gitui -l && grep asyncgit::revlog::async::revlog ~/.cache/gitui/gitui.log
Logging enabled. log written to: "/home/josh/.cache/gitui/gitui.log"
17:53:38 [TRACE] (4) scopetime: [scopetime/src/lib.rs:41] scopetime: 2378 ms [asyncgit::revlog::async::revlog] @asyncgit/src/revlog.rs:142

@extrawurst
Copy link
Owner

@Joshix-1 thanks, lgtm!

@extrawurst extrawurst merged commit 7335cd1 into extrawurst:master Feb 12, 2024
19 checks passed
@extrawurst extrawurst added this to the v0.25 milestone Feb 12, 2024
IndianBoy42 pushed a commit to IndianBoy42/gitui that referenced this pull request Jun 4, 2024
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.

None yet

3 participants