Should we stop recycling the compose window?

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

Should we stop recycling the compose window?

Joshua Cranmer-2
For over a decade, Thunderbird and SeaMonkey (and Mozilla Mail, back in
the day) has recycled compose windows. The code to do this is fairly
integrated into the entire compose infrastructure and requires several
hacks in various places to make things work correctly.

A change made on Monday caused our caching code to break (the short
explanation is that we hold onto in the compose window cache for
recycling isn't good enough to get a full window), lighting up our tree
in all sorts of colors that aren't a healthy green. There are two
options here: change the objects we hold onto or stop recycling the
windows. I have candidate patches for both of these options.

I'm going to take this as an opportunity to broach the possibility of
not recycling the compose window. There has existed a pref that dictates
the maximum number of compose windows to
save--mail.compose.max_recycled_windows--since the feature was first
implemented; this allows us to test the effect of not recycling without
actually committing any changes. I discovered today that I (or some
extension I once had) set this pref to 0; given that I don't ever recall
touching it, it's likely that I've had this pref set for several years
and never noticed anything untoward about it.

What are the ramifications of ripping this code out?
1. We have less reliance on low-level windowing code and have less risk
of a mozilla-central crashing us horribly.
2. The compose code becomes simpler and could possibly reduce bugs in
the long run.
3. One of the main stumbling blocks to compose-in-a-tab would be removed.
4. The time to start up a compose window may increase. By how much, I
don't know.

I myself am not sure that the regression in compose window startup time
is worth all of the pain that we get from trying to recycle windows. But
I am by no means an expert in the compose code, so I don't feel that I
have the right to unilaterally decide to make the switch. What are other
people's experiences in this regard?
Thoughts/comments/questions/concerns/flames/cheers?
_______________________________________________
dev-apps-thunderbird mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-apps-thunderbird
Reply | Threaded
Open this post in threaded view
|

Re: Should we stop recycling the compose window?

Blake Winton
On 25-07-12 17:03 , Joshua Cranmer wrote:
> What are the ramifications of ripping this code out?
> 4. The time to start up a compose window may increase. By how much, I
> don't know.

Can we measure it, and if so, how hard would it be?

Thanks,
Blake.

_______________________________________________
dev-apps-thunderbird mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-apps-thunderbird
Reply | Threaded
Open this post in threaded view
|

Re: Should we stop recycling the compose window?

Andrew Sutherland-3
On 07/25/2012 02:18 PM, Blake Winton wrote:
> On 25-07-12 17:03 , Joshua Cranmer wrote:
>> What are the ramifications of ripping this code out?
>> 4. The time to start up a compose window may increase. By how much, I
>> don't know.
>
> Can we measure it, and if so, how hard would it be?

Maybe we can just do both.  Since Joshua has patches for both, we could
just pre-load the compose window in general, but throw it away when
we're done.  (This is assuming that holding onto the window does not
have a high complexity on its own or cause massive window manager anger.)

Andrew

_______________________________________________
dev-apps-thunderbird mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-apps-thunderbird
Reply | Threaded
Open this post in threaded view
|

Re: Should we stop recycling the compose window?

Joshua Cranmer-2
In reply to this post by Blake Winton
On 7/25/2012 5:18 PM, Blake Winton wrote:
> On 25-07-12 17:03 , Joshua Cranmer wrote:
>> What are the ramifications of ripping this code out?
>> 4. The time to start up a compose window may increase. By how much, I
>> don't know.
>
> Can we measure it, and if so, how hard would it be?

I did some measurements by timing how long the composition mozmill tests
(which open the composition window several dozen times) took before and
after toggling the pref. After doing the tests in different orders and
with slightly different steps, it appears that *not* recycling the
window *decreases* the execution time by ~10s out of 4-5min. I'm not
sure how much I trust the numbers, but the suggestion that ripping out
the code might actually improve performance makes it look all the more
tempting..

Actual results, in order running the tests:
Recycled Windows = 1
real    5m56.989s
user    3m20.633s
sys     0m41.723s
Recycled Windows = 0
real    5m51.673s
user    3m15.976s
sys     0m44.215s

