Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

image/gif: Decode ingests all frames into memory despite only returning the first one #22199

Closed
artyom opened this issue Oct 10, 2017 · 2 comments
Closed

Comments

@artyom
Copy link
Member

@artyom artyom commented Oct 10, 2017

Please answer these questions before submitting your issue. Thanks!

What did you do?

  1. Save https://play.golang.org/p/6d9xGk8S6f and run it locally to produce bad.gif — 2.5Mb GIF file 1000x1000px and 1000 frames.

  2. Save https://play.golang.org/p/SSLx-1wd9N, compile and run it providing bad.gif as first argument, optionally getting its rusage (i.e. with time(1)):

     command time -l ./main bad.gif
    

What did you expect to see?

Decent memory usage, since gif.Decode is documented to only return image for the first frame:

Decode reads a GIF image from r and returns the first embedded image as an image.Image.

What did you see instead?

High memory usage because gif.decoder.decode saves all frames in memory despite being called from gif.Decode that only needs the first frame:

go/src/image/gif/reader.go

Lines 282 to 284 in b80029c

d.image = append(d.image, m)
d.delay = append(d.delay, d.delayTime)
d.disposal = append(d.disposal, d.disposalMethod)

On macOS time -l shows it used more than 800 Mb RSS:

 917983232  maximum resident set size

I have a straightforward optimization for this by making gif.decoder.decode() only store the first frame to gif.decoder.image slice when appropriate (gif.Decode call) while still decoding file to the end so any malformed input is properly handled. Please let me know if this approach is ok and I'll submit a CL.

Memory usage reported with the modifications applied is close to what I'd expect to see for a single image:

  60616704  maximum resident set size

System details

go version go1.9.1 darwin/amd64
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/tmp/go:/Users/artyom/go"
GORACE=""
GOROOT="/Users/artyom/Library/go"
GOTOOLDIR="/Users/artyom/Library/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/lb/3rk8rqs53czgb4v35w_342xc0000gn/T/go-build415198232=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOROOT/bin/go version: go version go1.9.1 darwin/amd64
GOROOT/bin/go tool compile -V: compile version go1.9.1
uname -v: Darwin Kernel Version 17.0.0: Thu Aug 24 21:48:19 PDT 2017; root:xnu-4570.1.46~2/RELEASE_X86_64
ProductName:	Mac OS X
ProductVersion:	10.13
BuildVersion:	17A405
lldb --version: lldb-900.0.45
  Swift-4.0
@artyom
Copy link
Member Author

@artyom artyom commented Oct 10, 2017

/cc @nigeltao

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 11, 2017

Change https://golang.org/cl/69890 mentions this issue: image/gif: make Decode only keep the first frame in memory

@gopherbot gopherbot closed this in 9ce43ce Oct 12, 2017
@golang golang locked and limited conversation to collaborators Oct 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.