While working on a project, I've noticed that goimports is very slow
whenever I used it on a file that references non-exported symbols found in
another file in the same package. After some digging, it looks like
goimports spends most of its time loading packages and listing directories
(from imports.Process) and it does this on every invocation. This has also
been noticed by other projects like vim-go, which switched from using
goimports to gofmt by default, due to the issues caused by the long runtime
required by goimports (vim freezing for several seconds on save).
I propose to add an opt-in cache to tools/imports, so programs like
goimports might decide to use this cache.
- Reduce time spent by programs relying on tools/imports, while still
providing correct results
- Cache must be opt-in
- Cache must be per-program (some programs might not need a cache since
they might do little processing or might not require fast execution)
- Setting up and saving the cache must the program's responsibility.
- Minimal overhead for callers not using the cache.
- Forward compatibility. It's a cache, so future breaking changes to the
cache format might just discard the old one and rebuild it.
Discarded alternative solutions:
- Loading the current package and checking its non-exported symbols. There
are already many tools that feed goimports from stdin, so it has no way to
check the package containing the current input file. This solution would
also not alleviate the problem of non existing symbols (e.g. a mistyped
New exposed functions in tools/imports
- LoadCache(r io.Reader) error
- SaveCache(w io.Writer) error
With these two functions, each program is responsible for determining where
it wants the cache to be stored as well as loading and saving it. Using
interfaces rather than filenames allows also out-of-fs caching (e.g.
memcache, redis, whatever the caller wants it to be).
Potential gains with naive implementation:
I've implemented a proof of concept cache which uses the directory mtime to
check whether a directory needs to be scanned again or not. I do understand
that this is not bullet proof, since changes to attributes in the files
won't update the directory mtime, keep in mind this is PoF.
All runs have been averaged between 5 times, after discarding the first run
(to minimize effects from in-kernel fs caching).
goimports (current implementation):
goimports (with cache, cold run with cache not built yet):
goimports (with cache populated):
Please, let me know what you think about this proposal.
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to firstname.lastname@example.org.
For more options, visit https://groups.google.com/d/optout.