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

Issue 613658 link

Starred by 47 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome
Pri: 0
Type: Bug



Sign in to add a comment

GfW/Gmail FE slowness in M50

Project Member Reported by ananthak@google.com, May 20 2016

Issue description

Folks from Gmail team have alerted me that the several GfW customers are experiencing slowness with Gmail on Chrome 50. 

https://b.corp.google.com/issues/28860580
https://bugs.chromium.org/p/chromium/issues/detail?id=612306

The test team is trying to reproduce the bug now on 50, 51, 52.

We need someone to investigate and identify the root cause. 

Ccing Melody to see if they are seeing any signal of this in conops reports.
 
Showing comments 34 - 133 of 133 Older
Cc: mkretzsc...@google.com
Thanks Alex. We'll need someone on Gmail to look into this a bit further. Adding Martin, but feel free to redirect to others.
Cc: -erikc...@chromium.org
This seems to be "Gmail" specific issue (Not related to Chrome). I have tested this on Chrome M49# 49.0.2623.112 and it reproduces the slowness/memory issue, which we didn't see on the same M49 before.

Thank you!
Labels: -Pri-1 -ReleaseBlock-Stable -M-51 Pri-2
Removing RBS and M-51 labels. Based on update on b/28860580 we don't think this issue was introduced by chrome at this point. I'm reducing priority as well, but won't close the bug until we have clarity on what happened.
Cc: vencel@google.com
Cc: mdawcewicz@google.com
Cc: sentaro@chromium.org
Cc: -mkretzsc...@google.com -melodychu@chromium.org -anan...@chromium.org -gov...@chromium.org -ncarter@google.com -manoranj...@chromium.org -jainabhi...@chromium.org -sshruthi@chromium.org -kotah@chromium.org -rohitj@google.com -skyos...@chromium.org -wfh@chromium.org -pbomm...@chromium.org -vencel@google.com -tinazh@chromium.org -alexclarke@chromium.org -saswat@chromium.org -mdawcewicz@google.com -rsch...@chromium.org -parisa@chromium.org -sentaro@chromium.org
Components: -Blink>Scheduling
Labels: -Restrict-View-Google -OS-Linux -OS-Windows -Performance -Type-Bug -Pri-2 -Hotlist-Enterprise -Hotlist-GoogleApps -OS-Chrome
Status: (was: Untriaged)
The network-offline bug in Gmail that caused the exponential timer creation was identified and fixed+cherry-picked. [We think the code (from Feb 2016) relied on some implementation detail that might have changed in 49 or 50.]

We don't believe that the offline bug is what's causing most of these reports, though.

Can someone from Chrome team look at the latest comments and timelines in http://b/28860580 ?
Cc: pbomm...@chromium.org melodychu@chromium.org ncarter@google.com manoranj...@chromium.org skyos...@chromium.org rohitj@google.com rsch...@chromium.org sshruthi@chromium.org mdawcewicz@google.com kotah@chromium.org parisa@chromium.org mkretzsc...@google.com anan...@chromium.org tinazh@chromium.org saswat@chromium.org sentaro@chromium.org wfh@chromium.org jainabhi...@chromium.org gov...@chromium.org vencel@google.com alexclarke@chromium.org
Components: Blink>Scheduling
Labels: Performance Restrict-View-Google OS-Linux OS-Windows Pri-2 Type-Bug
Labels: Hotlist-GoogleApps Hotlist-Enterprise OS-Chrome
Sorry about this metadata change. Monorail problem, I think. Filing a bug.

Comment 44 by o...@google.com, May 30 2016

Cc: o...@google.com

Comment 45 by sgud...@google.com, May 30 2016

Cc: cnes...@google.com
Labels: -Pri-2 Pri-1
Updating back to P1, Gmail team was able to identify actions which are causing the slowness - don't seem to be caused by Gmail. 
Can we plz get Chrome team to take a look again?

Copying here last 3 comments from our version of the bug in buganizer (b/28860580)

---
Vencel Bors  <vencel@google.com>  #100 May 27, 2016 03:06PM
Analyzed the data from #98 and compared it with a test account having ~250 labels, using Windows 7 with Chrome 50. I compared the time required for opening the move to menu.

