Closed Bug 156607 Opened 22 years ago Closed 22 years ago

Chimera performance slows with large numbers of tabs open

Categories

(Camino Graveyard :: Tabbed Browsing, defect)

Product:
Camino Graveyard
Component:
Tabbed Browsing
Platform:
PowerPC
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
Chimera0.4

People

(Reporter: Stuart.Lamble, Assigned: sfraser_bugs)

References

Details

(Keywords: perf)

Attachments

(1 file, 1 obsolete file)

revised patch
16.72 KB, patch
Details | Diff | Splinter Review 
I have a "daily web sites" bookmark group, containing web sites that I visit on
a daily basis (news/comics type thing.) This group contains 21 elements. In
Mozilla, the pages come up reasonably quickly (it isn't going to be lightning
fast, but it does a decent job, considering that it's all coming through a
single proxy that only allows a limited number of simultaneous downloads...) On
Chimera, though, everything slows to a crawl: opening tabs, loading pages,
rendering, switching tabs, closing tabs... The difference is _very_ noticeable
from Mozilla's performance.

FYI, the 21 URLs are (from left to right in the tabs):
  http://www.sluggy.com/
  http://www.ubersoft.net/
  http://www.absurdnotions.org/
  http://www.clanofthecats.com/cotc_dailycomic.html
  http://www.brunothebandit.com/
  http://www.ozyandmillie.com/
  http://www.gpf-comics.com/
  http://www.sheldoncomics.com/
  http://www.comicspage.com/helen/
  http://www.purrsia.com/freefall/default.htm
  http://www.redmeat.com/redmeat/current/index.html
  http://www.kevinandkell.com/index.html
  http://www.pvponline.com/
  http://www.fborfw.com/strip_fix/index.html
  http://www.unitedmedia.com/creators/wizardofid/
  http://www.unitedmedia.com/comics/dilbert/
  http://www.theregister.co.uk/
  http://www.slashdot.org/
  http://www.kuro5hin.org/
  http://www.arstechnica.com/
  http://www.appleturns.com/

There are a couple of other issues too, but they're going to be filed
separately. ;-)

Hardware: Apple iBook (500 MHz G3, 256 MB RAM, Mac OS X 10.1.5).

Cheers,

Stuart.
I'm surprised that having 21 tabs open is fast in mozilla :-)

Maybe we should shut down js, animations, plugins, reflows, etc. in tabs
Assignee: saari → pinkerton
Revising Summary. What could improve this performance? Since Chimera is Cocoa,
this won't be helped by Mozilla's future move to Carbon events, will it?

Stuart, does performance also slow with 21 windows open rather than tabs?
Keywords: perf
Summary: Tabbed browsing is _SLOW_. → Chimera performance slows with large numbers of tabs open
saari -- I'm not saying Mozilla is _fast_, just saying that its performance is
fast compared with Chimera's in a similar situation. (Actually, "lightning"
would probably be a better relative description :-)

Greg -- re: performance with 21 windows rather than 21 tabs: yes, it slows, but
nowhere _near_ as much. With the windows open, the browser is still usable. With
the tabs, I basically have to go to lunch each time I tell it to do something
(such as close a tab).

