Closed
Bug 19273
Opened 25 years ago
Closed 25 years ago
Pasting text is very slow (reflow)
Categories
(Core :: Layout, defect, P3)
Tracking
()
M16
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.
Updated•25 years ago
|
Target Milestone: M13
Comment 1•25 years ago
|
||
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.
Assignee | ||
Updated•25 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•25 years ago
|
||
doh. thats not hpw it's -supposed- to work. accepting bug.
Assignee | ||
Comment 3•25 years ago
|
||
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.
Assignee | ||
Comment 4•25 years ago
|
||
made some improvements, but leaving open for now (want to work on this some
more).
Assignee | ||
Updated•25 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 5•25 years ago
|
||
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.
Updated•25 years ago
|
Status: VERIFIED → REOPENED
Comment 7•25 years ago
|
||
It's still slow. Reopening to assign to the owners of incremental reflow.
Updated•25 years ago
|
Assignee: jfrancis → troy
Status: REOPENED → NEW
Component: Editor → Layout
Summary: [Perf] Pasting text is very slow → [Perf] Pasting text is very slow (reflow)
Comment 8•25 years ago
|
||
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 ago → 25 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...
Updated•25 years ago
|
Status: RESOLVED → REOPENED
Comment 10•25 years ago
|
||
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.
Updated•25 years ago
|
Assignee: troy → jfrancis
Status: REOPENED → NEW
Comment 11•25 years ago
|
||
Joe, you said you had profiling numbers showing that reflow was the problem?
Comment 12•25 years ago
|
||
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 ago → 25 years ago
Keywords: perf
Summary: [Perf] Pasting text is very slow (reflow) → Pasting text is very slow (reflow)
Comment 13•25 years ago
|
||
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!
Comment 14•25 years ago
|
||
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.
Reporter | ||
Comment 15•25 years ago
|
||
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)
Updated•25 years ago
|
Status: RESOLVED → REOPENED
Updated•25 years ago
|
Resolution: WORKSFORME → ---
Comment 16•25 years ago
|
||
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.
Comment 17•25 years ago
|
||
Clearing resolution.
Comment 18•25 years ago
|
||
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?
Assignee | ||
Comment 19•25 years ago
|
||
*** Bug 21588 has been marked as a duplicate of this bug. ***
Assignee | ||
Updated•25 years ago
|
Target Milestone: M13 → M14
Assignee | ||
Comment 20•25 years ago
|
||
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...
Reporter | ||
Comment 21•25 years ago
|
||
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?
Comment 22•25 years ago
|
||
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.
Assignee | ||
Comment 23•25 years ago
|
||
ok, I'm confused. How can you possibly coalesce reflows if they aren't
asynchronous?
Comment 24•25 years ago
|
||
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
Assignee | ||
Comment 26•25 years ago
|
||
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 ago → 25 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•