From 56510593ff66f901c5144f3c980c6c433ddb7859 Mon Sep 17 00:00:00 2001 From: Bill Thiede Date: Sun, 5 Jan 2014 17:49:59 -0800 Subject: [PATCH] pkg/server: set thumbnail RAM limit per-handler. Move from one pool for all thumbnail resizing to per-ImageHandler instace. Uses the value of "maxResizeBytes" if specified in the "handlerArgs" section of a "publish" or "ui" handler's low-level config. If we end up wanting to share pools, i.e. if all published handlers should share a pool, we'll need to do a bit more work. Implements suggestion from https://camlistore.org/r/1803 review. Further improves on https://camlistore.org/issue/316. Change-Id: Ia93fad119b546064173ac3e2c7f0ab8509744628 --- pkg/server/image.go | 36 ++++++++++++++++++++---------------- pkg/server/publish.go | 9 +++++++++ pkg/server/ui.go | 6 ++++++ 3 files changed, 35 insertions(+), 16 deletions(-) diff --git a/pkg/server/image.go b/pkg/server/image.go index e8963501d..7b7533e94 100644 --- a/pkg/server/image.go +++ b/pkg/server/image.go @@ -44,7 +44,17 @@ import ( _ "camlistore.org/third_party/github.com/nf/cr2" ) -const imageDebug = false +const ( + imageDebug = false + // This is the default maximum concurrent number of bytes we allocate for + // uncompressed pixel data while generating thumbnails. + // If a single image is larger than the configured size for an + // ImageHandler, we'll never successfully resize it. + // 256M is a max image of ~9.5kx9.5k*3. + // TODO(wathiede) move to pkg/constants when https://camlistore.org/r/1536 + // lands. + defaultMaxResizeBytes = 256 << 20 +) var ( imageBytesServedVar = expvar.NewInt("image-bytes-served") @@ -57,6 +67,7 @@ type ImageHandler struct { MaxWidth, MaxHeight int Square bool thumbMeta *thumbMeta // optional cache for scaled images + resizeSem *syncutil.Sem } func (ih *ImageHandler) storageSeekFetcher() blob.SeekFetcher { @@ -163,12 +174,6 @@ func (ih *ImageHandler) scaledCached(buf *bytes.Buffer, file blob.Ref) (format s // Gate the number of concurrent image resizes to limit RAM & CPU use. -// This is the maximum concurrent number of bytes we allocate for uncompressed -// pixel data while generating thumbnails. -const maxResizeBytes = 256 << 20 - -var resizeSem = syncutil.NewSem(maxResizeBytes) - type formatAndImage struct { format string image []byte @@ -221,19 +226,18 @@ func (ih *ImageHandler) scaleImage(fileRef blob.Ref) (*formatAndImage, error) { // TODO(wathiede): build a size table keyed by conf.ColorModel for // common color models for a more exact size estimate. - // This value is an estimate of the memory required to decode an image, - // for YCbCr images, i.e. JPEGs, it will often be higher, for RGBA PNGs - // it is low. + // This value is an estimate of the memory required to decode an image. + // PNGs range from 1-64 bits per pixel (not all of which are supported by + // the Go standard parser). JPEGs encoded in YCbCr 4:4:4 are 3 byte/pixel. + // For all other JPEGs this is an overestimate. For GIFs it is 3x larger + // than needed. How accurate this estimate is depends on the mix of + // images being resized concurrently. ramSize := int64(conf.Width) * int64(conf.Height) * 3 - // If a single image is larger than maxResizeBytes can hold, we'll never - // successfully resize it. - // TODO(wathiede): do we need a more graceful fallback? 256M is a max - // image of ~9.5kx9.5k*3. - if err = resizeSem.Acquire(ramSize); err != nil { + if err = ih.resizeSem.Acquire(ramSize); err != nil { return nil, err } - defer resizeSem.Release(ramSize) + defer ih.resizeSem.Release(ramSize) i, imConfig, err := images.Decode(tr, &images.DecodeOpts{ MaxWidth: ih.MaxWidth, diff --git a/pkg/server/publish.go b/pkg/server/publish.go index c2d2fe753..ee881b267 100644 --- a/pkg/server/publish.go +++ b/pkg/server/publish.go @@ -46,6 +46,7 @@ import ( "camlistore.org/pkg/publish" "camlistore.org/pkg/schema" "camlistore.org/pkg/search" + "camlistore.org/pkg/syncutil" "camlistore.org/pkg/types/camtypes" uistatic "camlistore.org/server/camlistored/ui" ) @@ -58,6 +59,8 @@ type PublishHandler struct { Storage blobserver.Storage // of blobRoot Cache blobserver.Storage // or nil + // Limit peak RAM used by concurrent image thumbnail calls. + resizeSem *syncutil.Sem thumbMeta *thumbMeta // optional cache of scaled images CSSFiles []string @@ -98,6 +101,7 @@ func newPublishFromConfig(ld blobserver.Loader, conf jsonconfig.Obj) (h http.Han bootstrapSignRoot := conf.OptionalString("devBootstrapPermanodeUsing", "") rootNode := conf.OptionalList("rootPermanode") ph.sourceRoot = conf.OptionalString("sourceRoot", "") + ph.resizeSem = syncutil.NewSem(int64(conf.OptionalInt("maxResizeBytes", defaultMaxResizeBytes))) if err = conf.Validate(); err != nil { return } @@ -303,6 +307,9 @@ type publishRequest struct { // A describe request that we can reuse, sharing its map of // blobs already described. dr *search.DescribeRequest + + // Limit peak RAM used by concurrent image thumbnail calls. + resizeSem *syncutil.Sem } func (ph *PublishHandler) NewRequest(rw http.ResponseWriter, req *http.Request) *publishRequest { @@ -326,6 +333,7 @@ func (ph *PublishHandler) NewRequest(rw http.ResponseWriter, req *http.Request) dr: ph.Search.NewDescribeRequest(), inSubjectChain: make(map[string]bool), subjectBasePath: "", + resizeSem: ph.resizeSem, } } @@ -941,6 +949,7 @@ func (pr *publishRequest) serveScaledImage(des *search.DescribedBlob, maxWidth, MaxHeight: maxHeight, Square: square, thumbMeta: pr.ph.thumbMeta, + resizeSem: pr.resizeSem, } th.ServeHTTP(pr.rw, pr.req, fileref) } diff --git a/pkg/server/ui.go b/pkg/server/ui.go index ba9026473..1026cbab1 100644 --- a/pkg/server/ui.go +++ b/pkg/server/ui.go @@ -38,6 +38,7 @@ import ( "camlistore.org/pkg/misc/closure" "camlistore.org/pkg/search" "camlistore.org/pkg/sorted" + "camlistore.org/pkg/syncutil" uistatic "camlistore.org/server/camlistored/ui" closurestatic "camlistore.org/server/camlistored/ui/closure" glitchstatic "camlistore.org/third_party/glitch" @@ -83,6 +84,8 @@ type UIHandler struct { // caching image thumbnails and other emphemeral data. Cache blobserver.Storage // or nil + // Limit peak RAM used by concurrent image thumbnail calls. + resizeSem *syncutil.Sem thumbMeta *thumbMeta // optional thumbnail key->blob.Ref cache // sourceRoot optionally specifies the path to root of Camlistore's @@ -116,6 +119,8 @@ func uiFromConfig(ld blobserver.Loader, conf jsonconfig.Obj) (h http.Handler, er prefix: ld.MyPrefix(), JSONSignRoot: conf.OptionalString("jsonSignRoot", ""), sourceRoot: conf.OptionalString("sourceRoot", ""), + resizeSem: syncutil.NewSem(int64(conf.OptionalInt("maxResizeBytes", + defaultMaxResizeBytes))), } pubRoots := conf.OptionalList("publishRoots") cachePrefix := conf.OptionalString("cache", "") @@ -512,6 +517,7 @@ func (ui *UIHandler) serveThumbnail(rw http.ResponseWriter, req *http.Request) { MaxWidth: width, MaxHeight: height, thumbMeta: ui.thumbMeta, + resizeSem: ui.resizeSem, } th.ServeHTTP(rw, req, blobref) }