One of these days, in my Copious Spare Time(tm), I'll have to recompile Chimera
and do some profiling... (as if I don't already have enough to do :)
See the attachment in bug 158169 for a sampler trace.
*** Bug 158169 has been marked as a duplicate of this bug. ***
I profiled this again with Sampler, testing on a simple page with a long page
title, and 6-7 tabs in the window. I profiled the time taken to add an
additional tab loading that test page.

Of 484 samples on the main thread, 119 end up in [NSTabView addTabViewItem] (on
the click handling stack), and 116 in [NSTabViewItem setLabel:] on a stack
called from pageload (nsDocument::SetTitle). So we can attribute 235 samples
directly to tab view manipulation. Another 50 stacks hit [NSTabViewItem
drawLabel] in two other code paths.

So it seems that we're hitting an NSTabView performance issue here. We might
have to override/implement our own tab widget.
Would it be worth raising a bug report with Apple on this? I can't see it
hurting, in any case. (And Apple would probably be glad to see Chimera as a
viable browser...)
richard writes...

"In short, during the same run through the event loop, we call
setNeedsDisplayInRect: on the area of the link, and then we call it again on the
progress text in the lower left.  On the next pass through the event loop, Cocoa
sees that two areas within the same view (probably the NSTabItem) have been
invalidated, unions the rects, and redraws the whole thing.  That means to
rollover a link near the top of a page, it has to redraw the whole damn page."

Are we sure it's cocoa doing it, not our own widget update code?
Status: NEW → ASSIGNED
Target Milestone: --- → Chimera0.4
Yeah, I'm pretty sure it's Cocoa that's causing this.  I've seen this behavior
in previous projects I've done.  If you invalidate two rects, Cocoa will union
the rects so it only has to call drawRect: once.

If I comment out the line that sets status text (and only that line), link
rollover performance is great.  In fact, far better in the debug build (when I'm
probably using more swap space than ram...), than in the nightlies.

Oh, and I don't think this is the right bug to be putting this in...  This
really has nothing to do with the number of tabs, it's quite apparent with only
a single browser view open.  Create a new bug?
you're right, it has nothing to do with this bug. my brain isn't working. sorry.
Target Milestone: Chimera0.4 → Future
For grins, I made up a CHTabViewItem class, and overrode the sizeOfLabel method
to make all equal sized tabs.  The speed increase is huge.  Even with 20 some
odd tabs open, a new blank tab can be added in a second or two - comparable to
Mozilla speed.  So Simon is right on the money.

Chimera already has a custom TabViewItem class -> CHIconTabViewItem.  Looks like
Matt Judy was setting it up so that you could put bookmark/favicon icons in the
tabs. 

So here's the question: do you want the tab views to have icons in the tabs or
not?  
It would be nice to experiment with some feedback in the tabs; showing spinning
arrows like mozilla during loading would be nice. We've also had requests for a
little close button on each tab. Another thing we need to show is drop feedback
when urls are dragged onto the tab.
Wow, nice catch.  We can get the icons and other things into the tabs later. 
Can you upload a patch that at least gets the speed fix into the nightlies?

As for statis icons in the tabs, that can quickly use up a lot of space.  How
about we go without the URL icon, but we can put a spinning arrow widget (or
spikey ball thingie, once Jaguar comes around) in there during loading.  It
shouldn't hurt to be adding/removing icons from the tab now that they will have
constant width, since it won't cause the tabs to shift around.

I'm not sure I want to have a close widget in each tab.  That would burn a lot
of space.  Perhaps a close tab toolbar item, or a single widget off to one side
that would close the active tab?
Attached patch faster tabs (obsolete) — — Splinter Review 
Here's the patch.  It's not perfect yet, but I think the flaws are minor
compared to the slow tab behavior.  

Good parts of this patch: 

Tab creation is fast, fast, fast with lots of tabs open.  Running sampler, all
those calls in NSTabView layout have disappeared.  All you see is the mozilla
layout code.  

If you want to display an icon/spinny loading thing/etc in the page tabs, just
call the setTabIcon label.  Display/layout of the label works OK when an icon
is present.

Bad parts of this patch:

First: Label text isn't centered in tab.  This violates Apple HIG.  So sue me.

Second: Label text truncation (ie, "Slashdot: News for Nerds" becomes
"Slash..."
when full text won't fit in the tab)  is erratic.  Sometimes tabs truncate
correctly.  Sometimes they don't.  I've gotten tired of making guesses as to
what number to put it to make this work all the time.  

Third: I only tested with one kind of image size : same as bookicon,
historyicon, searchicon.  If the image isn't this size, bad things might
happen.

Fourth: whatever else I haven't noticed.
Sorry - one more thing, which I should have mentioned before.  This patch
changes the header/license block on CHIconTabViewItem.h, .m, and
CHExtendedTabView.mm.
Matt Judy wrote the original code, and he had a couple different varieties of
license blocks on top, not all of which included the GPL/LGPL.  I've e-mailed
him & asked if he minds if the licensing gets changed, but I haven't heard back
from him yet.

Just so you know.  
Sorry for the spam:

Matt Judy said updating the licenses is A-OK with him.
Keywords: patch, review
<OT>Is comment 8 relevant to my bug 154947?</OT>
i don't think we want any GPL code in mozilla. is there any now? GPL is evil.
Our trilicense is MPL/GLP/LGLP, which isn't so evil; people can choose whichever
of the licenses they like (and use mozilla code in a GLP product).
And pinkerton's troll claims its first victim . . .
I'm new to Bugzilla so sorry if this is the wrong forum... Someone mentioned
some different ways of implementing a Close Tab widget. One suggestion was a 
widget off the side that would close the current tab. I don't vote for that one.
Mozilla used (uses?) it and it was confusing. Further it forces you to activate
the tab you want to close.

I do agree that widgets on all tabs would occupy to much space. But maybe, just
maybe, you could render the widget (I suggest a red Aqua closing-dot like the
windows have) only when the mouse cursor hovers on the tab? Off to the right of
the tab then, covering anything that might be showing there.
Attached patch revised patch — — Splinter Review 
Just like the other patch, but now labels are centered & things get ellipsed in
a reasonable manner.
Attachment #93117 - Attachment is obsolete: true
adding this to the blocker list (bug 147975) --i start experiencing the slowdown
with only 3-4 tabs open. :( fwiw, am on a b&w 450mhz G3.
Blocks: 147975
pulling back in. simon and i are looking at it
Target Milestone: Future → Chimera0.4
Taking
Assignee: pinkerton → sfraser
Status: ASSIGNED → NEW
Checked in changes to make all the tabs the same width, and put a cap of 16 on
the number of tabs per browser window. When at this limit, commands that would
open a new tab (e.g. view source) now open a new window. 'New tab' menu items
should disable appropriately.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Is the 16 tab limit likely to stay as a permanent fixture? Because if so, I
would consider that a bug -- I can think of several scenarios where it would
become very annoying, very quickly.

Yes, I understand some of the performance implications of this... but it would
play merry havoc with the way I prefer to browse the web. (I have no doubt that
there are other users in a similar boat. :)

Having had my gripe about parts of the fix, I would nonetheless like to thank
you guys for all the effort put into resolving the problem (even if it's not
_entirely_ to my satisfaction, it's a decent start.)

Cheers,

Stuart.
I'm curious, too - why put a limit on the number of tabs?

I understand that beyond a certain number, the tab labels become unreadable -
but I figured that if the user wanted to put 50 tabs on a page, that was his
choice and he could deal with trying to remember where everything was.
Initially I did because, with your patch, things started to freak out when the
tabs became very narrow. However, I fixed that, but thought that imposing an
upper limit was a sensible thing to do.

16 was chosen because, when the window is resized to its minimum width, the tabs
are small but not ridiculously so (even though they all show '...' as the title).
I'll quite happily take ridiculously small tabs in exchange for dropping the
limit, as long as I can still click on a specific one. Down to, say, five pixels
wide is fine by me. :-)

One other comment -- if you're going to start opening new windows rather than
new tabs (ignoring my personal dislike of this approach) at the limit, what
happens if the user asks for, say, five or ten new tabs? eg: browsing a news
site, opening articles that look interesting for later perusal. Will the user
get five or ten new windows, or one new window with five or ten tabs in it?

Hint: The first possibility is the wrong one, IMO. :-)

Hmm. Is this the right place for this sort of discussion? I'm suspecting not..
there was some improvement after this fix, but there are still perf problems
with loading pages in multiple tabs --see bug 159672.
Status: RESOLVED → VERIFIED
Component: General → Tabbed Browsing
QA Contact: winnie → sairuh
No longer blocks: 147975
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: