Commit 824d3f76 authored by fs's avatar fs Committed by Commit bot

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}
parent 1824ef1f
......@@ -36,6 +36,8 @@ DEFINE_TRACE(FilterData) {
void FilterData::Dispose() {
node_map = nullptr;
if (last_effect)
last_effect->DisposeImageFiltersRecursive();
last_effect = nullptr;
}
......@@ -45,8 +47,8 @@ LayoutSVGResourceFilter::LayoutSVGResourceFilter(SVGFilterElement* node)
LayoutSVGResourceFilter::~LayoutSVGResourceFilter() {}
void LayoutSVGResourceFilter::DisposeFilterMap() {
for (auto& filter : filter_)
filter.value->Dispose();
for (auto& entry : filter_)
entry.value->Dispose();
filter_.clear();
}
......@@ -76,9 +78,12 @@ void LayoutSVGResourceFilter::RemoveClientFromCache(
bool mark_for_invalidation) {
DCHECK(client);
bool filter_cached = filter_.Contains(client);
if (filter_cached)
filter_.erase(client);
auto entry = filter_.find(client);
bool filter_cached = entry != filter_.end();
if (filter_cached) {
entry->value->Dispose();
filter_.erase(entry);
}
// If the filter has a cached subtree, invalidate the associated display item.
if (mark_for_invalidation && filter_cached)
......
......@@ -93,7 +93,7 @@ void SVGFilterGraphNodeMap::InvalidateDependentEffects(FilterEffect* effect) {
if (!effect->HasImageFilter())
return;
effect->ClearResult();
effect->DisposeImageFilters();
FilterEffectSet& effect_references = this->EffectReferences(effect);
for (FilterEffect* effect_reference : effect_references)
......
......@@ -88,11 +88,19 @@ FilterEffect* FilterEffect::InputEffect(unsigned number) const {
return input_effects_.at(number).Get();
}
void FilterEffect::ClearResult() {
void FilterEffect::DisposeImageFilters() {
for (int i = 0; i < 4; i++)
image_filters_[i] = nullptr;
}
void FilterEffect::DisposeImageFiltersRecursive() {
if (!HasImageFilter())
return;
DisposeImageFilters();
for (auto& effect : input_effects_)
effect->DisposeImageFiltersRecursive();
}
Color FilterEffect::AdaptColorToOperatingColorSpace(const Color& device_color) {
// |deviceColor| is assumed to be DeviceRGB.
return ColorSpaceUtilities::ConvertColor(device_color, OperatingColorSpace());
......
......@@ -56,7 +56,8 @@ class PLATFORM_EXPORT FilterEffect
virtual ~FilterEffect();
DECLARE_VIRTUAL_TRACE();
void ClearResult();
void DisposeImageFilters();
void DisposeImageFiltersRecursive();
FilterEffectVector& InputEffects() { return input_effects_; }
FilterEffect* InputEffect(unsigned) const;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment