Skip to content

Commit edf8e6f

Browse files
committed
cmd/goimports, imports: optimize directory scanning and other things
This brings goimports from 160ms to 100ms on my laptop, and under 50ms on my Linux machine. Using cmd/trace, I noticed that filepath.Walk is inherently slow. See https://golang.org/issue/16399 for details. Instead, this CL introduces a new (private) filepath.Walk implementation, optimized for speed and avoiding unnecessary work. In addition to avoid an Lstat per file, it also reads directories concurrently. The old goimports code did that too, but now that logic is removed from goimports and the code is simplified. This also adds some profiling command line flags to goimports that I found useful. Updates golang/go#16367 (goimports is slow) Updates golang/go#16399 (filepath.Walk is slow) Change-Id: I708d570cbaad3fa9ad75a12054f5a932ee159b84 Reviewed-on: https://go-review.googlesource.com/25001 Reviewed-by: Andrew Gerrand <[email protected]> Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
1 parent caebc7a commit edf8e6f

File tree

9 files changed

+727
-111
lines changed

9 files changed

+727
-111
lines changed

cmd/goimports/goimports.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package main
66

77
import (
8+
"bufio"
89
"bytes"
910
"flag"
1011
"fmt"
@@ -16,6 +17,8 @@ import (
1617
"os/exec"
1718
"path/filepath"
1819
"runtime"
20+
"runtime/pprof"
21+
"runtime/trace"
1922
"strings"
2023

2124
"golang.org/x/tools/imports"
@@ -29,6 +32,11 @@ var (
2932
srcdir = flag.String("srcdir", "", "choose imports as if source code is from `dir`")
3033
verbose = flag.Bool("v", false, "verbose logging")
3134

35+
cpuProfile = flag.String("cpuprofile", "", "CPU profile output")
36+
memProfile = flag.String("memprofile", "", "memory profile output")
37+
memProfileRate = flag.Int("memrate", 0, "if > 0, sets runtime.MemProfileRate")
38+
traceProfile = flag.String("trace", "", "trace profile output")
39+
3240
options = &imports.Options{
3341
TabWidth: 8,
3442
TabIndent: true,
@@ -152,10 +160,50 @@ var parseFlags = func() []string {
152160
return flag.Args()
153161
}
154162

163+
func bufferedFileWriter(dest string) (w io.Writer, close func()) {
164+
f, err := os.Create(dest)
165+
if err != nil {
166+
log.Fatal(err)
167+
}
168+
bw := bufio.NewWriter(f)
169+
return bw, func() {
170+
if err := bw.Flush(); err != nil {
171+
log.Fatalf("error flushing %v: %v", dest, err)
172+
}
173+
if err := f.Close(); err != nil {
174+
log.Fatal(err)
175+
}
176+
}
177+
}
178+
155179
func gofmtMain() {
156180
flag.Usage = usage
157181
paths := parseFlags()
158182

183+
if *cpuProfile != "" {
184+
bw, flush := bufferedFileWriter(*cpuProfile)
185+
pprof.StartCPUProfile(bw)
186+
defer flush()
187+
defer pprof.StopCPUProfile()
188+
}
189+
if *traceProfile != "" {
190+
bw, flush := bufferedFileWriter(*traceProfile)
191+
trace.Start(bw)
192+
defer flush()
193+
defer trace.Stop()
194+
}
195+
if *memProfileRate > 0 {
196+
runtime.MemProfileRate = *memProfileRate
197+
bw, flush := bufferedFileWriter(*memProfile)
198+
defer func() {
199+
runtime.GC() // materialize all statistics
200+
if err := pprof.WriteHeapProfile(bw); err != nil {
201+
log.Fatal(err)
202+
}
203+
flush()
204+
}()
205+
}
206+
159207
if *verbose {
160208
log.SetFlags(log.LstdFlags | log.Lmicroseconds)
161209
imports.Debug = true

imports/fastwalk.go

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
// Copyright 2016 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// A faster implementation of filepath.Walk.
6+
//
7+
// filepath.Walk's design necessarily calls os.Lstat on each file,
8+
// even if the caller needs less info. And goimports only need to know
9+
// the type of each file. The kernel interface provides the type in
10+
// the Readdir call but the standard library ignored it.
11+
// fastwalk_unix.go contains a fork of the syscall routines.
12+
//
13+
// See golang.org/issue/16399
14+
15+
package imports
16+
17+
import (
18+
"errors"
19+
"os"
20+
"path/filepath"
21+
"runtime"
22+
)
23+
24+
// traverseLink is a sentinel error for fastWalk, similar to filepath.SkipDir.
25+
var traverseLink = errors.New("traverse symlink, assuming target is a directory")
26+
27+
// fastWalk walks the file tree rooted at root, calling walkFn for
28+
// each file or directory in the tree, including root.
29+
//
30+
// If fastWalk returns filepath.SkipDir, the directory is skipped.
31+
//
32+
// Unlike filepath.Walk:
33+
// * file stat calls must be done by the user.
34+
// The only provided metadata is the file type, which does not include
35+
// any permission bits.
36+
// * multiple goroutines stat the filesystem concurrently. The provided
37+
// walkFn must be safe for concurrent use.
38+
// * fastWalk can follow symlinks if walkFn returns the traverseLink
39+
// sentinel error. It is the walkFn's responsibility to prevent
40+
// fastWalk from going into symlink cycles.
41+
func fastWalk(root string, walkFn func(path string, typ os.FileMode) error) error {
42+
// TODO(bradfitz): make numWorkers configurable? We used a
43+
// minimum of 4 to give the kernel more info about multiple
44+
// things we want, in hopes its I/O scheduling can take
45+
// advantage of that. Hopefully most are in cache. Maybe 4 is
46+
// even too low of a minimum. Profile more.
47+
numWorkers := 4
48+
if n := runtime.NumCPU(); n > numWorkers {
49+
numWorkers = n
50+
}
51+
w := &walker{
52+
fn: walkFn,
53+
enqueuec: make(chan walkItem, numWorkers), // buffered for performance
54+
workc: make(chan walkItem, numWorkers), // buffered for performance
55+
donec: make(chan struct{}),
56+
57+
// buffered for correctness & not leaking goroutines:
58+
resc: make(chan error, numWorkers),
59+
}
60+
defer close(w.donec)
61+
// TODO(bradfitz): start the workers as needed? maybe not worth it.
62+
for i := 0; i < numWorkers; i++ {
63+
go w.doWork()
64+
}
65+
todo := []walkItem{{dir: root}}
66+
out := 0
67+
for {
68+
workc := w.workc
69+
var workItem walkItem
70+
if len(todo) == 0 {
71+
workc = nil
72+
} else {
73+
workItem = todo[len(todo)-1]
74+
}
75+
select {
76+
case workc <- workItem:
77+
todo = todo[:len(todo)-1]
78+
out++
79+
case it := <-w.enqueuec:
80+
todo = append(todo, it)
81+
case err := <-w.resc:
82+
out--
83+
if err != nil {
84+
return err
85+
}
86+
if out == 0 && len(todo) == 0 {
87+
// It's safe to quit here, as long as the buffered
88+
// enqueue channel isn't also readable, which might
89+
// happen if the worker sends both another unit of
90+
// work and its result before the other select was
91+
// scheduled and both w.resc and w.enqueuec were
92+
// readable.
93+
select {
94+
case it := <-w.enqueuec:
95+
todo = append(todo, it)
96+
default:
97+
return nil
98+
}
99+
}
100+
}
101+
}
102+
}
103+
104+
// doWork reads directories as instructed (via workc) and runs the
105+
// user's callback function.
106+
func (w *walker) doWork() {
107+
for {
108+
select {
109+
case <-w.donec:
110+
return
111+
case it := <-w.workc:
112+
w.resc <- w.walk(it.dir, !it.callbackDone)
113+
}
114+
}
115+
}
116+
117+
type walker struct {
118+
fn func(path string, typ os.FileMode) error
119+
120+
donec chan struct{} // closed on fastWalk's return
121+
workc chan walkItem // to workers
122+
enqueuec chan walkItem // from workers
123+
resc chan error // from workers
124+
}
125+
126+
type walkItem struct {
127+
dir string
128+
callbackDone bool // callback already called; don't do it again
129+
}
130+
131+
func (w *walker) enqueue(it walkItem) {
132+
select {
133+
case w.enqueuec <- it:
134+
case <-w.donec:
135+
}
136+
}
137+
138+
func (w *walker) onDirEnt(dirName, baseName string, typ os.FileMode) error {
139+
joined := dirName + string(os.PathSeparator) + baseName
140+
if typ == os.ModeDir {
141+
w.enqueue(walkItem{dir: joined})
142+
return nil
143+
}
144+
145+
err := w.fn(joined, typ)
146+
if typ == os.ModeSymlink {
147+
if err == traverseLink {
148+
// Set callbackDone so we don't call it twice for both the
149+
// symlink-as-symlink and the symlink-as-directory later:
150+
w.enqueue(walkItem{dir: joined, callbackDone: true})
151+
return nil
152+
}
153+
if err == filepath.SkipDir {
154+
// Permit SkipDir on symlinks too.
155+
return nil
156+
}
157+
}
158+
return err
159+
}
160+
func (w *walker) walk(root string, runUserCallback bool) error {
161+
if runUserCallback {
162+
err := w.fn(root, os.ModeDir)
163+
if err == filepath.SkipDir {
164+
return nil
165+
}
166+
if err != nil {
167+
return err
168+
}
169+
}
170+
171+
return readDir(root, w.onDirEnt)
172+
}

imports/fastwalk_dirent_fileno.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Copyright 2016 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// +build freebsd openbsd netbsd
6+
7+
package imports
8+
9+
import "syscall"
10+
11+
func direntInode(dirent *syscall.Dirent) uint64 {
12+
return uint64(dirent.Fileno)
13+
}

imports/fastwalk_dirent_ino.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Copyright 2016 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// +build linux darwin
6+
7+
package imports
8+
9+
import "syscall"
10+
11+
func direntInode(dirent *syscall.Dirent) uint64 {
12+
return uint64(dirent.Ino)
13+
}

imports/fastwalk_portable.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Copyright 2016 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// +build !linux,!darwin,!freebsd,!openbsd,!netbsd
6+
7+
package imports
8+
9+
import (
10+
"io/ioutil"
11+
"os"
12+
)
13+
14+
// readDir calls fn for each directory entry in dirName.
15+
// It does not descend into directories or follow symlinks.
16+
// If fn returns a non-nil error, readDir returns with that error
17+
// immediately.
18+
func readDir(dirName string, fn func(dirName, entName string, typ os.FileMode) error) error {
19+
fis, err := ioutil.ReadDir(dirName)
20+
if err != nil {
21+
return err
22+
}
23+
for _, fi := range fis {
24+
if err := fn(dirName, fi.Name(), fi.Mode()&os.ModeType); err != nil {
25+
return err
26+
}
27+
}
28+
return nil
29+
}

0 commit comments

Comments
 (0)