Is it possible to request a code review?

[Neia Neutuladh]

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.