On Wednesday, 13 December 2017 at 03:15:11 UTC, IM wrote:
https://code.dlang.org/packages/libdtasks
I'd get rid of the ITaskRunner interface and rename
AbstractTaskRunner to TaskRunner. Interfaces are fine when you
need them, but you rarely need them. You can just make
PostTaskAndReply non-final and it's the same. It's slightly
cheaper than calling the final method through an interface, even.
I'd also eliminate TaskSystem. You can use a thread-local
TaskRunner pretty much the same way, and it's a bit weird to have
policy based on thread ID. (Normally, you would have a
threadpool-like thing for each type of task. Like one for serving
web traffic, one for transcoding video, one for reading RSS
feeds, etc.) Thread IDs can be reused, so if you forget to clean
something up, you might end up reusing the wrong TaskRunner
later. A thread-local variable also reduces the work you have to
do to stop a thread.
In any case, the TaskRunner-per-thread thing is a policy decision
that's not really appropriate for a library to make. You
generally try to leave that to applications.
TaskQueue has a race condition, I think:
Thread 1 calls Pop(). It acquires the mutex.
Thread 1 observes there are no tasks available to pop. It waits
on the condition variable.
Thread 2 calls Push(). It tries to acquire the mutex, but the
mutex is taken. It blocks forever.
Reading code is generally easier when that code follows the style
guide for the language. https://dlang.org/dstyle.html and maybe
run dfmt on your code. For instance, when you write `atomicOp
!"+="`, it takes me about a full second to connect that to the
common style of `atomicOp!"+="`. If I worked with your code for
an hour, that would go away, but it's a bit of a speedbump for
new contributors.
One other point of note about formatting: I never regret using
braces around single-statement blocks. Like:
while (!done && tasks.length == 0)
{
condition.wait;
}
I just so frequently need to add logging or extra bookkeeping to
statements like that that it's not even worth the time to try to
omit braces.