Slow account: https://screenshot.googleplex.com/jfchgGNZmPL.png
Test account: https://screenshot.googleplex.com/zPVienOODXE

The function executed is goog.ui.MenuButton.setopen == Pj.Qh (based on ScriptZ gmail.main.en.Vo0OYQSaqo0.O). In case of a slow client the basic appendChild operation takes 28.3% (1400 ms) of total time required while on a test account the same operation takes 0.7% (1.2 ms) of the total time. While affects the other percentages too they still seem slower in general. I didn't find any other operation that is unexpectedly slow.

---
Vencel Bors  <vencel@google.com>  #101 May 27, 2016 06:30PM
Can someone from Chrome team look at the latest profiling and tracing data? It seems odd that adding a node takes 30 times more resources.

The timeline recordings containing the issue all seem to have a large number of nodes which could affect adding and removing new nodes.
Profile portrait of Carl Haverl

---
Carl Haverl  <carlanton@google.com>  #102 May 27, 2016 06:55PM
As Vencel says in #100, for bbva I noticed appendChild was surprisingly slow.  In addition, removeChild was slow for bbva.

Comment 46 by sgud...@google.com, May 30 2016

Cc: philr@google.com
There's not enough information to diagnose any possible chrome bugs.  Can you please take a chrome trace.

1. goto about:tracing
2. Click the record button
3. Select "Manually select settings" option
4. Click renderer.scheduler on the right hand pane

That will help diagnose if any in chromium is taking longer than expected.

Comment 48 by avic...@google.com, May 31 2016

alexclarke@chromium.org, we were unable to repro the issue, but we got the traces from a customer in b/28860580.

Will ask them for a chrome trace.

Comment 49 by avic...@google.com, May 31 2016

Please note that we have a new chrome trace (attached).
QA was able to repro with:
1. Reply to a bunch (10-20) of threads but don't actually send the replies, just keep them as drafts.
2. Refresh Gmail
3. Open a compose window, click on the emoji picker, note that it comes up instantly.
4. Goto drafts list
5. Click on each draft reply and then press back to go back to drafts list
6. Once you've done that for all your draft replies, open a new compose window
7. Click on the emoji picker and notice the lag for it to render the emoji picker.

We are trying to see whether this is dependent on Chrome version, or Gmail version.

Comment 50 by wfh@chromium.org, May 31 2016

If you are able to reliably reproduce this, perhaps you could try running a bisect - https://www.chromium.org/developers/bisect-builds-py

Thanks for the trace, I can see a few long running JS tasks (a 116ms timer* and a 528ms click handler**) which would cause the main thread to be janky.  I also noticed in the middle of the timer 23ms where spent in WebLocalFrameImpl::createChildframe, mostly running "chrome-extension://noondiphcddnnabmjcihcjfbhfklnnep/content_script_compiled.js"

Apart from that I can't see any slow dom operations in this trace

* 	
{frame: "0x7c7bf9f2830",
 functionName: "c",
 scriptId: "15575",
 scriptLine: 161,
 scriptName: "https://mail.google.com/_/scs/mail-static/_/js/k=gmail.main.en.Vo0OYQSaqo0.O/m=m_i,t,it/am=nlGvBMz_vT8Y1xgG6EpfrTv__ef7J22nl3v8f9JEiNSrgP6b_T-I_wO9aRsF/rt=h/d=1/rs=AHGWq9AONxnH0lDgOFBXLfJbglw8l8syQw"}

** {frame: "0x327c1b681e20",
 functionName: "",
 scriptId: "15574",
 scriptLine: 99,
 scriptName: "https://mail.google.com/_/scs/mail-static/_/js/k=gmail.main.en.Vo0OYQSaqo0.O/m=m_i,t,it/am=nlGvBMz_vT8Y1xgG6EpfrTv__ef7J22nl3v8f9JEiNSrgP6b_T-I_wO9aRsF/rt=h/d=1/rs=AHGWq9AONxnH0lDgOFBXLfJbglw8l8syQw"}

Comment 52 by avic...@google.com, May 31 2016

Cc: avic...@google.com
More details in b/28860580
Our QA team was able to repro this on old Gmail build on Ubuntu/Chrome51.
However, they cannot repro this on Windows/Chrome 48.

