scheduler remove lock for scheduled|resumed tasks

[jbaldwin]

• Use atomic waiter list instead of std::mutex + std::vector to pass tasks scheduled through to the coro::scheduler.

[jbaldwin]

@PyXiion been toying around with #404 and #446 solutions, this by no means will resolve both issues but I found in performance testing its somewhere between 5% to 10% quicker for scheduling tasks on a scheduler using inline.

[jbaldwin]

I simplified the m_size being decremented only here by the process calls for inline and thread pool, it was difficult to merge the two patterns until I realized the other decrements should have just been here all along.

[jbaldwin]

I updated this test to actually have some throughput for benchmarking, and removed most of the REQUIRE_THREAD_SAFE since they are a lock and affect benching

[jbaldwin]

This was a bug, added a test to fix it as well.

[PyXiion]

What if a partial read happens? write is guaranteed to be atomic (if the size of the data written is <= PIPE_BUF), but read is not

[jbaldwin]

yeah good point, the check needs to be perfectly divided by for the sizeof, in testing this doesn't seem to happen, but that doesn't mean it won't. Caching a partial read sucks too. 😬

[jbaldwin]

Yeah maybe this change isn't really worth it, there are some nice cleanups around these data structures but that is a real problem.

[PyXiion]

You can do something similar to read_exact in TCP, but read_aligned

auto pipe_t::read_aligned(void* buffer, std::size_t n, std::size_t align) -> long
{
    if (n == 0 || align == 0) return 0;
    
    std::size_t total_read = 0;
    char* ptr = static_cast<char*>(buffer);

    while (total_read < n) {
        ssize_t result = ::read(read_fd(), ptr + total_read, n - total_read);

        if (result < 0) {
            if (errno == EINTR) continue;
            return -1;
        }
        // EOF
        if (result == 0) break;

        total_read += result;

        if (total_read % align == 0) {
            return static_cast<ssize_t>(total_read);
        }
    }

    if (total_read % align != 0) {
        // If we hit EOF but aren't aligned, we have data corruption.
        std::cerr << "cry in cerr\n";
        return -1; 
    }

    return static_cast<long>(total_read);
}

But it looks a bit too much

[PyXiion]

Although it won't save us if an error happens midpointer ._.

[PyXiion]

Also, I think, this can completely break if we have multiple threads

Thread A calls read and the kernel gives it 12 bytes
Thread B calls read and the kernel gives it the remaining 4 bytes plus whatever comes next

Now they have 1,5 and 0,5 pointers and corrupted data

[jbaldwin]

Yeah all good points, for the scheduler the event loop is currently single threaded, but it would limit us to always be singly threaded on it. I'll update this PR to revert passing the schedule_operation* through the pipe, I think there are some other nice changes to keep here though.

[jbaldwin]

Ok I've reverted to using an atomic waiter list (still dropping the lock). In theory I believe this should work, but its possibly got an ABA problem?

[jbaldwin]

I think this works to get rid of one lock but I think getting rid of the timer lock could help a lot too. Probably need to create the poll info on schedule op and have the process event thread use the poll map exclusively. This could introduce some latency to the timers though.

[jbaldwin]

I'm gonna give this a merge to move forward, if we start seeing failures we can revert.