Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(11)

Issue 2846593008: Proactively dispose image filters for SVG filter chains (Closed)

Created:
6 months, 3 weeks ago by fs
Modified:
6 months, 3 weeks ago
Reviewers:
Stephen Chennney
CC:
ajuma+watch_chromium.org, blink-reviews, blink-reviews-layout_chromium.org, blink-reviews-platform-graphics_chromium.org, Rik, chromium-reviews, dshwang, drott+blinkwatch_chromium.org, krit, eae+blinkwatch, fmalita+watch_chromium.org, gyuyoung2, jchaffraix+rendering, Justin Novosad, kinuko+watch, kouhei+svg_chromium.org, leviw+renderwatch, pdr+svgwatchlist_chromium.org, pdr+graphicswatchlist_chromium.org, pdr+renderingwatchlist_chromium.org, rwlbuis, Stephen Chennney, szager+layoutwatch_chromium.org, zoltan1
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Proactively dispose image filters for SVG filter chains Because of the spanning of multiple heaps by the resources associated with FilterEffects [GCd] (SkImageFilter [mallocd]), the garbage collector only observes a relatively slow growth, while resources tied by or via the other heap can be substantial. Since we have fairly good control of the lifetimes here, we can try to dispose of our references to the resources on the other heap up front, and prevent growth due to (dead) GCd objects in limbo. Also rename FilterEffect::ClearResult to DisposeImageFilters to better match it does nowadays. BUG=610158 Review-Url: https://codereview.chromium.org/2846593008 Cr-Commit-Position: refs/heads/master@{#467983} Committed: https://chromium.googlesource.com/chromium/src/+/824d3f765ad699054db51cc21c7a9deb8d17a947

Patch Set 1 #

Messages

Total messages: 15 (9 generated)
fs
6 months, 3 weeks ago (2017-04-28 10:23:21 UTC) #7
Stephen Chennney
lgtm. We need one heap to rule them all, which is really hard when we ...
6 months, 3 weeks ago (2017-04-28 13:18:39 UTC) #8
f(malita)
On 2017/04/28 13:18:39, Stephen Chennney wrote: > The right approach, really, is to have Skia ...
6 months, 3 weeks ago (2017-04-28 13:29:44 UTC) #9
fs
On 2017/04/28 at 13:29:44, fmalita wrote: > On 2017/04/28 13:18:39, Stephen Chennney wrote: > > ...
6 months, 3 weeks ago (2017-04-28 13:43:59 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2846593008/1
6 months, 3 weeks ago (2017-04-28 13:44:32 UTC) #12
commit-bot: I haz the power
6 months, 3 weeks ago (2017-04-28 13:50:30 UTC) #15
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/824d3f765ad699054db51cc21c7a...

Powered by Google App Engine
This is Rietveld efc10ee0f