Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1928403002:
Enable generating SkImage with the same uniqueID in SkImageGenerator subclass (Closed)
|
Created:
4 years, 7 months ago by aleksandar.stojiljkovic Modified:
4 years, 7 months ago CC:
reviews_skia.org Base URL:
https://chromium.googlesource.com/skia.git@master Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
DescriptionEnable generating SkImage with the same uniqueID in SkImageGenerator subclass
Enable reusing uniqueID when instantiating SkImageGenerator subclasses enables
using uniqueID in client code to cache generated bitmaps with no need to keep
the reference to SkImageGenerator.
This is a bug fix for out of memory cause in chromium and 100% CPU usage
described in issue 165750#13:
- cache uses SkImage::uniqueID() to cache decoded bitmaps.
- every animation loop creates new SkImage instances.
- after decoding, bitmap copies are added to cache, filling it up with
duplicates of previous loops frames.
BUG=165750
Blink patch that depends on this:
https://codereview.chromium.org/1925533003/
"High CPU and increased memory usage fix for high-res (GIF, WEBP...) animations."
Committed: https://skia.googlesource.com/skia/+/95167758572eca33008dce3e8bebe2a7656a06c9
Patch Set 1 #Patch Set 2 : CL description #
Messages
Total messages: 17 (12 generated)
Description was changed from ========== Enable generating SkImage with the same uniqueID in SkImageGenerator subclass This is a bug fix for out of memory cause in chromium and 100% CPU usage described in issue 165750#13: - cc (SoftwareImageDecodeContoller) is using SkImage uniqueID to cache decoded bitmaps. - blink (BitmapImage) is in every animation loop creating new SkImage instances (blink::DecodingImageGenerator). - after decoding, bitmap copies are added to cache, filling it up with duplicates of previous loops frames. Similar issues happens also when Skia's discardable memory cache is used - blink::BitmapImage is, in order to save memory, not caching SkImages for large (arbitrary) GIF animations and creating new SkImage instances during animation playback. In case of using skia DM cache, approach in BitmapImage doesn't bring significant benefit to memory reduction as DM is disposing unpinned pixelrefs on it's own pace. BUG=165750 ========== to ========== Enable generating SkImage with the same uniqueID in SkImageGenerator subclass This is a bug fix for out of memory cause in chromium and 100% CPU usage described in issue 165750#13: - cc (SoftwareImageDecodeContoller) is using SkImage uniqueID to cache decoded bitmaps. - blink (BitmapImage) is in every animation loop creating new SkImage instances (blink::DecodingImageGenerator). - after decoding, bitmap copies are added to cache, filling it up with duplicates of previous loops frames. Similar issues happens also when Skia's discardable memory cache is used - blink::BitmapImage is, in order to save memory, not caching SkImages for large (arbitrary) GIF animations and creating new SkImage instances during animation playback. In case of using skia DM cache, approach in BitmapImage doesn't bring significant benefit to memory reduction as DM is disposing unpinned pixelrefs on it's own pace. BUG=165750 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== Enable generating SkImage with the same uniqueID in SkImageGenerator subclass This is a bug fix for out of memory cause in chromium and 100% CPU usage described in issue 165750#13: - cc (SoftwareImageDecodeContoller) is using SkImage uniqueID to cache decoded bitmaps. - blink (BitmapImage) is in every animation loop creating new SkImage instances (blink::DecodingImageGenerator). - after decoding, bitmap copies are added to cache, filling it up with duplicates of previous loops frames. Similar issues happens also when Skia's discardable memory cache is used - blink::BitmapImage is, in order to save memory, not caching SkImages for large (arbitrary) GIF animations and creating new SkImage instances during animation playback. In case of using skia DM cache, approach in BitmapImage doesn't bring significant benefit to memory reduction as DM is disposing unpinned pixelrefs on it's own pace. BUG=165750 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== This is a bug fix for out of memory cause in chromium and 100% CPU usage described in issue https://bugs.chromium.org/p/chromium/issues/detail?id=165750: - cc (SoftwareImageDecodeContoller) is using SkImage uniqueID to cache decoded bitmaps. - blink (BitmapImage) is in every animation loop creating new SkImage instances (blink::DecodingImageGenerator). - after decoding, bitmap copies are added to cache, filling it up with duplicates of previous loops frames. Similar issues happens also when Skia's discardable memory cache is used - blink::BitmapImage is, in order to save memory, not caching SkImages for large (arbitrary) GIF animations and creating new SkImage instances during animation playback. In case of using skia DM cache, approach in BitmapImage doesn't bring significant benefit to memory reduction as DM is disposing unpinned pixelrefs on it's own pace. BUG=165750 ==========
aleksandar.stojiljkovic@intel.com changed reviewers: + pkasting@chromium.org, reed@chromium.org
aleksandar.stojiljkovic@intel.com changed reviewers: - pkasting@chromium.org
Description was changed from ========== This is a bug fix for out of memory cause in chromium and 100% CPU usage described in issue https://bugs.chromium.org/p/chromium/issues/detail?id=165750: - cc (SoftwareImageDecodeContoller) is using SkImage uniqueID to cache decoded bitmaps. - blink (BitmapImage) is in every animation loop creating new SkImage instances (blink::DecodingImageGenerator). - after decoding, bitmap copies are added to cache, filling it up with duplicates of previous loops frames. Similar issues happens also when Skia's discardable memory cache is used - blink::BitmapImage is, in order to save memory, not caching SkImages for large (arbitrary) GIF animations and creating new SkImage instances during animation playback. In case of using skia DM cache, approach in BitmapImage doesn't bring significant benefit to memory reduction as DM is disposing unpinned pixelrefs on it's own pace. BUG=165750 ========== to ========== Blink patch that depends on this: https://codereview.chromium.org/1925533003/ "High CPU and increased memory usage fix for high-res (GIF, WEBP...) animations." This is a bug fix for out of memory cause in chromium and 100% CPU usage described in issue https://bugs.chromium.org/p/chromium/issues/detail?id=165750: - cc (SoftwareImageDecodeContoller) is using SkImage uniqueID to cache decoded bitmaps. - blink (BitmapImage) is in every animation loop creating new SkImage instances (blink::DecodingImageGenerator). - after decoding, bitmap copies are added to cache, filling it up with duplicates of previous loops frames. Similar issues happens also when Skia's discardable memory cache is used - blink::BitmapImage is, in order to save memory, not caching SkImages for large (arbitrary) GIF animations and creating new SkImage instances during animation playback. In case of using skia DM cache, approach in BitmapImage doesn't bring significant benefit to memory reduction as DM is disposing unpinned pixelrefs on it's own pace. BUG=165750 ==========
aleksandar.stojiljkovic@intel.com changed reviewers: + bsalomon@google.com
reed@google.com changed reviewers: + reed@google.com
lgtm the CL description is good, but more interesting perhaps in a blink CL or in a bug, than in this Skia CL. That said, I think the feature is well done. We should have a unittest for this in Skia, but I can add that after this lands, so as not to hold-the other CL up.
Description was changed from ========== Blink patch that depends on this: https://codereview.chromium.org/1925533003/ "High CPU and increased memory usage fix for high-res (GIF, WEBP...) animations." This is a bug fix for out of memory cause in chromium and 100% CPU usage described in issue https://bugs.chromium.org/p/chromium/issues/detail?id=165750: - cc (SoftwareImageDecodeContoller) is using SkImage uniqueID to cache decoded bitmaps. - blink (BitmapImage) is in every animation loop creating new SkImage instances (blink::DecodingImageGenerator). - after decoding, bitmap copies are added to cache, filling it up with duplicates of previous loops frames. Similar issues happens also when Skia's discardable memory cache is used - blink::BitmapImage is, in order to save memory, not caching SkImages for large (arbitrary) GIF animations and creating new SkImage instances during animation playback. In case of using skia DM cache, approach in BitmapImage doesn't bring significant benefit to memory reduction as DM is disposing unpinned pixelrefs on it's own pace. BUG=165750 ========== to ========== Blink patch that depends on this: https://codereview.chromium.org/1925533003/ "High CPU and increased memory usage fix for high-res (GIF, WEBP...) animations." This is a bug fix for out of memory cause in chromium and 100% CPU usage described in issue https://bugs.chromium.org/p/chromium/issues/detail?id=165750: - cc (SoftwareImageDecodeContoller) is using SkImage uniqueID to cache decoded bitmaps. - blink (BitmapImage) is in every animation loop creating new SkImage instances (blink::DecodingImageGenerator). - after decoding, bitmap copies are added to cache, filling it up with duplicates of previous loops frames. Similar issues happens also when Skia's discardable memory cache is used - blink::BitmapImage is, in order to save memory, not caching SkImages for large (arbitrary) GIF animations and creating new SkImage instances during animation playback. In case of using skia DM cache, approach in BitmapImage doesn't bring significant benefit to memory reduction as DM is disposing unpinned pixelrefs on it's own pace. BUG=165750 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== Blink patch that depends on this: https://codereview.chromium.org/1925533003/ "High CPU and increased memory usage fix for high-res (GIF, WEBP...) animations." This is a bug fix for out of memory cause in chromium and 100% CPU usage described in issue https://bugs.chromium.org/p/chromium/issues/detail?id=165750: - cc (SoftwareImageDecodeContoller) is using SkImage uniqueID to cache decoded bitmaps. - blink (BitmapImage) is in every animation loop creating new SkImage instances (blink::DecodingImageGenerator). - after decoding, bitmap copies are added to cache, filling it up with duplicates of previous loops frames. Similar issues happens also when Skia's discardable memory cache is used - blink::BitmapImage is, in order to save memory, not caching SkImages for large (arbitrary) GIF animations and creating new SkImage instances during animation playback. In case of using skia DM cache, approach in BitmapImage doesn't bring significant benefit to memory reduction as DM is disposing unpinned pixelrefs on it's own pace. BUG=165750 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Enable generating SkImage with the same uniqueID in SkImageGenerator subclass Enable reusing uniqueID when instantiating SkImageGenerator subclasses enables using uniqueID in client code to cache generated bitmaps with no need to keep the reference to SkImageGenerator. This is a bug fix for out of memory cause in chromium and 100% CPU usage described in issue 165750#13: - cache uses SkImage::uniqueID() to cache decoded bitmaps. - every animation loop creates new SkImage instances. - after decoding, bitmap copies are added to cache, filling it up with duplicates of previous loops frames. BUG=165750 Blink patch that depends on this: https://codereview.chromium.org/1925533003/ "High CPU and increased memory usage fix for high-res (GIF, WEBP...) animations." ==========
On 2016/05/01 18:29:22, reed1 wrote: > lgtm > > the CL description is good, but more interesting perhaps in a blink CL or in a > bug, than in this Skia CL. That said, I think the feature is well done. > > We should have a unittest for this in Skia, but I can add that after this lands, > so as not to hold-the other CL up. Thanks @reed.
The CQ bit was checked by aleksandar.stojiljkovic@intel.com
The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com Link to the patchset: https://codereview.chromium.org/1928403002/#ps20001 (title: "CL description")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1928403002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1928403002/20001
Message was sent while issue was closed.
Description was changed from
==========
Enable generating SkImage with the same uniqueID in SkImageGenerator subclass
Enable reusing uniqueID when instantiating SkImageGenerator subclasses enables
using uniqueID in client code to cache generated bitmaps with no need to keep
the reference to SkImageGenerator.
This is a bug fix for out of memory cause in chromium and 100% CPU usage
described in issue 165750#13:
- cache uses SkImage::uniqueID() to cache decoded bitmaps.
- every animation loop creates new SkImage instances.
- after decoding, bitmap copies are added to cache, filling it up with
duplicates of previous loops frames.
BUG=165750
Blink patch that depends on this:
https://codereview.chromium.org/1925533003/
"High CPU and increased memory usage fix for high-res (GIF, WEBP...)
animations."
==========
to
==========
Enable generating SkImage with the same uniqueID in SkImageGenerator subclass
Enable reusing uniqueID when instantiating SkImageGenerator subclasses enables
using uniqueID in client code to cache generated bitmaps with no need to keep
the reference to SkImageGenerator.
This is a bug fix for out of memory cause in chromium and 100% CPU usage
described in issue 165750#13:
- cache uses SkImage::uniqueID() to cache decoded bitmaps.
- every animation loop creates new SkImage instances.
- after decoding, bitmap copies are added to cache, filling it up with
duplicates of previous loops frames.
BUG=165750
Blink patch that depends on this:
https://codereview.chromium.org/1925533003/
"High CPU and increased memory usage fix for high-res (GIF, WEBP...)
animations."
Committed:
https://skia.googlesource.com/skia/+/95167758572eca33008dce3e8bebe2a7656a06c9
==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://skia.googlesource.com/skia/+/95167758572eca33008dce3e8bebe2a7656a06c9 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
