Closed Bug 615704 Opened 14 years ago Closed 14 years ago

TabCandy has a memory footprint of 1MB-2MB per tab (should be reduced)

Categories

(Firefox Graveyard :: Panorama, defect, P2)

Product:
Firefox Graveyard
Component:
Panorama
Type:
defect

Tracking

(blocking2.0 -)

Status:
RESOLVED WORKSFORME
Tracking Flags:
Tracking Status
blocking2.0 --- -

People

(Reporter: bugzilla.mozilla.org, Assigned: seanedunn)

References

Details

(Keywords: memory-footprint, perf, regression, Whiteboard: [WFM?]) 

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101130 Firefox/4.0b8pre
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101130 Firefox/4.0b8pre

If yo take a look at about:memory you'll see that there is an entry under content/canvas/2d_pixel_bytes. The amount of memory consumed seems to grow proportionally with the number of tabs, so i assume this is due to tabcandy's tab previews.

From the looks of it this data is uncompressed. Normal image data is stored in compressed form and only decompressed for rendering. The decompressed data is discarded after 10 seconds after the tab has been backgrounded (its docshell being marked as inactive).

So to save memory tabcandy should render the canvas objects to jpeg or png graphics and then discard the canvas to let the image discarding mechanism save memory when appropriate.

On a 300 tab session i'm seeing this: content/canvas/2d_pixel_bytes 294,924,880

Reproducible: Always

Steps to Reproduce:
1. Open a session with many tabs
2. Open panorama
3. Check memory usage with about:memory
Moving to -moz-element for thumbnail display (bug 597258) would also be an alternative solution.
Note that a fix for this may be related to bug 604699.
I did some more testing and it seems this bug is a bit more sneaky than i thought initially. It seems that tabcandy actually does discard the canvas memory, but oly when you open the tab view.

So here's a modified testing procedure:
1. Open a session with many tabs (80+)
2. Ctrl+Tab through all tabs
3. Note how there's about 1MB per tab in content/canvas/2d_pixel_bytes
4. Open the tab view, watch how the tab images change slightly (get more blurry?) one by one. For each preview image being processed this decreases the canvas memory by about 1MB.

So tabcandy accumulates memory for every tab that's touched and keeps it until the tab view is opened and then *slowly* processes it and releases the memory.

If one doesn't use the tab view or only cycles through it quickly (too fast for the processing to happen) then this essentially 1 MB of overhead.

-> changing summary
Summary: Tab previews should be stored as images, canvas objects should be discarded to save memory. → TabCandy incurs an overhead of ~1MB per tab until previews are refreshed in the tab view
Blocks: 598466
(In reply to comment #3)
Correction:
 
1. Open a session with many tabs (80+)
2. Open and close the tab view
3. Ctrl+Tab through all tabs
4. Note how there's about 1MB per tab in content/canvas/2d_pixel_bytes
5. Open the tab view, watch how the tab images change slightly (get more blurry?) one by one. For each preview image being processed this decreases the canvas memory by about 1MB.
Just for scale, by the way, memory usage for a graphics-and-script-heavy site like the ones in bug 598466 is about 5MB per tab in 3.6  So this bug is about 20% memory overhead.  Just in case "1MB" didn't sound like much.  ;)
Status: UNCONFIRMED → NEW
blocking2.0: --- → ?
Ever confirmed: true
And it gets even more interesting. After step 5 the content/canvas/2d_pixel_bytes pool is almost empty again but the malloc or any other figures don't go down. So I'm not sure what happens but i assume that the data is just shoved somewhere else?
blocking2.0: ? → ---
The 8472, not sure if you realised, but the blocking2.0 nomination (added by Boris) was just removed by your previous comment.
It didn't show a conflict for some reason, thx.
blocking2.0: --- → ?
Severity: normal → critical
Some additional numbers:

tested with 80 cad-comic tabs, methodjit=off, image discarding=off, hw acceleration=off

1. startup 
2. load all tabs (ctrl+tab)
3. wait for GC
mem use: 684/698/874

4. open tabcandy
5. wait for GC
mem use: 752/766/937

6. close tabcandy
7. ctrl+tab through all tabs 
8. wait for GC
mem use: 860/875/1045

9. open tabcandy
10. wait for tab preview refresh
11. wait for GC
mem use: 754/769/934



So we're actually seeing two issues here:
A) tabcandy itself, once initialized uses about 0,85MB per tab, permanently.
B) some canvas rendering used by tabcandy uses 1,3MB per tab, until panorama view is opend and kept open long enough to refresh all previews.

