22486 - Investigate creation of a msg compose/reply window performance

Status: VERIFIED WORKSFORME

(MailNews Core :: Composition, defect, P1)

Description

[Jean-Francois Ducarroz]

Are we really slow? why? and how can we fix that?

Comment 1

[ekrock's old account (dead)]

Corrected code name to the standard, [PERF]. (Don't use [PERFORMANCE].)

Comment 2

[leger]

Adding perf to keyword field.

Comment 3

[Phil Peterson]

Not an M13 stopper

Comment 4

[Phil Peterson]

Would like to improve this for beta1, but wouldn't hold for it.

Comment 5

[Jean-Francois Ducarroz]

Seem to be more a generic problem when opening a xul window. Move to M15

Comment 6

[selmer (gone)]

Mass moving to M16 to get these off the M15 radar.  Please let me know if this
is really an M15 stopper.

Comment 7

[selmer (gone)]

Not M16 stopper.  Marking M17.

Comment 8

[scottputterman]

moving to M18 and nominating for beta3

Comment 9

[lchiang]

What are the latest timings on this?

Comment 10

[Pratik Dhupia]

I think there are two ways one could time these. (1) is to open the compose 
window for the first time after starting the mail application (new mail session) 
and (2)is to open the compose window the second time in a single mail session. 

Here are the approximate values in seconds using build 2000-07-27-08 on Win95, 
Linux 6.1 and Mac OS ( performance machines )

Win95:
1) 12.23 sec
2) 7.21 sec

Linux: 
1) 18.32 sec
2) 9.67 sec

Mac: 
1) 25.12 sec
2) 12.24 sec

Comment 11

[scottputterman]

Mail triage marking [nsbeta3+]

Comment 12

[lchiang]

second pass: - per mail triage

Comment 13

[varada]

Adding mail1 keyword

Comment 14

[[e-mail] (gone - use [e-mail] instead)]

I've created bug 63759 which is a performance tracker bug for mail/news.

Comment 15

[scottputterman]

We are especially interested in reply performance. marking nsbeta1+ and moving
to mozilla0.8 though this will probably keep getting moved out as we do more
research.

Comment 16

[scottputterman]

changing priority.

Comment 17

[[e-mail] (gone - use [e-mail] instead)]

*** Bug 63551 has been marked as a duplicate of this bug. ***

Comment 18

[scottputterman]

moving to mozilla0.9.  Still looking into this but it's not going to be solved
by mozilla0.8 :)

Comment 19

[Jean-Francois Ducarroz]

Attachment: Add a timestamp feature to help use to get some good metrics when creating a compose window

Comment 20

[Jean-Francois Ducarroz]

This patch, which a similar one is already checked in the performance branch, let you output some timing numbers 
to the console or/and to a file. Which those numbers, you can track progress between version and let you see in 
which task (new window, initialization,  mime, editor, UI update) we are spending our time. To activate this 
feature, you need to add the following pref into you current profile:

   user_pref("mail.compose.trace_to_console", true);
and/or
   user_pref("mail.compose.trace_to_file", true);

the output file will be located into your profile folder, at the same level that the prefs.js file. One file is created 
per session, the name is msgcomposetrace.txt or msgcomposetrace-nn.txt
  

Comment 21

[Jean-Francois Ducarroz]

REMARK: this patch would have to be disabled before RTM at it consume space and time.

Comment 22

[Scott MacGregor]

Jean-Francois, I think we should be using PR_Logging to do most of this work for
you instead of adding so many ifdef's, and hand rolling your own file flushing code.

Let PR_Logging do all of that for you. To do that, look in nsImapProtocol.cpp
for IMAPMODULE or something like that. It'll show you how to set up a log
module. You can call yours MsgCompose or something like that. 

You'll still need if'defs for your new code to caculate the timing metrics but
all the actual data reporting should be done using the log module.

Let me know if you have questions and I can help.

Comment 23

[David :Bienvenu]

yes, I second mscott's comments - PR logging is the way to go.

Comment 24

[Jean-Francois Ducarroz]

no problem, I will do that... and correct the branch as well...

Comment 25

[Jean-Francois Ducarroz]

Attachment: New proposed fix, this time using PRLog

Comment 26

[David :Bienvenu]

r or sr=bienvenu

Comment 27

