Investigate creation of a msg compose/reply window performance

VERIFIED WORKSFORME

Status

defect
P1
critical
VERIFIED WORKSFORME
20 years ago
5 months ago

People

(Reporter: bugzilla, Assigned: bugzilla)

Tracking

(Depends on 2 bugs, {perf, topperf})

Trunk
mozilla1.2alpha
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(14 attachments)

10.07 KB, patch
Details | Diff | Splinter Review
7.22 KB, patch
Details | Diff | Splinter Review
388.27 KB, application/x-gzip
Details
16.71 KB, patch
Details | Diff | Splinter Review
19.18 KB, patch
Details | Diff | Splinter Review
1.37 KB, text/plain
Details
1.97 KB, text/plain
Details
1.97 KB, text/plain
Details
4.05 KB, text/plain
Details
1.97 KB, text/plain
Details
1.97 KB, text/plain
Details
4.21 KB, text/plain
Details
14.16 KB, patch
Details | Diff | Splinter Review
11.50 KB, patch
Details | Diff | Splinter Review
(Assignee)

Description

20 years ago
Are we really slow? why? and how can we fix that?
(Assignee)

Updated

20 years ago
Status: NEW → ASSIGNED
OS: Mac System 8.5 → All
Hardware: Macintosh → All
Target Milestone: M13
Summary: [PERFORMANCE] Investigate creation of a msg compose window performance → [PERF] Investigate creation of a msg compose window performance
Corrected code name to the standard, [PERF]. (Don't use [PERFORMANCE].)

Updated

20 years ago
Keywords: perf
Summary: [PERF] Investigate creation of a msg compose window performance → Investigate creation of a msg compose window performance

Comment 2

20 years ago
Adding perf to keyword field.

Updated

20 years ago
Target Milestone: M13 → M14

Comment 3

20 years ago
Not an M13 stopper

Updated

20 years ago
QA Contact: lchiang → suresh

Comment 4

20 years ago
Would like to improve this for beta1, but wouldn't hold for it.
(Assignee)

Comment 5

19 years ago
Seem to be more a generic problem when opening a xul window. Move to M15
Target Milestone: M14 → M15

Comment 6

19 years ago
Mass moving to M16 to get these off the M15 radar.  Please let me know if this
is really an M15 stopper.
Target Milestone: M15 → M16

Comment 7

19 years ago
Not M16 stopper.  Marking M17.
Target Milestone: M16 → M17

Comment 8

19 years ago
moving to M18 and nominating for beta3
Keywords: nsbeta3
Target Milestone: M17 → M18

Comment 9

19 years ago
What are the latest timings on this?
QA Contact: suresh → pratikd

Comment 10

19 years ago
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

19 years ago
Mail triage marking [nsbeta3+]
Whiteboard: [nsbeta3+]
Depends on: 49141
No longer depends on: 49141

Comment 12

19 years ago
second pass: - per mail triage
Whiteboard: [nsbeta3+] → [nsbeta3-][cut 8/28]

Comment 13

19 years ago
Adding mail1 keyword
Keywords: mail1

Updated

19 years ago
QA Contact: pratikd → esther
QA Contact: esther → stephend
I've created bug 63759 which is a performance tracker bug for mail/news.

Comment 15

19 years ago
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.
Keywords: nsbeta3nsbeta1
Whiteboard: [nsbeta3-][cut 8/28] → [nsbeta1+]
Target Milestone: M18 → mozilla0.8

Comment 16

19 years ago
changing priority.
Priority: P3 → P1
Summary: Investigate creation of a msg compose window performance → Investigate creation of a msg compose/reply window performance
*** Bug 63551 has been marked as a duplicate of this bug. ***

Comment 18

19 years ago
moving to mozilla0.9.  Still looking into this but it's not going to be solved
by mozilla0.8 :)
Target Milestone: mozilla0.8 → mozilla0.9
(Assignee)

Comment 20

18 years ago
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
  
(Assignee)

Comment 21

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

Comment 22

18 years ago
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

18 years ago
yes, I second mscott's comments - PR logging is the way to go.
(Assignee)

Comment 24

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

Comment 26

18 years ago
r or sr=bienvenu

Comment 28

18 years ago
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..
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

18 years ago
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

18 years ago
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.
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

18 years ago
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.
(Assignee)

Comment 34

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

Comment 35

18 years ago
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?)
Keywords: nsCatFood
(Assignee)

Updated

18 years ago
Depends on: 74901

Updated

18 years ago
Depends on: 74904

Comment 37

18 years ago
adding dependencies.
Depends on: 74911, 74913, 74915

Comment 38

18 years ago
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
Depends on: 74655, 74656, 74907, 74912, 74914
No longer depends on: 74911, 74913, 74915
(Assignee)

