Work queue & octree reinsertion optimizations.

Engine/Core/WorkQueue.cpp

@@ -75,7 +75,9 @@ public:
         #ifdef WIN32
         WaitForSingleObject(eventHandle_, INFINITE);
         #else
+        pthread_mutex_lock(&mutex_);
         pthread_cond_wait(&condition_, &mutex_);
+        pthread_mutex_unlock(&mutex_);
         #endif
     }
     
@@ -97,9 +99,10 @@ WorkQueue::WorkQueue(Context* context) :
     Object(context),
     impl_(new WorkQueueImpl()),
     started_(false),
-    shutDown_(false)
+    shutDown_(false),
+    numWaiting_(0)
 {
-    // Create worker threads and start them
+    // Create worker threads and start them. Leave one core free for the main thread
     unsigned numCores = GetNumCPUCores();
     for (unsigned i = 1; i < numCores; ++i)
     {
@@ -135,8 +138,10 @@ void WorkQueue::AddWorkItem(WorkItem* item)
     {
         queueLock_.Acquire();
         queue_.Push(item);
+        bool isWaiting = numWaiting_ > 0;
         queueLock_.Release();
-        impl_->Signal();
+        if (isWaiting)
+            impl_->Signal();
     }
 }
 
@@ -145,7 +150,8 @@ void WorkQueue::Start()
     if (!threads_.Empty() && !started_)
     {
         started_ = true;
-        for (unsigned i = 0; i < threads_.Size(); ++i)
+        unsigned numSignals = numWaiting_;
+        for (unsigned i = 0; i < numSignals; ++i)
             impl_->Signal();
     }
 }
@@ -165,7 +171,6 @@ void WorkQueue::Finish()
         for (;;)
         {
             WorkItem* item = 0;
-            bool allIdle = false;
             
             queueLock_.Acquire();
             if (!queue_.Empty())
@@ -173,13 +178,11 @@ void WorkQueue::Finish()
                 item = queue_.Front();
                 queue_.PopFront();
             }
-            else
-                allIdle = CheckIdle();
             queueLock_.Release();
             
             if (item)
                 item->Process(0);
-            else if (allIdle)
+            else if (numWaiting_ == threads_.Size())
                 break;
         }
     }
@@ -194,17 +197,16 @@ void WorkQueue::FinishAndStop()
 bool WorkQueue::IsCompleted()
 {
     queueLock_.Acquire();
-    bool queueEmpty = queue_.Empty();
-    bool allIdle = false;
-    if (queueEmpty)
-        allIdle = CheckIdle();
+    bool completed = queue_.Empty() && numWaiting_ == threads_.Size();
     queueLock_.Release();
     
-    return queueEmpty && allIdle;
+    return completed;
 }
 
-WorkItem* WorkQueue::GetNextWorkItem(WorkerThread* thread)
+WorkItem* WorkQueue::GetNextWorkItem()
 {
+    bool wasWaiting = false;
+    
     for (;;)
     {
         if (shutDown_)
@@ -212,34 +214,27 @@ WorkItem* WorkQueue::GetNextWorkItem(WorkerThread* thread)
         
         WorkItem* item = 0;
         queueLock_.Acquire();
+        if (wasWaiting)
+        {
+            --numWaiting_;
+            wasWaiting = false;
+        }
         if (started_ && !queue_.Empty())
         {
             item = queue_.Front();
             queue_.PopFront();
-            thread->SetWorking(true);
         }
-        queueLock_.Release();
-        
         if (item)
+        {
+            queueLock_.Release();
             return item;
+        }
         else
         {
-            thread->SetWorking(false);
+            ++numWaiting_;
+            queueLock_.Release();
             impl_->Wait();
+            wasWaiting = true;
         }
     }
 }
-
-bool WorkQueue::CheckIdle()
-{
-    bool allIdle = true;
-    for (unsigned i = 0; i < threads_.Size(); ++i)
-    {
-        if (threads_[i]->IsWorking())
-        {
-            allIdle = false;
-            break;
-        }
-    }
-    return allIdle;
-}

Engine/Core/WorkQueue.h

@@ -61,10 +61,8 @@ public:
     bool IsCompleted();
     
 private:
