Commit c4078a19 authored by Daniel Martí's avatar Daniel Martí Committed by Robert Griesemer

text/tabwriter: use a single defer per Write call

Lines with single cells prompt a flush. Unfortunately, a call to
Writer.Flush also means two defers, which is an expensive operation to
do if many lines consist of single cells.

This is common when formatting code with aligned comments. Most lines
aren't going to have any comments at all, so the performance hit is
going to be noticeable.

The Write method already has a "defer handlePanic" of its own, so we
don't need to worry about panics leaking out. The error will now mention
"Write" instead of "Flush" if a panic is encountered during that nested
flush, but arguably that's a good thing; the user called Write, not
Flush.

For the reset call, add a non-deferred call as part of flushNoDefers, as
that's still necessary. Otherwise, the exported Flush method still does
a "defer b.reset".

The current tabwriter benchmarks are unaffected, since they don't
contain many single-cell lines, and because lines are written one at a
time. For that reason, we add a benchmark which has both of these
characteristics.

name    old time/op    new time/op    delta
Code-8    2.72µs ± 0%    1.77µs ± 0%  -34.88%  (p=0.000 n=6+5)

name    old alloc/op   new alloc/op   delta
Code-8      648B ± 0%      648B ± 0%     ~     (all equal)

name    old allocs/op  new allocs/op  delta
Code-8      13.0 ± 0%      13.0 ± 0%     ~     (all equal)

Perhaps unsurprisingly, go/printer also gets a bit faster, as it too
buffers its output before writing it to tabwriter.

name     old time/op  new time/op  delta
Print-8  6.53ms ± 0%  6.39ms ± 0%  -2.22%  (p=0.008 n=5+5)

Change-Id: Ie01fea5ced43886a9eb796cb1e6c810f7a810853
Reviewed-on: https://go-review.googlesource.com/c/go/+/166797
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarRobert Griesemer <gri@golang.org>
parent 01d1dc41
......@@ -473,8 +473,12 @@ func (b *Writer) terminateCell(htab bool) int {
return len(*line)
}
func handlePanic(err *error, op string) {
func (b *Writer) handlePanic(err *error, op string) {
if e := recover(); e != nil {
if op == "Flush" {
// If Flush ran into a panic, we still need to reset.
b.reset()
}
if nerr, ok := e.(osError); ok {
*err = nerr.err
return
......@@ -491,10 +495,17 @@ func (b *Writer) Flush() error {
return b.flush()
}
// flush is the internal version of Flush, with a named return value which we
// don't want to expose.
func (b *Writer) flush() (err error) {
defer b.reset() // even in the presence of errors
defer handlePanic(&err, "Flush")
defer b.handlePanic(&err, "Flush")
return b.flushNoDefers()
}
// flushNoDefers is like flush, but without a deferred handlePanic call. This
// can be called from other methods which already have their own deferred
// handlePanic calls, such as Write, and avoid the extra defer work.
func (b *Writer) flushNoDefers() (err error) {
// add current cell if not empty
if b.cell.size > 0 {
if b.endChar != 0 {
......@@ -506,6 +517,7 @@ func (b *Writer) flush() (err error) {
// format contents of buffer
b.format(0, 0, len(b.lines))
b.reset()
return nil
}
......@@ -516,7 +528,7 @@ var hbar = []byte("---\n")
// while writing to the underlying output stream.
//
func (b *Writer) Write(buf []byte) (n int, err error) {
defer handlePanic(&err, "Write")
defer b.handlePanic(&err, "Write")
// split text into cells
n = 0
......@@ -539,7 +551,7 @@ func (b *Writer) Write(buf []byte) (n int, err error) {
// the formatting of the following lines (the last cell per
// line is ignored by format()), thus we can flush the
// Writer contents.
if err = b.Flush(); err != nil {
if err = b.flushNoDefers(); err != nil {
return
}
if ch == '\f' && b.flags&Debug != 0 {
......
......@@ -729,3 +729,27 @@ func BenchmarkRagged(b *testing.B) {
})
}
}
const codeSnippet = `
some command
foo # aligned
barbaz # comments
but
mostly
single
cell
lines
`
func BenchmarkCode(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
w := NewWriter(ioutil.Discard, 4, 4, 1, ' ', 0) // no particular reason for these settings
// The code is small, so it's reasonable for the tabwriter user
// to write it all at once, or buffer the writes.
w.Write([]byte(codeSnippet))
w.Flush()
}
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment