From 9b50d9dd2b079e678730b43514e9ad385d8b7ea3 Mon Sep 17 00:00:00 2001 From: Yuqian Li Date: Fri, 1 Dec 2017 14:16:15 -0500 Subject: Try to bypass the false-positive clang thread analysis warning This is identical to 70221 except for 3 lines inside SkFlexibleTaskGroup2D::work to bypass the false-positive warning. We cannot reproduce the error the android roller generated so we'll have to try and see. The detailed warning can be found in: https://skia-review.googlesource.com/c/skia/+/78720 TBR: mtklein@google.com Bug: skia: Change-Id: I3e2f414581dbd1398482cf45cf4f43eaf0535651 Reviewed-on: https://skia-review.googlesource.com/79321 Commit-Queue: Yuqian Li Reviewed-by: Yuqian Li --- src/core/SkThreadedBMPDevice.cpp | 222 ++++----------------------------------- 1 file changed, 19 insertions(+), 203 deletions(-) (limited to 'src/core/SkThreadedBMPDevice.cpp') diff --git a/src/core/SkThreadedBMPDevice.cpp b/src/core/SkThreadedBMPDevice.cpp index 608abb26e2..098c7216e8 100644 --- a/src/core/SkThreadedBMPDevice.cpp +++ b/src/core/SkThreadedBMPDevice.cpp @@ -11,202 +11,25 @@ #include "SkTaskGroup.h" #include "SkVertices.h" -#include -#include - -constexpr int MAX_CACHE_LINE = 64; - -// Some basic logics and data structures that are shared across the current experimental schedulers. -class TiledDrawSchedulerBase : public TiledDrawScheduler { -public: - TiledDrawSchedulerBase(int tiles, WorkFunc work) - : fTileCnt(tiles), fIsFinishing(false), fDrawCnt(0), fWork(std::move(work)) {} - - void signal() override { - fDrawCnt++; - } - void finish() override { - fIsFinishing.store(true, std::memory_order_relaxed); +void SkThreadedBMPDevice::DrawQueue::reset() { + if (fTasks) { + fTasks->finish(); } -protected: - const int fTileCnt; - std::atomic fIsFinishing; - std::atomic fDrawCnt; - WorkFunc fWork; -}; - -class TiledDrawSchedulerBySpinning : public TiledDrawSchedulerBase { -public: - TiledDrawSchedulerBySpinning(int tiles, WorkFunc work) - : TiledDrawSchedulerBase(tiles, std::move(work)), fScheduleData(tiles) {} - - void signal() final { this->TiledDrawSchedulerBase::signal(); } - void finish() final { this->TiledDrawSchedulerBase::finish(); } + fSize = 0; - bool next(int& tileIndex) final { - int& drawIndex = fScheduleData[tileIndex].fDrawIndex; - SkASSERT(drawIndex <= fDrawCnt); - while (true) { - bool isFinishing = fIsFinishing.load(std::memory_order_relaxed); - if (isFinishing && drawIndex >= fDrawCnt) { - return false; - } else if (drawIndex < fDrawCnt) { - fWork(tileIndex, drawIndex++); - return true; - } + // using TaskGroup2D = SkSpinningTaskGroup2D; + using TaskGroup2D = SkFlexibleTaskGroup2D; + auto draw2D = [this](int row, int column){ + SkThreadedBMPDevice::DrawElement& drawElement = fElements[column]; + if (!SkIRect::Intersects(fDevice->fTileBounds[row], drawElement.fDrawBounds)) { + return; } - } - -private: - // alignas(MAX_CACHE_LINE) to avoid false sharing by cache lines - struct alignas(MAX_CACHE_LINE) TileScheduleData { - TileScheduleData() : fDrawIndex(0) {} - - int fDrawIndex; // next draw index for this tile + drawElement.fDrawFn(fDevice->fTileBounds[row]); }; - - std::vector fScheduleData; -}; - -class TiledDrawSchedulerFlexible : public TiledDrawSchedulerBase { -public: - TiledDrawSchedulerFlexible(int tiles, WorkFunc work) - : TiledDrawSchedulerBase(tiles, std::move(work)), fScheduleData(tiles) {} - - void signal() final { this->TiledDrawSchedulerBase::signal(); } - void finish() final { this->TiledDrawSchedulerBase::finish(); } - - bool next(int& tileIndex) final { - int failCnt = 0; - while (true) { - TileScheduleData& scheduleData = fScheduleData[tileIndex]; - bool locked = scheduleData.fMutex.try_lock(); - bool processed = false; - - if (locked) { - if (scheduleData.fDrawIndex < fDrawCnt) { - fWork(tileIndex, scheduleData.fDrawIndex++); - processed = true; - } else { - failCnt += fIsFinishing.load(std::memory_order_relaxed); - } - scheduleData.fMutex.unlock(); - } - - if (processed) { - return true; - } else { - if (failCnt >= fTileCnt) { - return false; - } - tileIndex = (tileIndex + 1) % fTileCnt; - } - } - } - -private: - // alignas(MAX_CACHE_LINE) to avoid false sharing by cache lines - struct alignas(MAX_CACHE_LINE) TileScheduleData { - TileScheduleData() : fDrawIndex(0) {} - - int fDrawIndex; // next draw index for this tile - std::mutex fMutex; // the mutex for the thread to acquire - }; - - std::vector fScheduleData; -}; - -class TiledDrawSchedulerBySemaphores : public TiledDrawSchedulerBase { -public: - TiledDrawSchedulerBySemaphores(int tiles, WorkFunc work) - : TiledDrawSchedulerBase(tiles, std::move(work)), fScheduleData(tiles) {} - - - void signal() final { - this->TiledDrawSchedulerBase::signal(); - signalRoot(); - } - - void finish() final { - this->TiledDrawSchedulerBase::finish(); - signalRoot(); - } - - bool next(int& tileIndex) final { - SkASSERT(tileIndex >= 0 && tileIndex < fTileCnt); - TileScheduleData& scheduleData = fScheduleData[tileIndex]; - while (true) { - scheduleData.fSemaphore.wait(); - int leftChild = (tileIndex + 1) * 2 - 1; - int rightChild = leftChild + 1; - if (leftChild < fTileCnt) { - fScheduleData[leftChild].fSemaphore.signal(); - } - if (rightChild < fTileCnt) { - fScheduleData[rightChild].fSemaphore.signal(); - } - - bool isFinishing = fIsFinishing.load(std::memory_order_relaxed); - if (isFinishing && scheduleData.fDrawIndex >= fDrawCnt) { - return false; - } else { - SkASSERT(scheduleData.fDrawIndex < fDrawCnt); - fWork(tileIndex, scheduleData.fDrawIndex++); - return true; - } - } - } - -private: - // alignas(MAX_CACHE_LINE) to avoid false sharing by cache lines - struct alignas(MAX_CACHE_LINE) TileScheduleData { - TileScheduleData() : fDrawIndex(0) {} - - int fDrawIndex; - SkSemaphore fSemaphore; - }; - - void signalRoot() { - SkASSERT(fTileCnt > 0); - fScheduleData[0].fSemaphore.signal(); - } - - std::vector fScheduleData; -}; - -void SkThreadedBMPDevice::startThreads() { - SkASSERT(fQueueSize == 0); - - TiledDrawScheduler::WorkFunc work = [this](int tileIndex, int drawIndex){ - auto& element = fQueue[drawIndex]; - if (SkIRect::Intersects(fTileBounds[tileIndex], element.fDrawBounds)) { - element.fDrawFn(fTileBounds[tileIndex]); - } - }; - - // using Scheduler = TiledDrawSchedulerBySemaphores; - // using Scheduler = TiledDrawSchedulerBySpinning; - using Scheduler = TiledDrawSchedulerFlexible; - fScheduler.reset(new Scheduler(fTileCnt, work)); - - // We intentionally call the int parameter tileIndex although it ranges from 0 to fThreadCnt-1. - // For some schedulers (e.g., TiledDrawSchedulerBySemaphores and TiledDrawSchedulerBySpinning), - // fThreadCnt should be equal to fTileCnt so it doesn't make a difference. - // - // For TiledDrawSchedulerFlexible, the input tileIndex provides only a hint about which tile - // the current thread should draw; the scheduler may later modify that tileIndex to draw on - // another tile. - fTaskGroup->batch(fThreadCnt, [this](int tileIndex){ - while (fScheduler->next(tileIndex)) {} - }); -} - -void SkThreadedBMPDevice::finishThreads() { - fScheduler->finish(); - fTaskGroup->wait(); - fQueueSize = 0; - fScheduler.reset(nullptr); + fTasks.reset(new TaskGroup2D(draw2D, fDevice->fTileCnt, fDevice->fExecutor, + fDevice->fThreadCnt)); + fTasks->start(); } SkThreadedBMPDevice::SkThreadedBMPDevice(const SkBitmap& bitmap, @@ -216,6 +39,7 @@ SkThreadedBMPDevice::SkThreadedBMPDevice(const SkBitmap& bitmap, : INHERITED(bitmap) , fTileCnt(tiles) , fThreadCnt(threads <= 0 ? tiles : threads) + , fQueue(this) { if (executor == nullptr) { fInternalExecutor = SkExecutor::MakeFIFOThreadPool(fThreadCnt); @@ -230,14 +54,11 @@ SkThreadedBMPDevice::SkThreadedBMPDevice(const SkBitmap& bitmap, for(int tid = 0; tid < fTileCnt; ++tid, top += h) { fTileBounds.push_back(SkIRect::MakeLTRB(0, top, w, top + h)); } - fQueueSize = 0; - fTaskGroup.reset(new SkTaskGroup(*fExecutor)); - startThreads(); + fQueue.reset(); } void SkThreadedBMPDevice::flush() { - finishThreads(); - startThreads(); + fQueue.reset(); } // Having this captured in lambda seems to be faster than saving this in DrawElement @@ -279,20 +100,15 @@ SkIRect SkThreadedBMPDevice::transformDrawBounds(const SkRect& drawBounds) const // The do {...} while (false) is to enforce trailing semicolon as suggested by mtklein@ #define THREADED_DRAW(drawBounds, actualDrawCall) \ do { \ - if (fQueueSize == MAX_QUEUE_SIZE) { \ - this->flush(); \ - } \ DrawState ds(this); \ - SkASSERT(fQueueSize < MAX_QUEUE_SIZE); \ - fQueue[fQueueSize++] = { \ + fQueue.push({ \ this->transformDrawBounds(drawBounds), \ [=](const SkIRect& tileBounds) { \ SkRasterClip tileRC; \ SkDraw draw = ds.getThreadDraw(tileRC, tileBounds); \ draw.actualDrawCall; \ }, \ - }; \ - fScheduler->signal(); \ + }); \ } while (false) static inline SkRect get_fast_bounds(const SkRect& r, const SkPaint& p) { -- cgit v1.2.3