image/gif: write fewer, bigger blocks

The indexed bitmap of a frame is encoded into a GIF by first LZW
compression, and then packaged by a simple block mechanism.  Each block
of up-to-256 bytes starts with one byte, which indicates the size of the
block (0x01-0xff). The sequence of blocks is terminated by a 0x00.

While the format supports it, there is no good reason why any particular
image should be anything but a sequence of 255-byte blocks with one last
block less than 255-bytes.

The old blockWriter implementation would not buffer between Write()s,
meaning if the lzw Writer needs to flush more than one chunk of data via
a Write, multiple short blocks might exist in the middle of a stream.

Separate but related, the old implementation also forces lzw.NewWriter
to allocate a bufio.Writer because the blockWriter is not an
io.ByteWriter itself.  But, even though it doesn't effectively buffer
data between Writes, it does make extra copies of sub-blocks during the
course of writing them to the GIF's writer.

Now, the blockWriter shall continue to use the encoder's [256]byte buf,
but use it to effectively buffer a series of WriteByte calls from the
lzw Writer.  Once a WriteByte fills the buffer, the staged block is
Write()n to the underlying GIF writer.  After the lzw Writer is Closed,
the blockWriter should also be closed, which will flush any remaining
block along with the block terminator.

BenchmarkEncode indicates slight improvements:

name      old time/op    new time/op    delta
Encode-8    7.71ms ± 0%    7.38ms ± 0%   -4.27%  (p=0.008 n=5+5)

name      old speed      new speed      delta
Encode-8   159MB/s ± 0%   167MB/s ± 0%   +4.46%  (p=0.008 n=5+5)

name      old alloc/op   new alloc/op   delta
Encode-8    84.1kB ± 0%    80.0kB ± 0%   -4.94%  (p=0.008 n=5+5)

name      old allocs/op  new allocs/op  delta
Encode-8      9.00 ± 0%      7.00 ± 0%  -22.22%  (p=0.008 n=5+5)

Change-Id: I9eb9367d41d7c3d4d7f0adc9b720fc24fb50006a
Reviewed-on: https://go-review.googlesource.com/68351
Reviewed-by: Nigel Tao <nigeltao@golang.org>

diff --git a/src/image/gif/writer.go b/src/image/gif/writer.go
index fd8463ed08569ff07437f14eb4fcd77332c0870e..f26af8be478be253d8aa9e13462cbdb53f13cb61 100644 (file)
--- a/src/image/gif/writer.go
+++ b/src/image/gif/writer.go
@@ -70,25 +70,54 @@ type blockWriter struct {
        e *encoder
 }
 
-func (b blockWriter) Write(data []byte) (int, error) {
+func (b blockWriter) setup() {
+       b.e.buf[0] = 0
+}
+
+func (b blockWriter) Flush() error {
+       return b.e.err
+}
+
+func (b blockWriter) WriteByte(c byte) error {
        if b.e.err != nil {
-               return 0, b.e.err
+               return b.e.err
        }
-       if len(data) == 0 {
-               return 0, nil
+
+       // Append c to buffered sub-block.
+       b.e.buf[0]++
+       b.e.buf[b.e.buf[0]] = c
+       if b.e.buf[0] < 255 {
+               return nil
        }
-       total := 0
-       for total < len(data) {
-               n := copy(b.e.buf[1:256], data[total:])
-               total += n
-               b.e.buf[0] = uint8(n)
 
-               _, b.e.err = b.e.w.Write(b.e.buf[:n+1])
-               if b.e.err != nil {
-                       return 0, b.e.err
+       // Flush block
+       b.e.write(b.e.buf[:256])
+       b.e.buf[0] = 0
+       return b.e.err
+}
+
+// blockWriter must be an io.Writer for lzw.NewWriter, but this is never
+// actually called.
+func (b blockWriter) Write(data []byte) (int, error) {
+       for i, c := range data {
+               if err := b.WriteByte(c); err != nil {
+                       return i, err
                }
        }
-       return total, b.e.err
+       return len(data), nil
+}
+
+func (b blockWriter) close() {
+       // Write the block terminator (0x00), either by itself, or along with a
+       // pending sub-block.
+       if b.e.buf[0] == 0 {
+               b.e.writeByte(0)
+       } else {
+               n := uint(b.e.buf[0])
+               b.e.buf[n+1] = 0
+               b.e.write(b.e.buf[:n+2])
+       }
+       b.e.flush()
 }
 
 func (e *encoder) flush() {
@@ -301,7 +330,9 @@ func (e *encoder) writeImageBlock(pm *image.Paletted, delay int, disposal byte)
        }
        e.writeByte(uint8(litWidth)) // LZW Minimum Code Size.
 
-       lzww := lzw.NewWriter(blockWriter{e: e}, lzw.LSB, litWidth)
+       bw := blockWriter{e: e}
+       bw.setup()
+       lzww := lzw.NewWriter(bw, lzw.LSB, litWidth)
        if dx := b.Dx(); dx == pm.Stride {
                _, e.err = lzww.Write(pm.Pix[:dx*b.Dy()])
                if e.err != nil {
@@ -317,8 +348,8 @@ func (e *encoder) writeImageBlock(pm *image.Paletted, delay int, disposal byte)
                        }
                }
        }
-       lzww.Close()
-       e.writeByte(0x00) // Block Terminator.
+       lzww.Close() // flush to bw
+       bw.close()   // flush to e.w
 }
 
 // Options are the encoding parameters.

diff --git a/src/image/gif/writer_test.go b/src/image/gif/writer_test.go
index b48e53272e29fc1b548f4cd36c2ea4283cee3a6b..69042ec67439be592f149e61bf5e523cd57a2f2c 100644 (file)
--- a/src/image/gif/writer_test.go
+++ b/src/image/gif/writer_test.go
@@ -64,6 +64,10 @@ func averageDelta(m0, m1 image.Image) int64 {
        return sum / n
 }
 
+// lzw.NewWriter wants an interface which is basically the same thing as gif's
+// writer interface.  This ensures we're compatible.
+var _ writer = blockWriter{}
+
 var testCase = []struct {
        filename  string
        tolerance int64