Please note that we also got an external report saying:
"
we definitely notice a progressive slow down. I.e. the more draft replies you goto the worse the problem gets
the performance impact is on DOM operations specifically. We timed functions like appendChild and noticed that their invocation cost could increase by a factor of 10x. 
This impact on DOM operations doesn't have a huge impact on Gmail itself because it seems gmail does very few DOM operations in general
"

Comment 53 by avic...@google.com, May 31 2016

I was able to repro this with the same steps.
The code around the emoji picker is very inefficient and adds lots of elements in the DOM. However, the operations take insanely long, I had to stop collecting the trace, because my browser was crashing.

So please check the new report and see whether you see anything interesting from Chrome point of view.
Components: -Blink>Scheduling Blink>Layout
There is some very long running js in that trace!  The timer below ran for 2.5 seconds, there was also one very slow layout taking 1085ms to run FrameView::performLayout.  We'd need somebody from blink to take a look to see what happened there.  There are a number of 20-80ms layouts too. 

{frame: "0x14240769bee8",
 scriptId: "5909",
 scriptLine: 12893,
 scriptName: "https://mail.google.com/_/scs/mail-static/_/js/k=gmail.main.en.sPd6lfIFLNc.DU/m=m_i,t,it/am=nlGvDez_vT8Y1xgG6EpfrTv__ef7J_1nL_fw_4aJUI1fgPz__3-S_wf79zYO/rt=h/d=1/rs=AHGWq9DT1iiVD5dXohtS4kKhQng0LskX0w"}
More information now available in b/28860580.

A summary:
We are investigating the issue by piggy-backing on a feature which is poorly implemented in Gmail, but it reveals that dom operations are very slow.

When bisecting, we saw:

49.0.2623.54 - No Repro
49.0.2623.87 - No Repro

50.0.2661.75 - REPRO
50.0.2661.102 - REPRO

51.0.2704.63 - REPRO

In Chrome48 and Chrome50, I am using the same account, the same Gmail build, the same steps. Some stats I can see:
After opening 10 large drafts, Chrome48 stays around 50k nodes, while Chrome50 goes up to 80k.
After one day of using Gmail, Chrome50 went to 220k nodes. There, the operations slow down quite significantly.

In Chrome50, around 200k, a certain method takes 250ms
In Chrome48, around 50k, the same method takes 62ms

So I believe this boils down to the fact that the number of nodes are not gc in Chrome50.


I am not sure whether the number of nodes is relevant anymore. I can repro on Chrome50 with 55k nodes. With the same number of nodes in Chrome48 does not repro. See the attached timeline.
Components: Blink>MemoryAllocator>GarbageCollection
Status: Untriaged
Adding Oilpan label, as maybe it is Oilpan related given the node counts and Oilpan was enabled in m50, right?. Sorry if I'm misunderstanding the reporting information.

Maybe try with one of these m49 builds to confirm:
https://docs.google.com/document/d/1co1tfKBCTqHZUDok-xzGjwspOek1LBfduDk06bUW7EU/edit
schenney@ can somebody in your team help the triage for this?
We (Gmail team) don't have the right context of features Chrome team does.
We have a pretty good range now: https://chromium.googlesource.com/chromium/src/+log/56f66725674e6e4795e02b16c928372450061129..45f46be8dd032d12b198ca4e81d60b652b27dce6

You can see that "Enable Oilpan on ToT" is there in the change list.
What is our next step here?
Attaching chrome://tracing as per b/28860580#comment120 repro case.

Thank you!
Cc: haraken@chromium.org
+haraken@, could you take a look at this and see if it could be Oilpan-related?
Owner: haraken@chromium.org
Cc: sigbjo...@opera.com
Owner: keishi@chromium.org
Status: Assigned (was: Untriaged)
+keishi: Would you take a look at this (with high priority)?

+sigbjorn: Any idea is welcome.

Here is a summary of this bug:

Problem: When you click the emoji-picker in gmail (http://haraken.info/a/iHkpWUd9MRi.png), it takes a very long time to open the window. See #49 for the repro step. We identified that the issue is caused by enabling Oilpan. #45 is saying that this is caused by the slowness of appendChild.

The essence of the repro step is as follows:

1) Do something complicated and create a ton of Nodes (Note: I'm not sure if the culprit is the Node object or not)
2) Click the "Compose" button and click the emoji-picker button.

