There is kind of very special "leak" in BeOS. BBitmap, once allocated, stays listed in occupied memory until be_app is closed, be it destoryed or not. This is BeOS issue. Formally there is no danger, as BBitmaps belong to system resources, and, in theory, when some app needs memory, system should squeeze resources, allocated by gone bitmaps. But in practice things are worse, when you open lot of tabs with images, memory "ends" quite soon, and with sucking BeOS VM it leads to noticeable slowdown of BeZilla and even crash. So, we should reuse created BBitmaps whenever possible. Currently there are two places - nsImageBeOS and nsDrawingSurfaceBeOS - where BBitmap is used intensively

Do you think we should use the same bitmap instead of cloning in bug 294234 ?

Did some experiments with nsImageBeOS. Cases when sizes of "new" mImage and tmpbmp (background image tile) are same as previous, are quite frequent. And in most of such cases mImageCurrent is PR_TRUE. So, with reusing BBitmaps there we can also add real speed in drawing not only pages with (tiled) background, but also Chrome. Last is quite important for us, as we draw it asynchronously.

2 tqh. That's unclear at the moment, but one thing might be done even if we don't use instance from gfx. To have (pseudo)static DnD BBitmap in widget and reuse it if possible, filling with new data.

ah ok, btw I'll leave bug 294234 (for now) so if you want to take it and do something feel free to do so.

Fixing obvious case - temporary bitmap used for DrawTile. Not only keeping it alive until different size is requested, but also don't refill it until mImage is up-to-date. This is very frequent case in Chrome and pages with tiled background.

Comment on attachment 199653 [details] [diff] [review] patch r=thesuckiestemail@yahoo.se Maybe we should change the PRBool's to PRPackedBools?

First (i hope not last) patch landed in trunk: /cvsroot/mozilla/gfx/src/beos/nsImageBeOS.cpp,v <-- nsImageBeOS.cpp new revision: 1.35; previous revision: 1.34 /cvsroot/mozilla/gfx/src/beos/nsImageBeOS.h,v <-- nsImageBeOS.h new revision: 1.22; previous revision: 1.21

patch needs to be revisited. tiled background sometimes goes messed at scrolling, something like that already happened in past, but it was related rather to missing syncing at background paint. Will check if regression is related to this commit

my tiling code appeared to be too tricky to add yet another trick:( So for now rolling back partially - trying to keep BBitmap itself, but refilling it each time.

Comment on attachment 199787 [details] [diff] [review] patch - partial rollback r=thesuckiestemail@yahoo.se clearing the whole bitmap and then drawing looks like it 'might' be optimized later.

checked in https://bugzilla.mozilla.org/attachment.cgi?id=199787 Checking in mozilla/gfx/src/beos/nsImageBeOS.cpp; /cvsroot/mozilla/gfx/src/beos/nsImageBeOS.cpp,v <-- nsImageBeOS.cpp new revision: 1.36; previous revision: 1.35 done Checking in mozilla/gfx/src/beos/nsImageBeOS.h; /cvsroot/mozilla/gfx/src/beos/nsImageBeOS.h,v <-- nsImageBeOS.h new revision: 1.23; previous revision: 1.22 done

Now chnages for nsSurfaceBeOS. Proof of concept, yet non-elegant code. Using global "static" BBitmap (gBitmap) wherever possible, together with bool gBused flag. Currently gBitmap is created with 1280*1240 size, ideally it will be screen-sized, but it needs more tricky code. Observations: 1)Bitmapped surface is created twice at window create. gBitmap is reused 2)Bitmapped Init() is called at window resize. In most of cases gBitmap is also to be reused. 3)Some pages do try to create more bitmapped surfaces (eg http://www.nbc.com - 4 surfaces.). In this case additional mBitmap also can be reused.

sorry, size of bitmap in previous explanation must be 1280*1024

I guess the observations is with plain CVS code?

It is observation about how it may happen if we use CVS code, made using prinfs() in patched code:)

btw, code from last patch don't work at ZETA 1.1 on an AMD/64 3000+ from "installed" version: "You need a valid BApplication object before interacting with the app_server -skip- fcff8dec ec198c1a BBitmap::InitObject(BRect, color_space, unsigned long, long, screen_id) + 000002b2 -etc- " It seems that global objects at stack in modules are allocated to early - before be_app spinup. Maybe due way we do it, with fake ->Run suspended, and then reallowed from nsAppShell. So more fine approach is needed, like using pointer and allocating it once at some moment, like we do in JapaneseInlinePatch with beosIME object

I don't think it's a problem with how we handle BApp (it's hard to do any other way (except just getting rid of the extra thread). It's Mozilla that loads libraries early and somehow gfx seems to be needed. So the best way is to initialize the bitmap some other way than static. GetInstance maybe could be used?

allocating gBitmap at nsSurface constructor. Should be much safer in regard of previous problem

Comment on attachment 199653 [details] [diff] [review] patch Requesting permission to land on the BUGZILLA_1_8_BRANCH. This is a BeOS-only change and does not affect other platforms. Note: Sergei, apply these patches after bug 310845.

Comment on attachment 199787 [details] [diff] [review] patch - partial rollback Requesting permission to land on the BUGZILLA_1_8_BRANCH. This is a BeOS-only change and does not affect other platforms.

Comment on attachment 199653 [details] [diff] [review] patch a=schrep for drivers for BeOS only change.

Applied patch and partial rollback at once: Checking in mozilla/gfx/src/beos/nsImageBeOS.cpp; /cvsroot/mozilla/gfx/src/beos/nsImageBeOS.cpp,v <-- nsImageBeOS.cpp new revision: 1.34.8.1; previous revision: 1.34 done Checking in mozilla/gfx/src/beos/nsImageBeOS.h; /cvsroot/mozilla/gfx/src/beos/nsImageBeOS.h,v <-- nsImageBeOS.h new revision: 1.21.8.1; previous revision: 1.21 done there must be also some bug and patch with adds Sync() statements to nsImage.cpp, actually only really important sync must be added to DrawTile code. Probably that bug is related to wrong rendering of http://rezo.net site

That's bug 283225. I'll check later if that one is on my list (I think it is), and I'll get to it eventually.

Delete from dependency list

We don't support BeOS any more.

In the graveyard, code referred to doesn't exist to be fixed.