Updated

18 years ago
Depends on: 75618

Updated

18 years ago
Keywords: nsCatFood+

Updated

18 years ago
Keywords: nsCatFood

Comment 39

18 years ago
moving to 0.9.1
Target Milestone: mozilla0.9 → mozilla0.9.1

Comment 41

18 years ago
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.
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.

Updated

18 years ago
Depends on: 35294

Comment 43

18 years ago
Updating dependency since 74907 was dup of 35294.
No longer depends on: 74907

Comment 44

18 years ago
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

18 years ago
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.
Target Milestone: mozilla0.9.1 → mozilla0.9.2

Comment 47

18 years ago
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.
Joe: what are those numbers before these changes? How much does that last patch 
help us?

Comment 49

18 years ago

Comment 50

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

Comment 51

18 years ago
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

18 years ago
adding PDT+
Whiteboard: [nsbeta1+] → [nsbeta1+][PDT+]
(Assignee)

Comment 53

18 years ago
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

18 years ago
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.
(Assignee)

Comment 55

18 years ago
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)


A couple of my patches failed to apply, is this POSIXLY_CORRECT stuff I'm
running into?
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

18 years ago
Great job Joe!! I am looking forward to see this checked in!
(Assignee)

Comment 59

18 years ago
I tried jst patch on top of joe's one but I haven't see an improvement!
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.
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?
(Assignee)

Comment 65

18 years ago
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.
(Assignee)

Comment 66

18 years ago
Let me send you my test messages...
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.
The results I am about to attach with astound you all!  This is Windows 98 this
time, local folder, with a 1200 line message!
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

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

Comment 75

18 years ago
Please add eta info to status whiteboard.

Updated

18 years ago
Whiteboard: [nsbeta1+][PDT+] → [nsbeta1+][PDT+] need eta
(Assignee)

Comment 76

18 years ago
We still have apparently some issue to solve, no eta yet...
Whiteboard: [nsbeta1+][PDT+] need eta → [nsbeta1+][PDT+] No ETA yet

Comment 77

18 years ago
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.
Keywords: nsBranch
Whiteboard: [nsbeta1+][PDT+] No ETA yet → [nsbeta1+]No ETA yet

Comment 78

18 years ago
Moving to 0.9.3
Target Milestone: mozilla0.9.2 → mozilla0.9.3

Comment 79

18 years ago
removing the nsbranch. We'll try to land this on the trunk instead.
Keywords: nsBranch

Updated

18 years ago
Depends on: 89624

Comment 80

18 years ago
moving to 0.9.4
Target Milestone: mozilla0.9.3 → mozilla0.9.4
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

18 years ago
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

18 years ago
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.
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

18 years ago
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

18 years ago
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?

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

18 years ago
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

18 years ago
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

18 years ago
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). 
Can we have a new patch without all the #ifdef EDITOR_MAC_INSTRUMENTATION stuff 
please?
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.
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

18 years ago
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?
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
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

18 years ago
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

18 years ago
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

18 years ago
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
Target Milestone: mozilla0.9.4 → mozilla0.9.5

Comment 102

18 years ago
moving to 0.9.6
Target Milestone: mozilla0.9.5 → mozilla0.9.6

Updated

18 years ago
Depends on: 101480

Updated

18 years ago
Blocks: 104166

Comment 103

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

Comment 104

18 years ago
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

18 years ago
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.
Target Milestone: mozilla0.9.6 → mozilla0.9.7

Updated

18 years ago
Keywords: mail1nsbeta1+
Whiteboard: [nsbeta1+]No ETA yet

Comment 106

18 years ago
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
Severity: normal → critical
Something regressed recently (since my 11-9-2001 build testing).  Taking a look
at when exactly this regressed...
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.
Wild stab, but I see bug 105561, which contains a fix to nsEditorShell.cpp.  I'm
assuming we in mailnews use this?
Charley, any ideas?  Think your changes might be affecting this?

Comment 112

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

Comment 113

18 years ago
Clickable link: bug 110355

Comment 114

18 years ago
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

18 years ago
Investigation will continue in 0.9.8
Target Milestone: mozilla0.9.7 → mozilla0.9.8
(Assignee)

Comment 116

18 years ago
Move to 0.9.9. We are still working on it...
Target Milestone: mozilla0.9.8 → mozilla0.9.9

Comment 117

17 years ago
Moving out. There are individual bugs we'll look at.
Keywords: nsbeta1+nsbeta1-
Target Milestone: mozilla0.9.9 → mozilla1.2

Comment 118

16 years ago
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → WORKSFORME
I concur, verified.
Status: RESOLVED → VERIFIED
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.