Reducing the number of attributes in the thread-pane will reduce footprint and reduce the time it takes to construct content. Using the "class" attribute rather than arbitrary attributes for style rules will reduce the amount of time required to match style rules. Current (2000-09-21) profiling shows that about 25% of the time in mail scrolling is spent matching style rules, and 15% of the time is spent creating the "base" XUL content (not including anonymous content created from XBL).

You'll need to apply the last patch in bug 46134 to try out these changes.

Here is what I see: Before After Change SelectorMatches (calls) 363,545 231,010 36% nsXULElement::SetAttribute (calls) 5,958 5,188 13% nsXULElement::GetAttribute (calls) 247,598 116,147 53% style resolution (usec) 369,593 184,020 50% content construction (usec) 184,020 160,948 12% This is doing "page down" scrolling from the top to the bottom of an unthreaded message pane with 107 messages in it. It's hard for me to estimate the "real gain" here because I'm not sure of a good metric for measuring scrolling performance. Some comments: - We reduce the calls to SelectorMatches by ~1/3 because we no longer need to enumerate rules: they're all hashed by class. - Reducing calls to SetAttribute means less bloat, because we store fewer strings. - Reducing calls to GetAttribute() is a win with respect to the amount of string copying that we do.

This seems really cool. Is there any chance this will get into NS 6.0 (e.g., an exception from the PDT)? I'll try it out anyway.

I agree that this is great. I'll nominate for rtm. Thanks for looking at this.

Can someone make a stab at estimating the benefit? How perceptible is the improvement? (Maybe it's most noticable on Linux where we need the most help.) I think we can get a plus on this if we have a reasonable story about the benefit.

once chris fixes the multi-value attribute thing, this is a very low-impact fix that will likely have a minor benefit, but a benefit none the less. Risk is very low - possible odd highlighting or display in the threadpane, which will be very obvious. I think we should take it. Thanks a ton for doing the work, chris.

rtm+ to get on PDT radar, not sure the benefit is justified though.

I think the above patch covers the remaining skins. I've checked the fix for bug 46134 in on the trunk.

I did some very unscientific "end user" measurement: timed how long it took me to page down through an unthreaded news group with 3,100 message (n.p.m.mac). On my Linux box, I saw time drop from 102s to 93s, or about a 9% improvement.

Marking rtm++. Please land this as soon as possible so that we have time to fix any problems this might cause.

Assuming I applied the patches correctly there looks to be a problem. The thread pane icon only works with the last attribute in the class. With the current patch, imapDeleted is the only icon that shows up correctly. If I move MessageType to the end, it works but then you can't see the new state icons because the Status attribute isn't working. The easiest way to see this is if you open up a news folder, it will have the same icon as a mail folder. Also, if you send yourself a new mail, it should have the "new mail" icon, and it currently doesn't.

reassigning to waterson.

Downgrading status b/c scottip sez still problems with the patch.

Sure enough, there's a bug: it's in string code. I've filed bug 55143 and added a dependency. I can work around the problem with the patch I'm about to attach.

Sure enough, yet another bug in the template builder! When >1 variable is in the attribute, we were "forgetting" that an earlier variable was impacted by a change. Need to logically-or the result from IsVarInSet() with previous values.

a = hyatt

oo.. sexy. a=ben (XUL stuff)

Fixes checked in to the trunk.

re-marking rtm++

fixes checked in on branch

Using branch build dated 10-16-00 on win98, mac and linux Not sure how I can test this bug fix, so I will use performace results for scrolling through a 500 msg POP account: Windows: On 9-21 arrow= 61.19 seconds, dragging =1.86 seconds. On 10-16 arrow= 86.84 seconds, dragging =2.73 seconds. Linux: On 8-30 arrow= 62.47, dragging = 3.61 On 10-16 arrow=93.14, dragging = 2.24 Mac: On 8-30 arrow=73.80, dragging = 2.35 On 10-16 arrow=72.87, dragging =1.72 Per these performance results there is some improvement on Mac, but win and linux is worse. Not sure if other bugs caused this performance issue it. If I should test this in another way, please provide a test scenario.

Yeah, I think your testing may be confounded by some other tree widget changes from hyatt and evaughan that have gone in. There is a known performance regression with scrolling; can't recall the bug number though.

esther: if you're bored ;-), you could try comparing branch builds from 10-10-2000 and 10-11-2000 (since the fix went into the branch on 10-10-2000). I don't think that any of the new tree stuff went into the branch until later...

The other bug is http://bugzilla.mozilla.org/show_bug.cgi?id=56683, although it is a two-headed beast right now: one part is specific to scrolling in an HTML page (slower due to line-height scrolled), and the other is a win98/win2k scrolling speed difference in an IMAP message folder. However, for the same build, it's hard to reconcile the numbers I got on that bug, with the numbers posted here. We should get together to figure out the reason for the difference.

I'll add the url to the Performance results so you can check the numbers. We're not bored, we have lots to do, but we need to try to verify the rtm++ bugs. This one is a tough one because we don't have specific numbers for the poor performance, and the time lapse between fix and verification has allowed other bugs to creep in. Now we just need to know that the performance is acceptable for rtm per performance test results for mail.

The difference in the times for scrolling is due to system config. Our testing for performance is with a 166 mhz 64MB RAM, the test morrison did is with a 500mhz 128MB RAM. Bug 56683 mentioned above is for scrolling speed in both the browser and 3-pane mail window it was logged on 10-14. I will verify this one since 56683 is more current.