This actually makes an overall memory increase of 0,85 to 2,15MB per tab, depending on usage patterns.


Canvas objects generated by switching to a tab are the obvious cause for case B), but case A) will need investigating, although i also assume that it's related to the tab previews.
Blocks: 604213
Summary: TabCandy incurs an overhead of ~1MB per tab until previews are refreshed in the tab view → TabCandy has a memory footprint of 1MB-2MB per tab (should be reduced)
Blocks: 608028
Priority: -- → P2
Blocking+ on this, rolls back a non-trivial amount of the progress made in memory reduction.
blocking2.0: ? → final+
Assigning to Sean, as this will likely have perf implications. Will switching from Canvas to image improve or degrade our performance? 

Note that we already do have images we use to show thumbnails before the pages have loaded enough for us to use the canvas... the patch for this bug should probably take advantage of those images. 

By the way, any help anyone can give on profiling Panorama's memory usage (beyond what The_8472 has already provided) would be of great help!
Assignee: nobody → seanedunn
Are you keeping around full-sized canvases containing snapshots of tabs? It seems to me you should just keep around thumbnails. They wouldn't consume much memory even if you don't compress them. They're probably faster to create too.
blocking2.0: final+ → betaN+
Keywords: perf
(In reply to comment #12)
> Are you keeping around full-sized canvases containing snapshots of tabs? It
> seems to me you should just keep around thumbnails. They wouldn't consume much
> memory even if you don't compress them. They're probably faster to create too.

No, we're just keeping around thumbnail-sized canvases. My understanding is that they also render faster (for resizes and moves) than images would; is that true?

Is there any way we can get a more fine-grained look at what's using this memory?
(In reply to comment #13)
> No, we're just keeping around thumbnail-sized canvases.

Great!

> My understanding is
> that they also render faster (for resizes and moves) than images would; is that
> true?

Yes!

> Is there any way we can get a more fine-grained look at what's using this
> memory?

about:memory is a good start. Getting a cycle-collector dump before and after would be good too. On Mac 10.6, I hear Instruments can do this pretty well. dbaron should have some more advice.
Keywords: regression
Version: unspecified → Trunk
Is there any progress on this bug?  Memory consumption in Firefox 4.0 is worrying me...
No, I haven't started looking at this.
We'd be happy for some help from someone who has an understanding of how Firefox's image memory is handled!
(In reply to comment #17)
> We'd be happy for some help from someone who has an understanding of how
> Firefox's image memory is handled!

What are the questions you want that person to answer?  It's not very clear here.

The big question I think we need to answer is what is responsible for the 0.85 MB usage The 8472 has identified?  An overhead when you're viewing the Panorama screen is one thing, but a substantial per tab penalty is much worse IMO.

Also, were these measurements really done with image discarding off?  That seems odd.
> Also, were these measurements really done with image discarding off? 

Yes, to eliminate it as a confounding variable (because its implementation has been in flux over the last few years, and we were trying to get historical numbers, not just current ones).
Keywords: footprint
(In reply to comment #18)
> The big question I think we need to answer is what is responsible for the 0.85
> MB usage The 8472 has identified?

I had a closer look at the tabcandy view with the DOM Inspector and some things seem to be odd.

1. each thumbnail has a canvas element *and* an img element

This means that data is kept redundantly.

2. the img element has a dataURI as src attribute

That means the image is kept in a) inflated dataURI form b) compressed form (for image discarding) c) uncompressed form when it's rendered.
Maybe using mozGetAsFile on the canvas element and shoving them into the cache might eliminate one redundancy.

3. The canvas element is resized via its height/width attributes instead of CSS

If i read the canvas spec correctly this means the underlying storage of the canvas has to be changed too to match the new dimensions instead of just using the renderer's scaling when it's drawn. Not to mention that i have no idea why there even is a tiny canvas element with the same contents as the resized image. While this doesn't waste any further memory it seems inefficient.


(In reply to comment #18)
> An overhead when you're viewing the Panorama
> screen is one thing, but a substantial per tab penalty is much worse IMO.

You misunderstood the issue here. The 1,3MB per tab are quasi-permanent because they only go away if you leave the panorama view open and start accumulating again when you change through tabs without opening the panorama view.
I.e. they're not the memory footprint of viewing panorama. They're the memory overhead you get while NOT viewing panorama.
(In reply to comment #20)
> (In reply to comment #18)
> > The big question I think we need to answer is what is responsible for the 0.85
> > MB usage The 8472 has identified?
> 
> I had a closer look at the tabcandy view with the DOM Inspector and some things
> seem to be odd.
> 
> 1. each thumbnail has a canvas element *and* an img element
> 
> This means that data is kept redundantly.
> 
> 2. the img element has a dataURI as src attribute
> 
> That means the image is kept in a) inflated dataURI form b) compressed form
> (for image discarding) c) uncompressed form when it's rendered.
> Maybe using mozGetAsFile on the canvas element and shoving them into the cache
> might eliminate one redundancy.
> 
> 3. The canvas element is resized via its height/width attributes instead of CSS
> 
> If i read the canvas spec correctly this means the underlying storage of the
> canvas has to be changed too to match the new dimensions instead of just using
> the renderer's scaling when it's drawn. Not to mention that i have no idea why
> there even is a tiny canvas element with the same contents as the resized
> image. While this doesn't waste any further memory it seems inefficient.

This is excellent analysis, thanks for this!  I'd also suggest using mozGetAsFile to extract the data from the <canvas>, we can then use the createObjectURL stuff to avoid needing a ridiculously long url.  File objects are easy to persist too :-)

I'm also somewhat curious how much time we spend on the main thread encoding PNGs for TabCandy.

> (In reply to comment #18)
> > An overhead when you're viewing the Panorama
> > screen is one thing, but a substantial per tab penalty is much worse IMO.
> 
> You misunderstood the issue here. The 1,3MB per tab are quasi-permanent because
> they only go away if you leave the panorama view open and start accumulating
> again when you change through tabs without opening the panorama view.
> I.e. they're not the memory footprint of viewing panorama. They're the memory
> overhead you get while NOT viewing panorama.

Ah, guess it's all bad then :-/
(In reply to comment #20)
> I had a closer look at the tabcandy view with the DOM Inspector and some things
> seem to be odd.

Thanks for digging in!

> 1. each thumbnail has a canvas element *and* an img element

Good point! The img is the cached image from last session; it's used until the tab's page has rendered and we can generate a live thumbnail (which goes into the canvas). 

Theoretically this should all be able to happen in the canvas (using the cached data at first and then the live data later), but I believe we had some trouble getting the canvas to play nicely with the cached data when it was first implemented; perhaps we can get that sorted out now. At the very least, we can make sure the img and the canvas are never both around at the same time; at the moment we do keep both elements in memory.

Between the two (img and canvas), we prefer canvas, as it's our understanding that it's better suited to the hardware accelleration we get from css transitions.

> That means the image is kept in a) inflated dataURI form b) compressed form
> (for image discarding) c) uncompressed form when it's rendered.

Worse yet, we're also keeping a copy of the dataURI outside of the img; that definitely needs to be fixed.

> Maybe using mozGetAsFile on the canvas element and shoving them into the cache
> might eliminate one redundancy.

This would be a way to convert from canvas to img? I think we'd like to go the other way.

> 3. The canvas element is resized via its height/width attributes instead of CSS

We scale the canvas with both width/height and css. We use css to stretch its pixels immediately upon user action, but then when we get a chance, we change its width/height to match the css so we can have an exact 1:1 pixel rendering for the desired size.
(In reply to comment #21)
> This is excellent analysis, thanks for this!  I'd also suggest using
> mozGetAsFile to extract the data from the <canvas>, we can then use the
> createObjectURL stuff to avoid needing a ridiculously long url.  File objects
> are easy to persist too :-)

Sounds like this could be used for our thumbnail cache (see bug 604699).

> I'm also somewhat curious how much time we spend on the main thread encoding
> PNGs for TabCandy.

Encoding pngs? We update our canvas thumbnails while the app is running, but only when Panorama is up. We cache them all as dataURIs when the app quits.
> This would be a way to convert from canvas to img? I think we'd like to go the
> other way.

mozGetAsFile is a way to go from canvas to img, yes.  Going the other way is called drawImage.
Sounds like some new bugs need to be filed! :)  Great analysis, The 8472.
(In reply to comment #23)
> > I'm also somewhat curious how much time we spend on the main thread encoding
> > PNGs for TabCandy.
> 
> Encoding pngs? We update our canvas thumbnails while the app is running, but
> only when Panorama is up. We cache them all as dataURIs when the app quits.

so you keep the raw, uncompressed, full-sized canvas element around for each tab until panorama is opened? This matches my assumptions wrt. memory increase B)
It needs to be fixed, e.g. by rendering them on a different thread if that's possible.

(In reply to comment #22)
> Between the two (img and canvas), we prefer canvas, as it's our understanding
> that it's better suited to the hardware accelleration we get from css
> transitions.

That would seem odd, since images are decompressed into 32bit RGBA format when they're rendered and i assume canvas elements use a similar format for internal storage. So i don't see why canvas elements would be faster. Especially considering that websites consist to 99% of images over canvas elements it would seem silly to optimize the rendering engine for canvas elements.

The main issue here is that canvas is uncompressed storage while images are subject to discarding and thus are kept in compressed form when not needed. So from a memory standpoint images are preferable.

And why do we need canvas elements at all? There is no fancy drawing going on as far as i can see.


> > That means the image is kept in a) inflated dataURI form b) compressed form
> > (for image discarding) c) uncompressed form when it's rendered.
> 
> Worse yet, we're also keeping a copy of the dataURI outside of the img; that
> definitely needs to be fixed.
Well, i would hope those are the same string objects. If they aren't that would be a lot of wasted storage.

> > Maybe using mozGetAsFile on the canvas element and shoving them into the cache
> > might eliminate one redundancy.
> 
> This would be a way to convert from canvas to img? I think we'd like to go the
> other way.

It would avoid the inflated base64 form, that was my main issue.

> We scale the canvas with both width/height and css. We use css to stretch its
> pixels immediately upon user action, but then when we get a chance, we change
> its width/height to match the css so we can have an exact 1:1 pixel rendering
> for the desired size.

Um, isn't downsizing a lossy operation? Where do you get the data from when you expand them again?
> Especially considering that websites consist to 99% of images over canvas
> elements it would seem silly to optimize the rendering engine for canvas
> elements.

Because canvas elements are less common and tend to be updated a lot more, the optimization tradeoffs are different.  In practice, canvas has its own hardware-composited layer, and at least on Linux and Windows is often stored in the graphics memory, not in main RAM.  But yes, canvas is always uncompressed RGBA data.

> There is no fancy drawing going on as far as i can see.

Apart from the whole "create a snapshot of a window" fancy drawing bit, you mean?  That's done via the drawWindow API on canvas.

> Well, i would hope those are the same string objects.

They may or may not be depending on where things are stored.  Generally, XPCOM strings can share memory with each other, and JS strings can share memory with each other, but XPCOM and JS can't share memory with each other. So if you have a long string |str| and do img.setAttribute("src", str), that does a string copy.

> Where do you get the data from when you expand them again?