-    /// Block until a work item is available and return it. Item may be null. Called by the worker threads.
-    WorkItem* GetNextWorkItem(WorkerThread* thread);
-    /// Check if all worker threads are idle.
-    bool CheckIdle();
+    /// Block until a work item is available and return it. May return null. Called by the worker threads.
+    WorkItem* GetNextWorkItem();
     
     /// Work queue implementation. Contains the operating system-specific signaling mechanism.
     WorkQueueImpl* impl_;
@@ -74,6 +72,8 @@ private:
     List<WorkItem*> queue_;
     /// Queue spinlock.
     SpinLock queueLock_;
+    /// Number of waiting threads.
+    unsigned numWaiting_;
     /// Started flag.
     volatile bool started_;
     /// Shutting down flag.

Engine/Core/WorkerThread.cpp

@@ -28,8 +28,7 @@
 
 WorkerThread::WorkerThread(WorkQueue* owner, unsigned index) :
     owner_(owner),
-    index_(index),
-    working_(false)
+    index_(index)
 {
 }
 
@@ -37,7 +36,7 @@ void WorkerThread::ThreadFunction()
 {
     while (shouldRun_)
     {
-        WorkItem* item = owner_->GetNextWorkItem(this);
+        WorkItem* item = owner_->GetNextWorkItem();
         if (item)
             item->Process(index_);
     }

Engine/Core/WorkerThread.h

@@ -40,16 +40,10 @@ public:
     
     /// Return thread index.
     unsigned GetIndex() const { return index_; }
-    /// Set working flag.
-    void SetWorking(bool enable) { working_ = enable; }
-    /// Return whether is working.
-    bool IsWorking() const { return working_; }
     
 private:
     /// Work queue.
     WorkQueue* owner_;
     /// Thread index.
     unsigned index_;
-    /// Working flag.
-    volatile bool working_;
 };

Engine/Graphics/Drawable.cpp

@@ -158,7 +158,7 @@ void Drawable::SetOccluder(bool enable)
 
 void Drawable::MarkForUpdate()
 {
-    if (octant_)
+    if (octant_ && !updateQueued_)
         octant_->GetRoot()->QueueUpdate(this);
 }
 
@@ -284,7 +284,7 @@ void Drawable::OnMarkedDirty(Node* node)
     {
         worldBoundingBoxDirty_ = true;
         zone_.Reset();
-        if (octant_)
+        if (octant_ && !reinsertionQueued_)
             octant_->GetRoot()->QueueReinsertion(this);
     }
 }

Engine/Graphics/Drawable.h

@@ -35,6 +35,7 @@ static const unsigned DRAWABLE_ANY = 0xff;
 static const unsigned DEFAULT_VIEWMASK = M_MAX_UNSIGNED;
 static const unsigned DEFAULT_LIGHTMASK = M_MAX_UNSIGNED;
 static const unsigned DEFAULT_ZONEMASK = M_MAX_UNSIGNED;
+static const int DRAWABLES_PER_WORK_ITEM = 16;
 
 struct Batch;
 class Camera;

Engine/Graphics/Octree.cpp

@@ -39,7 +39,6 @@
 
 static const float DEFAULT_OCTREE_SIZE = 1000.0f;
 static const int DEFAULT_OCTREE_LEVELS = 8;
