Skip to content
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

x/tools/go/packages: use goroutine only for load-ready packages in (*loader).refine #41224

Open
nu50218 opened this issue Sep 4, 2020 · 1 comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@nu50218
Copy link
Contributor

nu50218 commented Sep 4, 2020

(*loader).refine uses (*loader).loadRecursive internally, which may produce a large amount of goroutine as the import graph grows.

With code like following, you can use goroutine only for packages that are ready to load. (existing code)

// Load type data and syntax if needed, starting at
// the initial packages (roots of the import DAG).
if ld.Mode&NeedTypes != 0 || ld.Mode&NeedSyntax != 0 {
	visited := make(map[*loaderPackage]bool, len(ld.pkgs))
	var visit func(pkg *loaderPackage)
	visit = func(pkg *loaderPackage) {
		for _, imp := range pkg.Imports {
			imp := ld.pkgs[imp.ID]
			if !visited[imp] {
				visited[imp] = true
				visit(imp)
			}
		}
	}
	for _, lpkg := range initial {
		visited[lpkg] = true
		visit(lpkg)
	}

	var wg sync.WaitGroup
	waitNum := make(map[*loaderPackage]*int32, len(visited))
	importedBy := make(map[*loaderPackage][]*loaderPackage, len(visited))

	for lpkg := range visited {
		if !visited[lpkg] {
			continue
		}
		l := int32(len(lpkg.Imports))
		waitNum[lpkg] = &l
		for _, imp := range lpkg.Imports {
			p := ld.pkgs[imp.ID]
			importedBy[p] = append(importedBy[p], lpkg)
		}
	}

	var loadRec func(*loaderPackage)
	loadRec = func(lpkg *loaderPackage) {
		lpkg.loadOnce.Do(func() {
			ld.loadPackage(lpkg)
			for _, lpkg2 := range importedBy[lpkg] {
				if atomic.AddInt32(waitNum[lpkg2], -1) == 0 {
					lpkg2 := lpkg2
					wg.Add(1)
					go func() {
						loadRec(lpkg2)
						wg.Done()
					}()
				}
			}
		})
	}

	for lpkg := range visited {
		if atomic.LoadInt32(waitNum[lpkg]) == 0 {
			lpkg := lpkg
			wg.Add(1)
			go func() {
				loadRec(lpkg)
				wg.Done()
			}()
		}
	}
	wg.Wait()
}
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Sep 4, 2020
@gopherbot gopherbot added this to the Unreleased milestone Sep 4, 2020
@stamblerre
Copy link
Contributor

stamblerre commented Sep 4, 2020

/cc @matloob for go/packages

@nu50218: If you're interested in contributing your patch to the project, please feel free to do so by following these steps: https://golang.org/doc/contribute.html.

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

4 participants