35295 - PERF: edit rules postproccessing slow on large pastes.

Status: RESOLVED INCOMPLETE

(Core :: DOM: Editor, defect, P3)

Description

[Joe Francis]

the big culprit here is whitespace conversion.  Scott mentioned that there are 
stream-based strings in place now.  I could use these to convert whitespace on 
the fly as I am first inserting it into the dom - this should save some time.  
dont know how much...

Comment 1

[Joe Francis]

i accept Bugzilla as my lord and savior.  

Comment 2

[Joe Francis]

m17

Comment 3

[leger]

Adding perf keyword

Comment 4

[rubydoo123]

moving to m19

Comment 5

[rubydoo123]

please do not nominate for nsbeta3 -- this will be addressed after crashes, 
regressions, correctness and polish bugs have been resolved, all Composer perf 
bugs have been set to m19/m20

Comment 6

[rubydoo123]

reviewed by Bijal and beppe, setting to nsbeta3+, p2

Comment 7

[rubydoo123]

paste is such a high profile function, after much discussion, we are bumping
this up to p1 & rtm+

Comment 8

[rubydoo123]

Joe, please include the required information per the rtm checkin rules

Comment 9

[Phil Peterson]

PDT marking [rtm-] since the performance impact is not quantified. Please
renominate if/when the user impact is explained in the bug.

Comment 10

[rubydoo123]

will have to wait till post-rtm

Comment 11

[Joe Francis]

Ok, so this change is expiremental, but it does reduce the amount of 
editor code involved in quoting a mail reply tremendously.  Johnny, this 
patch uses InsertBefore() or AppendChild(), as appropriate, of a docFrag 
to put everything into the dom tree at once.

You can use this as a test bed for improving docFrag insertion speed, 
assuming you construct the right test message (one with a lot of nodes 
that are children of the docFrag).

Note that regular text insertion in the editor still does not use docFrags.  
That work will be more difficult, so I want to wait until docFrags are fast 
before tackling it.  So use HTML mail compose for your test.  

Comment 12

[Joe Francis]

Attachment: diffs of mozilla/editor

Comment 13

[Joe Francis]

Attachment: diffs for editor/base

Comment 14

[Joe Francis]

updated diffs.  this is actually the wrong bug to be putting them in.  I'll
track down the mail reply perf tracking bug and put them there too.

Comment 15

[Joe Francis]

Attachment: diffs for editor

Comment 16

[Joe Francis]

I left out an .idl file in earlier diffs.  This one should have everything you 
need.

Comment 17

[Simon Fraser [no longer active]]

nsHTMLEditor::InsertHTMLWithCharset() has this comment:
+  // nuke the undo/redo stack
Should this be implemented?

+NS_IMETHODIMP nsHTMLEditor::PasteHTML(const nsAReadableString & aInString)
+{
   nsAutoString charset;
-  return InsertHTMLWithCharsetAndContext(aInputString, charset, aContextStr, 
aInfoStr);
+  return PasteHTMLWithCharset(aInString, charset);
 }

Better to use NS_LITERAL_STRING("") in place of an empty nsAutoString. Ditto in 
PasteHTMLWithContext(), and in PasteHTMLWithCharset().

+#ifdef EDITOR_MAC_INSTRUMENTATION
+        INST_TRACE("EndReflowBatching");       
+#endif
Personally, I don't think we should litter the tree with instrumentation 
droppings.

r/sr on the changes if you address these issues.

Comment 18

[Joe Francis]

the swami says: things that will not land in 099!

Comment 19

[Kevin McCluskey (gone)]

Moving to Mozilla1.1. Engineers are overloaded working on higher priority bugs.

Comment 20

[rubydoo123]

removing myself from the cc list

Comment 21

[Joe Francis]

The days of having a half dozen milestones out in front of us to divide bugs 
between seem to be gone, though I dont know why.  Lumping everything together as 
far out as I can.  I'll pull back things that I am working on as I go.

Comment 22

[Joe Francis]

[ushing these out as far as bugzilla will let me.  I'll pull them back as I work
on them.

Comment 23

[Wayne Mery (:wsmwk)]

still a worthy patch?  (and if so, why major?)

Comment 24

[Wayne Mery (:wsmwk)]

Joe, was the perf gain ever quantified?
bug 35292 is the only bug to have checked in a patch for paste performance

Comment 25

[Wayne Mery (:wsmwk)]

not clear to me if this perf issue still exists, or if any of the related bugs have/describe testcase in detail (on cursory examination, i'm not finding any testcase files)

bug 28783 comment 0 has one description. And several detailed comments about code. note bug 74912 comment 13.

Comment 26

[Makoto Kato [:m_kato]]

old bug