-
Notifications
You must be signed in to change notification settings - Fork 385
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
perf: load existing BUILD files concurrently #1928
base: master
Are you sure you want to change the base?
Conversation
a69754a
to
66d4849
Compare
@@ -363,6 +363,9 @@ func resolveFileInfo(wc *walkConfig, dir, rel string, ent fs.DirEntry) fs.DirEnt | |||
type pathTrie struct { | |||
children map[string]*pathTrie | |||
entry *fs.DirEntry | |||
|
|||
buildFileErr error |
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.
This could probably be done different or better, but persisting this error
here keeps the scope of this PR minimal. We can think about changing it more in the future.
@@ -167,7 +167,7 @@ func TestCustomBuildName(t *testing.T) { | |||
} | |||
}) | |||
want := []string{ | |||
"sub/BUILD.test", | |||
"sub/BUILD.bazel", |
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.
Why would this happen? 🤔
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.
Oh, because ValidBuildFileNames
can be changed with # gazelle:build_file_name
in addition to via the -build_file_name
cli arg 😢
Maybe that means this change is not possible?
66d4849
to
c5def77
Compare
Now that the fs is walked concurrently we can also load BUILDs concurrently at the same time, that is primarily the
rule.LoadFile
method.If
ReadBuildFilesDir
is set then this may have a more significant performance gain because the extraos.ReadDir
call will also be run concurrently now.What type of PR is this?
What package or component does this PR mostly affect?
What does this PR do? Why is it needed?
Performance