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: Robert Griesemer <gri@golang.org>

diff --git a/src/text/tabwriter/tabwriter.go b/src/text/tabwriter/tabwriter.go
index 36d999b411ee0bea1d6f5287f90abcefe83763c3..bd45cddecb75de368ed6ddc7152e139206cbbfde 100644 (file)
--- a/src/text/tabwriter/tabwriter.go
+++ b/src/text/tabwriter/tabwriter.go
@@ -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 {

diff --git a/src/text/tabwriter/tabwriter_test.go b/src/text/tabwriter/tabwriter_test.go
index 07bae0ca0b2bf673d28a2510f3b122803e3dce0f..6a97d4c4273d4248e6886955fd0f82b0a105c4ff 100644 (file)
--- a/src/text/tabwriter/tabwriter_test.go
+++ b/src/text/tabwriter/tabwriter_test.go
@@ -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()
+       }
+}