As far as I experimented, it looks like that the emoji-picker can take an arbitrarily long time depending on when the last Oilpan's GC ran. If the Oilpan's GC hasn't run for a long time, it takes a very long time. This indicates that the culprit would be related to weak processing. I suspect that appendChild is doing something that scans a HeapHashSet<WeakMember<X>> and the cost depends on the size of the hash set. If Oilpan's GC hasn't run for a long time, the hash set grows up, making the appendChild extremely slow. I haven't identified what weak members are causing the issue though.

This is very high priority.


Cc: keishi@chromium.org
Owner: yosin@chromium.org
OK, I identified the root cause. It's the size of Document::m_ranges.

Easier repro step:

1) Click the "Compose" button.
2) Write some in the email. It creates a lot of ranges. Document::m_ranges grows up.
3) Click the emoji-picker button. It takes a long time because each appendChild needs to iterate the Document::m_ranges. Document::m_ranges is not cleared until the next Oilpan's GC happens.

yosin@: I think we've worked on promptly detaching Range objects as much as possible, but this bug indicates it's not enough. Can you take a look at this?



Cc: yoichio@chromium.org
Cc: rmcilroy@chromium.org
I attempt to which C++ function creates a lot of Range object by chrome:://tracing
with http://crrev.com/2030713003

I saw 192 Range object after typing 10+ characters. But, I've not yet to identify the function.
Cc: oilpan-reviews@chromium.org tkonch...@chromium.org
 Issue 613816  has been merged into this issue.
I got 255 Range object by typing "are you ready to go?" 20 characters, 3 times.
39 Range objects are from Selection#getRangeAt().

I attempt to identify rest of them.
127 Range objects comes from Selection#getRangeAt

See attached trace which contains "Range ctor Node" and "Range ctor Document" events.
More details:

Range ctor Document 3
Range ctor Node = 255
= Total 258 Range object create during typing "are you ready go?\n" 3 times

Document::createRange 3
DOMSelection#getRangeAt 149

So, I can explain 152 Range objects, but not other 156 Range objects yet. :-<

Owner: keishi@chromium.org
When we stop growing Document::m_ranges, opening emoji-picker is still slow.
So, it seems # of ranges doesn't matter.

Assign to keishi@, to check Node#apendChild
Project Member

Comment 73 by sheriffbot@chromium.org, Jun 2 2016

Labels: Hotlist-Google
Cc: tkent@chromium.org
m_ranges has a larger amount of deleted entries, I notice. A sparse table to iterate over.
i.e., for weak hash tables which are pruned during weak processing and the code otherwise (nearly never) explicitly removes entries, shouldShrink() won't be consulted and tables with low load factors will remain. I'm not sure that's intentional.
Here is my theory so far.

The problem seems to be that when Gmail has over 60K nodes, appendChild is 40x slower.

My experiment steps were:
1. Open gmail
2. Open 4 compose windows each with lots of content (in my case a copy&paste of theverge.com)
3. From devtools timeline pane confirm that the node count is over 60K
4. Run the following from the devtools console

console.time("bench");
for (i = 0; i < 1000; i++)
document.body.appendChild(document.createElement("div"));
console.timeEnd("bench");

5. The time for me is around 40~100ms. When oilpan is off, it is around 2ms.

I took a trace of this (attached image) and it revealed that Document::updateRangesAfterChildrenChanged is slow.

However the size of Document::m_ranges is around 5 so it isn't the number of Ranges that is at fault.

I'm going to investigate how updating a Range could be related to dom tree size.
Screen Shot 2016-06-03 at 12.28.32 AM.png
28.4 KB View Download
The Document::m_ranges.size seems to vary wildly. By interacting with the compose window I saw Document::m_ranges.size as big as 70k.

Range::nodeChildrenChanged doesn't really do anything (related to the dom tree) so I'm thinking haraken@'s original theory was right. Document::m_ranges.size can become to big if gc isn't triggering properly.
Does m_ranges.capacity() decrease after those high peaks?
I can easily make Document::m_ranges.size exceed 60K.

