Tasks are dispatched with invalid ranges

[cstamford]

See the attached code for a minimal repro. I am new to this API so if I am making an obvious mistake, please do let me know. This bug appears to occur reliably when TASK_COUNT >= 50 and when TASK_COUNT is 5000 it will always happen.

Context / use case: I am developing a terrain streaming system. On world load I spawn about 5000~ tasks which procedurally generate each terrain cell's heightmap, then another 5000~ with various dependencies to generate the meshes.

#include "TaskScheduler.h"

int main(int, char**)
{
    static enki::TaskScheduler s_scheduler;
    s_scheduler.Initialize();

    static constexpr size_t TASK_RANGE = 65*65;
    static constexpr size_t TASK_COUNT = 50;

    struct Repro : public enki::ITaskSet
    {
        Repro() : enki::ITaskSet(TASK_RANGE) {};
        virtual void ExecuteRange( enki::TaskSetPartition range, uint32_t) override
        {
            if (range.start > TASK_RANGE || range.end > TASK_RANGE)
            {
                __debugbreak(); // bug?!
            }
        }
    };

    std::vector<std::unique_ptr<Repro>> jobs;

    for (size_t i = 0; i < TASK_COUNT; ++i)
    {
        jobs.emplace_back(std::make_unique<Repro>());
    }

    for (std::unique_ptr<Repro>& job : jobs)
    {
        s_scheduler.AddTaskSetToPipe(job.get());
    }

    s_scheduler.WaitforAll();
}

From a preliminary search of the library code, the problem appears to occur only when the task buffer is full and it must be executed immediately. The code does not properly handle ranges that are not divisible by the range step, so what should be the final step overruns into an infinite loop.

https://github.com/dougbinks/enkiTS/blob/master/src/TaskScheduler.cpp#L656

I have removed this block - it doesn't make sense to me, if we were simply executing the same job immediately because there was no room, why would we have to tweak the range that we've already calculated? This fixed the problem for me. I did verify that every element of the range was being generated with the following test:

struct Repro : public enki::ITaskSet
{
    std::vector<int> data;
    Repro() : enki::ITaskSet(TASK_RANGE) { data.resize(TASK_RANGE); };
    virtual void ExecuteRange( enki::TaskSetPartition range, uint32_t) override
    {
        if (range.start > TASK_RANGE || range.end > TASK_RANGE)
        {
            __debugbreak(); // bug?!
        }

        for (size_t i = range.start; i < range.end; ++i)
        {
            data[i] = 1;
        }
    }
};

... later

for (std::unique_ptr<Repro>& job : jobs)
{
    int count = 0;
    for (size_t i = 0; i < TASK_RANGE; ++i)
    {
        count += job->data[i];
    }
    ASSERT(count == TASK_RANGE);
}

[dougbinks]

Many thanks for this report and especially for a simple reproduction - it seems my own tests for this didn't trigger the exact flow required to get the issue.

I've added your reproduction as part of the test suite, fixed the issue and added an extra assert.

[cstamford]

Thanks, that seems to have fixed it for me!

[dougbinks]

Excellent, I'll close this issue and the PR then, but please do re-open them if there are any further problems with this.

You should watch #62 if you are interested in an example of how to add lots of tasks.