-
Notifications
You must be signed in to change notification settings - Fork 18k
go/format: add benchmarks #26528
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
Comments
Previously calling format.Source would take up most of the runtime (see golang/go#26528 for more info). But most of the file is already formatted correctly, we mostly need to use go/format to whitespace align the maps per the Go guidelines. Calling go/format on just this part of the file yields a significant performance improvement by avoiding calling it on the entire body of the file. Use a bytes.Buffer as a file argument instead of an io.Writer which allows using WriteByte and WriteString in places. Avoid calling fmt.Fprintf with no arguments. name old time/op new time/op delta Bindata-4 558ms ± 1% 216ms ± 6% -61.34% (p=0.008 n=5+5) name old speed new speed delta Bindata-4 12.3MB/s ± 1% 31.8MB/s ± 6% +159.03% (p=0.008 n=5+5) name old alloc/op new alloc/op delta Bindata-4 353MB ± 0% 162MB ± 0% -54.21% (p=0.008 n=5+5) name old allocs/op new allocs/op delta Bindata-4 46.1k ± 0% 15.7k ± 0% -66.00% (p=0.008 n=5+5)
I suspect go/format performance hotspots vary a lot based on input, with a long tail of pathological problematic inputs. Instead of a generic “add benchmarks” issue, which is hard to fix, let’s instead file performance issues for specific inputs, like go-bindata-style output. We don’t want to add a large file to the repo, but I suspect you could create a suitable input (for reproduction purposes) on the fly, in memory. Also, have you tried with 1.11 betas yet? I made some improvements to text/tabwriter this cycle that might help. |
I have, thanks! I'm not sure about the benchmarking output. When we use format.Source on the whole file, it looks slower. 1.9.7: https://travis-ci.org/kevinburke/go-bindata/jobs/406748540 I just merged a change that only calls format.Source on a small part of the file. With that change it looks like there's an improvement between 1.9.7 and tip. https://travis-ci.org/kevinburke/go-bindata/jobs/406757117 |
So what are my options here for a benchmark? I am guessing these are not possible:
What about downloading a large file from the public internet, and skipping the benchmark if the download fails? This one for example would be suitable. https://github.com/kevinburke/go-bindata/blob/master/testdata/assets/bindata.go Otherwise, I suppose I could check in a medium size photo (or use an existing one in the git repository), and manually reconstruct the process of building the bindata file. |
Skimming the beginning of that file, I'm guessing you could get similar performance behavior with just something like: package p
var x1 = []byte("some very very very long string created with bytes.Repeat")
var x2 = []byte("another long string")
var x3 = []byte("as many of these as you need to recreate your performance characteristics") That should be easy to create in memory with a plain old for loop. In any case, I'd start there and use pprof profiles to convince yourself that the cpu/memory behavior is similar. If it isn't, tweak the simple generated file until it is. If somehow (which I would find very surprising) the contents of the strings involved matter, just generate random bytes using math/rand. |
Change https://golang.org/cl/125437 mentions this issue: |
@kevinburke I used to use |
Change https://golang.org/cl/127928 mentions this issue: |
Change https://golang.org/cl/153642 mentions this issue: |
For now, this CL adds as a first benchmark the formatting of a 10,000 element array literal. It is easy to add additional test cases as we see fit. name time/op Format/array1-10000-4 26.7ms ± 7% name speed Format/array1-10000-4 2.43MB/s ± 6% name alloc/op Format/array1-10000-4 5.52MB ± 0% name allocs/op Format/array1-10000-4 119k ± 0% Updates #26528. Change-Id: Ic8ec8f70160d122b877740412d4d4406f5f4b345 Reviewed-on: https://go-review.googlesource.com/c/153642 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Previously calling format.Source would take up most of the runtime (see golang/go#26528 for more info). But most of the file is already formatted correctly, we mostly need to use go/format to whitespace align the maps per the Go guidelines. Calling go/format on just this part of the file yields a significant performance improvement by avoiding calling it on the entire body of the file. Use a bytes.Buffer as a file argument instead of an io.Writer which allows using WriteByte and WriteString in places. Avoid calling fmt.Fprintf with no arguments. name old time/op new time/op delta Bindata-4 558ms ± 1% 216ms ± 6% -61.34% (p=0.008 n=5+5) name old speed new speed delta Bindata-4 12.3MB/s ± 1% 31.8MB/s ± 6% +159.03% (p=0.008 n=5+5) name old alloc/op new alloc/op delta Bindata-4 353MB ± 0% 162MB ± 0% -54.21% (p=0.008 n=5+5) name old allocs/op new allocs/op delta Bindata-4 46.1k ± 0% 15.7k ± 0% -66.00% (p=0.008 n=5+5)
I maintain a (fork of a) tool called
go-bindata
that compiles static files into a binary. Often the compiled file is quite large - tens of megabytes are not uncommon. I callformat.Source
on this generated file to avoid gofmt thrash. However, I noticed recently that callingformat.Source
takes 60% of the runtime of the go-bindata program.Currently there are no benchmarks for
format.Source
. It might be good to add some to see if there are quick wins to get that function to return more quickly.A starting point may be the benchmark here: https://github.com/kevinburke/go-bindata/blob/master/benchmark_test.go. Committing an extremely large file to source might not be the best way to do this, though.
The text was updated successfully, but these errors were encountered: