Proactively dispose image filters for SVG filter chains

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

[fs, 2017-04-28 08:18:58]

The CQ bit was checked by fs@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-28 08:19:18]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[fs, 2017-04-28 08:25:58]

Description was changed from

==========
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 reousrces 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
==========

to

==========
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
==========

[commit-bot: I haz the power, 2017-04-28 10:14:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-28 10:14:41]

Dry run: This issue passed the CQ dry run.

[fs, 2017-04-28 10:23:19]

fs@opera.com changed reviewers:
+ schenney@chromium.org

[Stephen Chennney, 2017-04-28 13:18:39]

lgtm. We need one heap to rule them all, which is really hard when we have lots
of third_party software.

The right approach, really, is to have Skia provide an API the lets an
application provide it's own memory managers for Skia's allocations. We did that
back in the game engine days.

[f(malita), 2017-04-28 13:29:44]

On 2017/04/28 13:18:39, Stephen Chennney wrote:
> The right approach, really, is to have Skia provide an API the lets an
> application provide it's own memory managers for Skia's allocations. We did
that
> back in the game engine days.

That mechanism exists, but it follows the explicit allocation/deallocation model
- so switching to a GC heap is prolly not on the table:
https://cs.chromium.org/chromium/src/skia/ext/SkMemory_new_handler.cpp?l=63

[fs, 2017-04-28 13:43:59]

On 2017/04/28 at 13:29:44, fmalita wrote:
> On 2017/04/28 13:18:39, Stephen Chennney wrote:
> > The right approach, really, is to have Skia provide an API the lets an
> > application provide it's own memory managers for Skia's allocations. We did
that
> > back in the game engine days.
> 
> That mechanism exists, but it follows the explicit allocation/deallocation
model - so switching to a GC heap is prolly not on the table:
https://cs.chromium.org/chromium/src/skia/ext/SkMemory_new_handler.cpp?l=63

Well, explicit allocation/deallocation isn't a problem per se (you could
implement the deallocation operation as a no-op), but that's really limping
along more than anything else... =) I somehow suspect that there could be other
things at play here though, like "willingness" to evict things from caches that
are still referenced et.c. Not even sure it really makes sense to have
FilterEffect on the GC heap in the first place (there are preciously few objects
referenced within the chain that are on the GC-heap - the chain itself
disregarded - so it's more about avoiding dynamic GC roots, and the risk with
leaking GC sub-objectgraphs because of their use.)

[fs, 2017-04-28 13:44:10]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2017-04-28 13:44:32]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-28 13:50:16]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1493387050244660, "parent_rev":
"1824ef1f2ac1b1e233bbe1934d8d2060aa9f42a8", "commit_rev":
"824d3f765ad699054db51cc21c7a9deb8d17a947"}

[commit-bot: I haz the power, 2017-04-28 13:50:28]

Description was changed from

==========
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
==========

to

==========
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/+/824d3f765ad699054db51cc21c7a...
==========

[commit-bot: I haz the power, 2017-04-28 13:50:30]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/824d3f765ad699054db51cc21c7a...