New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 135160 link

Starred by 11 users

Issue metadata

Status: Verified
Owner:
Closed: Jul 2012
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 0
Type: Bug-Regression

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment

Noticeable performance degradation in M20 using RTL UI (Omnibox, wrench menu & tabs switching)

Reported by omaret@chromium.org, Jun 29 2012

Issue description

Chrome Version       : 20.0.1132.47
OS Version: 6.1 (Windows 7, Windows Server 2008 R2)

What steps will reproduce the problem?
1. Open Chrome M20 and switch to an RTL language UI (ex. Arabic)
2. Try typing quickly in omnibox. There's a noticeable delay between keypress and character display. This delay stacks and ends up being too long. (The delay doesn't seem to depend on input (i.e: it's reproduced when typing English.). You need to type quickly to reproduce it (you can type gibberish quickly)
3. Try clicking the wrench icon and note how slowly it opens
4. Switching between tabs has become slower

What is the expected result?
Typing in omnibox should be much more responsive
Switching tabs should be more seamless
Open wrench menu should be faster

What happens instead?
Slow switching between tabs
Wrench Menu opens slowly
Noticeable delay in omnibox

Please provide any additional information below. Attach a screenshot if
possible.

UserAgentString: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/536.11 (KHTML, like Gecko) Chrome/20.0.1132.47 Safari/536.11



 

Comment 1 by omaret@chromium.org, Jun 29 2012

I've also noticed an enormous overall delay in the browser when I'm trying to download a file and haven't dismissed the notification message about "Save" or "Cancel".

Comment 2 by omaret@chromium.org, Jun 29 2012

Labels: -Pri-2 Pri-0

Comment 3 by dharani@google.com, Jun 29 2012

Labels: Stability-Performance Action-BisectNeeded

Comment 4 by jtan@chromium.org, Jun 29 2012

Cc: jeremy@chromium.org xji@chromium.org
Labels: -Area-Undefined Area-Internals

Comment 5 by jtan@chromium.org, Jun 29 2012

Per Omar, users reported that this happened after updating to Chrome 20. 

Comment 6 by dharani@google.com, Jun 29 2012

Cc: jar@chromium.org tonyg@chromium.org bashi@chromium.org jam...@chromium.org simonjam@chromium.org nduca@chromium.org

Comment 7 by jam...@chromium.org, Jun 29 2012

Could someone who's seeing this issue try grabbing a trace (http://www.chromium.org/developers/how-tos/trace-event-profiling-tool/recording-tracing-runs) and see if it's illuminating?  (If it isn't, we should add more instrumentation)

Comment 8 by xji@chromium.org, Jun 29 2012

Cc: msw@chromium.org asvitk...@chromium.org
Mike, Alexei,
Are you aware of anything we did in M20 that might cause this performance degradation?
M20 is the first release that enabled use_canvas_skia=1.

It is possible that the font fallback code in render_text_win.cc is less performant than the previous path - although this should also manifest in other language UIs like CJK.

Of note: use_canvas_skia=1 shouldn't affect omnibox drawing since that uses a native control.

Are we able to repro this and if so, could we profile this? (I've previously had good luck using Very Sleepy for profiling - http://www.codersnotes.com/sleepy.)
Although, now that I think about it, we do cache the last successful fallback font that was used, so off the top of my head, I wouldn't think that font fallback would be to blame for perf since it wouldn't happen once cached.

Comment 11 by msw@chromium.org, Jun 29 2012

Alexei, do you have that CL offhand? Any chance we could bisect around that to see if it's the issue at all?
RenderText could definitely use some performance profiling and tuning, especially on Windows.
I bisected and found this range:

http://build.chromium.org/f/chromium/perf/dashboard/ui/changelog.html?url=/trunk/src&range=135198%3a135211

However, the perf difference I observed was very subtle (I tried it by switching between two NTP tabs), so it's possible I got the bisection wrong. Can anyone confirm the bisection result?

That range does include two RenderText CLs. I will look at them more closely to see if they could be to blame...
I bisected again and got the same result. So I guess it is indeed in that range.

Comment 15 by xji@chromium.org, Jun 29 2012

I tried manually install 135229 and 135192, 135229 is slower when open wrench menu. (open wrench menu seems the  most easiest way to repro).
Here is the WebKit changelog as well:

http://trac.webkit.org/log/trunk/?rev=115990&stop_rev=115982&verbose=on&limit=10000

(From a glance, I don't see anything interesting there.)

I guess the next step is to see if it repros on TOT and if reverting one of the CLs in that range brings perf back to normal.
Labels: -Type-Bug -Action-BisectNeeded Type-Regression
Status: Untriaged
It is broken between 19.0.1084.56 and 20.0.1132.43

Please find the bisect info:
WEBKIT CHANGELOG URL:
http://trac.webkit.org/log/trunk/?rev=115990&stop_rev=115982&verbose=on
CHANGELOG URL:
http://build.chromium.org/f/chromium/perf/dashboard/ui/changelog.html?url=/trunk
/src&range=135198:135211
Built at revision:
http://src.chromium.org/viewvc/chrome?view=rev&revision=135211


I am currently building locally to see if I can reproduce on TOT. If so, I'll try reverting some of the CLs in that range to see if they are responsible.
Owner: asvitk...@chromium.org
Status: Assigned
I've repro'd this on TOT and confirmed that reverting http://crrev.com/135206 fixes the regression.

It is not clear to me why that CL has such a big effect on perf, though. Will investigate.
Cc: js...@chromium.org
The problem is the code that's checking for missing glyphs is hitting a false positive and thus determining that there are missing glyphs when that's not actually the case.

This causes it to go through the complete font fallback chain (and probably never find an appropriate font). And since it doesn't find a font, the font cache doesn't help, and it does this for every string that's drawn.

That would explain the perf hit.

Now, the question is how do we change that logic to not detect these cases.

+jshin who was on the thread were we originally decided to implement the logic that way.
Ah, so the problem was that code was treating the characters inserted by WrapStringWithRTLFormatting() as missing glyphs. I've got a fix - will sent out a CL shortly!
Labels: Mstone-20
Status: Started

Comment 24 by dharani@google.com, Jun 29 2012

Excellent work asvitkine@! Let's get this change baked in canary before we hit M20.

Comment 25 by jtan@chromium.org, Jun 29 2012

Labels: Hotlist-ConOps

Comment 26 by msw@chromium.org, Jun 29 2012

Agreed, this looks like a good fix, thanks for the quick turnaround, Alexei!
The fix should also be merged to M21 after being verified in Canary.
Project Member

Comment 27 by bugdroid1@chromium.org, Jun 30 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=145046

------------------------------------------------------------------------
r145046 | asvitkine@chromium.org | Fri Jun 29 17:57:22 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/ui/gfx/render_text_win.cc?r1=145046&r2=145045&pathrev=145046

Ignore Unicode BiDi control characters in missing glyphs check in RenderTextWin.

BUG= 135160 
TEST=Open chrome.exe with --lang=he and bring up context menu. There should be
no lag opening t.

Review URL: https://chromiumcodereview.appspot.com/10696058
------------------------------------------------------------------------

Comment 28 by Deleted ...@, Jun 30 2012

yes sem

Comment 29 by Deleted ...@, Jul 1 2012


Chrome browser after the last update became very slowly.. what is the solution ?

Google chrome latest version 20.0.1132.47 in English is fast but in Arabic really so slow and laggy. please fix it.
GoogleChrome.png
37.2 KB View Download
Labels: Action-FeedbackNeeded
The fix has landed on Canary and will be merged back to M20 and M21 assuming everything is well on Canary.

Can someone verify that performance is back to normal with Chrome Canary?  (22.0.1191.1)
(Chrome Canary can be installed from here: https://tools.google.com/dlpage/chromesxs/)
 Issue 135358  has been merged into this issue.

Comment 34 by Deleted ...@, Jul 1 2012

me to google :$
Labels: -Action-FeedbackNeeded Merge-Requested
I've verified that the performance problem is not seen with ‪22.0.1192.0 canary‬ which has the fix.

Requesting merge to M20.

(Will also later request merge to M21 once the M21 branch re-opens for merges.)

Comment 36 by Deleted ...@, Jul 3 2012

Google chrome latest version 20.0.1132.47 in English is fast but in Arabic really so slow and laggy. please fix it.

Comment 37 by Deleted ...@, Jul 3 2012

Google chrome latest version 20.0.1132.47 in English is fast but in Arabic really so slow and laggy. please fix it.

Comment 38 by Deleted ...@, Jul 3 2012

Google chrome latest version 20.0.1132.47 in English is fast but in Arabic really so slow and laggy. please fix it.
Labels: -Merge-Requested Merge-Approved
Project Member

Comment 40 by bugdroid1@chromium.org, Jul 3 2012

Labels: merge-merged-1132
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=145297

------------------------------------------------------------------------
r145297 | asvitkine@chromium.org | Tue Jul 03 07:12:05 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/1132/src/ui/gfx/render_text_win.cc?r1=145297&r2=145296&pathrev=145297

Merge 145046 - Ignore Unicode BiDi control characters in missing glyphs check in RenderTextWin.

BUG= 135160 
TEST=Open chrome.exe with --lang=he and bring up context menu. There should be
no lag opening t.

Review URL: https://chromiumcodereview.appspot.com/10696058

TBR=asvitkine@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10698078
------------------------------------------------------------------------
Merged to M20.

(Keeping bug open to also merge to M21 when that branch re-opens.)
Great work, guys!

Is there any way to apply this patch to an existing Chrome 20.0.1132.47 installation? Or do we have to wait for 21?
It will arrive in an update to Chrome 20.
Great. Can't wait... :)
Thanks again!

Comment 45 by Deleted ...@, Jul 4 2012

chrome 20 is too slow
Cc: sky@chromium.org willchan@chromium.org
 Issue 132199  has been merged into this issue.
 Issue 132199  has been merged into this issue.

Comment 48 by Deleted ...@, Jul 5 2012

pls write  Arabic 
Labels: -Mstone-20 -Merge-Approved Mstone-21 Merge-Requested
Requesting merge to M21 beta too. (It has been merged to M20 already.)

Comment 50 by kareng@google.com, Jul 9 2012

Labels: -Merge-Requested Merge-Approved
go ahead.
Project Member

Comment 51 by bugdroid1@chromium.org, Jul 9 2012

Labels: -Merge-Approved merge-merged-1180
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=145772

------------------------------------------------------------------------
r145772 | asvitkine@chromium.org | Mon Jul 09 16:00:17 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/1180/src/ui/gfx/render_text_win.cc?r1=145772&r2=145771&pathrev=145772

Merge 145046 - Ignore Unicode BiDi control characters in missing glyphs check in RenderTextWin.

BUG= 135160 
TEST=Open chrome.exe with --lang=he and bring up context menu. There should be
no lag opening t.

Review URL: https://chromiumcodereview.appspot.com/10696058

TBR=asvitkine@chromium.org
------------------------------------------------------------------------
Status: Fixed
Status: Verified
Verified with Chrome 20.0.1132.57 (גירסה רשמית 145807)/ Win7
Project Member

Comment 54 by bugdroid1@chromium.org, Oct 13 2012

Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Project Member

Comment 55 by bugdroid1@chromium.org, Mar 9 2013

Labels: -Type-Regression -Area-Internals -Stability-Performance -Mstone-21 Type-Bug-Regression M-21 Cr-Internals Performance
Project Member

Comment 56 by bugdroid1@chromium.org, Mar 14 2013

Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue

Sign in to add a comment