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

Issue 134153006: TextAutosizer: avoid unnecessary calls to containerShouldBeAutosized. (Closed)

Created:
6 years, 11 months ago by timvolodine
Modified:
6 years, 11 months ago
Reviewers:
johnme, pdr., skobes
CC:
blink-reviews, bemjb+rendering_chromium.org, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering, zoltan1
Visibility:
Public.

Description

TextAutosizer: avoid unnecessary calls to containerShouldBeAutosized. This patch adds a simple check to ensure that the multiplier > 1 before calling containerShouldBeAutosized() inside processContainer(). If the multiplier is exactly 1 the return value of containerShouldBeAutosized does not matter. Due to the cascading structure of the algorithm quite a few unnecessary calls to containerShouldBeAutosized are avoided in this way, reducing the text autosizing times by up to 30% on some websites. Reduction of measured time spent in text autosizing (in %): amazon.co.uk - 17% www.nytimes.com - 27% www.answers.com - 31% www.yahoo.com - 16% http://en.wikipedia.org/wiki/Cat - 9% BUG= Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=165479

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -1 line) Patch
M Source/core/rendering/TextAutosizer.cpp View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 6 (0 generated)
timvolodine
6 years, 11 months ago (2014-01-21 19:37:17 UTC) #1
timvolodine
for more timings: https://docs.google.com/a/google.com/spreadsheets/d/1H_ilEGa4SjFsuWv9Hu0W6QxgL1ErPbOspZELaRQrcio/edit#gid=1979910635
6 years, 11 months ago (2014-01-21 19:37:46 UTC) #2
pdr.
On 2014/01/21 19:37:46, timvolodine wrote: > for more timings: > https://docs.google.com/a/google.com/spreadsheets/d/1H_ilEGa4SjFsuWv9Hu0W6QxgL1ErPbOspZELaRQrcio/edit#gid=1979910635 LGTM
6 years, 11 months ago (2014-01-21 19:39:10 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/timvolodine@chromium.org/134153006/1
6 years, 11 months ago (2014-01-21 19:40:09 UTC) #4
commit-bot: I haz the power
Change committed as 165479
6 years, 11 months ago (2014-01-21 20:49:59 UTC) #5
pdr.
6 years, 11 months ago (2014-01-21 21:15:15 UTC) #6
Message was sent while issue was closed.
On 2014/01/21 20:49:59, I haz the power (commit-bot) wrote:
> Change committed as 165479

Lets keep an eye on
https://chromeperf.appspot.com/report?masters=ChromiumPerf&bots=android-nexus...

If this doesn't show a perf win, we need to fix our tests :)

Powered by Google App Engine
This is Rietveld 408576698