Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(105)

Issue 7071060: Fix leaks in blurrect benchs. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 8 months ago by bsalomon
Modified:
11 years, 8 months ago
Reviewers:
Humper, reed1
CC:
skia-review_googlegroups.com, EricB
Base URL:
http://skia.googlecode.com/svn/trunk/
Visibility:
Public.

Description

Fix leaks in blurrect benchs and gm. Committed: https://code.google.com/p/skia/source/detail?r=7142

Patch Set 1 #

Patch Set 2 : #

Total comments: 1

Patch Set 3 : #

Patch Set 4 : #

Patch Set 5 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -18 lines) Patch
M bench/BlurRectBench.cpp View 1 2 3 3 chunks +9 lines, -1 line 0 comments Download
M gm/blurrect.cpp View 1 2 3 4 5 chunks +15 lines, -13 lines 0 comments Download
M src/effects/SkBlurMask.cpp View 1 2 3 4 chunks +3 lines, -4 lines 0 comments Download

Messages

Total messages: 7
bsalomon
These leaks are causing the Android bench runs to get killed by OOM.
11 years, 8 months ago (2013-01-10 20:35:28 UTC) #1
Humper
otherwise lgtm https://codereview.appspot.com/7071060/diff/2001/bench/BlurRectBench.cpp File bench/BlurRectBench.cpp (right): https://codereview.appspot.com/7071060/diff/2001/bench/BlurRectBench.cpp#newcode94 bench/BlurRectBench.cpp:94: // BlurRect delete's its result?! Are these ...
11 years, 8 months ago (2013-01-10 20:52:22 UTC) #2
bsalomon
On 2013/01/10 20:52:22, Humper wrote: > otherwise lgtm > > https://codereview.appspot.com/7071060/diff/2001/bench/BlurRectBench.cpp > File bench/BlurRectBench.cpp (right): ...
11 years, 8 months ago (2013-01-10 21:06:53 UTC) #3
bsalomon
In patchset 4 BlurRect no longer frees the mask it generates.
11 years, 8 months ago (2013-01-10 22:07:19 UTC) #4
bsalomon
Patchset 5 also fixes leaks in the blur rect GMs.
11 years, 8 months ago (2013-01-10 22:21:04 UTC) #5
bsalomon
Can this get another look? I (and valgrind) think is an improvement over patchset 2.
11 years, 8 months ago (2013-01-11 15:19:19 UTC) #6
Humper
11 years, 8 months ago (2013-01-11 20:54:00 UTC) #7
On 2013/01/11 15:19:19, bsalomon wrote:
> Can this get another look? I (and valgrind) think is an improvement over
> patchset 2.

lgtm
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b