1
real    3m21.625s
user    2m10.684s
sys     0m40.951s
0
real    3m13.782s
user    2m11.404s
sys     0m39.490s

No recycling 1:
real    4m39.063s
user    3m34.529s
sys     0m43.843s

No recycling 2:
real    4m44.029s
user    3m38.886s
sys     0m44.603s

Recycling 1:
real    4m48.861s
user    3m41.642s
sys     0m44.903s

Recycling 2:
real    4m54.801s
user    3m49.030s
sys     0m43.739s

[Tested on my laptop in a Linux VM w/ 4/8 cores and 6/8G of mem. Build
is debug, near-tip w/patch to fix stuff applied.]
_______________________________________________
dev-apps-thunderbird mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-apps-thunderbird
Reply | Threaded
Open this post in threaded view
|

Re: Should we stop recycling the compose window?

Andreas Tscharner-2
In reply to this post by Joshua Cranmer-2
On 25.07.2012 23:03, Joshua Cranmer wrote:

[snip]
> What are the ramifications of ripping this code out?
> 1. We have less reliance on low-level windowing code and have less risk
> of a mozilla-central crashing us horribly.
> 2. The compose code becomes simpler and could possibly reduce bugs in
> the long run.
> 3. One of the main stumbling blocks to compose-in-a-tab would be removed.
> 4. The time to start up a compose window may increase. By how much, I
> don't know.

Rip it out. The advantages outweigh the disadvantage. And if your
measures are correct, it is not a disadvantage at all.

Best regards
        Andreas
--
Andreas Tscharner                                   <[hidden email]>
----------------------------------------------------------------------
"Intruder on level one. All Aliens please proceed to level one."
                                       -- Call in "Alien: Resurrection"
_______________________________________________
dev-apps-thunderbird mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-apps-thunderbird
Reply | Threaded
Open this post in threaded view
|

Re: Should we stop recycling the compose window?

Ron K.
In reply to this post by Joshua Cranmer-2
Joshua Cranmer on 7/25/2012 5:03 PM, keyboarded a reply:

> For over a decade, Thunderbird and SeaMonkey (and Mozilla Mail, back in
> the day) has recycled compose windows. The code to do this is fairly
> integrated into the entire compose infrastructure and requires several
> hacks in various places to make things work correctly.
>
>
>
> What are the ramifications of ripping this code out?
> 1. We have less reliance on low-level windowing code and have less risk
> of a mozilla-central crashing us horribly.
> 2. The compose code becomes simpler and could possibly reduce bugs in
> the long run.
> 3. One of the main stumbling blocks to compose-in-a-tab would be removed.
> 4. The time to start up a compose window may increase. By how much, I
> don't know.
>

I see enough advantage, from your comments, that removal of recycling is
desirable. What I wonder is how creaky that code is and the quality of
its documentation. I suspect some of the coding practices are no longer
the best, based on the codes age. The reliance on hacks concerns me as
they likely make the code fragile.

For documentation reasons the ripout should be a new bug and I would
like to be CC: on it for QA testing when it lands.

--
Ron K.
Who is General Failure, and why is he searching my HDD?
Kernel Restore reported Major Error used BSOD to msg the enemy!
_______________________________________________
dev-apps-thunderbird mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-apps-thunderbird
Reply | Threaded
Open this post in threaded view
|

Re: Should we stop recycling the compose window?

Jonathan Kamens-4
In reply to this post by Joshua Cranmer-2
I absolutely vote for no longer recycling compose windows.

If Firefox doesn't recycle browser windows, and surely they
are just as heavyweight to initialize as compose windows if
not more so, then I see no justification for Thunderbird to
continue recycling compose windows.

Ripping out the recycling functionality will make the code
smaller, simpler, and less complicated. It'll eliminate the
need for an event (compose-window-init) and all the baggage
associated with it in add-ons that use the compose window.

I honestly see no reason to preserve the recycling
functionality.
_______________________________________________
dev-apps-thunderbird mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-apps-thunderbird
Reply | Threaded
Open this post in threaded view
|

Re: Should we stop recycling the compose window?

Philip Chee
On Fri, 27 Jul 2012 00:55:17 +0000 (UTC), Jonathan Kamens wrote:

> I honestly see no reason to preserve the recycling
> functionality.

Well there goes our green certification.

Phil

--
Philip Chee <[hidden email]>, <[hidden email]>
http://flashblock.mozdev.org/ http://xsidebar.mozdev.org
Guard us from the she-wolf and the wolf, and guard us from the thief,
oh Night, and so be good for us to pass.
_______________________________________________
dev-apps-thunderbird mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-apps-thunderbird
WBT
Reply | Threaded
Open this post in threaded view
|

Re: Should we stop recycling the compose window?

WBT
>Well there goes our green certification.

Sounds like that went out when "A change made on Monday caused our caching
code to break (the short explanation is that we hold onto in the compose
window cache for recycling isn't good enough to get a full window),
lighting up our tree in all sorts of colors that aren't a healthy green."

-

Not recycling the compose window may fix another bug that I've observed
frequently, where after closing Thunderbird, the process remains active and
consumes a fairly steady 42-50% of CPU (which may be the max it can get on
a 2-core machine), though all windows have closed.  Attempts to restart
Thunderbird from this state always result in a "Thunderbird is running but
not responding" error, and I have to go in through Windows Task Manager to
kill it.  I haven't been able to nail down the precise triggering causes &
conditions for reproducibility, but the current hypothesis was somewhere
around "this happens when the last open window is a compose window and the
message is sent" or "the last two windows are a main window and compose
window, and the main window is closed at <some particular stage of
sending>."

I suspect the bug may be related to recycling and fixed by tearing that out
correctly.

-WBT


On Fri, Jul 27, 2012 at 2:10 AM, Philip Chee <[hidden email]> wrote:

> On Fri, 27 Jul 2012 00:55:17 +0000 (UTC), Jonathan Kamens wrote:
>
> > I honestly see no reason to preserve the recycling
> > functionality.
>
> Well there goes our green certification.
>
> Phil
>
> --
> Philip Chee <[hidden email]>, <[hidden email]>
> http://flashblock.mozdev.org/ http://xsidebar.mozdev.org
> Guard us from the she-wolf and the wolf, and guard us from the thief,
> oh Night, and so be good for us to pass.
> _______________________________________________
> dev-apps-thunderbird mailing list
> [hidden email]
> https://lists.mozilla.org/listinfo/dev-apps-thunderbird
>
_______________________________________________
dev-apps-thunderbird mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-apps-thunderbird
Reply | Threaded
Open this post in threaded view
|

Re: Should we stop recycling the compose window?

David Bienvenu
In reply to this post by Joshua Cranmer-2
On 7/25/2012 2:03 PM, Joshua Cranmer wrote:
>
> What are the ramifications of ripping this code out?
> 1. We have less reliance on low-level windowing code and have less risk of a mozilla-central crashing us horribly.
> 2. The compose code becomes simpler and could possibly reduce bugs in the long run.
> 3. One of the main stumbling blocks to compose-in-a-tab would be removed.
> 4. The time to start up a compose window may increase. By how much, I don't know.

5. Any memory leaks in the compose window code will become much more serious issues.

I'm all for getting rid of the caching, but the memory leaks will absolutely need to be fixed.

- David
_______________________________________________
dev-apps-thunderbird mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-apps-thunderbird
Reply | Threaded
Open this post in threaded view
|

Re: Should we stop recycling the compose window?

Mark Banner-4
On 30/07/2012 00:29, David Bienvenu wrote:
> On 7/25/2012 2:03 PM, Joshua Cranmer wrote:
>> 4. The time to start up a compose window may increase. By how much, I
>> don't know.

Without performance tests, I think the best way to measure this would be
via telemetry hooks - I'm guessing we should be able to measure time for
non-cached windows versus time for cached windows (preferably those just
for new messages not replying/forwarding attachments etc to avoid time
for rending and extra set up).

> 5. Any memory leaks in the compose window code will become much more
> serious issues.
>
> I'm all for getting rid of the caching, but the memory leaks will
> absolutely need to be fixed.

Agreed, there's definitely some at the moment that we'd need to fix up.

Mark.
_______________________________________________
dev-apps-thunderbird mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-apps-thunderbird