[(not reading, please use [e-mail] instead)]

r=sspitzer

Comment 28

[Håkan Waara]

Ok, I looked some in messengercompose.xul to see if it was the XUL/JS that is 
responsible for the long loading time.

But I disabled various parts of the code, to see if the loading went faster - 
but it didn't! So I'm pretty convinced it's the C++.

But I found one thing that might be of interest: should we really 
include "EditorCommandsDebug.js" in messengercompose.xul? It looks like 
something that's not needed which hogs CPU time.

If I decided to hack on mscompose performance in C++, where should I look? Has 
anyone found any critical code that could be optimized since this bug was filed 
in 1999?     Thanks..

Comment 29

[Brendan Eich [:brendan]]

In my few-days-old dogfood optimized Linux builds, I'm seeing huge delays (with
a black content area, no painting) waiting for a compose message to dismiss
after I hit Send.  Sorry if this is known and reported as another bug.  Someone
rent me a clue, or tell me to update my build?

/be

Comment 30

[Håkan Waara]

Brendan, the MsgCompose window is horribly inefficent and slow in many ways. 

Why, I don't know but I think MsgCompose should be the next target to do a major
cleanup on after the outliner stuff hits the trunk.

Comment 31

[Håkan Waara]

Hey, I have an idea!

Has anyone profiled the msgcompose window opening/closing to see where all the 
time is spent? That would help very much.

Comment 32

[Brendan Eich [:brendan]]

I only commented because this long (many seconds) black/non-painting compose
window that I see after replying and hitting Send is a fairly recent problem,
say in the last few weeks, in my optimized dogfood home-built Mozilla
experience.  Maybe I was just lucky earlier?

/be

Comment 33

[Oliver Klee]

Brendan, I started seeing this delay a few days ago as well. But I think that
should be another bug as it seems to be a bad regression.

Comment 34

[Jean-Francois Ducarroz]

I've just check in the timeStamp function. I'll post on mozilla.org instruction
how to use it...

Comment 35

[Håkan Waara]

