forked from bazel-contrib/bazel-gazelle
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(resolve): optimize applying overrides to be efficient
This PR addresses the inefficiency of processing `# gazelle:resolve` directives. Previously, the system iterated over a slice for each import to find matching directives, which is very inefficient. This PR introduces two major improvements: 1. **Transition from Slice to Map**: We've shifted from using a slice to a map for storing override configurations. This change reduces lookup time complexity from O(n) to O(1) (or O(length of chain of enclosing dirs containing resolves)). 2. **Parent-Child Config Structure**: To maintain directory-specific configurations without the overhead of copying maps (which is very inefficient), we've implemented a parent-child relationship in configurations. This structure allows each directory to have its unique overrides while inheriting unmodified settings from its parent. We can't use a single shared map, because this could mistakenly apply directives from one directory to another. `regexp` overrides are always copied and evaluated inline. Note on GC: to avoid a really complex object tree, the parent/child relationship skips all ancestors without `# gazelle:resolve` directives, greatly reducing the complexity. Note of Regexp: although we need to iterate through these due to the matching logic, we could at least have a `map: lang -> []overrides` to improve the lookup some. Fixes bazel-contrib#1688
- Loading branch information
1 parent
999b74f
commit 073699e
Showing
3 changed files
with
235 additions
and
75 deletions.
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| package resolve | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/bazelbuild/bazel-gazelle/rule" | ||
| "github.com/bazelbuild/bazel-gazelle/config" | ||
| "github.com/bazelbuild/bazel-gazelle/label" | ||
| "github.com/google/go-cmp/cmp" | ||
| ) | ||
|
|
||
| func TestFindRuleWithOverride_ParentTraversal(t *testing.T) { | ||
| rootCfg := getConfig(t, "", []rule.Directive{ | ||
| {Key: "resolve", Value: "go go github.com/root/repo @com_example//root:replacement"}, | ||
| {Key: "resolve_regexp", Value: "go ^github.com/root/(.*)$ @com_example//regexp:replacement"}, | ||
| }, nil) | ||
|
|
||
| childCfg := getConfig(t, "child/rel", []rule.Directive{ | ||
| {Key: "resolve", Value: "go github.com/child/repo //some/local/child:replacement"}, | ||
| {Key: "resolve_regexp", Value: "go ^github.com/child/(.*)$ relative/child/regexp"}, | ||
| }, rootCfg) | ||
|
|
||
| secondChildCfg := getConfig(t, "second/child/rel", nil, rootCfg) | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| cfg *config.Config | ||
| importSpec ImportSpec | ||
| lang string | ||
| want label.Label | ||
| wantFound bool | ||
| }{ | ||
| { | ||
| name: "Child exact match", | ||
| cfg: childCfg, | ||
| importSpec: ImportSpec{Lang: "go", Imp: "github.com/child/repo"}, | ||
| lang: "go", | ||
| want: getTestLabel(t, "//some/local/child:replacement"), | ||
| wantFound: true, | ||
| }, | ||
| { | ||
| name: "Child regexp match", | ||
| cfg: childCfg, | ||
| importSpec: ImportSpec{Lang: "go", Imp: "github.com/child/other"}, | ||
| lang: "go", | ||
| want: getTestLabel(t, "//child/rel:relative/child/regexp"), | ||
| wantFound: true, | ||
| }, | ||
| { | ||
| name: "Root exact match from child", | ||
| cfg: childCfg, | ||
| importSpec: ImportSpec{Lang: "go", Imp: "github.com/root/repo"}, | ||
| lang: "go", | ||
| want: getTestLabel(t, "@com_example//root:replacement"), | ||
| wantFound: true, | ||
| }, | ||
| { | ||
| name: "Root regexp match from child", | ||
| cfg: childCfg, | ||
| importSpec: ImportSpec{Lang: "go", Imp: "github.com/root/some"}, | ||
| lang: "go", | ||
| want: getTestLabel(t, "@com_example//regexp:replacement"), | ||
| wantFound: true, | ||
| }, | ||
| { | ||
| name: "No match in child or root", | ||
| cfg: childCfg, | ||
| importSpec: ImportSpec{Lang: "go", Imp: "github.com/nonexistent/repo"}, | ||
| lang: "go", | ||
| want: label.NoLabel, | ||
| wantFound: false, | ||
| }, | ||
| { | ||
| name: "Second child does not find child directive", | ||
| cfg: secondChildCfg, | ||
| importSpec: ImportSpec{Lang: "go", Imp: "github.com/child/repo"}, | ||
| lang: "go", | ||
| want: label.NoLabel, | ||
| wantFound: false, | ||
| }, | ||
| { | ||
| name: "Second child finds root directive", | ||
| cfg: secondChildCfg, | ||
| importSpec: ImportSpec{Lang: "go", Imp: "github.com/root/repo"}, | ||
| lang: "go", | ||
| want: getTestLabel(t, "@com_example//root:replacement"), | ||
| wantFound: true, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| got, found := FindRuleWithOverride(tt.cfg, tt.importSpec, tt.lang) | ||
| if found != tt.wantFound { | ||
| t.Fatalf("FindRuleWithOverride() found = %v, wantFound %v", found, tt.wantFound) | ||
| } | ||
| if diff := cmp.Diff(tt.want, got); diff != "" { | ||
| t.Errorf("FindRuleWithOverride() mismatch (-want +got):\n%s", diff) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func getConfig(t *testing.T, path string, directives []rule.Directive, parent *config.Config) *config.Config { | ||
| cfg := &config.Config{ | ||
| Exts: map[string]interface{}{}, | ||
| } | ||
| configurer := &Configurer{} | ||
| configurer.RegisterFlags(nil, "", cfg) | ||
| configurer.CheckFlags(nil, cfg) | ||
|
|
||
| if parent != nil { | ||
| cfg.Exts[resolveName] = parent.Exts[resolveName] | ||
| } | ||
|
|
||
| configurer.Configure(cfg, path, &rule.File{Directives: directives}) | ||
| return cfg | ||
| } | ||
|
|
||
| func getTestLabel(t *testing.T, str string) label.Label { | ||
| l, err := label.Parse(str) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| return l | ||
| } |