Presumably drawWindow on the window the canvas is supposed to be showing.
(In reply to comment #27)
> > Especially considering that websites consist to 99% of images over canvas
> > elements it would seem silly to optimize the rendering engine for canvas
> > elements.
> 
> Because canvas elements are less common and tend to be updated a lot more, the
> optimization tradeoffs are different.  In practice, canvas has its own
> hardware-composited layer, and at least on Linux and Windows is often stored in
> the graphics memory, not in main RAM.

The thing is that tabcandy is more like a resizeable image gallery than some fancy, real-time drawing. So it might actually be the wrong optimization tradeoff and images would be more suited to what it's doing.
And if we want live-previews we are better off with -moz-element since that can be optimized with layers/hardware-compositing instead of first drawing into a canvas and then displaying it.

So as far as i can see it canvas is the wrong choice here.

> But yes, canvas is always uncompressed RGBA data.

So at least when it comes to memory-footprint keeping any canvas elements around is a bad idea.


> > There is no fancy drawing going on as far as i can see.
> 
> Apart from the whole "create a snapshot of a window" fancy drawing bit, you
> mean?  That's done via the drawWindow API on canvas.

That doesn't happen all too often (per tab). Or at least it shouldn't (see next point). Which means that for the drawing a canvas is good/necessary. But for all the other performance characteristics of tabcandy image objects should be better.


> They may or may not be depending on where things are stored.  Generally, XPCOM
> strings can share memory with each other, and JS strings can share memory with
> each other, but XPCOM and JS can't share memory with each other. So if you have
> a long string |str| and do img.setAttribute("src", str), that does a string
> copy.

From what i gather they're kept in javascript. So it indeed is duplicate memory usage.

> > Where do you get the data from when you expand them again?
> 
> Presumably drawWindow on the window the canvas is supposed to be showing.

If all windows have to be drawn every time a group is resized this would seem to be horrible performance-wise.
Instead windows should be drawn to a canvas, the canvas should be converted into a compressed picture with a semi-decent resolution (maybe even jpeg instead of png) and that picture should be resized with CSS scaling.

This would remove one scaling operation (right now a window has to be scaled into a canvas and then the canvas itself is scaled into the tabcandy layout) and it would remove loads of uncompressed image storage.
(In reply to comment #22)
> Theoretically this should all be able to happen in the canvas (using the cached
> data at first and then the live data later), but I believe we had some trouble
> getting the canvas to play nicely with the cached data when it was first
> implemented; perhaps we can get that sorted out now. At the very least, we can
> make sure the img and the canvas are never both around at the same time; at the
> moment we do keep both elements in memory.

Please just use canvas!

The per-tab cost when Panorama is not being used should be at most the size of the thumbnail canvas's image storage, e.g. 320x400x4, about 0.5MB.
(In reply to comment #29)
> Please just use canvas!
> 
> The per-tab cost when Panorama is not being used should be at most the size of
> the thumbnail canvas's image storage, e.g. 320x400x4, about 0.5MB.

why not 

page -> draw to canvas -> convert canvas to image with compression?

This would save more memory or allow us to keep a higher resolution with the same memory footprint.
That would be slower, but might be better overall.
Whiteboard: [hardblocker]
Depends on: 617454
Even at only 0.5 MB per tab, that's a lot of overhead for Panorama, especially if it's not going to be used*. I take it on-demand rendering of the thumbnails is too slow?

* If only it were an add-on I could uninstall ;-)
It actually won't render any thumbnails until you activate it at least once, if I understand correctly.

But yes. More compression would make sense.
Ah, I missed the correction in comment 4. Just tested and confirmed that they're not created until one brings up Panorama for the first time.

Then we hang on to them (in the form of an <img src="data:...">) until the next time Panorama is shown?

Yeah, it'd be nice if we could cache them in compressed form (bug 604699?), and just drawImage() that onto the <canvas> as a place-holder until we can draw the live page there.

Do we hang onto Panorama's DOM, or does that get rebuilt? Specifically, are we keeping the <canvas> objects around while it's not visible? Looks like we do (judging by the rendering speed on my two year old iMac). Could we get away with creating them on demand? Probably with some sort of place-holder for the height/width so things don't jump around.
(Meant to add this to the previous comment: And thanks!)
Note that 0.5mb per tab is on the order of 10% of our per-tab memory usage or so, at least for the site in bug 598466 (which has tons of JS, images, ads, etc).
(In reply to comment #33)
> It actually won't render any thumbnails until you activate it at least once, if
> I understand correctly.

Yes, but activating it by accident is easy if you hit the tab context menu -> move to group menu for example... that already loads TC.

(In reply to comment #35)
> Then we hang on to them (in the form of an <img src="data:...">) until the next
> time Panorama is shown?

Once TC has been loaded the images + dataURIs stay forever.


> Do we hang onto Panorama's DOM, or does that get rebuilt? Specifically, are we
> keeping the <canvas> objects around while it's not visible? Looks like we do
> (judging by the rendering speed on my two year old iMac). Could we get away
> with creating them on demand? Probably with some sort of place-holder for the
> height/width so things don't jump around.

The DOM stays as it is when you close TC. I don't think drawing everything to a canvas every time you open TC is a good idea since it would not scale up to many open tabs.
So what if we (mostly) keep doing what we're currently doing, display an <img> (created when showing Panorama) from the cached (compressed?) thumbnail, and (destructively) swap it out for a (newly created) <canvas> once the live page has been rendered onto it?

I'm assuming here that creating an image element for an image already in the cache is fast enough, which it may well not be. Though I'm not sure why drawing an image onto an <img> surface would be that much faster than doing the same onto a <canvas> surface.
We've figured out why we're getting an average of 2mb rather than < 0.5mb... it's a subtle interaction between our thumbnail update code and our zooming code; you end up keeping around zoomed versions of all the tabs you visit when you're not in Panorama. I believe Sean is working on the patch.
(In reply to comment #40)
> We've figured out why we're getting an average of 2mb rather than < 0.5mb...
> it's a subtle interaction between our thumbnail update code and our zooming
> code; you end up keeping around zoomed versions of all the tabs you visit when
> you're not in Panorama. I believe Sean is working on the patch.

That was causing increase B) described in comment 9. Increase A) still has to be solved.
(In reply to comment #41)
> That was causing increase B) described in comment 9. Increase A) still has to
> be solved.

There's also the duplicate dataURI, and the fact we keep both the canvas and img around at the same time. Fixing those should reduce our footprint as much as we're going to be able to for Fx4.
blocker=hardblocker
Severity: critical → blocker
Severity: blocker → critical
Sean, do you have a patch (even a WIP one)?  If so, can you please attach it here?  I can also take a look at this if you need me to, but I'd rather not do any duplicate work you might have already done...
I don't have a patch, only the above list of things I need to start removing. I'm starting this bug as soon as I get 567029 finished (almost there).
(In reply to comment #45)
> I don't have a patch, only the above list of things I need to start removing.
> I'm starting this bug as soon as I get 567029 finished (almost there).

OK, let me know if you need help.
New repro to check whether this bug still exists, based on conversation with The_8472:

Preparation:
1. Use the latest nightly build for Windows 32 bit if possible.
2. Create 40 tabs based on going to http://www.cad-comic.com/cad/ and ctrl/option-clicking the RANDOM button 40 times.
3. Close Firefox.

Test:
1. Open Firefox, and go to about:memory on the first tab, reloading until memory settles (< 30sec). Note the amount of memory given for malloc/allocated and malloc/mapped.
2. Cycle all 40 tabs by repeatedly pressing ctrl-tab.
3. Ctrl-shift-e into Panorama, and wait for all thumbnails to update.
4. Ctrl-shift-e back to the browser, and ctrl-tab through all 40 tabs again.
5. Let memory settle again and note the same memory given in malloc/allocated and malloc/mapped.
Addendum to above: Expand window to full screen and resize Panorama group to fill screen.
<The_8472> "malloc/allocated
[14:44] <The_8472> and malloc/mapped."
[14:44] <The_8472> the allocator does not capture everything.
[14:44] <The_8472> there's lots of stuff bypassing malloc
[14:44] <The_8472> you need to measure the private bytes at least.
[14:45] <The_8472> resident set (working set on windows) and virtual memory would be good too
(In reply to comment #47)
> Test:
> 1. Open Firefox, and go to about:memory on the first tab, reloading until
> memory settles (< 30sec). Note the amount of memory given for malloc/allocated
> and malloc/mapped.

Here is the numbers that I got (using Firefox 32 bit on Windows 7 x64): 300/341.

> 2. Cycle all 40 tabs by repeatedly pressing ctrl-tab.
> 3. Ctrl-shift-e into Panorama, and wait for all thumbnails to update.
> 4. Ctrl-shift-e back to the browser, and ctrl-tab through all 40 tabs again.
> 5. Let memory settle again and note the same memory given in malloc/allocated
> and malloc/mapped.

Here is the numbers that I got in this case: 303/348.

Looks like the bug doesn't show up any more.  What exactly fixed this?  Do we get different numbers with older nighlies?
FWIW, I get the same result on beta9 as comment 50.
Looking at private bytes, it stabilizes around 448MB after loading the tabs, and at about 447MB after opening and closing Panorama and letting things stabilize.
That all said, I don't want to close it all without determining what's changed that might have fixed this.
Whiteboard: [hardblocker] → [hardblocker][WFM?]
(In reply to comment #53)
> That all said, I don't want to close it all without determining what's changed
> that might have fixed this.

We had an unfortunate gotcha. We weren't updating our thumbnail canvases while panorama was in the background, except if it was the thumbnail for the currently selected page. We also resize the thumbnail for the currently selected page (with CSS) to about 2/3 the size of the window, to facilitate the zoom down animation. Neither of these things are really a problem on their own; it's the combination: we were resizing the thumbnail up and then allowing it to resize its canvas to match when it was the selected tab, but then when it was no longer the selected tab we resized it down again with CSS but didn't allow it to resize its canvas. In that way, if you cycled through a lot of tabs without returning to Panorama, those large canvases would just pile up.

Bug 609685 has fixed this by not updating even the selected tab (which we decided we didn't need to do anyway). Because of this, the thumbnails will continue to grow and shrink via CSS but their canvases will never be resized when you're outside of Panorama.

For what it's worth, I'm seeing basically the same numbers (within the range of noise) before and after with the repro steps in comment 47.

I think this bug is resolved. If we find additional memory issues we can file them as new bugs.
After some more testing it seems like this issue is present in session that i've been using for testing for a long time now but is not present in a fresh one.

I've disabled all extensions/plugins in both sessions and the difference still remains. The baseline usage is about the same too when i use identical settings. But in one session panorama does pull in an additional 100MB or so while it doesn't in the new one.

Does anyone have an idea how i could isolate the issue?
Any interesting non-default pref values in the profile that shows the problem?
Profile without memory increase:

  Modified Preferences

      Name

      Value

        browser.places.smartBookmarksVersion
        2

        browser.startup.homepage_override.buildID
        20110119030331

        browser.startup.homepage_override.mstone
        rv:2.0b10pre

        extensions.lastAppVersion
        4.0b10pre

        javascript.options.methodjit.content
        false

        network.cookie.prefsMigrated
        true

        places.history.expiration.transient_current_max_pages
        193225

        privacy.sanitize.migrateFx3Prefs
        true



###########################################################


Profile with memory increase:



  Modified Preferences

      Name

      Value

        accessibility.typeaheadfind.flashBar
        0

        browser.places.importBookmarksHTML
        false

        browser.places.importDefaults
        false

        browser.places.migratePostDataAnnotations
        false

        browser.places.smartBookmarksVersion
        2

        browser.places.updateRecentTagsUri
        false

        browser.startup.homepage_override.buildID
        20110119030331

        browser.startup.homepage_override.mstone
        rv:2.0b10pre

        extensions.lastAppVersion
        4.0b10pre

        general.useragent.extra.microsoftdotnet
        (.NET CLR 3.5.30729)

        javascript.options.methodjit.content
        false

        network.cookie.prefsMigrated
        true

        places.history.expiration.transient_current_max_pages
        193225

        places.last_vacuum
        1288597033

        privacy.sanitize.migrateFx3Prefs
        true

        security.enable_java
        false
Resolving bug per comment 54.
Status: NEW → UNCONFIRMED
Ever confirmed: false
Status: UNCONFIRMED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment 55 indicates that this isn't resolved.
if Bug 609685 fixed this at least set the dependency
(In reply to comment #59)
> Comment 55 indicates that this isn't resolved.

It's resolved for everyone who has tested it (Sean, Ehsan, The_8472, me) except for an old session of The_8472's. At the very least we should change it to "UNCONFIRMED" and remove blocking, yes?
Depends on: 609685
I don't know if it should still be blocking, depends on what makes The_8472's profile special.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: FIXED → ---
nominating for unblocking
blocking2.0: betaN+ → ?
(In reply to comment #63)
> nominating for unblocking

This is the wrong thing to do, Ian.  Based on the information we have here, this problem has been solved just for one case, and we still need to figure out what other cases can cause a regression.

The_8472, can you try copying your sessionstore.js over to the fresh profile and see if the same problem persists?
Moving this to [softblocker] though, based on the fact that it might not be a "for everyone" problem. Ian: to get soft/hard re-evaluated, just remove the [softblocker]/[hardblocker] from the whiteboard.
blocking2.0: ? → betaN+
Whiteboard: [hardblocker][WFM?] → [softblocker][WFM?]
(In reply to comment #64)
> The_8472, can you try copying your sessionstore.js over to the fresh profile
> and see if the same problem persists?

Mhh, it seems that copying the sessionstore does also move the problem to the new profile. But i can't attach it, it's too large for bugzilla.
bugspam. Moving b10 to b11
Blocks: 627096
bugspam. Removing b10
No longer blocks: 608028
(In reply to comment #66)
> (In reply to comment #64)
> > The_8472, can you try copying your sessionstore.js over to the fresh profile
> > and see if the same problem persists?
> 
> Mhh, it seems that copying the sessionstore does also move the problem to the
> new profile. But i can't attach it, it's too large for bugzilla.

Maybe you can compress it using bzip2 or something?  Or you could mail it to me and I will host it on my people.mozilla.com account?  Please beware that it could contain some private information...
sessionstore causing the problem: http://infinite-source.de/8472/sessionstore.js.gz
(In reply to comment #70)
> sessionstore causing the problem:
> http://infinite-source.de/8472/sessionstore.js.gz

That file is polluted with image data, bug 604699 :(
(In reply to comment #71)
> That file is polluted with image data, bug 604699 :(
That does not explain the size increase when opening panorama view. The sessionstore has to be loaded anyway to restore the session and the file is only 12MB in size, so even keeping it in memory in its entirety wouldn't explain an overhead of almost 100MB when opening panorama.
(In reply to comment #72)
> (In reply to comment #71)
> > That file is polluted with image data, bug 604699 :(
> That does not explain the size increase when opening panorama view. The
> sessionstore has to be loaded anyway to restore the session and the file is
> only 12MB in size, so even keeping it in memory in its entirety wouldn't
> explain an overhead of almost 100MB when opening panorama.

As far as "only 12MB in size" is concerned, a large sessionstore.js used to be between 100 and 200 KB in the pre-Panorama days, as far as I remember.

Most of that data (i.e. the image data) is probably held in memory multiple times, by the session restore code and by panorama. That still doesn't explain the 100 MB increase, though.
Could a measurement be done of how much the image data expands in memory when pixmapped?
Is there a good reason that this blocks betaN?  If not, it should be moved over to final+.
Whiteboard: [softblocker][WFM?] → [softblocker][WFM?][final?]
blocking2.0: betaN+ → final+
Whiteboard: [softblocker][WFM?][final?] → [softblocker][WFM?]
No longer depends on: 617454
[bugspam: betaN -> final]
Blocks: 585689
No longer blocks: 627096
Nominating for unblocking. We've yet to come across the regression for this bug with other users. (removed softblocker white board per comment 65)
Whiteboard: [softblocker][WFM?] → [WFM?]
Agreed
blocking2.0: final+ → -
In light of no longer blocking, I'm closing this bug as WFM. We can re-open it later if this regresses or we find new edge cases.
Status: REOPENED → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → WORKSFORME
uhm, has anyone even tested it with the profile I have provided in comment 71?
(In reply to comment #79)
> In light of no longer blocking, I'm closing this bug as WFM. We can re-open it
> later if this regresses or we find new edge cases.

I don't see the logic of that - the bar for being a blocker is rather higher than the bar for merely being a confirmed bug.  As for "we've yet to come across the bug for other users" - short of doing the diagnostic work that the8472 has done, how would users "come across" it?  There are certainly still a lot of people complaining that 4b12 uses a lot more memory than 3.6.  Nobody has actually tried to reproduce this bug, as far as I can tell...
I have loaded this file, and I have inspected its contents to see what might be causing the problem.

The 12MB consists of 50 or so tabs, each tab containing a very large image file, on the scale of 500Kb each, URL encoded. A 500Kb encoded png would be quite large in memory. I don't know why they were so large in the specific rev of FF that The_8472 was using.

Upon running this once and exiting, the sessionstore.js becomes 1MB in size, the majority of which is "referrer" tags. At this point, all images have been loaded from their URL and stored in disk cache. Because of the addition of the image caching code, the large image data was never used by tabcandy.

So we have a possible reason why the memory was so huge (large images saved), and we have a reason why this specific bug will have a minimized effect on users (the images are never reconstituted into image buffers, and the data is filtered from the sessionstore.js on first load).
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.