This is (after the perf-branch lands) the worst performance problem in MailNews
IMHO. Nominating for nscatfood...(BTW, isn't this bug also a topperf?)

Comment 36

[[e-mail] (gone - use [e-mail] instead)]

Without a doubt, topperf.

Comment 37

[scottputterman]

adding dependencies.

Comment 38

[Joe Francis]

depends on the following bugs:
74904: ReplaceSubstring is O(n^2)
74901: Stop converting linefeed during a reply
74655: Stop converting linefeeds in editor InsertHTML (was: 40% of reply time in 
mailcompose spent converting linefeeds)
74656: 10% of reply time spent in O(n^2) line walking algorithm in nsLineBox
74907: Need efficient insertion of large flat docfrags into document
74912: nsHTMLEditor::InsertCitedQuotation should use fast docFrag insertion when 
available
74914: Remove unneeded editor postprocessing for mailcite insertion

Comment 39

[scottputterman]

moving to 0.9.1

Comment 40

[Chris Waterson]

Attachment: text version of quantify output, gzip'd text

Comment 41

[Chris Waterson]

Just profiled text insertion from the clipboard into an HTML editor window (is 
there a better bug to put this stuff in?) Looks like there's some low-hanging 
fruit here, but an editor person would be best to look at it.

Comment 42

[[e-mail] (gone - use [e-mail] instead)]

Just an update from my standpoint (from my original data in bug 74656) - Varada
and I will be looking into why msgcompose:5 logging is not working on my machine.

Comment 43

[selmer (gone)]

Updating dependency since 74907 was dup of 35294.

Comment 44

[selmer (gone)]

Does this have any chance of being completed this week?  Seems like it's time to
admit defeat again and move this to m.9.2... :-(   :-(

Comment 45

[scottputterman]

well, this is just a tracking bug. Hopefully some of the bugs that this is
dependent on that are still marked 0.9.1 will get fixed.

Comment 46

[Joe Francis]

Attachment: diffs for editor/base

Comment 47

[Joe Francis]

with the attached diffs, editor becomes a pipe through which much
mail/content/layout love flows.  Nothing is going on in editor-proper now during
creating of a mail reply compose window.  

On a 300 MHz G3 mac portable, doing a reply on debug tip with these patches, to
a message that has 4000 lines of text separated by 4000 break nodes, I get this:

>31  seconds  to respond to a 4000 line message
7 seconds from mail compose creating an editor to when it hands off the quoted
message to editor
1.6 seconds for parser to convert html stream into a document fragment
18.4 seconds for content to insert the document fragment into the document
3.45 seconds for layout to reflow the document
0.45 seconds for painting document

Chris, note that this code path is now different from pasting, so pasting is no
longer a fair test for this (though improving paste perf would be great too!).

Improving docfrag insertion into the document, and speeding up mail groveling
through the message, look like the two biggest opportunities here.

Comment 48

[Simon Fraser [no longer active]]

Joe: what are those numbers before these changes? How much does that last patch 
help us?

Comment 49

[Joe Francis]

Attachment: diffs of editor

Comment 50

[Joe Francis]

I left out an .idl file in the earlier diffs.  This set should have everything.

Comment 51

[Joe Francis]

Simon asks a good question. 
 
with my patch:
mail compose "NotifyDocuemntCreated()" to mail compose done: 30 seconds
 
without my patch:
mail compose "NotifyDocuemntCreated()" to mail compose done: 49 seconds

This speedup is a combination of not going through some postprocessing and also 
not using the more complicated paste code to do the insertion of the quote.

Comment 52

[scottputterman]

adding PDT+

Comment 53

[Jean-Francois Ducarroz]

I did some test on Mac using today debug build with or without joe's patch and
with Communicator 4.75 Release. The message I am replying to is 272K with about
9100 lines.

      total time to open the compose (insert data into editor)
4.75:              39   sec (n/a)
0.9.2 no patch:    87.5 sec (80.1)
0.9.2 with patch:  16.8 sec (9.8)

Very good job Joe, looks like we are not anymore exponential...

Comment 54

[scottputterman]

This is great!  It will be interesting to see some stats from some smaller sized
messages. Even better, JF was using a debug build without jst's patches.  Thanks
Joe.

Comment 55

[Jean-Francois Ducarroz]

same test with a smaller message: 33K for 1200 lines:

 total time to open the compose (insert data into editor)
4.75:               4   sec (n/a)
0.9.2 no patch:    12.1 sec (6.9)
0.9.2 with patch:   6.4 sec (1.4)

Comment 56

[[e-mail] (gone - use [e-mail] instead)]

A couple of my patches failed to apply, is this POSIXLY_CORRECT stuff I'm
running into?

Comment 57

[[e-mail] (gone - use [e-mail] instead)]

No worries, I had selected 2 files in my cut and paste (which caused patching to
fail).  A 36k message on my Win2K box (733mhz Pentium III with 384 megs of RAM)
takes 3 seconds to bring up and focus the reply window's insertion point.  I
will test a trunk build tomorrow on this same machine, but obviously this is too
high-end to be the main focus.  I will also export this build to the lab, where
my   266 MHz Pentium II, 128M RAM machine lives (the performance-testing
machien), and this should get us the solid numbers we need.

Comment 58

[Håkan Waara]

Great job Joe!! I am looking forward to see this checked in!

Comment 59

[Jean-Francois Ducarroz]

I tried jst patch on top of joe's one but I haven't see an improvement!

Comment 60

[[e-mail] (gone - use [e-mail] instead)]

Attachment: Before and after, Win2K, Pentium III 733mhz 384MB Ram (not my perf machine)

Comment 61

[[e-mail] (gone - use [e-mail] instead)]

I feel like a complete loser, forgot to apply the IDL patch, so now I will run 
stricly on the perf machine now with my new build.  Sorry for the delays.

Comment 62

[[e-mail] (gone - use [e-mail] instead)]

Attachment: Results for a 36k message on Win2k trunk 2001-06-08-04

Comment 63

[[e-mail] (gone - use [e-mail] instead)]

Attachment: Results for a 36k message with the 7 patches on Win2k fresh CVS

Comment 64

[[e-mail] (gone - use [e-mail] instead)]

I'm not seeing the same numbers that everyone else is seeing, am I not going
about this the correct way? JFD, what are the machine specs that you tested
these 7 patches on?  I just apply, do a depend and build, right?

Comment 65

[Jean-Francois Ducarroz]

I am using a Mac PowerBook G3/500Mhz/256Mb.

Stephen, how many lines does you test message has? Are you loading the message
from a POP account or IMAP? The test message should be on you local folder or in
a pop account to avoid to have to load it from the net.

Comment 66

[Jean-Francois Ducarroz]

Let me send you my test messages...

Comment 67

[[e-mail] (gone - use [e-mail] instead)]

Sigh, yet again.  I was using IMAP.  Not that I doubt you, but once we fetch the
message body, don't we cache it for insertion, or do we pull from the server,
yet again?  I will try POP3 with the messages you send me.

Comment 68

[[e-mail] (gone - use [e-mail] instead)]

The results I am about to attach with astound you all!  This is Windows 98 this
time, local folder, with a 1200 line message!

Comment 69

[[e-mail] (gone - use [e-mail] instead)]

Attachment: Conclusive results on a Win98 266 with 128 MB of RAM, 1200 line 34k message!

Comment 70

[[e-mail] (gone - use [e-mail] instead)]

To summarize the results I found:

OS/Machine config: Windows 98, Pentium II 266 mhz, 128 megs of RAM.
Message info: 1200 line message, 34k in size.
Builds: 2001-06-08-04 (*unpatched* trunk) vs. fresh CVS (*patched trunk).

---------------------------------------------------------------
Un-patched                        |                     Patched                 
---------------------------------------------------------------
[Run # 1] 12.04 seconds           |                 7.3 seconds
[Run # 2] 10.21 seconds           |                 5.5 seconds
[Run # 3] 10.40 seconds           |                 5.5 seconds
---------------------------------------------------------------
 40% improvement on the 1st run, 47 on the other 2 runs.

Comment 71

[Håkan Waara]

You might want to let npm.performance know about this amazing improvement!

Comment 72

[[e-mail] (gone - use [e-mail] instead)]

Attachment: Reply Perf Branch builds' msgcompose:5 log output

Comment 73

[[e-mail] (gone - use [e-mail] instead)]

Attachment: 2001061104 trunk build

Comment 74

[[e-mail] (gone - use [e-mail] instead)]

Attachment: Comparison between builds using 14k and 22k messages.

Comment 75

[selmer (gone)]

Please add eta info to status whiteboard.

Comment 76

[Jean-Francois Ducarroz]

We still have apparently some issue to solve, no eta yet...

Comment 77

[selmer (gone)]

It's getting late to take this in for 092.  Removing PDT+ and adding nsBranch. 
We'd like to see this on the trunk a little before making a decision to put it
on the branch.

Comment 78

[scottputterman]

Moving to 0.9.3

Comment 79

[scottputterman]

removing the nsbranch. We'll try to land this on the trunk instead.

Comment 80

[scottputterman]

moving to 0.9.4

Comment 81

[(not reading, please use [e-mail] instead)]

varada / ducarroz:

I don't think the last patch contains the problem I was worried about.

I was worried that to gain performance jfrancis removed the code that turned
newlines into <br>.  (see bug #67334.  that code lives in
editor/base/nsTextEditRules.cpp, see
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=37065 for a partial
patch that disables it.)

we need that hack so that after doing reply, the user can put the cursor on
blank lines.  with out it, you'd get dead spots.

ducarroz / varada:  

before you land this patch, make sure to test editor for regressions.  please
double check that bugs #20613 and #21121 haven't come back.  (those bugs are
what the newline -> <br> hack was for.)

Comment 82

[varada]

This has been tested by both mailnewsqa and editorqa for any possible and
obvious regressions. I would like to check this in today and require an R and SR
from folks in the Editor Team.
Akkana /Simon ?

Comment 83

[Akkana Peck]

The patch is way off in line numbers in nsHTMLDataTransfer.cpp and two of the
hunks failed.  I guessed where to put the first of the two (though I might have
guessed wrong, there are a lot of similar code blocks around there) but couldn't
find the match to the second one.  I'd feel more comfortable if I had a patch
that matched the current tree, so I knew I was reviewing the right changes.

All the other files patch okay, it's just nsHTMLDataTransfer.cpp.
I was using the 6/5/01 patch since that seemed to be the last one with all the
diffs.  If that's the wrong patch, please advise.

Comment 84

[varada]

Attachment: old patch updated on trunk

Comment 85

[Simon Fraser [no longer active]]

Comments:
+#ifdef EDITOR_MAC_INSTRUMENTATION
+        INST_TRACE("EndReflowBatching");       
+#endif

I don't think this instrumentation stuff should be checked in. It's mac-only, and 
intrusive. It also bloats the diff.

+  // force IME commit; set up rules sniffing and batching
+  ForceCompositionEnd();
+{
+  nsAutoEditBatch beginBatching(this);
...
+  // nuke the undo/redo stack
+}  
+  return res;

Bad indentation.

   nsAutoString charset;
-  return InsertHTMLWithCharsetAndContext(aInputString, charset, aContextStr, 
aInfoStr);
+  return PasteHTMLWithCharset(aInString, charset);

and

+  nsAutoString charset;
+  return PasteHTMLWithCharsetAndContext(aInputString, charset, aContextStr, 
aInfoStr);

and

+  return PasteHTMLWithCharsetAndContext(aInputString, aCharset, nsAutoString(), 
nsAutoString());

Empty strings are more efficiently supplied by NS_LITERAL_STRING(""), not 
nsAutoString.

Fix those, and sr=sfraser

Comment 86

[Akkana Peck]

I agree with Simon about the mac ifdefs.  If you want the instrumenting code to
stay in there, at least remove all the ifdefs sprinkled throughout the file and
use them once at the beginning of the file, so that other platforms could
implement the instrumentation (if it's useful) and the file isn't cluttered with
platform-specific ifdefs.  Why is it included in nsEditorShell.cpp, which has no
other changes?

I'm a little worried about removing kOpInsertQuotation from the list of
operations that do linebreak translation.  Does this mean that insert quotation
(ctrl-middleclick) in plaintext editors will be broken, or has this also been
modified to do the appropriate translation?

Why does InsertHTML become undoable?  Isn't that going to break undo for lots of
other routines which call InsertHTML?

Comment 87

[Akkana Peck]

Continued (had to submit last bit that because mozilla stopped letting me set
the caret to the end) ...

I'm confused about the new method PasteHTML which does not paste, but instead
inserts.  A method called "paste" should get info from the clipboard, not an
argument passed in, shouldn't it?  Or am I misunderstanding what it does?  A
routine called paste which doesn't actually refer to the clipboard would be a
nightmare for people who have to maintain these routines (those of us on the
editor team) and people doing triage on editor bugs (our boss).

Why did you add nsICaret.h to nsHTMLEditor.cpp when there were no other changes
except for instrumentation calls?

Comment 88

[Simon Fraser [no longer active]]

akk: note that most of these diffs are joe's, so ours is not to reason why. I do, 
however, agree with your comments.

Comment 89

[Akkana Peck]

It is mine to reason why when I'm asked to review, and to be honest, I'm worried
that this is the wrong solution to the problem.  (The right solution would be to
fix the caret-on-blank-lines problem, but perhaps there's something in between.)  

Aside from that, this is code that I sometimes have to maintain, and I'm not
happy having routines that claim to do one thing but actually do something else.
 I know I'm going to trip over that if this is checked in, and bugs will get
mis-filed to the wrong person because of misunderstandings, and I'm sure new
people coming in and trying to understand the code will also be confused by it.

I can't approve this the way it is.  If the routines were renamed to make it
clear what their purpose was (I'm not entirely clear at this point what the
difference is between the two routines -- can anyone explain?), I'd feel better
about the patch.   I do understand that mail reply has a performance problem
that needs to be fixed, but let's make sure we're doing it the right way and not
filling the editor with code that will be harder to maintain.

Comment 90

[Scott MacGregor]

I wouldn't presume to have mailnews folks changing core editor routines, but I'm
sure we could find someone to rename the routines Joe added here. =) Any
suggestions? 

Comment 91

[Scott MacGregor]

However, if you don't think Joe's solution is correct then that's another matter
and maybe we should have a meeting Monday to hash out what should be done to
make it right. (I just sent email about that trying to set it up). 

Comment 92

[Simon Fraser [no longer active]]

Can we have a new patch without all the #ifdef EDITOR_MAC_INSTRUMENTATION stuff 
please?

Comment 93

[kinmoz]

Attachment: Editor diffs minus the EDITOR_MAC_INSTRUMENTATION code.

Comment 94

[Simon Fraser [no longer active]]

About the method names:
What Joe has done here is to add new routines for HTML insertion. The existing 
code was kept for Paste, hence all the Paste methods do, actually, get called 
when pasting from the clipboard. The new code is called on HTML insertion, e.g. 
when mailnews inserted quoted text at reply time.

The real difference between the routines is that the Paste code breaks the data 
up and inserts them node-by-node into the document, whereas the insert code just
creates a document fragment, and slams that in. If we decide to choose new 
routine names, they should reflect this.

Comment 95

[(not reading, please use [e-mail] instead)]

I just talked to sfraser, and there's more than just renaming the names.

1)  this patch breaks editor.  with it, composer in html source mode, you'd get
the "dead line" problem, where you can't put the cursor on certain lines or
table cells.

2)  this patch leaves message reply with the same "dead line" problem.

so sfraser suggest we rethink this.  either we fix this patch continue to insert
<br> (to prevent the "dead line" problem), and figure out how to do that more
efficiently, or fix #85505, which will mean we don't have to insert <br>s anymore.

note, #85505 is known to be hard. 

FYI:  jfrancis comes back in 5 days.

Comment 96

[Akkana Peck]

Kin and I have been discussing simpler fixes.  We can get the code cleaner by
keeping the op changes Joe had, and keeping his new routine but giving it a new
name (SimpleInsertHTMLWithCharset or something) which only mail would call. 
This routine would have two differences from the current html:
- Avoid the node by node insertion of doc frag contents
  (this would also mean that it's not undo-able, but mail wouldn't care)
- Avoid calling ReplaceNewlines, which, yes, means that you won't be able to
click on blank lines inside a preformatted block in mail (see next paragraph).

I'm a little confused about parts of this bug -- the discussion in it has never
been clear whether we're talking about plaintext mail or html.  Joe's patch
seems to change html insertion exclusively; but the newline-to-br substitution
affects plaintext, not html (it will affect anything inserted into a plaintext
compose window, which shouldn't have anything to do with Joe's patch since mail
shouldn't be calling InsertHTML in that case anyway, and it will affect
InsertQuotation into an html editor when you're inserting a text/plain quoted
block, which also doesn't go through InsertHTML).  I'm not clear what path would
end up with mail calling InsertHTML on a long html block which needs to be
newline-to-br substituted.

None of the folks quoting performance numbers in this bug ever says (or at least
I can't find it) which compose window they're using, and what sort of message
they're replying to (plain or html, and if it's html, what sort of stuff is in
it).  Could someone clarify what exactly we're trying to fix here?

Comment 97

[(not reading, please use [e-mail] instead)]

ok, stephend has a test case he uses for his numbers.

see:

http://www.mozilla.org/mailnews/win_performance_results.html
"Click Reply All to a 2kb HTML message sent to 5 recipients. (Due to "Brutal
Sharing" of XUL windows, I now log the 2nd time it takes to open this window.)"

here is the message, save the file as a local mbox file (as a local folder) for
testing.

http://www.mozilla.org/mailnews/2khtmlmessage.eml

Comment 98

[(not reading, please use [e-mail] instead)]

actually, that message is pretty small.  I'm not sure that message truly shows
the "slow reply to large message problem" we're trying to address.

looking at the perf numbers, we're 4 seconds to reply to that and our target is
2 seconds.

for such a small message, the problem might be else where.

there are two issues:

1)  new compose window speed (not on the chart, but we do care about it.) 
recently blake landed some changes that really help in that department.  there's
still more to do.

2)  reply speed

the time to reply will improve if new compose window speed improves, but I think
what akk / kin / jfrancis / sfraser have been focusing on is the reply part,
especially of large messages.

akkana, kin, smfr, jfrancis:  I'll work on getting you a valid, bigger test case
that exposes the perf problem in editor, so we can work on that.

Comment 99

[Joe Francis]

So, I'm back now.  To address a few points that have cropped up...

1) I never really meant for this patch to be the polished "lets check it in" 
patch.  That's why I left in the instrumentation.  I though it might be useful in 
the interim, while folks were still working on this.  

2) Simon is right about the names, IIRC.  I don't think I introduced "PasteFoo()" 
in any new place where it wasn't before.  I agree we could have these (and many 
other) functions named better in the editor.  I don't believe my patch made 
matters any worse in that regard, though.

