hrev54353 adds 2 changesets to branch 'master'
old head: b344d252e91b332260d1b57cfec8f890faa5aaeb
new head: 176d0e1041b0792b41c4bfe80c8cc4ed728184f9
overview:
https://git.haiku-os.org/haiku/log/?qt=range&q=176d0e1041b0+%5Eb344d252e91b
----------------------------------------------------------------------------
67ace0bfab2e: app_server: Use RecursiveLocker in AlphaMask instead of BLocker.
This avoids creaing a semaphore where it is not needed, especially
as most of these locks are never used from another thread (in the
reports in #16246, there are thousands of semaphores from this
with only a small handful having a "last acquirer" != 0.)
176d0e1041b0: app_server: Make use of BReferenceable in AlphaMask.
Extracted from https://review.haiku-os.org/c/haiku/+/2695
with a few minor tweaks.
May help with #16246, but I could only reproduce it intermittently.
[ Augustin Cavalier <waddlesplash@xxxxxxxxx> ]
----------------------------------------------------------------------------
2 files changed, 31 insertions(+), 37 deletions(-)
src/servers/app/drawing/AlphaMask.cpp | 59 ++++++++++++++-----------------
src/servers/app/drawing/AlphaMask.h | 9 +++--
############################################################################
Commit: 67ace0bfab2e04beb4174ac4e86fa05903211164
URL: https://git.haiku-os.org/haiku/commit/?id=67ace0bfab2e
Author: Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date: Sat Jun 20 23:46:22 2020 UTC
Ticket: https://dev.haiku-os.org/ticket/16246
app_server: Use RecursiveLocker in AlphaMask instead of BLocker.
This avoids creaing a semaphore where it is not needed, especially
as most of these locks are never used from another thread (in the
reports in #16246, there are thousands of semaphores from this
with only a small handful having a "last acquirer" != 0.)
----------------------------------------------------------------------------
diff --git a/src/servers/app/drawing/AlphaMask.cpp
b/src/servers/app/drawing/AlphaMask.cpp
index 4393444ee3..eac6ace222 100644
--- a/src/servers/app/drawing/AlphaMask.cpp
+++ b/src/servers/app/drawing/AlphaMask.cpp
@@ -45,6 +45,8 @@ AlphaMask::AlphaMask(AlphaMask* previousMask, bool inverse)
fMask(),
fScanline(fMask)
{
+ recursive_lock_init(&fLock, "AlphaMask");
+
if (previousMask != NULL)
atomic_add(&previousMask->fNextMaskCount, 1);
}
@@ -67,6 +69,8 @@ AlphaMask::AlphaMask(AlphaMask* previousMask, AlphaMask*
other)
fMask(other->fMask),
fScanline(fMask)
{
+ recursive_lock_init(&fLock, "AlphaMask");
+
fMask.attach(fBuffer);
if (previousMask != NULL)
@@ -92,6 +96,7 @@ AlphaMask::AlphaMask(uint8 backgroundOpacity)
fMask(),
fScanline(fMask)
{
+ recursive_lock_init(&fLock, "AlphaMask");
}
@@ -101,13 +106,15 @@ AlphaMask::~AlphaMask()
fBits->ReleaseReference();
if (fPreviousMask.Get() != NULL)
atomic_add(&fPreviousMask->fNextMaskCount, -1);
+
+ recursive_lock_destroy(&fLock);
}
IntPoint
AlphaMask::SetCanvasGeometry(IntPoint origin, IntRect bounds)
{
- AutoLocker<BLocker> locker(fLock);
+ RecursiveLocker locker(fLock);
if (origin == fCanvasOrigin && bounds.Width() == fCanvasBounds.Width()
&& bounds.Height() == fCanvasBounds.Height())
@@ -165,8 +172,8 @@ AlphaMask::_CreateTemporaryBitmap(BRect bounds) const
void
AlphaMask::_Generate()
{
- AutoLocker<BLocker> locker(fLock);
- AutoLocker<BLocker> previousLocker;
+ RecursiveLocker locker(fLock);
+ RecursiveLocker previousLocker;
if (fPreviousMask != NULL)
previousLocker.SetTo(fPreviousMask->fLock, false);
@@ -495,7 +502,7 @@ ShapeAlphaMask::Create(AlphaMask* previousMask, const
shape_data& shape,
// TODO: don't make a new mask if the cache entry has no
drawstate
// using it anymore, because then we ca just immediately reuse
it
AlphaMask* cachedMask = mask;
- AutoLocker<BLocker> locker(mask->fLock);
+ RecursiveLocker locker(mask->fLock);
mask = new(std::nothrow) ShapeAlphaMask(previousMask, mask);
cachedMask->ReleaseReference();
}
diff --git a/src/servers/app/drawing/AlphaMask.h
b/src/servers/app/drawing/AlphaMask.h
index 9156c23582..8ae7a72911 100644
--- a/src/servers/app/drawing/AlphaMask.h
+++ b/src/servers/app/drawing/AlphaMask.h
@@ -7,6 +7,7 @@
#define ALPHA_MASK_H
#include <Referenceable.h>
+#include <locks.h>
#include "agg_clipped_alpha_mask.h"
#include "ServerPicture.h"
@@ -15,8 +16,6 @@
#include "drawing/Painter/defines.h"
#include "IntRect.h"
-#include <Locker.h>
-
class BShape;
class ServerBitmap;
@@ -65,7 +64,7 @@ protected:
BReference<AlphaMask> fPreviousMask;
IntRect fBounds;
bool fClippedToCanvas;
- BLocker fLock;
+ recursive_lock fLock;
private:
friend class AlphaMaskCache;
############################################################################
Revision: hrev54353
Commit: 176d0e1041b0792b41c4bfe80c8cc4ed728184f9
URL: https://git.haiku-os.org/haiku/commit/?id=176d0e1041b0
Author: Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date: Sun Jun 21 00:02:35 2020 UTC
Ticket: https://dev.haiku-os.org/ticket/16246
app_server: Make use of BReferenceable in AlphaMask.
Extracted from https://review.haiku-os.org/c/haiku/+/2695
with a few minor tweaks.
May help with #16246, but I could only reproduce it intermittently.
----------------------------------------------------------------------------
diff --git a/src/servers/app/drawing/AlphaMask.cpp
b/src/servers/app/drawing/AlphaMask.cpp
index eac6ace222..21d15df5ff 100644
--- a/src/servers/app/drawing/AlphaMask.cpp
+++ b/src/servers/app/drawing/AlphaMask.cpp
@@ -75,7 +75,6 @@ AlphaMask::AlphaMask(AlphaMask* previousMask, AlphaMask*
other)
if (previousMask != NULL)
atomic_add(&previousMask->fNextMaskCount, 1);
- fBits->AcquireReference();
}
@@ -102,8 +101,6 @@ AlphaMask::AlphaMask(uint8 backgroundOpacity)
AlphaMask::~AlphaMask()
{
- if (fBits != NULL)
- fBits->ReleaseReference();
if (fPreviousMask.Get() != NULL)
atomic_add(&fPreviousMask->fNextMaskCount, -1);
@@ -153,19 +150,17 @@ AlphaMask::BitmapSize() const
ServerBitmap*
AlphaMask::_CreateTemporaryBitmap(BRect bounds) const
{
- UtilityBitmap* bitmap = new(std::nothrow) UtilityBitmap(bounds,
- B_RGBA32, 0);
+ BReference<UtilityBitmap> bitmap(new(std::nothrow) UtilityBitmap(bounds,
+ B_RGBA32, 0), true);
if (bitmap == NULL)
return NULL;
- if (!bitmap->IsValid()) {
- delete bitmap;
+ if (!bitmap->IsValid())
return NULL;
- }
memset(bitmap->Bits(), fBackgroundOpacity, bitmap->BitsLength());
- return bitmap;
+ return bitmap.Detach();
}
@@ -184,9 +179,7 @@ AlphaMask::_Generate()
return;
}
- if (fBits != NULL)
- fBits->ReleaseReference();
- fBits = new(std::nothrow) UtilityBitmap(fBounds, B_GRAY8, 0);
+ fBits.SetTo(new(std::nothrow) UtilityBitmap(fBounds, B_GRAY8, 0), true);
if (fBits == NULL)
return;
@@ -339,17 +332,16 @@ VectorAlphaMask<VectorMaskType>::_RenderSource(const
IntRect& canvasBounds)
if (!fBounds.IsValid())
return NULL;
- ServerBitmap* bitmap = _CreateTemporaryBitmap(fBounds);
+ BReference<ServerBitmap> bitmap(_CreateTemporaryBitmap(fBounds), true);
if (bitmap == NULL)
return NULL;
// Render the picture to the bitmap
BitmapHWInterface interface(bitmap);
DrawingEngine* engine = interface.CreateDrawingEngine();
- if (engine == NULL) {
- bitmap->ReleaseReference();
+ if (engine == NULL)
return NULL;
- }
+
engine->SetRendererOffset(fBounds.left, fBounds.top);
OffscreenCanvas canvas(engine,
@@ -374,7 +366,7 @@ VectorAlphaMask<VectorMaskType>::_RenderSource(const
IntRect& canvasBounds)
canvas.PopState();
delete engine;
- return bitmap;
+ return bitmap.Detach();
}
@@ -458,7 +450,7 @@ ShapeAlphaMask::ShapeAlphaMask(AlphaMask* previousMask,
const shape_data& shape, BPoint where, bool inverse)
:
VectorAlphaMask<ShapeAlphaMask>(previousMask, where, inverse),
- fShape(new(std::nothrow) shape_data(shape))
+ fShape(new(std::nothrow) shape_data(shape), true)
{
if (fDrawState == NULL)
fDrawState = new(std::nothrow) DrawState();
@@ -474,13 +466,11 @@ ShapeAlphaMask::ShapeAlphaMask(AlphaMask* previousMask,
fShape(other->fShape),
fShapeBounds(other->fShapeBounds)
{
- fShape->AcquireReference();
}
ShapeAlphaMask::~ShapeAlphaMask()
{
- fShape->ReleaseReference();
}
@@ -489,25 +479,23 @@ ShapeAlphaMask::Create(AlphaMask* previousMask, const
shape_data& shape,
BPoint where, bool inverse)
{
// Look if we have a suitable cached mask
- ShapeAlphaMask* mask = AlphaMaskCache::Default()->Get(shape,
previousMask,
- inverse);
+ BReference<ShapeAlphaMask> mask(AlphaMaskCache::Default()->Get(shape,
+ previousMask, inverse), true);
if (mask == NULL) {
// No cached mask, create new one
- mask = new(std::nothrow) ShapeAlphaMask(previousMask, shape,
- BPoint(0, 0), inverse);
+ mask.SetTo(new(std::nothrow) ShapeAlphaMask(previousMask, shape,
+ BPoint(0, 0), inverse), true);
} else {
// Create new mask which reuses the parameters and the mask
bitmap
// of the cache entry
// TODO: don't make a new mask if the cache entry has no
drawstate
// using it anymore, because then we ca just immediately reuse
it
- AlphaMask* cachedMask = mask;
RecursiveLocker locker(mask->fLock);
- mask = new(std::nothrow) ShapeAlphaMask(previousMask, mask);
- cachedMask->ReleaseReference();
+ mask.SetTo(new(std::nothrow) ShapeAlphaMask(previousMask,
mask), true);
}
- return mask;
+ return mask.Detach();
}
diff --git a/src/servers/app/drawing/AlphaMask.h
b/src/servers/app/drawing/AlphaMask.h
index 8ae7a72911..05620a1faf 100644
--- a/src/servers/app/drawing/AlphaMask.h
+++ b/src/servers/app/drawing/AlphaMask.h
@@ -82,7 +82,7 @@ private:
// one
in the cache, without being
// in
the cache itself
- UtilityBitmap* fBits;
+ BReference<UtilityBitmap> fBits;
agg::rendering_buffer fBuffer;
agg::clipped_alpha_mask fMask;
scanline_unpacked_masked_type fScanline;
@@ -172,7 +172,7 @@ private:
private:
friend class AlphaMaskCache;
- shape_data* fShape;
+ BReference<shape_data> fShape;
BRect fShapeBounds;
static DrawState* fDrawState;
};
