Skip to content

Commit

Permalink
internal/gopathwalk: use filepath.WalkDir instead of internal/fastwalk
Browse files Browse the repository at this point in the history
golang.org/x/tools/internal/fastwalk adds a lot of complexity to avoid
unnecessary calls to os.Stat. It also adds very limited concurrency
(a maximum of 4 goroutines).

filepath.WalkDir, added in Go 1.16, also avoids unnecessary calls to
os.Stat, and is part of the standard library (so it adds much less
complexity to x/tools). Switching to that substantially simplifies the
implementation of internal/gopathwalk, at the cost of losing the small
amount of concurrency added by fastwalk.

The internal/gopathwalk package does not appear to have any benchmarks
of its own, but the benchmark impact shows a ~14.5% increase in the
imports.ScanModCache benchmark on Linux with a warm filesystem cache,
but that can be reduced to ~5.5% by also removing an apparently
unnecessary call to os.SameFile (in followup CL 508507).

To me, the decrease in code complexity seems worth the slight increase
in latency.

	~/x/tools$ benchstat cl508505.txt cl508506.txt
	goos: linux
	goarch: amd64
	pkg: golang.org/x/tools/internal/imports
	cpu: AMD EPYC 7B12
	                │ cl508505.txt │            cl508506.txt             │
	                │    sec/op    │   sec/op     vs base                │
	ScanModCache-24    231.0m ± 1%   264.4m ± 1%  +14.47% (p=0.000 n=10)

Fixes golang/go#58035.

Change-Id: Iffcdab4e9aea11c0f0d7a87904935635e20b2fe7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/508506
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
  • Loading branch information
Bryan C. Mills authored and gopherbot committed Oct 30, 2023
1 parent 9cf559c commit 1762c80
Show file tree
Hide file tree
Showing 10 changed files with 87 additions and 848 deletions.
196 changes: 0 additions & 196 deletions internal/fastwalk/fastwalk.go

This file was deleted.

119 changes: 0 additions & 119 deletions internal/fastwalk/fastwalk_darwin.go

This file was deleted.

14 changes: 0 additions & 14 deletions internal/fastwalk/fastwalk_dirent_fileno.go

This file was deleted.

15 changes: 0 additions & 15 deletions internal/fastwalk/fastwalk_dirent_ino.go

This file was deleted.

14 changes: 0 additions & 14 deletions internal/fastwalk/fastwalk_dirent_namlen_bsd.go

This file was deleted.

29 changes: 0 additions & 29 deletions internal/fastwalk/fastwalk_dirent_namlen_linux.go

This file was deleted.

0 comments on commit 1762c80

Please sign in to comment.