Closed Bug 19273 Opened 25 years ago Closed 25 years ago

Pasting text is very slow (reflow)

Categories

(Core :: Layout, defect, P3)

All
Mac System 8.5
defect

Tracking

()

VERIFIED DUPLICATE of bug 28783

People

(Reporter: sfraser_bugs, Assigned: mozeditor)

References

Details

(Keywords: perf)

Paste a few lines of text into Composer -- it can take several seconds, depending on how much you are pasting. The reason it's so slow is that nsHTMLEditRules::WillInsertText() is doing an InsertTextTxn for each character in the clipboard, which in turn does update batching etc. But we are going to be reflowing for each insert, which is not good.
Target Milestone: M13
I think the deadline for M12 is pretty soon and I know Joe has a lot of stuff he is trying to land before then so I'm initially triaging this bug to be M13. It should be done sooner rather than later since it could involve some rearchitecting to insert the text in chunks rather than single characters.
Status: NEW → ASSIGNED
doh. thats not hpw it's -supposed- to work. accepting bug.
I fixed the "one character pasted at a time" bug. But I'm leaving this open, because paste still is butt slow. Since we can see the caret boogying around, I assume somehow selection updating isn't getting turned off during the paste, though from the code it certainly appears to be. I also have some ideas for making other improvements to the paste code.
made some improvements, but leaving open for now (want to work on this some more).
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
i've investigated this further. profiling reveals that reflow is the culprit. There is a pref that you can add to prefs.js: user_pref("layout.reflow.asynch",true); which seems to speed things up quite a bit. I believe it ends up coellescing a bunch of reflows that would otherwise be synchronously repeated. closing this out for now.
Status: RESOLVED → VERIFIED
verified in 1/13 build.
Status: VERIFIED → REOPENED
It's still slow. Reopening to assign to the owners of incremental reflow.
Assignee: jfrancis → troy
Status: REOPENED → NEW
Component: Editor → Layout
Summary: [Perf] Pasting text is very slow → [Perf] Pasting text is very slow (reflow)
Reassigning to layout since the remaining performance issue is in reflow. This may be a DUP of an existing bug on incremental reflow performance.
Status: NEW → RESOLVED
Closed: 25 years ago25 years ago
Resolution: FIXED → WORKSFORME
I pasted text in and it looked fine (layed out quickly) to me so marking this WORKSFORME. Please don't reopen this without some actual numbers to back up the assertion that reflow is very slow...
Status: RESOLVED → REOPENED
Reopening -- Joe says he has profiling numbers. Troy, seeming fast on one test (how many lines of text did you post?) on one platform is not an indication that performance is fine everywhere.
Assignee: troy → jfrancis
Status: REOPENED → NEW
Joe, you said you had profiling numbers showing that reflow was the problem?
Be reasonable. What do you expect me to do with a bug like that? There's no test case, and no numbers to substantiate the claim...
Status: NEW → RESOLVED
Closed: 25 years ago25 years ago
Keywords: perf
Summary: [Perf] Pasting text is very slow (reflow) → Pasting text is very slow (reflow)
Editting has had much improvement lately. Setting to Resolved/WorksForMe instead of Reopened. sujay, how is this looking to you? Please mark Verified if performance is better. Or reopen if you think this is impacting Dogfood. Thanks!
Turns out Jan only checked it with a couple of lines, and not on Linux (not clear whether she tried it on Windows). With a debug build from 1/12, it took me roughly 4 seconds to paste 40 lines of plaintext. On 1/14 it's just under 3 seconds. I'd still be interested in seeing the performance numbers on Mac. But I'll let Sujay make the call based on what he sees in the release builds.
pasting 40 lines takes about 2 seconds on a fast Mac. I think to get a good handle on this, we should testing large pastes (2 or more pages)
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Good idea. So here's the test I used, with a 1/14 debug build: Run mozilla -edit. View EditorInitPage.html in a plaintext editor, and copy the whole file (175 lines). Paste the contents into the editor, timing on a stopwatch. I count 18 seconds on my 1/14 debug build before I see anything in the window. That seems slow enough to justify reopening. Lots of people copy/paste whole web pages.
Clearing resolution.
An interesting question is why is it so slow. Certainly we can lay the exact same document out very quickly That suggests to me anyway that we're probably do a whole heck of a lot of incremental reflows. Most of which are probably unnecessary How many DOM calls do you make when inserting the text, and how many ContentInserted/ContentAppended() calls does the content sink make to the frame construction code?
*** Bug 21588 has been marked as a duplicate of this bug. ***
Target Milestone: M13 → M14
are we talking with asynch reflow or without? it's a fact that asynch reflow helps a lot. big kudos to nisheeth. nisheeth, you should use text pasting in the editor as a performance scenario for you reflow coellescing, if you weren't already. paste is still too slow even with nisheeth's work, but relow is no longer the dominant cause then. here are some numbers i have: pasting 5 lines of text (I couldn't use a bunch of text because it overwhelmed my profiler). ignore absolute values of the numbers - u cant trust them anyway. it's the relative values that are important. This was from a debug mac build, thursday jan 13, with profiling enabled. with asynch reflow OFF: paste took about 550 millisecs. 40% of that time was entirely inside of 48 calls to PresShell::ProcessReflowCommands() with asynch on: paste took about 310 milliseconds just shy of 30% was entirely inside 60 calls to nsDOMSelection::Collapse() 15% of the time was spent in 433 calls to CreateInstance. It looks like a lot of the expense here is hash table lookup in FindFactory(). About 5% was inside 124 calls to nsContentSubtreeIterator::Init() I noted that a lot of time was spent in CreateInstance, specifically in the case of the selection code creating a range. Since nsRangeList is already in bed with nsRange anyway (it's in the same library, and was including the implementation header for nsRange already), I tried replacing all those CreateInstances() with direct calls to NS_NewRange(). This dropped the amount of time we spent in CreateInstance() by almost 30 milliseconds, and dropped the total number of calls to it from 433 to 303. The profiles of the synch/asynch cases are very different. If asynch reflow is going to be on by default anytime soon, we should forget about trying to improve the synch reflow case and concentrate on the asynch case. I can send/show these profiles to whoever wants them. You will need a mac and the MW Profile app (which is included on the codewarrior cds). To address Troy's question on the number of dom calls: 24 InsertData calls on text nodes for my 5 line paste, along with . nsDocument::ContentChanged() got hit 32 times. I can improve the editor efficiency (it can make fewer calls). It's nowhere near as bad as it used to be but it's certainly not minimal. There were 10 calls to nsGenericHTMLContainerElement::InsertBefore() and 1 to RemoveChild(). Given that I have to break the paste out into 5 text nodes and 5 br nodes, this is already minimal. I'm m14'ing this puppy, and adding mjudge to cc list. Hope this helps...
So is async reflow going to become the default at some point? Is it valid to triage this bug based on performance with async reflow on?
Async reflow was sort of a temporary measure. The reason I asked about how many reflow commands were getting generated is that Nisheeth is working on reflow command coalescing which is more general purpose. Part of the changes have been checked in. He's working on changes to the block code to handle content appended/inserted and then content changed of text. Once all those changes are in you should see a noticeable improvement Nisheeth, let's make sure text frame is changed to use ReflowDirtyChild() just like the image frame code was changed.
ok, I'm confused. How can you possibly coalesce reflows if they aren't asynchronous?
The pref name is a little misleading. It's asynchronous in either case. The part that differs is in how the reflow command coalescing works. What we were doing was have the pres shell coalesce the reflow commands. Now we let the frames themselves coalesce the reflow commands. This allows greater flexibility, and it allows coalescing to occur in a hierarchical manner
m16
Target Milestone: M14 → M16
Blocks: 26388
i'm duping this over to 28783 - its the same issue *** This bug has been marked as a duplicate of 28783 ***
Status: REOPENED → RESOLVED
Closed: 25 years ago25 years ago
Resolution: --- → DUPLICATE
verified in 2/22 build.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.