which would need to build the pkgIndex? (e.g. having an undeclared variable
or func call)
Just for reference, I do think that there's more people experiencing this
issue. vim-go's developer received so many complaints that he changed the
default go formatter tool from goimports to gofmt,
see https://github.com/fatih/vim-go/issues/144#issuecomment-59341725
On Tuesday, April 28, 2015 at 1:30:27 PM UTC+1, David Crawshaw wrote:
Before adding a layer I would like to understand how to reproduce this
slowness. With a large GOPATH on both linux and darwin I cannot get
goimports to take more than a fraction of a second. What is in your
GOPATH?
On Tue, Apr 28, 2015 at 7:26 AM, Alberto García Hierro
<[email protected] <javascript:>> wrote:
--Before adding a layer I would like to understand how to reproduce this
slowness. With a large GOPATH on both linux and darwin I cannot get
goimports to take more than a fraction of a second. What is in your
GOPATH?
On Tue, Apr 28, 2015 at 7:26 AM, Alberto García Hierro
<[email protected] <javascript:>> wrote:
Hi,
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.
Goals:
- 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.
Non-goals:
- 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
variable).
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):
real 0m5.198s
goimports (with cache, cold run with cache not built yet):
real 0m5.972s
goimports (with cache populated):
real 0m0.110s
Please, let me know what you think about this proposal.
Regards,
Alberto
--
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 [email protected] <javascript:>.
For more options, visit https://groups.google.com/d/optout.
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.
Goals:
- 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.
Non-goals:
- 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
variable).
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):
real 0m5.198s
goimports (with cache, cold run with cache not built yet):
real 0m5.972s
goimports (with cache populated):
real 0m0.110s
Please, let me know what you think about this proposal.
Regards,
Alberto
--
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 [email protected] <javascript:>.
For more options, visit https://groups.google.com/d/optout.
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 [email protected].
For more options, visit https://groups.google.com/d/optout.
