200657 - BeOS Locking needs to be rewritten in GFX

Status: RESOLVED FIXED

(Core Graveyard :: GFX: BeOS, defect)

Description

[Sergei Dolgov]

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.

Comment 1

[Paul]

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?

Comment 2

[Sergei Dolgov]

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

Comment 3

[Sergei Dolgov]

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

Comment 4

[Sergei Dolgov]

Attachment: Locking rewrite and cleanup

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 5

[Sergei Dolgov]

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

review request

Comment 6

[tqh]

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

Havn't looked into formatting, but the patch seems ok.

Comment 7

[Niels Reedijk]

Checked in by timeless. Thanks.