diff --git a/src/cmd/compile/internal/types2/self_test.go b/src/cmd/compile/internal/types2/self_test.go index e68d52db42..3c8bec1c45 100644 --- a/src/cmd/compile/internal/types2/self_test.go +++ b/src/cmd/compile/internal/types2/self_test.go @@ -100,7 +100,7 @@ func runbench(b *testing.B, path string, ignoreFuncBodies, writeInfo bool) { } func pkgFiles(path string) ([]*syntax.File, error) { - filenames, err := pkgFilenames(path) // from stdlib_test.go + filenames, err := pkgFilenames(path, true) // from stdlib_test.go if err != nil { return nil, err } diff --git a/src/cmd/compile/internal/types2/stdlib_test.go b/src/cmd/compile/internal/types2/stdlib_test.go index 404e1636ae..9a03526b68 100644 --- a/src/cmd/compile/internal/types2/stdlib_test.go +++ b/src/cmd/compile/internal/types2/stdlib_test.go @@ -10,12 +10,15 @@ package types2_test import ( "bytes" "cmd/compile/internal/syntax" + "errors" "fmt" "go/build" "internal/testenv" "os" "path/filepath" + "runtime" "strings" + "sync" "testing" "time" @@ -25,17 +28,130 @@ import ( var stdLibImporter = defaultImporter() func TestStdlib(t *testing.T) { + if testing.Short() { + t.Skip("skipping in short mode") + } + testenv.MustHaveGoBuild(t) - pkgCount := 0 - duration := walkPkgDirs(filepath.Join(testenv.GOROOT(t), "src"), func(dir string, filenames []string) { - typecheckFiles(t, dir, filenames) - pkgCount++ + // Collect non-test files. + dirFiles := make(map[string][]string) + root := filepath.Join(testenv.GOROOT(t), "src") + walkPkgDirs(root, func(dir string, filenames []string) { + dirFiles[dir] = filenames }, t.Error) - if testing.Verbose() { - fmt.Println(pkgCount, "packages typechecked in", duration) + c := &stdlibChecker{ + dirFiles: dirFiles, + pkgs: make(map[string]*futurePackage), } + + start := time.Now() + + // Though we read files while parsing, type-checking is otherwise CPU bound. + // + // This doesn't achieve great CPU utilization as many packages may block + // waiting for a common import, but in combination with the non-deterministic + // map iteration below this should provide decent coverage of concurrent + // type-checking (see golang/go#47729). + cpulimit := make(chan struct{}, runtime.GOMAXPROCS(0)) + var wg sync.WaitGroup + + for dir := range dirFiles { + dir := dir + + cpulimit <- struct{}{} + wg.Add(1) + go func() { + defer func() { + wg.Done() + <-cpulimit + }() + + _, err := c.getDirPackage(dir) + if err != nil { + t.Errorf("error checking %s: %v", dir, err) + } + }() + } + + wg.Wait() + + if testing.Verbose() { + fmt.Println(len(dirFiles), "packages typechecked in", time.Since(start)) + } +} + +// stdlibChecker implements concurrent type-checking of the packages defined by +// dirFiles, which must define a closed set of packages (such as GOROOT/src). +type stdlibChecker struct { + dirFiles map[string][]string // non-test files per directory; must be pre-populated + + mu sync.Mutex + pkgs map[string]*futurePackage // future cache of type-checking results +} + +// A futurePackage is a future result of type-checking. +type futurePackage struct { + done chan struct{} // guards pkg and err + pkg *Package + err error +} + +func (c *stdlibChecker) Import(path string) (*Package, error) { + panic("unimplemented: use ImportFrom") +} + +func (c *stdlibChecker) ImportFrom(path, dir string, _ ImportMode) (*Package, error) { + if path == "unsafe" { + // unsafe cannot be type checked normally. + return Unsafe, nil + } + + p, err := build.Default.Import(path, dir, build.FindOnly) + if err != nil { + return nil, err + } + + pkg, err := c.getDirPackage(p.Dir) + if pkg != nil { + // As long as pkg is non-nil, avoid redundant errors related to failed + // imports. TestStdlib will collect errors once for each package. + return pkg, nil + } + return nil, err +} + +// getDirPackage gets the package defined in dir from the future cache. +// +// If this is the first goroutine requesting the package, getDirPackage +// type-checks. +func (c *stdlibChecker) getDirPackage(dir string) (*Package, error) { + c.mu.Lock() + fut, ok := c.pkgs[dir] + if !ok { + // First request for this package dir; type check. + fut = &futurePackage{ + done: make(chan struct{}), + } + c.pkgs[dir] = fut + files, ok := c.dirFiles[dir] + c.mu.Unlock() + if !ok { + fut.err = fmt.Errorf("no files for %s", dir) + } else { + // Using dir as the package path here may be inconsistent with the behavior + // of a normal importer, but is sufficient as dir is by construction unique + // to this package. + fut.pkg, fut.err = typecheckFiles(dir, files, c) + } + close(fut.done) + } else { + // Otherwise, await the result. + c.mu.Unlock() + <-fut.done + } + return fut.pkg, fut.err } // firstComment returns the contents of the first non-empty comment in @@ -230,34 +346,51 @@ var excluded = map[string]bool{ "crypto/internal/bigmod/_asm": true, } +// printPackageMu synchronizes the printing of type-checked package files in +// the typecheckFiles function. +// +// Without synchronization, package files may be interleaved during concurrent +// type-checking. +var printPackageMu sync.Mutex + // typecheckFiles typechecks the given package files. -func typecheckFiles(t *testing.T, path string, filenames []string) { - // parse package files +func typecheckFiles(path string, filenames []string, importer Importer) (*Package, error) { + // Parse package files. var files []*syntax.File for _, filename := range filenames { - errh := func(err error) { t.Error(err) } + var errs []error + errh := func(err error) { errs = append(errs, err) } file, err := syntax.ParseFile(filename, errh, nil, 0) if err != nil { - return - } - - if testing.Verbose() { - if len(files) == 0 { - fmt.Println("package", file.PkgName.Value) - } - fmt.Println("\t", filename) + return nil, errors.Join(errs...) } files = append(files, file) } - // typecheck package files + if testing.Verbose() { + printPackageMu.Lock() + fmt.Println("package", files[0].PkgName.Value) + for _, filename := range filenames { + fmt.Println("\t", filename) + } + printPackageMu.Unlock() + } + + // Typecheck package files. + var errs []error conf := Config{ - Error: func(err error) { t.Error(err) }, - Importer: stdLibImporter, + Error: func(err error) { + errs = append(errs, err) + }, + Importer: importer, } info := Info{Uses: make(map[*syntax.Name]Object)} - conf.Check(path, files, &info) + pkg, _ := conf.Check(path, files, &info) + err := errors.Join(errs...) + if err != nil { + return pkg, err + } // Perform checks of API invariants. @@ -268,16 +401,18 @@ func typecheckFiles(t *testing.T, path string, filenames []string) { if predeclared == (obj.Pkg() != nil) { posn := id.Pos() if predeclared { - t.Errorf("%s: predeclared object with package: %s", posn, obj) + return nil, fmt.Errorf("%s: predeclared object with package: %s", posn, obj) } else { - t.Errorf("%s: user-defined object without package: %s", posn, obj) + return nil, fmt.Errorf("%s: user-defined object without package: %s", posn, obj) } } } + + return pkg, nil } // pkgFilenames returns the list of package filenames for the given directory. -func pkgFilenames(dir string) ([]string, error) { +func pkgFilenames(dir string, includeTest bool) ([]string, error) { ctxt := build.Default ctxt.CgoEnabled = false pkg, err := ctxt.ImportDir(dir, 0) @@ -294,31 +429,25 @@ func pkgFilenames(dir string) ([]string, error) { for _, name := range pkg.GoFiles { filenames = append(filenames, filepath.Join(pkg.Dir, name)) } - for _, name := range pkg.TestGoFiles { - filenames = append(filenames, filepath.Join(pkg.Dir, name)) + if includeTest { + for _, name := range pkg.TestGoFiles { + filenames = append(filenames, filepath.Join(pkg.Dir, name)) + } } return filenames, nil } -func walkPkgDirs(dir string, pkgh func(dir string, filenames []string), errh func(args ...interface{})) time.Duration { - w := walker{time.Now(), 10 * time.Millisecond, pkgh, errh} +func walkPkgDirs(dir string, pkgh func(dir string, filenames []string), errh func(args ...interface{})) { + w := walker{pkgh, errh} w.walk(dir) - return time.Since(w.start) } type walker struct { - start time.Time - dmax time.Duration - pkgh func(dir string, filenames []string) - errh func(args ...interface{}) + pkgh func(dir string, filenames []string) + errh func(args ...any) } func (w *walker) walk(dir string) { - // limit run time for short tests - if testing.Short() && time.Since(w.start) >= w.dmax { - return - } - files, err := os.ReadDir(dir) if err != nil { w.errh(err) @@ -326,7 +455,9 @@ func (w *walker) walk(dir string) { } // apply pkgh to the files in directory dir - pkgFiles, err := pkgFilenames(dir) + + // Don't get test files as these packages are imported. + pkgFiles, err := pkgFilenames(dir, false) if err != nil { w.errh(err) return diff --git a/src/go/types/self_test.go b/src/go/types/self_test.go index a63f2b74f5..27fa75652a 100644 --- a/src/go/types/self_test.go +++ b/src/go/types/self_test.go @@ -104,7 +104,7 @@ func runbench(b *testing.B, path string, ignoreFuncBodies, writeInfo bool) { } func pkgFiles(fset *token.FileSet, path string) ([]*ast.File, error) { - filenames, err := pkgFilenames(path) // from stdlib_test.go + filenames, err := pkgFilenames(path, true) // from stdlib_test.go if err != nil { return nil, err } diff --git a/src/go/types/stdlib_test.go b/src/go/types/stdlib_test.go index 82f22de836..770d3bf52a 100644 --- a/src/go/types/stdlib_test.go +++ b/src/go/types/stdlib_test.go @@ -8,6 +8,7 @@ package types_test import ( + "errors" "fmt" "go/ast" "go/build" @@ -18,7 +19,9 @@ import ( "internal/testenv" "os" "path/filepath" + "runtime" "strings" + "sync" "testing" "time" @@ -36,17 +39,130 @@ import ( var stdLibImporter = importer.ForCompiler(token.NewFileSet(), "source", nil) func TestStdlib(t *testing.T) { + if testing.Short() { + t.Skip("skipping in short mode") + } + testenv.MustHaveGoBuild(t) - pkgCount := 0 - duration := walkPkgDirs(filepath.Join(testenv.GOROOT(t), "src"), func(dir string, filenames []string) { - typecheckFiles(t, dir, filenames) - pkgCount++ + // Collect non-test files. + dirFiles := make(map[string][]string) + root := filepath.Join(testenv.GOROOT(t), "src") + walkPkgDirs(root, func(dir string, filenames []string) { + dirFiles[dir] = filenames }, t.Error) - if testing.Verbose() { - fmt.Println(pkgCount, "packages typechecked in", duration) + c := &stdlibChecker{ + dirFiles: dirFiles, + pkgs: make(map[string]*futurePackage), } + + start := time.Now() + + // Though we read files while parsing, type-checking is otherwise CPU bound. + // + // This doesn't achieve great CPU utilization as many packages may block + // waiting for a common import, but in combination with the non-deterministic + // map iteration below this should provide decent coverage of concurrent + // type-checking (see golang/go#47729). + cpulimit := make(chan struct{}, runtime.GOMAXPROCS(0)) + var wg sync.WaitGroup + + for dir := range dirFiles { + dir := dir + + cpulimit <- struct{}{} + wg.Add(1) + go func() { + defer func() { + wg.Done() + <-cpulimit + }() + + _, err := c.getDirPackage(dir) + if err != nil { + t.Errorf("error checking %s: %v", dir, err) + } + }() + } + + wg.Wait() + + if testing.Verbose() { + fmt.Println(len(dirFiles), "packages typechecked in", time.Since(start)) + } +} + +// stdlibChecker implements concurrent type-checking of the packages defined by +// dirFiles, which must define a closed set of packages (such as GOROOT/src). +type stdlibChecker struct { + dirFiles map[string][]string // non-test files per directory; must be pre-populated + + mu sync.Mutex + pkgs map[string]*futurePackage // future cache of type-checking results +} + +// A futurePackage is a future result of type-checking. +type futurePackage struct { + done chan struct{} // guards pkg and err + pkg *Package + err error +} + +func (c *stdlibChecker) Import(path string) (*Package, error) { + panic("unimplemented: use ImportFrom") +} + +func (c *stdlibChecker) ImportFrom(path, dir string, _ ImportMode) (*Package, error) { + if path == "unsafe" { + // unsafe cannot be type checked normally. + return Unsafe, nil + } + + p, err := build.Default.Import(path, dir, build.FindOnly) + if err != nil { + return nil, err + } + + pkg, err := c.getDirPackage(p.Dir) + if pkg != nil { + // As long as pkg is non-nil, avoid redundant errors related to failed + // imports. TestStdlib will collect errors once for each package. + return pkg, nil + } + return nil, err +} + +// getDirPackage gets the package defined in dir from the future cache. +// +// If this is the first goroutine requesting the package, getDirPackage +// type-checks. +func (c *stdlibChecker) getDirPackage(dir string) (*Package, error) { + c.mu.Lock() + fut, ok := c.pkgs[dir] + if !ok { + // First request for this package dir; type check. + fut = &futurePackage{ + done: make(chan struct{}), + } + c.pkgs[dir] = fut + files, ok := c.dirFiles[dir] + c.mu.Unlock() + if !ok { + fut.err = fmt.Errorf("no files for %s", dir) + } else { + // Using dir as the package path here may be inconsistent with the behavior + // of a normal importer, but is sufficient as dir is by construction unique + // to this package. + fut.pkg, fut.err = typecheckFiles(dir, files, c) + } + close(fut.done) + } else { + // Otherwise, await the result. + c.mu.Unlock() + <-fut.done + } + return fut.pkg, fut.err } // firstComment returns the contents of the first non-empty comment in @@ -232,46 +348,51 @@ var excluded = map[string]bool{ "crypto/internal/bigmod/_asm": true, } +// printPackageMu synchronizes the printing of type-checked package files in +// the typecheckFiles function. +// +// Without synchronization, package files may be interleaved during concurrent +// type-checking. +var printPackageMu sync.Mutex + // typecheckFiles typechecks the given package files. -func typecheckFiles(t *testing.T, path string, filenames []string) { +func typecheckFiles(path string, filenames []string, importer Importer) (*Package, error) { fset := token.NewFileSet() - // parse package files + // Parse package files. var files []*ast.File for _, filename := range filenames { file, err := parser.ParseFile(fset, filename, nil, parser.AllErrors) if err != nil { - // the parser error may be a list of individual errors; report them all - if list, ok := err.(scanner.ErrorList); ok { - for _, err := range list { - t.Error(err) - } - return - } - t.Error(err) - return - } - - if testing.Verbose() { - if len(files) == 0 { - fmt.Println("package", file.Name.Name) - } - fmt.Println("\t", filename) + return nil, err } files = append(files, file) } - // typecheck package files + if testing.Verbose() { + printPackageMu.Lock() + fmt.Println("package", files[0].Name.Name) + for _, filename := range filenames { + fmt.Println("\t", filename) + } + printPackageMu.Unlock() + } + + // Typecheck package files. + var errs []error conf := Config{ Error: func(err error) { - t.Helper() - t.Error(err) + errs = append(errs, err) }, - Importer: stdLibImporter, + Importer: importer, } info := Info{Uses: make(map[*ast.Ident]Object)} - conf.Check(path, fset, files, &info) + pkg, _ := conf.Check(path, fset, files, &info) + err := errors.Join(errs...) + if err != nil { + return pkg, err + } // Perform checks of API invariants. @@ -282,16 +403,18 @@ func typecheckFiles(t *testing.T, path string, filenames []string) { if predeclared == (obj.Pkg() != nil) { posn := fset.Position(id.Pos()) if predeclared { - t.Errorf("%s: predeclared object with package: %s", posn, obj) + return nil, fmt.Errorf("%s: predeclared object with package: %s", posn, obj) } else { - t.Errorf("%s: user-defined object without package: %s", posn, obj) + return nil, fmt.Errorf("%s: user-defined object without package: %s", posn, obj) } } } + + return pkg, nil } // pkgFilenames returns the list of package filenames for the given directory. -func pkgFilenames(dir string) ([]string, error) { +func pkgFilenames(dir string, includeTest bool) ([]string, error) { ctxt := build.Default ctxt.CgoEnabled = false pkg, err := ctxt.ImportDir(dir, 0) @@ -308,31 +431,25 @@ func pkgFilenames(dir string) ([]string, error) { for _, name := range pkg.GoFiles { filenames = append(filenames, filepath.Join(pkg.Dir, name)) } - for _, name := range pkg.TestGoFiles { - filenames = append(filenames, filepath.Join(pkg.Dir, name)) + if includeTest { + for _, name := range pkg.TestGoFiles { + filenames = append(filenames, filepath.Join(pkg.Dir, name)) + } } return filenames, nil } -func walkPkgDirs(dir string, pkgh func(dir string, filenames []string), errh func(args ...any)) time.Duration { - w := walker{time.Now(), 10 * time.Millisecond, pkgh, errh} +func walkPkgDirs(dir string, pkgh func(dir string, filenames []string), errh func(args ...any)) { + w := walker{pkgh, errh} w.walk(dir) - return time.Since(w.start) } type walker struct { - start time.Time - dmax time.Duration - pkgh func(dir string, filenames []string) - errh func(args ...any) + pkgh func(dir string, filenames []string) + errh func(args ...any) } func (w *walker) walk(dir string) { - // limit run time for short tests - if testing.Short() && time.Since(w.start) >= w.dmax { - return - } - files, err := os.ReadDir(dir) if err != nil { w.errh(err) @@ -340,7 +457,9 @@ func (w *walker) walk(dir string) { } // apply pkgh to the files in directory dir - pkgFiles, err := pkgFilenames(dir) + + // Don't get test files as these packages are imported. + pkgFiles, err := pkgFilenames(dir, false) if err != nil { w.errh(err) return