Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(50)

Issue 549163002: Improve performance in onDraw() by creating RectF object only once. (Closed)

Created:
6 years, 3 months ago by wajahat
Modified:
6 years, 3 months ago
Reviewers:
Yaron, nilesh
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Creating RectF as member variable to save unnecessary allocation on each invocation of onDraw When onDraw is called, everytime RectF is created which allocates new object. To compute the rect, it needs to be created only once and re-used every time when onDraw() is called altering its values, else too many RectF objects could affect performance. This issue is found from below lint warning: ../../../tmp/tmppzTHqO/0/PopupZoomer.java:466 Avoid object allocations during draw/layout operations (preallocate and reuse instead): DrawAllocation [warning] RectF rect = new RectF(); Bug=None. Committed: https://crrev.com/fc16325bd1737582a405c84f2fd7355a1a1964d2 Cr-Commit-Position: refs/heads/master@{#294349}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Changes Incorporated #

Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -13 lines) Patch
M content/public/android/java/src/org/chromium/content/browser/PopupZoomer.java View 1 3 chunks +19 lines, -13 lines 0 comments Download

Messages

Total messages: 10 (2 generated)
wajahat
Please Review.
6 years, 3 months ago (2014-09-08 12:30:16 UTC) #2
Yaron
On 2014/09/08 12:30:16, wajahat wrote: > Please Review. How did you come across this? Have ...
6 years, 3 months ago (2014-09-08 23:16:31 UTC) #3
wajahat
On 2014/09/08 23:16:31, Yaron wrote: > On 2014/09/08 12:30:16, wajahat wrote: > > Please Review. ...
6 years, 3 months ago (2014-09-09 14:28:13 UTC) #4
Yaron
lgtm https://codereview.chromium.org/549163002/diff/1/content/public/android/java/src/org/chromium/content/browser/PopupZoomer.java File content/public/android/java/src/org/chromium/content/browser/PopupZoomer.java (right): https://codereview.chromium.org/549163002/diff/1/content/public/android/java/src/org/chromium/content/browser/PopupZoomer.java#newcode125 content/public/android/java/src/org/chromium/content/browser/PopupZoomer.java:125: // These bounds are computed in onDraw(). perhaps ...
6 years, 3 months ago (2014-09-10 01:37:16 UTC) #5
wajahat
Thank you for review. Changes have been incorporated as suggested. PTAL! https://codereview.chromium.org/549163002/diff/1/content/public/android/java/src/org/chromium/content/browser/PopupZoomer.java File content/public/android/java/src/org/chromium/content/browser/PopupZoomer.java (right): ...
6 years, 3 months ago (2014-09-10 11:34:53 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patchset/549163002/20001
6 years, 3 months ago (2014-09-11 06:45:06 UTC) #8
commit-bot: I haz the power
Committed patchset #2 (id:20001) as 87a9b5b342519f76b857bde7124ac06c0edd8e3a
6 years, 3 months ago (2014-09-11 07:43:34 UTC) #9
commit-bot: I haz the power
6 years, 3 months ago (2014-09-11 07:56:40 UTC) #10
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/fc16325bd1737582a405c84f2fd7355a1a1964d2
Cr-Commit-Position: refs/heads/master@{#294349}

Powered by Google App Engine
This is Rietveld 408576698