git: use dulwich for .gitignore #4809
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In my tests `Git._ignore` is taking around ~65seconds for the whole test suit(~50K invocations). The reason is that `gitpython` is launching a real `git check-ignore` process, that takes time and resources. Switching to `pygit2` or `dulwich` makes it drop to ~5seconds, saving us almost a minute of the test run time and a lot of resources by not launching any processes. I've chosen `dulwich` over `pygit2` here because it is much more pythonic and as long as the performance is on-par with `pygit2` it is better for us to use `dulwich`.
efiop
added
testing
efiop
commented
Oct 29, 2020
| @@ -50,6 +50,7 @@ def run(self): | |||
| "colorama>=0.3.9", | |||
| "configobj>=5.0.6", | |||
| "gitpython>3", | |||
| "dulwich>=0.20.6", | |||
There was a problem hiding this comment.
The only problem is that both dulwich and pygit don't have wheels for 3.9, but that is solvable.
efiop
commented
Oct 29, 2020
| return True | ||
| except GitCommandError: | ||
| return False | ||
| repo = Repo(self.root_dir) |
There was a problem hiding this comment.
Wasteful to have a repo instance created each time here, but doing so for drop-in simplicity. Likely dulwich will replace everything except Git.clone() in the near future and so Repo will move to __init__.
efiop
changed the title
|
Indeed, looks like the tests on windows are more stable now. Will be continuing the migration for other methods in the further PRs. EDIT: still flaky https://github.com/iterative/dvc/runs/1329409665?check_suite_focus=true |
victorskl
added a commit
to umccr/umccrise
that referenced
this pull request
Nov 26, 2020
* Latest dvc 1.10 re-use dulwich for .gitignore iterative/dvc#4809 * Regression: reintroduce issue reported in iterative/dvc#2010 * Resolution: pin dvc version to 1.9.1 for now * Build image is docker:dind Alpine Linux
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In my tests
Git._ignoreis taking around ~65seconds for the whole testsuit(~50K invocations). The reason is that
gitpythonis launching a realgit check-ignoreprocess, that takes time and resources.Switching to
pygit2ordulwichmakes it drop to ~5seconds, saving usalmost a minute of the test run time and a lot of resources by not
launching any processes.
I've chosen
dulwichoverpygit2here because it is much morepythonic and the performance is on-par with
pygit2.Related to #2215
❗ I have followed the Contributing to DVC checklist.
📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here.
Thank you for the contribution - we'll try to review it as soon as possible. 🙏