-static const int DRAWABLES_PER_WORKITEM = 8;
 
 inline bool CompareRayQueryResults(const RayQueryResult& lhs, const RayQueryResult& rhs)
 {
@@ -53,8 +52,9 @@ Octant::Octant(const BoundingBox& box, unsigned level, Octant* parent, Octree* r
     root_(root),
     numDrawables_(0)
 {
-    Vector3 halfSize = worldBoundingBox_.Size() * 0.5f;
-    cullingBox_ = BoundingBox(worldBoundingBox_.min_ - halfSize, worldBoundingBox_.max_ + halfSize);
+    center_ = worldBoundingBox_.Center();
+    halfSize_ = worldBoundingBox_.Size() * 0.5f;
+    cullingBox_ = BoundingBox(worldBoundingBox_.min_ - halfSize_, worldBoundingBox_.max_ + halfSize_);
     
     for (unsigned i = 0; i < NUM_OCTANTS; ++i)
         children_[i] = 0;
@@ -112,15 +112,15 @@ void Octant::DeleteChild(Octant* octant)
     }
 }
 
-void Octant::InsertDrawable(Drawable* drawable)
+void Octant::InsertDrawable(Drawable* drawable, const Vector3& boxCenter, const Vector3& boxSize)
 {
     // If size OK or outside, stop recursion & insert here
-    if (CheckDrawableSize(drawable) || cullingBox_.IsInside(drawable->GetWorldBoundingBox()) != INSIDE)
+    if (CheckDrawableSize(boxSize) || cullingBox_.IsInside(drawable->GetWorldBoundingBox()) != INSIDE)
     {
-        if (drawable->octant_ != this)
+        Octant* oldOctant = drawable->octant_;
+        if (oldOctant != this)
         {
             // Add first, then remove, because drawable count going to zero deletes the octree branch in question
-            Octant* oldOctant = drawable->octant_;
             AddDrawable(drawable);
             if (oldOctant)
                 oldOctant->RemoveDrawable(drawable, false);
@@ -128,25 +128,19 @@ void Octant::InsertDrawable(Drawable* drawable)
         return;
     }
     
-    Vector3 octantCenter = worldBoundingBox_.Center();
-    Vector3 drawableCenter = drawable->GetWorldBoundingBox().Center();
-    unsigned x = drawableCenter.x_ < octantCenter.x_ ? 0 : 1;
-    unsigned y = drawableCenter.y_ < octantCenter.y_ ? 0 : 2;
-    unsigned z = drawableCenter.z_ < octantCenter.z_ ? 0 : 4;
-    GetOrCreateChild(x + y + z)->InsertDrawable(drawable);
+    unsigned x = boxCenter.x_ < center_.x_ ? 0 : 1;
+    unsigned y = boxCenter.y_ < center_.y_ ? 0 : 2;
+    unsigned z = boxCenter.z_ < center_.z_ ? 0 : 4;
+    GetOrCreateChild(x + y + z)->InsertDrawable(drawable, boxCenter, boxSize);
 }
 
-bool Octant::CheckDrawableSize(Drawable* drawable) const
+bool Octant::CheckDrawableSize(const Vector3& boxSize) const
 {
     // If max split level, size always OK
-    if (level_ == root_->GetNumLevels())
+    if (level_ != root_->GetNumLevels())
+        return boxSize.x_ >= halfSize_.x_ || boxSize.y_ >= halfSize_.y_ || boxSize.z_ >= halfSize_.z_;
+    else
         return true;
-    
-    Vector3 octantHalfSize = worldBoundingBox_.Size() * 0.5;
-    Vector3 drawableSize = drawable->GetWorldBoundingBox().Size();
-    
-    return drawableSize.x_ >= octantHalfSize.x_ || drawableSize.y_ >= octantHalfSize.y_ || drawableSize.z_ >=
-        octantHalfSize.z_;
 }
 
 void Octant::ResetRoot()
@@ -390,27 +384,21 @@ void Octree::GetUnculledDrawables(PODVector<Drawable*>& dest, unsigned char draw
 
 void Octree::QueueUpdate(Drawable* drawable)
 {
-    if (!drawable->updateQueued_)
-    {
-        drawableUpdates_.Push(drawable);
-        drawable->updateQueued_ = true;
-    }
+    drawableUpdates_.Push(drawable);
+    drawable->updateQueued_ = true;
 }
 
 void Octree::QueueReinsertion(Drawable* drawable)
 {
-    if (!drawable->reinsertionQueued_)
+    if (scene_ && scene_->IsThreadedUpdate())
     {
-        if (scene_ && scene_->IsThreadedUpdate())
-        {
-            reinsertionLock_.Acquire();
-            drawableReinsertions_.Push(drawable);
-            reinsertionLock_.Release();
-        }
-        else
-            drawableReinsertions_.Push(drawable);
-        drawable->reinsertionQueued_ = true;
+        reinsertionLock_.Acquire();
+        drawableReinsertions_.Push(drawable);
+        reinsertionLock_.Release();
     }
+    else
+        drawableReinsertions_.Push(drawable);
+    drawable->reinsertionQueued_ = true;
 }
 
 void Octree::CancelUpdate(Drawable* drawable)
@@ -474,12 +462,8 @@ void Octree::UpdateDrawables(const FrameInfo& frame)
         }
         
         PODVector<Drawable*>::Iterator end = start;
-        int count = 0;
-        while (count < DRAWABLES_PER_WORKITEM && end != drawableUpdates_.End())
-        {
-            ++count;
+        while (end - start < DRAWABLES_PER_WORK_ITEM && end != drawableUpdates_.End())
             ++end;
-        }
         
         item->frame_ = &frame;
         item->start_ = start;
@@ -515,31 +499,28 @@ void Octree::ReinsertDrawables(const FrameInfo& frame)
     for (PODVector<Drawable*>::Iterator i = drawableReinsertions_.Begin(); i != drawableReinsertions_.End(); ++i)
     {
         Drawable* drawable = *i;
-        Octant* octant = drawable->GetOctant();
+        const BoundingBox& box = drawable->GetWorldBoundingBox();
+        Vector3 boxCenter = box.Center();
+        Vector3 boxSize = box.Size();
         
+        Octant* octant = drawable->GetOctant();
         if (octant)
         {
-            bool reinsert = false;
-            
             if (octant == this)
             {
                 // Handle root octant as special case: if outside the root, do not reinsert
-                if (GetCullingBox().IsInside(drawable->GetWorldBoundingBox()) == INSIDE && !CheckDrawableSize(drawable))
-                    reinsert = true;
+                if (GetCullingBox().IsInside(box) == INSIDE && !CheckDrawableSize(boxSize))
+                    InsertDrawable(drawable, boxCenter, boxSize);
             }
             else
             {
                 // Otherwise reinsert if outside current octant or if size does not fit octant size
-                if (octant->GetCullingBox().IsInside(drawable->GetWorldBoundingBox()) != INSIDE ||
-                    !octant->CheckDrawableSize(drawable))
-                    reinsert = true;
+                if (octant->GetCullingBox().IsInside(box) != INSIDE || !octant->CheckDrawableSize(boxSize))
+                    InsertDrawable(drawable, boxCenter, boxSize);
             }
-            
-            if (reinsert)
-                InsertDrawable(drawable);
         }
         else
-            InsertDrawable(drawable);
+            InsertDrawable(drawable, boxCenter, boxSize);
         
         drawable->reinsertionQueued_ = false;
     }

Engine/Graphics/Octree.h

@@ -73,9 +73,9 @@ public:
     /// Delete child octant by pointer.
     void DeleteChild(Octant* octant);
     /// Insert a drawable object by checking for fit recursively.
-    void InsertDrawable(Drawable* drawable);
+    void InsertDrawable(Drawable* drawable, const Vector3& boxCenter, const Vector3& boxSize);
     /// Check if a drawable object fits.
-    bool CheckDrawableSize(Drawable* drawable) const;
+    bool CheckDrawableSize(const Vector3& boxSize) const;
     
     /// Add a drawable object to this octant.
     void AddDrawable(Drawable* drawable)
@@ -152,6 +152,10 @@ protected:
     BoundingBox worldBoundingBox_;
     /// Bounding box used for drawable object fitting.
     BoundingBox cullingBox_;
+    /// World bounding box center.
+    Vector3 center_;
+    /// World bounding box half size.
+    Vector3 halfSize_;
     /// Subdivision level.
     unsigned level_;
     /// Parent octant.

Engine/Graphics/View.cpp

@@ -57,8 +57,6 @@ static const Vector3 directions[] =
     Vector3(0.0f, 0.0f, -1.0f)
 };
 
-static const int DRAWABLES_PER_WORKITEM = 8;
-
 OBJECTTYPESTATIC(View);
 
 View::View(Context* context) :
@@ -622,12 +620,8 @@ void View::UpdateGeometries()
             }
             
             PODVector<Drawable*>::Iterator end = start;
-            int count = 0;
-            while (count < DRAWABLES_PER_WORKITEM && end != threadedGeometries_.End())
-            {
-                ++count;
+            while (end - start < DRAWABLES_PER_WORK_ITEM && end != threadedGeometries_.End())
                 ++end;
-            }
             
             item->frame_ = &frame_;
             item->start_ = start;