From 129e827c28965ddfc1ae43d1d325638343b0c40d Mon Sep 17 00:00:00 2001 From: Travis Grigsby Date: Thu, 30 Mar 2023 13:19:25 -0700 Subject: [PATCH 1/8] adding image pool and buffer pool to options --- decoder/decoder.go | 26 +++++----- decoder/decoder_benchmark_test.go | 84 +++++++++++++++++++++++++++++++ decoder/decoder_test.go | 5 +- decoder/options.go | 14 ++++++ 4 files changed, 113 insertions(+), 16 deletions(-) create mode 100644 decoder/decoder_benchmark_test.go diff --git a/decoder/decoder.go b/decoder/decoder.go index 6854560..1f4bcae 100644 --- a/decoder/decoder.go +++ b/decoder/decoder.go @@ -29,6 +29,7 @@ package decoder */ import "C" import ( + "bytes" "errors" "fmt" "image" @@ -47,22 +48,22 @@ type Decoder struct { sPtr C.size_t } -// NewDecoder return new decoder instance func NewDecoder(r io.Reader, options *Options) (d *Decoder, err error) { - var data []byte - - if data, err = io.ReadAll(r); err != nil { - return nil, err + if options == nil { + options = &Options{} } - if len(data) == 0 { - return nil, errors.New("data is empty") + if options.imageFactory == nil { + options.imageFactory = &DefaultImageFactory{} } - if options == nil { - options = &Options{} + buf := bytes.NewBuffer(options.buffer) + + if _, err = io.Copy(buf, r); err != nil { + return nil, err } - d = &Decoder{data: data, options: options} + + d = &Decoder{data: buf.Bytes(), options: options} if d.config, err = d.options.GetConfig(); err != nil { return nil, err @@ -87,10 +88,7 @@ func (d *Decoder) Decode() (image.Image, error) { d.config.output.colorspace = C.MODE_RGBA d.config.output.is_external_memory = 1 - img := image.NewNRGBA(image.Rectangle{Max: image.Point{ - X: int(d.config.output.width), - Y: int(d.config.output.height), - }}) + img := d.options.imageFactory.Get(int(d.config.output.width), int(d.config.output.height)) buff := (*C.WebPRGBABuffer)(unsafe.Pointer(&d.config.output.u[0])) buff.stride = C.int(img.Stride) diff --git a/decoder/decoder_benchmark_test.go b/decoder/decoder_benchmark_test.go new file mode 100644 index 0000000..1eacfad --- /dev/null +++ b/decoder/decoder_benchmark_test.go @@ -0,0 +1,84 @@ +package decoder + +import ( + "bytes" + "image" + "os" + "testing" + + "github.com/kolesa-team/go-webp/pool" +) + +func BenchmarkDecodeOneSizeOnly(b *testing.B) { + b.ResetTimer() + b.ReportAllocs() + + data := loadImage(b) + + nrgbaPool := pool.NewNRGBAOneSizePool(512, 512) + bufferPool := pool.NewBufferPool(512 * 512) + + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + buf := bufferPool.Get() + decoder, err := NewDecoder(bytes.NewReader(data), &Options{imageFactory: nrgbaPool, buffer: buf}) + img, err := decoder.Decode() + if err != nil { + b.Fatal(err) + } + nrgbaPool.Put(img.(*image.NRGBA)) + bufferPool.Put(buf) + } + }) +} + +func loadImage(b *testing.B) []byte { + filename := "../test_data/images/100x150_lossless.webp" + // filename := "../test_data/images/composite.webp" + data, err := os.ReadFile(filename) + if err != nil { + b.Fatal(err) + } + return data +} + +func BenchmarkDecodeAnySize(b *testing.B) { + b.ResetTimer() + b.ReportAllocs() + + data := loadImage(b) + + nrgbaPool := pool.NewNRGBAMultiPool() + bufferPool := pool.NewBufferPool(512 * 512) + + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + buf := bufferPool.Get() + decoder, err := NewDecoder(bytes.NewReader(data), &Options{imageFactory: nrgbaPool, buffer: buf}) + img, err := decoder.Decode() + if err != nil { + b.Fatal(err) + } + nrgbaPool.Put(img.(*image.NRGBA)) + bufferPool.Put(buf) + } + }) +} + +func BenchmarkDecodeOriginal(b *testing.B) { + b.ResetTimer() + b.ReportAllocs() + + data := loadImage(b) + + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + decoder, err := NewDecoder(bytes.NewReader(data), &Options{}) + img, err := decoder.Decode() + if err != nil { + b.Fatal(err) + } + _ = img + } + }) +} diff --git a/decoder/decoder_test.go b/decoder/decoder_test.go index d1ba0ae..b96b267 100644 --- a/decoder/decoder_test.go +++ b/decoder/decoder_test.go @@ -22,11 +22,12 @@ package decoder import ( - "github.com/kolesa-team/go-webp/utils" - "github.com/stretchr/testify/require" "image" "os" "testing" + + "github.com/kolesa-team/go-webp/utils" + "github.com/stretchr/testify/require" ) func TestNewDecoder(t *testing.T) { diff --git a/decoder/options.go b/decoder/options.go index 311a725..73e8775 100644 --- a/decoder/options.go +++ b/decoder/options.go @@ -41,6 +41,10 @@ type Options struct { Flip bool DitheringStrength int AlphaDitheringStrength int + + // These two are optimizations that require a little extra work on the caller side. + imageFactory ImageFactory // if nil, DefaultImageFactory will be used + buffer []byte // temp buffer to store data from reader. If nil, default buffer will be used } // GetConfig build WebPDecoderConfig for libwebp @@ -89,3 +93,13 @@ func (o *Options) GetConfig() (*C.WebPDecoderConfig, error) { return &config, nil } + +type ImageFactory interface { + Get(width, height int) *image.NRGBA +} + +type DefaultImageFactory struct{} + +func (d *DefaultImageFactory) Get(width, height int) *image.NRGBA { + return image.NewNRGBA(image.Rect(0, 0, width, height)) +} From 1d3590de04970a7e000552d804894a45be55a567 Mon Sep 17 00:00:00 2001 From: Travis Grigsby Date: Thu, 30 Mar 2023 13:32:38 -0700 Subject: [PATCH 2/8] simplifying after some more testing --- decoder/decoder_benchmark_test.go | 38 +++++++------------------------ 1 file changed, 8 insertions(+), 30 deletions(-) diff --git a/decoder/decoder_benchmark_test.go b/decoder/decoder_benchmark_test.go index 1eacfad..0a19205 100644 --- a/decoder/decoder_benchmark_test.go +++ b/decoder/decoder_benchmark_test.go @@ -9,32 +9,8 @@ import ( "github.com/kolesa-team/go-webp/pool" ) -func BenchmarkDecodeOneSizeOnly(b *testing.B) { - b.ResetTimer() - b.ReportAllocs() - - data := loadImage(b) - - nrgbaPool := pool.NewNRGBAOneSizePool(512, 512) - bufferPool := pool.NewBufferPool(512 * 512) - - b.RunParallel(func(pb *testing.PB) { - for pb.Next() { - buf := bufferPool.Get() - decoder, err := NewDecoder(bytes.NewReader(data), &Options{imageFactory: nrgbaPool, buffer: buf}) - img, err := decoder.Decode() - if err != nil { - b.Fatal(err) - } - nrgbaPool.Put(img.(*image.NRGBA)) - bufferPool.Put(buf) - } - }) -} - func loadImage(b *testing.B) []byte { filename := "../test_data/images/100x150_lossless.webp" - // filename := "../test_data/images/composite.webp" data, err := os.ReadFile(filename) if err != nil { b.Fatal(err) @@ -42,30 +18,32 @@ func loadImage(b *testing.B) []byte { return data } -func BenchmarkDecodeAnySize(b *testing.B) { +func BenchmarkDecodePooled(b *testing.B) { b.ResetTimer() b.ReportAllocs() data := loadImage(b) - nrgbaPool := pool.NewNRGBAMultiPool() - bufferPool := pool.NewBufferPool(512 * 512) + imagePool := pool.NewSimplePool() + bufferPool := pool.NewBufferPool() b.RunParallel(func(pb *testing.PB) { for pb.Next() { buf := bufferPool.Get() - decoder, err := NewDecoder(bytes.NewReader(data), &Options{imageFactory: nrgbaPool, buffer: buf}) + decoder, err := NewDecoder(bytes.NewReader(data), &Options{imageFactory: imagePool, buffer: buf}) img, err := decoder.Decode() if err != nil { b.Fatal(err) } - nrgbaPool.Put(img.(*image.NRGBA)) + + // put everything back + imagePool.Put(img.(*image.NRGBA)) bufferPool.Put(buf) } }) } -func BenchmarkDecodeOriginal(b *testing.B) { +func BenchmarkDecodeUnPooled(b *testing.B) { b.ResetTimer() b.ReportAllocs() From 9d9f7c0beb3ddefb4abb668da96917354fc2c93f Mon Sep 17 00:00:00 2001 From: Travis Grigsby Date: Thu, 30 Mar 2023 13:33:57 -0700 Subject: [PATCH 3/8] moving some things around --- decoder/decoder_benchmark_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/decoder/decoder_benchmark_test.go b/decoder/decoder_benchmark_test.go index 0a19205..65cd85c 100644 --- a/decoder/decoder_benchmark_test.go +++ b/decoder/decoder_benchmark_test.go @@ -5,8 +5,6 @@ import ( "image" "os" "testing" - - "github.com/kolesa-team/go-webp/pool" ) func loadImage(b *testing.B) []byte { @@ -24,8 +22,8 @@ func BenchmarkDecodePooled(b *testing.B) { data := loadImage(b) - imagePool := pool.NewSimplePool() - bufferPool := pool.NewBufferPool() + imagePool := NewImagePool() + bufferPool := NewBufferPool() b.RunParallel(func(pb *testing.PB) { for pb.Next() { From 75612fbfbebf1998fad0a2ea6f1afa7e72b862f8 Mon Sep 17 00:00:00 2001 From: Travis Grigsby Date: Thu, 30 Mar 2023 13:46:16 -0700 Subject: [PATCH 4/8] adding some comments --- decoder/options.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/decoder/options.go b/decoder/options.go index 73e8775..b419cec 100644 --- a/decoder/options.go +++ b/decoder/options.go @@ -43,8 +43,14 @@ type Options struct { AlphaDitheringStrength int // These two are optimizations that require a little extra work on the caller side. - imageFactory ImageFactory // if nil, DefaultImageFactory will be used - buffer []byte // temp buffer to store data from reader. If nil, default buffer will be used + + // if nil, DefaultImageFactory will be used. If non-nil, decode will return an image that must be put back into the pool + // when you're done with it + imageFactory ImageFactory + // if nil, a default buffer will be used. If non-nil, decode will use this buffer to store data from the reader. + // The idea is that this buffer be reused, so either pass this back in next time you call decode, or put it back into + // a pool when you're done with it. + buffer []byte } // GetConfig build WebPDecoderConfig for libwebp From 8bb1218ba818913ca3d5be3f4c41276757e85df5 Mon Sep 17 00:00:00 2001 From: Travis Grigsby Date: Thu, 30 Mar 2023 13:47:40 -0700 Subject: [PATCH 5/8] moving things around --- decoder/pool.go | 75 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 decoder/pool.go diff --git a/decoder/pool.go b/decoder/pool.go new file mode 100644 index 0000000..e6e03dd --- /dev/null +++ b/decoder/pool.go @@ -0,0 +1,75 @@ +package decoder + +import ( + "image" + "sync" + "sync/atomic" +) + +type ImagePool struct { + poolMap map[int]*sync.Pool + lock *sync.Mutex + Count int64 +} + +func NewImagePool() *ImagePool { + return &ImagePool{ + poolMap: make(map[int]*sync.Pool), + lock: &sync.Mutex{}, + } +} + +func (n *ImagePool) Get(width, height int) *image.NRGBA { + dimPool := n.getPool(width, height) + + img := dimPool.Get().(*image.NRGBA) + img.Rect.Max.X = width + img.Rect.Max.Y = height + return img +} + +func (n *ImagePool) getPool(width int, height int) *sync.Pool { + dim := width * height + + n.lock.Lock() + dimPool, ok := n.poolMap[dim] + if !ok { + atomic.AddInt64(&n.Count, 1) + dimPool = &sync.Pool{ + New: func() interface{} { + return image.NewNRGBA(image.Rect(0, 0, width, height)) + }, + } + n.poolMap[dim] = dimPool + } + n.lock.Unlock() + return dimPool +} + +func (n *ImagePool) Put(img *image.NRGBA) { + dimPool := n.getPool(img.Rect.Dx(), img.Rect.Dy()) + dimPool.Put(img) +} + +type BufferPool struct { + pool *sync.Pool // pointer because noCopy +} + +func NewBufferPool() *BufferPool { + return &BufferPool{ + pool: &sync.Pool{ + New: func() interface{} { + return make([]byte, 0, 1024) + }, + }, + } +} + +func (b *BufferPool) Get() []byte { + return b.pool.Get().([]byte) +} + +func (b *BufferPool) Put(buf []byte) { + buf = buf[:0] + b.pool.Put(buf) +} From 26d0161efa3df50f702ae71ba6cdda18292d3170 Mon Sep 17 00:00:00 2001 From: Travis Grigsby Date: Thu, 30 Mar 2023 13:52:37 -0700 Subject: [PATCH 6/8] woops --- decoder/decoder.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/decoder/decoder.go b/decoder/decoder.go index 1f4bcae..eae2356 100644 --- a/decoder/decoder.go +++ b/decoder/decoder.go @@ -63,6 +63,10 @@ func NewDecoder(r io.Reader, options *Options) (d *Decoder, err error) { return nil, err } + if len(buf.Bytes()) == 0 { + return nil, errors.New("data is empty") + } + d = &Decoder{data: buf.Bytes(), options: options} if d.config, err = d.options.GetConfig(); err != nil { From 2776c42412a74b09ed1b280a129b1b650bac5f70 Mon Sep 17 00:00:00 2001 From: Travis Grigsby Date: Thu, 30 Mar 2023 15:20:33 -0700 Subject: [PATCH 7/8] exposing private fields --- decoder/options.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/decoder/options.go b/decoder/options.go index b419cec..250b5e7 100644 --- a/decoder/options.go +++ b/decoder/options.go @@ -46,11 +46,11 @@ type Options struct { // if nil, DefaultImageFactory will be used. If non-nil, decode will return an image that must be put back into the pool // when you're done with it - imageFactory ImageFactory + ImageFactory ImageFactory // if nil, a default buffer will be used. If non-nil, decode will use this buffer to store data from the reader. // The idea is that this buffer be reused, so either pass this back in next time you call decode, or put it back into // a pool when you're done with it. - buffer []byte + Buffer []byte } // GetConfig build WebPDecoderConfig for libwebp From a88289c4e5c447f18f3539488cbc71da7f3e1a26 Mon Sep 17 00:00:00 2001 From: Travis Grigsby Date: Thu, 30 Mar 2023 15:40:17 -0700 Subject: [PATCH 8/8] whoops --- decoder/decoder.go | 8 ++++---- decoder/decoder_benchmark_test.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/decoder/decoder.go b/decoder/decoder.go index eae2356..77f43f7 100644 --- a/decoder/decoder.go +++ b/decoder/decoder.go @@ -53,11 +53,11 @@ func NewDecoder(r io.Reader, options *Options) (d *Decoder, err error) { options = &Options{} } - if options.imageFactory == nil { - options.imageFactory = &DefaultImageFactory{} + if options.ImageFactory == nil { + options.ImageFactory = &DefaultImageFactory{} } - buf := bytes.NewBuffer(options.buffer) + buf := bytes.NewBuffer(options.Buffer) if _, err = io.Copy(buf, r); err != nil { return nil, err @@ -92,7 +92,7 @@ func (d *Decoder) Decode() (image.Image, error) { d.config.output.colorspace = C.MODE_RGBA d.config.output.is_external_memory = 1 - img := d.options.imageFactory.Get(int(d.config.output.width), int(d.config.output.height)) + img := d.options.ImageFactory.Get(int(d.config.output.width), int(d.config.output.height)) buff := (*C.WebPRGBABuffer)(unsafe.Pointer(&d.config.output.u[0])) buff.stride = C.int(img.Stride) diff --git a/decoder/decoder_benchmark_test.go b/decoder/decoder_benchmark_test.go index 65cd85c..04eefd7 100644 --- a/decoder/decoder_benchmark_test.go +++ b/decoder/decoder_benchmark_test.go @@ -28,7 +28,7 @@ func BenchmarkDecodePooled(b *testing.B) { b.RunParallel(func(pb *testing.PB) { for pb.Next() { buf := bufferPool.Get() - decoder, err := NewDecoder(bytes.NewReader(data), &Options{imageFactory: imagePool, buffer: buf}) + decoder, err := NewDecoder(bytes.NewReader(data), &Options{ImageFactory: imagePool, Buffer: buf}) img, err := decoder.Decode() if err != nil { b.Fatal(err)