mirror of
https://github.com/gohugoio/hugo.git
synced 2024-11-07 20:30:36 -05:00
resource: Fix multi-threaded image processing issue
When doing something like this with the same image from a partial used in, say, both the home page and the single page: ```bash {{ with $img }} {{ $big := .Fill "1024x512 top" }} {{ $small := $big.Resize "512x" }} {{ end }} ``` There would be timing issues making Hugo in some cases try to process the same image with the same instructions in parallel. You would experience errors of type: ```bash png: invalid format: not enough pixel data ``` This commit works around that by adding a mutex per image. This should also improve the performance, sligthly, as it avoids duplicate work. The current workaround before this fix is to always operate on the original: ```bash {{ with $img }} {{ $big := .Fill "1024x512 top" }} {{ $small := .Fill "512x256 top" }} {{ end }} ``` Fixes #4404
This commit is contained in:
parent
53dac9a506
commit
58382e9572
4 changed files with 120 additions and 5 deletions
|
@ -112,6 +112,9 @@ type Image struct {
|
||||||
|
|
||||||
copyToDestinationInit sync.Once
|
copyToDestinationInit sync.Once
|
||||||
|
|
||||||
|
// Lock used when creating alternate versions of this image.
|
||||||
|
createMu sync.Mutex
|
||||||
|
|
||||||
imaging *Imaging
|
imaging *Imaging
|
||||||
|
|
||||||
hash string
|
hash string
|
||||||
|
|
|
@ -28,7 +28,8 @@ type imageCache struct {
|
||||||
absCacheDir string
|
absCacheDir string
|
||||||
pathSpec *helpers.PathSpec
|
pathSpec *helpers.PathSpec
|
||||||
mu sync.RWMutex
|
mu sync.RWMutex
|
||||||
store map[string]*Image
|
|
||||||
|
store map[string]*Image
|
||||||
}
|
}
|
||||||
|
|
||||||
func (c *imageCache) isInCache(key string) bool {
|
func (c *imageCache) isInCache(key string) bool {
|
||||||
|
@ -69,6 +70,11 @@ func (c *imageCache) getOrCreate(
|
||||||
}
|
}
|
||||||
|
|
||||||
// Now look in the file cache.
|
// Now look in the file cache.
|
||||||
|
// Multiple Go routines can invoke same operation on the same image, so
|
||||||
|
// we need to make sure this is serialized per source image.
|
||||||
|
parent.createMu.Lock()
|
||||||
|
defer parent.createMu.Unlock()
|
||||||
|
|
||||||
cacheFilename := filepath.Join(c.absCacheDir, key)
|
cacheFilename := filepath.Join(c.absCacheDir, key)
|
||||||
|
|
||||||
// The definition of this counter is not that we have processed that amount
|
// The definition of this counter is not that we have processed that amount
|
||||||
|
|
|
@ -15,8 +15,12 @@ package resource
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"math/rand"
|
||||||
|
"strconv"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
|
"sync"
|
||||||
|
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
@ -141,6 +145,51 @@ func TestImageTransformLongFilename(t *testing.T) {
|
||||||
assert.Equal("/a/_hu59e56ffff1bc1d8d122b1403d34e039f_90587_c876768085288f41211f768147ba2647.jpg", resized.RelPermalink())
|
assert.Equal("/a/_hu59e56ffff1bc1d8d122b1403d34e039f_90587_c876768085288f41211f768147ba2647.jpg", resized.RelPermalink())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestImageTransformConcurrent(t *testing.T) {
|
||||||
|
|
||||||
|
var wg sync.WaitGroup
|
||||||
|
|
||||||
|
assert := require.New(t)
|
||||||
|
|
||||||
|
spec := newTestResourceOsFs(assert)
|
||||||
|
|
||||||
|
image := fetchImageForSpec(spec, assert, "sunset.jpg")
|
||||||
|
|
||||||
|
for i := 0; i < 4; i++ {
|
||||||
|
wg.Add(1)
|
||||||
|
go func(id int) {
|
||||||
|
defer wg.Done()
|
||||||
|
for j := 0; j < 5; j++ {
|
||||||
|
img := image
|
||||||
|
for k := 0; k < 2; k++ {
|
||||||
|
r1, err := img.Resize(fmt.Sprintf("%dx", id-k))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if r1.Width() != id-k {
|
||||||
|
t.Fatalf("Width: %d:%d", r1.Width(), j)
|
||||||
|
}
|
||||||
|
|
||||||
|
r2, err := r1.Resize(fmt.Sprintf("%dx", id-k-1))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
_, err = r2.decodeSource()
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal("Err decode:", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
img = r1
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}(i + 20)
|
||||||
|
}
|
||||||
|
|
||||||
|
wg.Wait()
|
||||||
|
}
|
||||||
|
|
||||||
func TestDecodeImaging(t *testing.T) {
|
func TestDecodeImaging(t *testing.T) {
|
||||||
assert := require.New(t)
|
assert := require.New(t)
|
||||||
m := map[string]interface{}{
|
m := map[string]interface{}{
|
||||||
|
@ -208,3 +257,22 @@ func TestImageWithMetadata(t *testing.T) {
|
||||||
assert.Equal("Sunset #1", resized.Name())
|
assert.Equal("Sunset #1", resized.Name())
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func BenchmarkResizeParallel(b *testing.B) {
|
||||||
|
assert := require.New(b)
|
||||||
|
img := fetchSunset(assert)
|
||||||
|
|
||||||
|
b.RunParallel(func(pb *testing.PB) {
|
||||||
|
for pb.Next() {
|
||||||
|
w := rand.Intn(10) + 10
|
||||||
|
resized, err := img.Resize(strconv.Itoa(w) + "x")
|
||||||
|
if err != nil {
|
||||||
|
b.Fatal(err)
|
||||||
|
}
|
||||||
|
_, err = resized.Resize(strconv.Itoa(w-1) + "x")
|
||||||
|
if err != nil {
|
||||||
|
b.Fatal(err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
|
@ -6,8 +6,11 @@ import (
|
||||||
|
|
||||||
"image"
|
"image"
|
||||||
"io"
|
"io"
|
||||||
|
"io/ioutil"
|
||||||
"os"
|
"os"
|
||||||
"path"
|
"path"
|
||||||
|
"runtime"
|
||||||
|
"strings"
|
||||||
|
|
||||||
"github.com/gohugoio/hugo/helpers"
|
"github.com/gohugoio/hugo/helpers"
|
||||||
"github.com/gohugoio/hugo/hugofs"
|
"github.com/gohugoio/hugo/hugofs"
|
||||||
|
@ -45,17 +48,53 @@ func newTestResourceSpecForBaseURL(assert *require.Assertions, baseURL string) *
|
||||||
return spec
|
return spec
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func newTestResourceOsFs(assert *require.Assertions) *Spec {
|
||||||
|
cfg := viper.New()
|
||||||
|
cfg.Set("baseURL", "https://example.com")
|
||||||
|
|
||||||
|
workDir, err := ioutil.TempDir("", "hugores")
|
||||||
|
|
||||||
|
if runtime.GOOS == "darwin" && !strings.HasPrefix(workDir, "/private") {
|
||||||
|
// To get the entry folder in line with the rest. This its a little bit
|
||||||
|
// mysterious, but so be it.
|
||||||
|
workDir = "/private" + workDir
|
||||||
|
}
|
||||||
|
|
||||||
|
contentDir := "base"
|
||||||
|
cfg.Set("workingDir", workDir)
|
||||||
|
cfg.Set("contentDir", contentDir)
|
||||||
|
cfg.Set("resourceDir", filepath.Join(workDir, "res"))
|
||||||
|
|
||||||
|
fs := hugofs.NewFrom(hugofs.Os, cfg)
|
||||||
|
fs.Destination = &afero.MemMapFs{}
|
||||||
|
|
||||||
|
s, err := helpers.NewPathSpec(fs, cfg)
|
||||||
|
|
||||||
|
assert.NoError(err)
|
||||||
|
|
||||||
|
spec, err := NewSpec(s, media.DefaultTypes)
|
||||||
|
assert.NoError(err)
|
||||||
|
return spec
|
||||||
|
|
||||||
|
}
|
||||||
|
|
||||||
func fetchSunset(assert *require.Assertions) *Image {
|
func fetchSunset(assert *require.Assertions) *Image {
|
||||||
return fetchImage(assert, "sunset.jpg")
|
return fetchImage(assert, "sunset.jpg")
|
||||||
}
|
}
|
||||||
|
|
||||||
func fetchImage(assert *require.Assertions, name string) *Image {
|
func fetchImage(assert *require.Assertions, name string) *Image {
|
||||||
|
spec := newTestResourceSpec(assert)
|
||||||
|
return fetchImageForSpec(spec, assert, name)
|
||||||
|
}
|
||||||
|
|
||||||
|
func fetchImageForSpec(spec *Spec, assert *require.Assertions, name string) *Image {
|
||||||
src, err := os.Open("testdata/" + name)
|
src, err := os.Open("testdata/" + name)
|
||||||
assert.NoError(err)
|
assert.NoError(err)
|
||||||
|
|
||||||
spec := newTestResourceSpec(assert)
|
workingDir := spec.Cfg.GetString("workingDir")
|
||||||
|
f := filepath.Join(workingDir, name)
|
||||||
|
|
||||||
out, err := spec.Fs.Source.Create("/b/" + name)
|
out, err := spec.Fs.Source.Create(f)
|
||||||
assert.NoError(err)
|
assert.NoError(err)
|
||||||
_, err = io.Copy(out, src)
|
_, err = io.Copy(out, src)
|
||||||
out.Close()
|
out.Close()
|
||||||
|
@ -66,11 +105,10 @@ func fetchImage(assert *require.Assertions, name string) *Image {
|
||||||
return path.Join("/a", s)
|
return path.Join("/a", s)
|
||||||
}
|
}
|
||||||
|
|
||||||
r, err := spec.NewResourceFromFilename(factory, "/public", "/b/"+name, name)
|
r, err := spec.NewResourceFromFilename(factory, "/public", f, name)
|
||||||
assert.NoError(err)
|
assert.NoError(err)
|
||||||
assert.IsType(&Image{}, r)
|
assert.IsType(&Image{}, r)
|
||||||
return r.(*Image)
|
return r.(*Image)
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func assertFileCache(assert *require.Assertions, fs *hugofs.Fs, filename string, width, height int) {
|
func assertFileCache(assert *require.Assertions, fs *hugofs.Fs, filename string, width, height int) {
|
||||||
|
|
Loading…
Reference in a new issue