-
Notifications
You must be signed in to change notification settings - Fork 12
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
refactor(injector): new API with better performance (#234)
Create a new `injector.Injector` API that does not rely on `decorator.Load` (internally using `packages.Load`), instead using the basic `go/types` API to type-check the AST nodes in order to obtain the `Uses` map that is sufficient to build an import-managing `decorator.Decorator` instance. The package name resolution is done by parsing type information from the archives mentioned in the `importcfg` file. This change removes unnecessary compilation of un-instrumented archives which are not useful, and hence saves time and disk space. --- The new API also does not consider `PreserveLineInfo` to be optional (this was never exposed to end-users anyway), so a bunch of test reference files have changed to now include line directives. --------- Co-authored-by: Eliott Bouhana <[email protected]>
- Loading branch information
1 parent
b4bdb4f
commit 62d7333
Showing
28 changed files
with
824 additions
and
470 deletions.
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
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,34 @@ | ||
| // Unless explicitly stated otherwise all files in this repository are licensed | ||
| // under the Apache License Version 2.0. | ||
| // This product includes software developed at Datadog (https://www.datadoghq.com/). | ||
| // Copyright 2023-present Datadog, Inc. | ||
|
|
||
| package injector | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "go/ast" | ||
| "go/importer" | ||
| "go/token" | ||
| "go/types" | ||
| "runtime" | ||
| ) | ||
|
|
||
| // typeCheck runs the Go type checker on the provided files, and returns the | ||
| // Uses type information map that is built in the process. | ||
| func (i *Injector) typeCheck(fset *token.FileSet, files []*ast.File) (map[*ast.Ident]types.Object, error) { | ||
| pkg := types.NewPackage(i.ImportPath, i.Name) | ||
| typeInfo := types.Info{Uses: make(map[*ast.Ident]types.Object)} | ||
|
|
||
| checkerCfg := types.Config{ | ||
| GoVersion: i.GoVersion, | ||
| Importer: importer.ForCompiler(fset, runtime.Compiler, i.Lookup), | ||
| } | ||
| checker := types.NewChecker(&checkerCfg, fset, pkg, &typeInfo) | ||
|
|
||
| if err := checker.Files(files); err != nil { | ||
| return nil, fmt.Errorf("type-checking files: %w", err) | ||
| } | ||
|
|
||
| return typeInfo.Uses, nil | ||
| } |
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,115 @@ | ||
| // Unless explicitly stated otherwise all files in this repository are licensed | ||
| // under the Apache License Version 2.0. | ||
| // This product includes software developed at Datadog (https://www.datadoghq.com/). | ||
| // Copyright 2023-present Datadog, Inc. | ||
|
|
||
| package injector | ||
|
|
||
| import ( | ||
| "go/token" | ||
| "strconv" | ||
|
|
||
| "github.com/dave/dst" | ||
| "github.com/dave/dst/dstutil" | ||
| ) | ||
|
|
||
| // canonicalizeImports works around the issue detailed in https://github.com/dave/dst/issues/45 | ||
| // where dave/dst improperly handles multiple imports of the same package with different aliases, | ||
| // resulting in invalid output source code. | ||
| // | ||
| // To do so, it modifies the AST file so that it only includes a single import per path, using the | ||
| // first non-empty alias found. | ||
| func canonicalizeImports(file *dst.File) { | ||
| specsByPath := importSpecsByImportPath(file) | ||
|
|
||
| retain := filterExtraneousImports(specsByPath) | ||
|
|
||
| file.Imports = file.Imports[:0] // Re-use the backing store, we'll keep <= what was there. | ||
| for spec := range retain { | ||
| file.Imports = append(file.Imports, spec) | ||
| } | ||
|
|
||
| filterDecls(file, retain) | ||
| } | ||
|
|
||
| func importSpecsByImportPath(file *dst.File) map[string][]*dst.ImportSpec { | ||
| byPath := make(map[string][]*dst.ImportSpec, len(file.Imports)) | ||
|
|
||
| for _, imp := range file.Imports { | ||
| path, err := strconv.Unquote(imp.Path.Value) | ||
| if err != nil { | ||
| continue | ||
| } | ||
| list := append(byPath[path], imp) | ||
| byPath[path] = list | ||
| } | ||
|
|
||
| return byPath | ||
| } | ||
|
|
||
| func filterExtraneousImports(byPath map[string][]*dst.ImportSpec) map[*dst.ImportSpec]struct{} { | ||
| result := make(map[*dst.ImportSpec]struct{}, len(byPath)) | ||
|
|
||
| for _, specs := range byPath { | ||
| retain := specs[0] | ||
| for _, spec := range specs[1:] { | ||
| if (spec.Name == nil && (retain.Name == nil || retain.Name.Name != "_")) || spec.Name.Name == "_" { | ||
| continue | ||
| } | ||
| retain = spec | ||
| break | ||
| } | ||
| result[retain] = struct{}{} | ||
| } | ||
|
|
||
| return result | ||
| } | ||
|
|
||
| func filterDecls(file *dst.File, retain map[*dst.ImportSpec]struct{}) { | ||
| dstutil.Apply( | ||
| file, | ||
| func(csor *dstutil.Cursor) bool { | ||
| switch node := csor.Node().(type) { | ||
| case *dst.GenDecl: | ||
| // Only visit the children of `import` declarations. | ||
| return node.Tok == token.IMPORT | ||
| case *dst.ImportSpec: | ||
| // Filter out ImportSpec entries to keep only those in retain | ||
| if _, ret := retain[node]; !ret { | ||
| csor.Delete() | ||
| } | ||
| // No need to traverse children. | ||
| return false | ||
| case dst.Decl: | ||
| // No need to visit any other kind of declaration | ||
| return false | ||
| default: | ||
| // Visit other node types (e.g, the *ast.File) | ||
| return true | ||
| } | ||
| }, | ||
| func(csor *dstutil.Cursor) bool { | ||
| switch node := csor.Node().(type) { | ||
| case *dst.GenDecl: | ||
| if node.Tok != token.IMPORT { | ||
| // Imports are before any other kind of declaration, we can abort traversal as soon as we | ||
| // find a declaration that is not an `import` declaration. | ||
| return false | ||
| } | ||
|
|
||
| if len(node.Specs) == 0 { | ||
| csor.Delete() | ||
| } | ||
| // Proceed with the rest of the nodes (there may be more imports). | ||
| return true | ||
| case dst.Decl: | ||
| // Imports are before any other kind of declaration, we can abort traversal as soon as we | ||
| // find a declaration that is not an `import` declaration. | ||
| return false | ||
| default: | ||
| // Proceed with the rest of the nodes (there may be imports down there). | ||
| return true | ||
| } | ||
| }, | ||
| ) | ||
| } |
Oops, something went wrong.