Native locking, especailly in nsRenderingContext is heavily ineffective now. It happnes on each drawing primitive method called, even if drawing is performed on ofscreen surface, BBitmap in BeOS case. As threadsafe reliable solution locking may be moved into nsDrawingSurface. We can lock this BBitmap once, on creation, and unlock also once, before realeasing/destruction. Though, it needs some care about non-buffered drawing in nsRenderingContext, which may happen while we're rendering GUI and also in CopyOffScreenBits method, but is is also easily implementable in nsDrawingSurface. Only question is rather ideological - should we create new method in nsDrawSurface or use (with some changes) existing nsDrawingSurface::Lock()/Unlock() which currently in BeOS implementation is rather useless. P.S. Second addition to nsDrawingSurface may be intelligent threadsafe storage for arrays/objects required by nsRenderingContext - like strings, BPoints, bitmaps etc whose currently are allocated by "new", but this is maybe subject for another bug.

Sergei, this bug is assigned to me. Were you planning on writing a patch for this, or do you want me to look into it?

Wondering if we can also reserve surface/bitmap in nsToolkit, as it is done in GTK port, and thus to remove locking overhead in whole BeOS code, not only in GFX

People, i'm planning to land most of those "jetgfx" patches from my builds in next month, before i leave in USA for 2 months. So acive participation of all FireFox and Mozilla for BeOS builders is needed. First take will be published today and commented at blog. Really, all this lays here since Mozilla 1.4 (partially used in last Paul's 1.5b build, which is also last half-official BeOS build at mozilla.org). Time to start to bring FireFox to same speed as in http://bebits.com/bob/18210/mozilla-i586-pc-beos-bone-full-1.7a-bthreads-21.tar.gz If someone knows e-mail of LoLL/Jeannot and mmadia, please add those to CC field

Ok, here is first patch from planned GFX-BeOS rewrite. 2 years to reach bugzilla. 1)Locking rewritten. Get rid of Lock/UnlockLooper() in nsRenderingContextBeOS and nsImageBeOS. Two methods added to nsDrawingSurfaceBeOS instead: Lock/UnlockDrawable() - called via nsRenderingContextBeOS::LockAndUpdateView()/UnlockView(). Surface is aware of actual Drawable - is it backbuffer BBitmap or BWindow. We are locking now backbuffer BBitmap for lifetime, and LockDrawable calls LockLoper() only for case "if (!mBitmap)", so - only for BWindow-attached BView. Thus we avoid extensive and expensive locking while rendering pages at backbuffer. Maybe we can avoid nested locking also for BWindow-attached views, but this is for future. As side-cleanup, Lock/UnlockLooper() removed from nsSurfaceBeOS::Lock/Unlock() - it has no sense there and is awaiting for transparency patch. 2)As UpdateAndLockView() was rewritten, some gfx-context related things were cleaned up in nsRenderingContextBeOS: a)Native mRGB_color added for future use. Addition of native patterns planned, in order to implement LineStyles b)Clipping region things adjusted - added 0-contrain in UpdateAndLockView in case !mClippingRegion, method CreateDrawingSurface() now always calls GC-update, because previous version sometimes "imported" wrong clipping, also CreateClipRegion() replaced by more intelligent method borrowed from GTK implementation (it also gets rid of do_CreateInstance, which is newest mozilla development trend, for speed reasons). c)Also, as part of GC cleanup, deprecated duEmulateBold things removed from nsRenderingContextBeOS and nsFontMetricsBeOS. Nobody uses it, and it was mistakenly taken in our port from X11/GTK to workaround some Xft (?) issues. 3)Style unified in nsRenderingContextBeOS. Sorry, we use BeIDE, nor emacs neither vim.

Comment on attachment 172044 [details] [diff] [review] Locking rewrite and cleanup review request

Comment on attachment 172044 [details] [diff] [review] Locking rewrite and cleanup Havn't looked into formatting, but the patch seems ok.

Checked in by timeless. Thanks.