-
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
Proposal: Lazy indexing for faster subdirectory updates #1891
Comments
Cc @linzhp @tyler-french for Über's "conventions" approach |
First thought... can we avoid any type of cli flag to opt in/out of this? A cli flag means everywhere gazelle is invoked needs to fully understand which gazelle languages are being used and if they need indexing, if lazy indexing is enough etc. The first idea that comes to mind would be doing it based on a language interface and gazelle can decide what to do based on which languages are known, which are enabled, maybe how they are configured. Just adding EDIT: I guess this |
With the current |
I guess my Maybe both are necessary? The use case I keep thinking of is npm packages, and maybe pip is similar? I think maven is also similar, maybe go mod can be implemented the same way as well. We need to parse a lockfile, which might be in a subdirectory, and then that data can be used by a resolver to resolve package names. The package resolver can use |
High level thought: I'm basically in favor of this proposal. It could speed up Gazelle dramatically in large repos while not being too "magical" or requiring a lot of configuration from end users. I haven't yet thought about the details though, and I do think we need to consider what configuration will look like for a few languages.
The current default is for Gazelle to index everything ( Some historical context about indexing, though enough time has passed that my memory is a little muddled: I added indexing to deal with GOPATH and vendoring. At the time, people could put a |
To give a bit of an update on this -- I've been using this for about 4 months to roll out adoption of python gazelle and we see runtimes of ~0.7-1.5s when running in a subdirectory with fewer dependencies on other packages. It has been working well enough that we were willing to maintain our own patch on gazelle to support it. (We wrap the python lang extension and use A global "use the index or not" configuration isn't what we needed. We still don't index to generate golang targets because the default conventions are strong enough and we historically avoided indexing because of the time penalty in our monorepo. To support lazy indexing for python and no indexing for go, I wrapped the golang language extension. We set the config struct's Assuming lazy becomes the default mode, I think we have options. There could be a Alternatively, if the |
I mentioned this in the PR... but doesn't the API need the ability to index directories that are not subdirectories? What if imports could be relative such as |
I wonder if we could do something more general: like
I think all paths in |
Problem
Currently, gazelle operates in indexing or non-indexing mode. When indexing is enabled, gazelle will walk the entire repo, using the
Imports
method of language extensions to build up a rule index. For large repos, this can lead to a substantial slowdown compared to operating in a non-indexing mode when trying to rungazelle
in a subdirectory (~10s -> 0.8s in my case).The time penalty from indexing is so great that there are ongoing discussions and work to allow gazelle to save and load its index to the disk - #1181
Not all languages are able to operate without indexing due to ambiguities in mapping source code import statements to the bazel target labels that provide those imports. For example, it is not possible to determine python target labels from import statements. As an example, how can you map the following imports to bazel target labels without an index?
Even though you cannot explicitly determine the target labels for each of those imports, you can reasonably guess which packages they would be in (
common
,common/math
,common/math/foo
). Because of common conventions for directory structure matching import path structure, we don't need to index the entire repository to resolve these imports. We only need to index the packages that we think the targets would be in.Proposal
I would like to add a new
-lazy_index
flag to gazelle. When this flag is enabled (with-index=true
), gazelle will not visit all directories in the repo. Instead, it will only visit the directories to update. TheGenerateResult
struct would gain aPkgsToIndex
field that language extensions should populate. Once gazelle is finished generating rules for a given subdir, it would look at all of thePkgsToIndex
, then index them prior to finalizing the ruleIndex and calling language extensionResolve
methods.For our above python code example, this leads to a result like,
Notably, it is okay for rules to suggest indexing packages that don't exist. We can have gazelle ignore directories that don't exist in the workspace when an extension asks to index them.
Benchmarking
I hacked together an implementation of lazy indexing with rules_python. In a repository with ~25k directories, it reduced the runtime of running gazelle in a single subdirectory from
5-10s
to~1s
(depending on the directory and how much indexing was avoidable).The gazelle changes can be seen in #1892.
The rules_python changes can be seen in alex-torok/rules_python#1
Questions / Challenges
PkgsToIndex
field to theGenerateResult
sound good, or should this be an entirely separate method?GenerateResult
, as the language extension should have all of the context it needs to make the recommendation to gazelle. Alternatively, there could be a new interface that extensions implement where we pass in the GenerateResult and have it return the packages to index.This solution does pose problems for repos that don't have a package directory structure that matches the language import path structure. For example,
a.b.c
could come from//my/repo/is/wierd
. In this case, people could write a language extension that has a nearly-no-opGenerate
method that returns a fixed set of nonconventional packages to index. Alternatively, there could be a-lazy_index_include=some/path
to force additional packages (and their subdirs) to be indexed.The text was updated successfully, but these errors were encountered: