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

git: Only reset Git index if non-empty #3748

Merged
merged 1 commit into from
May 27, 2024

Conversation

mlcui-corp
Copy link
Collaborator

In chromium/src.git, this gives an approximate ~0.83s speedup for commands if the Git index is empty, and a ~0.14s slowdown if the Git index is non-empty.

As most users using jj will likely be using jj to write to a colocated repo - and therefore avoid modifying the Git index - this should be a general speedup for most colocated checkouts.

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

Copy link
Collaborator

@yuja yuja left a comment

Choose a reason for hiding this comment

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

Out of curiosity, do you see any performance improvement in the gix version?
https://github.com/yuja/jj/tree/push-vpsmwmxpokyx

I haven't pushed the gix version because it adds more code, needs cleanup, and lacks testing. If it's faster than the current libgit2 impl, maybe I should restart the rewrite.

lib/src/git.rs Outdated Show resolved Hide resolved
In chromium/src.git, this gives an approximate ~0.83s speedup for
commands if the Git index is empty, and a ~0.14s slowdown if the Git
index is non-empty.

As most users using jj will likely be using jj to write to a colocated
repo - and therefore avoid modifying the Git index - this should be a
general speedup for most colocated checkouts.
@mlcui-corp
Copy link
Collaborator Author

mlcui-corp commented May 27, 2024

@yuja The gix implementation is noticeably faster! With this PR, it takes around ~3.0s to do a jj abandon when @ is empty. With your gix implementation, it takes ~2.4s.

However, it does cause weird things with the state of the git repo - a git status afterwards causes a long "Refresh index" operation (~80s, then ~36s, then ~36s) to occur, so I don't think it's feasible to use as-is. Do you know why that happens?

I also noticed that my p10k git prompt shows a very large number of modified files when it is in this weird state, which is fixed after a (slow) git status. This is reproducible on martinvonz/jj as well:

~/git/jj @e6219a87 !4 ❯ jj new u
Working copy now at: ztvr 036e (empty) (no description set)
Parent commit      : uwxu 9e62 push-uwxuluvskttx | git: Only reset Git index if non-empty
Added 0 files, modified 4 files, removed 0 files

~/git/jj @9e62cb1b !361 ❯ git status
HEAD detached from e6219a87
nothing to commit, working tree clean

~/git/jj @9e62cb1b ❯

Copy link
Collaborator

@yuja yuja left a comment

Choose a reason for hiding this comment

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

The gix implementation is noticeably faster! With this PR, it takes around ~3.0s to do a jj abandon when @ is empty. With your gix implementation, it takes ~2.4s.

However, it does cause weird things with the state of the git repo - a git status afterwards causes a long "Refresh index" operation (~80s, then ~36s, then ~36s) to occur,

Perhaps, that's because my version doesn't preserve mtime/ctime stored in the index. I haven't noticed it because I don't use git for working-copy management.

Thanks for testing. I think this PR is good workaround. We'll need some diffing (to copy back mtime/ctime) anyway if we switch to gix.

@mlcui-corp mlcui-corp merged commit a075a5c into martinvonz:main May 27, 2024
16 checks passed
mlcui-corp added a commit to mlcui-corp/jj that referenced this pull request May 27, 2024
This accounts for commit message-only changes which do not change the
tree ID, as well as checkouts between commits with identical tree
contents.

See martinvonz#3748 for previous work on avoiding resetting HEAD when the
*commits* are identical.
mlcui-corp added a commit to mlcui-corp/jj that referenced this pull request May 28, 2024
This accounts for commit message-only changes which do not change the
tree ID, as well as checkouts between commits with identical tree
contents.

See martinvonz#3748 for previous work on avoiding resetting HEAD when the
*commits* are identical.
mlcui-corp added a commit to mlcui-corp/jj that referenced this pull request May 29, 2024
This accounts for commit message-only changes which do not change the
tree ID, as well as checkouts between commits with identical tree
contents.

See martinvonz#3748 for previous work on avoiding resetting HEAD when the
*commits* are identical.
mlcui-corp added a commit to mlcui-corp/jj that referenced this pull request May 29, 2024
This accounts for commit message-only changes which do not change the
tree ID, as well as checkouts between commits with identical tree
contents.

See martinvonz#3748 for previous work on avoiding resetting HEAD when the
*commits* are identical.
mlcui-corp added a commit that referenced this pull request May 29, 2024
This accounts for commit message-only changes which do not change the
tree ID, as well as checkouts between commits with identical tree
contents.

See #3748 for previous work on avoiding resetting HEAD when the
*commits* are identical.
@mlcui-corp mlcui-corp deleted the push-uwxuluvskttx branch July 1, 2024 03:18
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.

2 participants