3) The regression with blank lines shouldn't surprise anyone, as I stated 
repeated all along in both mail and in the newsgroups that this was the tradeoff 
for this patch.  If someone(s) will tackle the caret problems on blank lines, a 
whole host of editor issues would go away.  I've been saying that for 2 years, 
and I have no reason to think it will ever happen.  Maybe I was wrong to ever 
work around the problem (perhaps that's why it never happened), but I wanted to 
ship a product.

Comment 100

[Joe Francis]

Hey, what can I say, it was a great sabbatical;  that's the only explanation for 
the sunshine shooting out of my butt.  So I was wrong about point 2 above: I did 
introduce "PasteFoo" names for methods that don't paste.  What was I thinking?  
Of course we should change those.

I'll attach a cleaner diff in a while.  Since this seems to be on the backburner 
for now, it may be a few days.

Comment 101

[Scott MacGregor]

we're still working on this but it sounds like we aren't going to have something
for emojo / .9.4.  Moving over to .9.5

Comment 102

[scottputterman]

moving to 0.9.6

Comment 103

[selmer (gone)]

096 is almost over.  Are we really ever going to fix this?

Comment 104

[Joe Francis]

I was thinking of closing this out, but I left it open as a reminder to check on
dependencies.

I recently landed some changes that are a portion of what was in this patch.  I
dont turn off the br replacement step, because mail needs that, and I do go
through the txn manager (unlike this patch) which probably loses back a very
small prtion of the perf gain, but I do still skip all the gratuitous paste code. 

So my guess is you will see a fairly significant perf gain if you test again
with today's builds.  

For some related operations, like paste, bug 35294 will help quite a bit when
and if it lands.  Other than that a lot of the various problems we identified so
long ago seem to have been fixed and landed.  Woohoo!

Comment 105

[scottputterman]

moving to 0.9.7 for continued work.  We shouldn't close it out.  Let's keep
using it as a meta bug for compose perf and adding bugs as dependencies.

Comment 106

[Judson Valeski]

I just pulled a trunk build today (it's been awhile). This bug is begging me to
move back to the 6.2 build. Trunk mail compose/reply window creation is
un-usably slow IMO.

bumping severity to critical

Comment 107

[[e-mail] (gone - use [e-mail] instead)]

Something regressed recently (since my 11-9-2001 build testing).  Taking a look
at when exactly this regressed...

Comment 108

[[e-mail] (gone - use [e-mail] instead)]

Here is what I've found so far.

This was Win98, PII 266, 128 megs of RAM.

                        BUILD DATE

Reply All/5 rcps 12      13     14      15
---------------------------------------------------
1st window     4.10     5.06    4.02   7.49      
2nd window     2.91     2.83    2.89   7.61
3rd window     2.89     2.85    2.87   7.68

Indeed, this is critical.  What is going on?  I'm going to start looking for
checkins that might have caused this.

Comment 109

[[e-mail] (gone - use [e-mail] instead)]

Wild stab, but I see bug 105561, which contains a fix to nsEditorShell.cpp.  I'm
assuming we in mailnews use this?

Comment 110

[[e-mail] (gone - use [e-mail] instead)]

Or maybe bug 104195?

Comment 111

[[e-mail] (gone - use [e-mail] instead)]

Charley, any ideas?  Think your changes might be affecting this?

Comment 112

[Scott MacGregor]

Please continue the discussion of this recent regression in today's build in
110355. 

Comment 113

[Charles Manske]

Clickable link: bug 110355

Comment 114

[Akkana Peck]

Is the regression in html compose, plaintext, or both?
(Sorry, asking here because no one's cc'ed on the other bug yet.  Please answer
in bug 110355.)

Comment 115

[scottputterman]

Investigation will continue in 0.9.8

Comment 116

[Jean-Francois Ducarroz]

Move to 0.9.9. We are still working on it...

Comment 117

[scottputterman]

Moving out. There are individual bugs we'll look at.

Comment 118

[David :Bienvenu]

marking wfm - we've done a lot for compose window performance, especially
caching a compose window...we can open a new bug for any remaining work.
Thunderbird will also help with its simpler ui.

Comment 119

[[e-mail] (gone - use [e-mail] instead)]

I concur, verified.