You're right m_ranges.capacity() does not shrink after Blink GC.
I can make m_range very sparse like size 2 and capacity 524288.
I need to confirm but appendChild when m_range is (size 2 capacity 524288) is faster than m_range at (size 24233 capacity 524288) but still slower than expected.
So capacity may be part of the problem.
https://codereview.chromium.org/2034883002/ adds weak hash table shrinking, which will at least avoid skipping past a number of empty slots when iterating m_ranges.
I commented out m_ranges.add(range); from Document::attachRange and opening the emoji dialog became much much faster, probably fixed.

We should focus on the size and capacity of m_ranges.
Yeah, I tried something just like 2034883002 with additional BlinkGCs but I didn't see much of a difference.
selection.getRangeAt(N) is spec'ed iirc to return the same object across multiple calls, which Blink doesn't implement. That is contributing to the m_ranges buildup here.
Cc: braydenhass@google.com
Labels: -Pri-1 Pri-0
Given the severity, raising the priority to P0.

keishi@ is assigned to the bug but let's collaborate together and fix it ASAP.


Cc: esprehn@chromium.org
+Elliott: Any input is welcome. In short, this is P0. The root cause is that Document::m_ranges becomes too big because the entries are not weakly-processed until the next Oilpan's GC is triggered. appendChild() needs to iterate Document::m_ranges, which makes appendChild() extremely slower when Gmail creates a lot of ranges.



Is there any concern about making this bug public? Gmail customers want to be informed of our status.

Cc: bnutter@chromium.org
A couple of questions:

- Are most of the Range objects created by selection.getRangeAt()?

- If no (i.e., most of the Range objects are created internally in Blink), we should investigate where they come from and replace them with EphemeralRange.

- If yes, I can come up with the following solutions:

a) Fix Gmail not to use selection.getRangeAt().
b) Fix Blink so that selection.getRangeAt() returns the same object.
c) Disable Oilpan.
d) Add some hack to reduce # of objects in Document::m_ranges when a lot of Range objects are created (e.g., force Oilpan's GC after each V8's minor GC when Document::m_ranges becomes big).

I don't think c) is realistic. It has a risk of breaking other things more terribly.

Is b) realistic? What are the behaviors of other browsers? If other browsers is already returning the same object, b) can be an option.

I don't know how realistic a) would be. I'll discuss with the Gmail team once we're convinced that most of the Range objects come from selection.getRangeAt().

d) is worth considering.


I haven't checked where the Ranges come from. I was investigating how to do d) because I don't know how practical the other solutions you listed are. For d) I am trying to find the right gc timing to solve the regression.
yosin@ proposed another solution:

e) Resurrect Range::detach(). Fix Gmail so that it explicitly calls Range::detach() for ranges Gmail no longer needs.

Either way, the key would be where the majority of the Range objects comes from. (I want to investigate this but need to know how to reliably repro the issue.)

I think it makes sense to explore d) in parallel.

I am able to 100% repro using these steps

1. Open gmail
2. Open 4 compose windows each with lots of content (in my case a copy&paste of theverge.com)
3. From devtools timeline pane confirm that the node count is over 60K
4. Open new compose window and click on emoji button
5. emoji dialog will take ~10 seconds to appear

Comment 94 by wfh@chromium.org, Jun 3 2016

re: comment #88 I don't think we can open this bug given the amount of data attached to it, but I recommend perhaps opening a public bug that users can follow and marking this one as a blocker to that.
wfh@: I've received multiple strong requests from enterprise customers to open this bug (e.g., https://bugs.chromium.org/p/chromium/issues/detail?id=614856#c10). What can we do there?

The bug itself is not confidential. Maybe can we drop the attached files and make it public?



Comment 96 by wfh@chromium.org, Jun 3 2016

If this bug in particular must be opened then all attachments can just be deleted and it should be fine.
Re #93: Hmm, I cannot repro the bug with the steps though.

yosin@: Would you try the step in #93 and investigate where the majority of the Range objects come from?

Since we already identified the root cause, it would be okay to drop all attachements in the history. I'll delete them and make the bug public.


Labels: -Restrict-View-Google
keishi@: Maybe what we can do is to force an Oilpan's GC at the end of V8's minor and major GCs when Document::m_ranges is too big. The Oilpan's GC would make sense only after V8's GCs are triggered (because the range wrapper keeps a strong reference to the C++ Range object).




