On Sun, 17 Jul 2016 23:37:12 GMT, Øivind Loe wrote:

On Sun, 17 Jul 2016 23:20:09 GMT, Øivind Loe wrote:

This came up recently and I couldn't remember why I chose runTask, but
I think that runWorkerTask would be the right choice. That would not
be a real thread per spawn, but tasks would be mapped to a thread pool
where the number of threads equals the number of total hardware threads
by default.

Another possibility would be to make this configurable and to provide
the choices runTask, runWorkerTask and new Thread.

The problem here is that unit-threaded should not be dependent on vibe.d.. With this new definition of spawn(..), unit-threaded suddenly has to become aware of vibe.d in order to work, and have a special case if vibe.d is present.

I looked at using 'new Task', but my impression is that then the 'Tid' struct would not be generated, and the newly created struct would not be able to receive messages using std.concurrency.

Preferably, vibe.d should not change the function of std.concurrency (improving on it is ok).

Do you have any ideas on how I can address this?

The problem is that std.concurrency would generally be incompatible with vibe.d if it wouldn't integrate at all. Using runTask is the only solution that is more or less @safe (because an unshared delegate is passed to spawn()), while runWorkerTask probably comes closest to what someone would expect if CPU-level concurrency is desired. new Thread would be the equivalent of the default behavior, but it's rather inefficient and having a Task is quite convenient.

So a configuration option probably wouldn't hurt.

As a hack, I am now using runWorkerTask_unsafe(..) in spawn, as the safe version would not take the delegate. I also had to change this function to public in order to get access to it.

Can you please sort this out for the next vibe.d version?

You'd have to cast(shared) the delegate, so that runWorkerTask accepts it. It's quite unfortunate that all of Phobos' threading is not taking shared into account.

I've switched spawn to use runWorkerTask on Git master now (5709950), but I'll also add way to configure the behavior.