-
Notifications
You must be signed in to change notification settings - Fork 273
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
feat(vcs): add new git repo scanning method to improve resolution speed #4642
Conversation
@edvald is this ready for review? There are a few test failures. |
I need to look at those, pushed just last night after working on this on my SF flight. I'd say it's ready for preliminary review maybe. |
72e8734
to
a8f5255
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! This was a fun read : )
I don't have any further comments on the code as things stand, but I've yet to do manual testing on some larger repos to examine performance more closely. Will do so early this week.
4eb4a68
to
1b7bdc6
Compare
This adds a new experimental git repo scanning method, enabled by setting `scan.git.mode: repo` in the project config or by setting `GARDEN_GIT_SCAN_MODE=repo`. It can still do with quite a bit of optimization but the general idea is to do far fewer (albeit generally more expensive) calls to git. For now I don't think it makes sense to make this the default since there are pros and cons to either approach, but for large repositories with many actions/modules this is likely to be more efficient. I suggest we collect user feedback and do IRL tests before diving in and optimizing further. One obvious optimization will be to index the repository files more efficiently, instead of just filtering on a straight list of paths. It would also make sense to delay hashing files that aren't in the index until they are called upon.
I've rebased the PR. @thsig can we get this merged please? It does include performance optimizations even for the default scanning method. We can always iterate on the newer experimental version. |
Yeah, sounds good. I've just approved it. |
This adds a new experimental git repo scanning method, enabled by setting
scan.git.mode: repo
in the project config or by settingGARDEN_GIT_SCAN_MODE=repo
.It can still do with quite a bit of optimization but the general idea is to
do far fewer (albeit generally more expensive) calls to git.
For now I don't think it makes sense to make this the default since there are
pros and cons to either approach, but for large repositories with many
actions/modules this is likely to be more efficient.
I suggest we collect user feedback and do IRL tests before diving in and
optimizing further. One obvious optimization will be to index the repository
files more efficiently, instead of just filtering on a straight list of paths.
It would also make sense to delay hashing files that aren't in the index until
they are called upon.
This is the first step to resolve #3452