Yes I will do that.
I can now reliably repro the bug with the following step:

1. Open gmail
2. Open 4 compose windows each with lots of content (in my case a copy&paste of theverge.com). *Use Ctrl+Shit+v to copy the data, not Ctrl+v*.
3. From devtools timeline pane confirm that the node count is over 60K
4. Open new compose window and click on emoji button
5. emoji dialog will take ~10 seconds to appear

Labels: -Hotlist-GoogleApps -Hotlist-Google
Labels: Hotlist-GoogleApps Hotlist-Google
tkent@: I dropped the RVG label in #99 (you won't be able to see the label in the bottom form) but it's still marked as RVG. Am I doing something wrong?


Labels: allpublic
Owner: yosin@chromium.org
yosin@ identified the root cause. Would you update the status here?

I get 60K Range object just pasting theverge.com in one composed window as c#102.
We found the criminal, DocumentMarkerController::updateMarkerRenderedRect().
It creates Range object for each misspell word. It seems there are bug in spell checker.

Anyway, I'm creating simple patch to dispose Range object in updateMarkerRenderedRect(), since it is definitely temporary use. 

Status: Started (was: Assigned)
Committing... http://crrev.com/2034753003 as temporary solution. :-<

I confirmed that 2034753003 solves the emoji dialog slowness (using repro steps from Comment 102)
Project Member

Comment 111 by bugdroid1@chromium.org, Jun 3 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/544e48708ee9286a58dfa24e0f29e30cece34f3c

commit 544e48708ee9286a58dfa24e0f29e30cece34f3c
Author: yosin <yosin@chromium.org>
Date: Fri Jun 03 04:20:25 2016

Make updateMarkerRenderedRect() to dispose temporary Range object after use

This patch makes |updateMarkerRenderedRect()| in |DocumentMarkerController| to
dispose temporary |Range| object after use to prevent increasing number of
active |Range| object for avoiding slow down of DOM mutation operations.

Note: This patch is a temporary solution, we'll have |EphemeralRange| version
later.

BUG= 613658 
TEST=n/a; This is temporary solution, we'll have another solution later.

