Adequately split work among RenderViewCommandBuilder

Try to give at least 100 entities per worker.
Ideally we'd find a way to only add the required number
of jobs but ThreadPooler doesn't easily allow that.

Change-Id: Ieaf21b66eefd6c3e3b85b949917ea93b73834838
Reviewed-by: Sean Harmer <sean.harmer@kdab.com>

6 files changed, 69 insertions, 37 deletions

diff --git a/src/render/renderers/opengl/jobs/renderviewcommandbuilderjob.cpp b/src/render/renderers/opengl/jobs/renderviewcommandbuilderjob.cpp
index c6bc20423..091d49ef5 100644
--- a/src/render/renderers/opengl/jobs/renderviewcommandbuilderjob.cpp
+++ b/src/render/renderers/opengl/jobs/renderviewcommandbuilderjob.cpp
@@ -53,6 +53,8 @@ int renderViewInstanceCounter = 0;
 
 RenderViewCommandBuilderJob::RenderViewCommandBuilderJob()
     : Qt3DCore::QAspectJob()
+    , m_offset(0)
+    , m_count(0)
     , m_renderView(nullptr)
 {
     SET_JOB_RUN_STAT_TYPE(this, JobTypes::RenderViewCommandBuilder, renderViewInstanceCounter++)
@@ -61,11 +63,14 @@ RenderViewCommandBuilderJob::RenderViewCommandBuilderJob()
 void RenderViewCommandBuilderJob::run()
 {
     if (!m_renderView->noDraw()) {
+        if (m_count == 0)
+            return;
+
         const bool isDraw = !m_renderView->isCompute();
         if (isDraw)
-            m_commandData = m_renderView->buildDrawRenderCommands(m_entities);
+            m_commandData = m_renderView->buildDrawRenderCommands(m_entities, m_offset, m_count);
         else
-            m_commandData = m_renderView->buildComputeRenderCommands(m_entities);
+            m_commandData = m_renderView->buildComputeRenderCommands(m_entities, m_offset, m_count);
     }
 }
 
diff --git a/src/render/renderers/opengl/jobs/renderviewcommandbuilderjob_p.h b/src/render/renderers/opengl/jobs/renderviewcommandbuilderjob_p.h
index 9f45a8005..556c7f241 100644
--- a/src/render/renderers/opengl/jobs/renderviewcommandbuilderjob_p.h
+++ b/src/render/renderers/opengl/jobs/renderviewcommandbuilderjob_p.h
@@ -67,12 +67,19 @@ public:
     RenderViewCommandBuilderJob();
 
     inline void setRenderView(RenderView *rv) Q_DECL_NOTHROW { m_renderView = rv; }
-    inline void setEntities(const QVector<Entity *> &entities) { m_entities = entities; }
+    inline void setEntities(const QVector<Entity *> &entities, int offset, int count)
+    {
+        m_offset = offset;
+        m_count = count;
+        m_entities = entities;
+    }
     inline EntityRenderCommandData &commandData() { return m_commandData; }
 
     void run() final;
 
 private:
+    int m_offset;
+    int m_count;
     RenderView *m_renderView;
     QVector<Entity *> m_entities;
     EntityRenderCommandData m_commandData;
diff --git a/src/render/renderers/opengl/jobs/renderviewcommandupdaterjob.cpp b/src/render/renderers/opengl/jobs/renderviewcommandupdaterjob.cpp
index 95c352dbe..6d6ae7853 100644
--- a/src/render/renderers/opengl/jobs/renderviewcommandupdaterjob.cpp
+++ b/src/render/renderers/opengl/jobs/renderviewcommandupdaterjob.cpp
@@ -58,6 +58,7 @@ RenderViewCommandUpdaterJob::RenderViewCommandUpdaterJob()
     , m_count(0)
     , m_renderView(nullptr)
     , m_renderer(nullptr)
+    , m_renderables(nullptr)
 {
     SET_JOB_RUN_STAT_TYPE(this, JobTypes::RenderCommandUpdater, renderViewInstanceCounter++);
 }
@@ -67,6 +68,8 @@ void RenderViewCommandUpdaterJob::run()
     // Build RenderCommand should perform the culling as we have no way to determine
     // if a child has a mesh in the view frustum while its parent isn't contained in it.
     if (!m_renderView->noDraw()) {
+        if (m_count == 0)
+            return;
         // Update Render Commands (Uniform Change, Depth Change)
         m_renderView->updateRenderCommand(m_renderables, m_offset, m_count);
     }
diff --git a/src/render/renderers/opengl/renderer/renderview.cpp b/src/render/renderers/opengl/renderer/renderview.cpp
index e761507ce..2eabec4ea 100644
--- a/src/render/renderers/opengl/renderer/renderview.cpp
+++ b/src/render/renderers/opengl/renderer/renderview.cpp
@@ -621,13 +621,16 @@ void RenderView::addClearBuffers(const ClearBuffers *cb) {
 }
 
 // If we are there, we know that entity had a GeometryRenderer + Material
-EntityRenderCommandData RenderView::buildDrawRenderCommands(const QVector<Entity *> &entities) const
+EntityRenderCommandData RenderView::buildDrawRenderCommands(const QVector<Entity *> &entities,
+                                                            int offset, int count) const
 {
     EntityRenderCommandData commands;
 
-    commands.reserve(entities.size());
+    commands.reserve(count);
 
-    for (Entity *entity : entities) {
+    for (int i = 0; i < count; ++i) {
+        const int idx = offset + i;
+        Entity *entity = entities.at(idx);
         GeometryRenderer *geometryRenderer = nullptr;
         HGeometryRenderer geometryRendererHandle = entity->componentHandle<GeometryRenderer>();
 
@@ -672,7 +675,8 @@ EntityRenderCommandData RenderView::buildDrawRenderCommands(const QVector<Entity
     return commands;
 }
 
-EntityRenderCommandData RenderView::buildComputeRenderCommands(const QVector<Entity *> &entities) const
+EntityRenderCommandData RenderView::buildComputeRenderCommands(const QVector<Entity *> &entities,
+                                                               int offset, int count) const
 {
     // If the RenderView contains only a ComputeDispatch then it cares about
     // A ComputeDispatch is also implicitely a NoDraw operation
@@ -681,9 +685,11 @@ EntityRenderCommandData RenderView::buildComputeRenderCommands(const QVector<Ent
     // material/effect/technique/parameters/filters/
     EntityRenderCommandData commands;
 
-    commands.reserve(entities.size());
+    commands.reserve(count);
 
-    for (Entity *entity : entities) {
+    for (int i = 0; i < count; ++i) {
+        const int idx = offset + i;
+        Entity *entity = entities.at(idx);
         ComputeCommand *computeJob = nullptr;
         HComputeCommand computeCommandHandle = entity->componentHandle<ComputeCommand>();
         if ((computeJob = nodeManagers()->computeJobManager()->data(computeCommandHandle)) != nullptr
diff --git a/src/render/renderers/opengl/renderer/renderview_p.h b/src/render/renderers/opengl/renderer/renderview_p.h
index 0d0a1dbf9..c7dc37a2c 100644
--- a/src/render/renderers/opengl/renderer/renderview_p.h
+++ b/src/render/renderers/opengl/renderer/renderview_p.h
@@ -227,8 +227,10 @@ public:
 
     RenderPassList passesAndParameters(ParameterInfoList *parameter, Entity *node, bool useDefaultMaterials = true);
 
-    EntityRenderCommandData buildDrawRenderCommands(const QVector<Entity *> &entities) const;
-    EntityRenderCommandData buildComputeRenderCommands(const QVector<Entity *> &entities) const;
+    EntityRenderCommandData buildDrawRenderCommands(const QVector<Entity *> &entities,
+                                                    int offset, int count) const;
+    EntityRenderCommandData buildComputeRenderCommands(const QVector<Entity *> &entities,
+                                                       int offset, int count) const;
 
 
     void updateRenderCommand(EntityRenderCommandData *renderCommandData,
diff --git a/src/render/renderers/opengl/renderer/renderviewbuilder.cpp b/src/render/renderers/opengl/renderer/renderviewbuilder.cpp
index 2bf2759fb..9f7589ecc 100644
--- a/src/render/renderers/opengl/renderer/renderviewbuilder.cpp
+++ b/src/render/renderers/opengl/renderer/renderviewbuilder.cpp
@@ -50,10 +50,18 @@ namespace Render {
 // In some cases having less jobs is better (especially on fast cpus where
 // splitting just adds more overhead). Ideally, we should try to set the value
 // depending on the platform/CPU/nbr of cores
-const int RenderViewBuilder::m_optimalParallelJobCount = std::max(std::min(4, QThread::idealThreadCount()), 2);
+const int RenderViewBuilder::m_optimalParallelJobCount = QThread::idealThreadCount();
 
 namespace {
 
+int findIdealNumberOfWorkers(int elementCount, int packetSize = 100)
+{
+    if (elementCount == 0 || packetSize == 0)
+        return 0;
+    return std::min(std::max(elementCount / packetSize, 1), RenderViewBuilder::optimalJobCount());
+}
+
+
 class SyncPreCommandBuilding
 {
 public:
@@ -82,16 +90,16 @@ public:
 
         lock.unlock();
 
-        // Split among the number of command builders
-        int i = 0;
-        const int m = RenderViewBuilder::optimalJobCount() - 1;
-        const int packetSize = entities.size() / RenderViewBuilder::optimalJobCount();
-        while (i < m) {
+        // Split among the ideal number of command builders
+        const int idealPacketSize = std::min(std::max(100, entities.size() / RenderViewBuilder::optimalJobCount()), entities.size());
+        // Try to split work into an ideal number of workers
+        const int m = findIdealNumberOfWorkers(entities.size(), idealPacketSize);
+
+        for (int i = 0; i < m; ++i) {
             const RenderViewCommandBuilderJobPtr renderViewCommandBuilder = m_renderViewCommandBuilderJobs.at(i);
-            renderViewCommandBuilder->setEntities(entities.mid(i * packetSize, packetSize));
-            ++i;
+            const int count = (i == m - 1) ? entities.size() - (i * idealPacketSize) : idealPacketSize;
+            renderViewCommandBuilder->setEntities(entities, i * idealPacketSize, count);
         }
-        m_renderViewCommandBuilderJobs.at(i)->setEntities(entities.mid(i * packetSize, packetSize + entities.size() % (m + 1)));
     }
 
 private:
@@ -118,16 +126,19 @@ public:
         RenderView *rv = m_renderViewJob->renderView();
 
         const EntityRenderCommandData *commandData = m_renderViewCommandUpdaterJobs.first()->renderables();
-        const QVector<RenderCommand> commands = std::move(commandData->commands);
 
-        // TO DO: Cache the EntityRenderCommandData so that it can be reused if nothing in the scene has changed
-        delete commandData;
+        if (commandData != nullptr) {
+            const QVector<RenderCommand> commands = std::move(commandData->commands);
 
-        rv->setCommands(commands);
+            // TO DO: Cache the EntityRenderCommandData so that it can be reused if nothing in the scene has changed
+            delete commandData;
 
-        // TO DO: Find way to store commands once or at least only when required
-        // Sort the commands
-        rv->sort();
+            rv->setCommands(commands);
+
+            // TO DO: Find way to store commands once or at least only when required
+            // Sort the commands
+            rv->sort();
+        }
 
         // Enqueue our fully populated RenderView with the RenderThread
         m_renderer->enqueueRenderView(rv, m_renderViewJob->submitOrderIndex());
@@ -274,12 +285,12 @@ public:
                         commandData += std::move(renderViewCommandBuilder->commandData());
                 }
 
+
                 // Store new cache
                 RendererCache::LeafNodeData &writableCacheForLeaf = cache->leafNodeCache[m_leafNode];
                 writableCacheForLeaf.renderCommandData = std::move(commandData);
             }
             const EntityRenderCommandData commandData = dataCacheForLeaf.renderCommandData;
-
             const QVector<Entity *> filteredEntities = dataCacheForLeaf.filterEntitiesByLayer;
             QVector<Entity *> renderableEntities = isDraw ? cache->renderableEntities : cache->computeEntities;
             QVector<LightSource> lightSources = cache->gatheredLights;
@@ -339,17 +350,15 @@ public:
             }
 
             // Split among the number of command builders
-            int i = 0;
-            const int m = RenderViewBuilder::optimalJobCount() - 1;
-            const int packetSize = filteredCommandData->size() / RenderViewBuilder::optimalJobCount();
-            while (i < m) {
+            // The idealPacketSize is at least 100 entities per worker
+            const int idealPacketSize = std::min(std::max(100, filteredCommandData->size() / RenderViewBuilder::optimalJobCount()), filteredCommandData->size());
+            const int m = findIdealNumberOfWorkers(filteredCommandData->size(), idealPacketSize);
+
+            for (int i = 0; i < m; ++i) {
                 const RenderViewCommandUpdaterJobPtr renderViewCommandBuilder = m_renderViewCommandUpdaterJobs.at(i);
-                renderViewCommandBuilder->setRenderables(filteredCommandData, i * packetSize, packetSize);
-                ++i;
+                const int count = (i == m - 1) ? filteredCommandData->size() - (i * idealPacketSize) : idealPacketSize;
+                renderViewCommandBuilder->setRenderables(filteredCommandData, i * idealPacketSize, count);
             }
-            m_renderViewCommandUpdaterJobs.at(i)->setRenderables(filteredCommandData,
-                                                                 i * packetSize,
-                                                                 packetSize + filteredCommandData->size() % (m + 1));
         }
     }