Review-Url: https://codereview.chromium.org/2034753003
Cr-Commit-Position: refs/heads/master@{#397626}

[modify] https://crrev.com/544e48708ee9286a58dfa24e0f29e30cece34f3c/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp

yosin@: Thanks for the quick fix!!! Would you merge it to M50, M51 and M52? We need to merge it by the end of today.


Labels: Merge-Request-51 Merge-Request-52 Merge-Request-50
By disabling Chrome's spell check in "chrome://settings/languages" , would this bypass the DocumentMarkerController::updateMarkerRenderedRect() issue ?
>#c114, yes, if you disable spell checking, spell check related Range objects aren't created since DocumentMarkerController::updateMarkerRenderedRect().
Project Member

Comment 116 by bugdroid1@chromium.org, Jun 3 2016

Labels: merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cafbc06082dc96e18194b5972b79e10e9efd4f42

commit cafbc06082dc96e18194b5972b79e10e9efd4f42
Author: Alex Mineer <amineer@chromium.org>
Date: Fri Jun 03 16:17:24 2016

Make updateMarkerRenderedRect() to dispose temporary Range object after use

This patch makes |updateMarkerRenderedRect()| in |DocumentMarkerController| to
dispose temporary |Range| object after use to prevent increasing number of
active |Range| object for avoiding slow down of DOM mutation operations.

Note: This patch is a temporary solution, we'll have |EphemeralRange| version
later.

BUG= 613658 
TEST=n/a; This is temporary solution, we'll have another solution later.

(cherry picked from commit 544e48708ee9286a58dfa24e0f29e30cece34f3c)

Review-Url: https://codereview.chromium.org/2034753003
Cr-Original-Commit-Position: refs/heads/master@{#397626}
Cr-Commit-Position: refs/branch-heads/2704@{#698}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/cafbc06082dc96e18194b5972b79e10e9efd4f42/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp

Per comment #116, this is already merged to M51. Is there anything pending here? If not, please remove "Merge-Request-51" label. Thank you.

Comment 118 by wfh@chromium.org, Jun 3 2016

Longer term I'd like to discuss how we could have detected this slow down by our metrics coverage - are there any metrics currently that would have detected this (can someone link them, so I can take a looK?), and if not, can we add some?
Labels: TE-Verified-51.0.2704.83 TE-Verified-M51
Tested the fix (per c#116) on Latest M51 Chrome#51.0.2704.83 (Win7 64-bit OS) and i am not seeing any lag after clicking emoji picker in compose window.

Followed the repro steps per c#49 as well as #93.

Thank you!
Labels: -Merge-Request-52 Merge-Approved-52
Approving merge to M52 branch 2743 as it is already verified on M51 (see comment #119).
(per c#119) When can we expect Chrome#51.0.2704.83 or greater to be released? 51.0.2704.79 is the current released version.
Re #118: We have a performance test for editing in textarea-edit.html (https://cs.chromium.org/chromium/src/third_party/WebKit/PerformanceTests/DOM/textarea-edit.html). However, the test was not adding pressure on the path we're discussing here.

We can add another performance test, but in practice it's not that realistic to add micro-benchmarks for all call paths.

A better idea would be to add performance tests for user interactions in gmail (we already have some, but I think it's just composing a lot of emails).

Project Member

Comment 123 by bugdroid1@chromium.org, Jun 4 2016

Labels: -merge-approved-52 merge-merged-2743
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/803a3ac5b77116151f02189ede18b22731de4579

commit 803a3ac5b77116151f02189ede18b22731de4579
Author: Kentaro Hara <haraken@chromium.org>
Date: Sat Jun 04 00:42:21 2016

Make updateMarkerRenderedRect() to dispose temporary Range object after use

This patch makes |updateMarkerRenderedRect()| in |DocumentMarkerController| to
dispose temporary |Range| object after use to prevent increasing number of
active |Range| object for avoiding slow down of DOM mutation operations.

Note: This patch is a temporary solution, we'll have |EphemeralRange| version
later.

BUG= 613658 
TEST=n/a; This is temporary solution, we'll have another solution later.

Review-Url: https://codereview.chromium.org/2034753003
Cr-Commit-Position: refs/heads/master@{#397626}
(cherry picked from commit 544e48708ee9286a58dfa24e0f29e30cece34f3c)

Review URL: https://codereview.chromium.org/2041493003 .

Cr-Commit-Position: refs/branch-heads/2743@{#225}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/803a3ac5b77116151f02189ede18b22731de4579/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp

Components: -Blink>Layout Blink>Editing
Labels: -Merge-Request-50 Merge-Review-50 Hotlist-Merge-Review
[Automated comment] There appears to be on-going work (i.e. bugroid changes), needs manual review.
Labels: -Merge-Request-51 Merge-Review-51
[Automated comment] There appears to be on-going work (i.e. bugroid changes), needs manual review.
Labels: -Merge-Request-50 Merge-Review-50 Hotlist-Merge-Review
[Automated comment] There appears to be on-going work (i.e. bugroid changes), needs manual review.
Status: Fixed (was: Started)
Labels: -Merge-Review-51
This has been merged into M51. Removing the merge review label.
Labels: TE-Verified-51.0.2704.84
Verified this issue on Ubuntu 14.04 and Windows 7 using chrome latest M51-51.0.2704.84 by following steps mentioned below

1. Opened Gmail
1. Replied to a bunch (10-20) of threads and kept them in drafts
2. Refreshed Gmail
3. Opened a compose window, clicked on the emoji picker and observed it comes up instantly.
4. Switched to drafts list
5. Clicked on each draft reply and then pressed back to go back to drafts list
6. Opened a new compose window
7. Clicked on the emoji picker and observed no lag to the emoji picker.

The fix is working as expected, Hence adding TE-Verified label.
This patch is now pushing out to stable channel in version 51.0.2704.84 for Desktop (Win,Mac & Linux).
Labels: TE-Verified-52.0.2743.33 TE-Verified-M52
Verified this issue on Ubuntu 14.04 and Windows 7 using chrome latest M52-52.0.2743.33 as well by following steps mentioned in the above comment. Observed no lag to the emoji picker. Hence adding TE-Verified label.

Labels: -Hotlist-GoogleApps Hotlist-Partner-GSuite
Showing comments 34 - 133 of 133 Older

Sign in to add a comment