review of std.parallelism

[Andrei Alexandrescu]

On 03/18/2011 11:40 PM, dsimcha wrote:
> Thanks for the advice. You mentioned in the past that the documentation
> was inadequate but didn't give enough specifics as to how until now. As
> the author of the library, things seem obvious to me that don't seem
> obvious to anyone else, so I don't feel that I'm in a good position to
> judge the quality of the documentation and where it needs improvement. I
> plan to fix most of the issues you raised, but I've left comments for
> the few that I can't/won't fix or believe are based on misunderstandings
> below.

Great, thanks.

> On 3/18/2011 11:29 PM, Andrei Alexandrescu wrote:
>> 1. Library proper:
>>
>> * "In the case of non-random access ranges, parallel foreach is still
>> usable but buffers lazily to an array..." Wouldn't strided processing
>> help? If e.g. 4 threads the first works on 0, 4, 8, ... second works on
>> 1, 5, 9, ... and so on.
>
> You can have this if you want, by setting the work unit size to 1.
> Setting it to a larger size just causes more elements to be buffered,
> which may be more efficient in some cases.

Got it.

>> * Why not make workerIndex a ulong and be done with it?
>
> I doubt anyone's really going to create anywhere near 4 billion TaskPool
> threads over the lifetime of a program. Part of the point of TaskPool is
> recycling threads rather than paying the overhead of creating and
> destroying them. Using a ulong on a 32-bit architecture would make
> worker-local storage substantially slower. workerIndex is how
> worker-local storage works under the hood, so it needs to be fast.

If you're confident that overflow won't occur, you may want to eliminate 
that detail from the docs. It throws off the reader.

>  > * No example for workerIndex and why it's useful.
>
> It should just be private. The fact that it's public is an artifact of
> when I was designing worker-local storage and didn't know how it was
> going to work yet. I never thought to revisit this until now. It really
> isn't useful to client code.

It could be public and undocumented.

>> * Is stop() really trusted or just unsafe? If it's forcibly killing
>> threads then its unsafe.
>
> It's not forcibly killing threads. As the documentation states, it has
> no effect on jobs already executing, only ones in the queue.
> Furthermore, it's needed unless makeDaemon is called. Speaking of which,
> makeDaemon and makeAngel should probably be trusted, too.

Great. The more safe/trusted, the better.

>> * defaultPoolThreads - should it be a @property?
>
> Yes. In spirit it's a global variable. It requires some extra
> machinations, though, to be threadsafe, which is why it's not
> implemented as a simple global variable.

Then it should be a @property. I think ddoc doesn't reflect that, but an 
example could.

>> * No example for task().
>
> ???? Yes there is, for both flavors, though these could admittedly be
> improved. Only the safe version doesn't have an example, and this is
> just a more restricted version of the function pointer case, so it seems
> silly to make a separate example for it.

Towards the bottom of the document there are overloads of task that 
don't have examples.

>> * What is 'run' in the definition of safe task()?
>
> It's just the run() adapter function. Isn't that obvious?

I'm referring to this:

Task!(run,TypeTuple!(F,Args)) task(F, Args...)(scope F delegateOrFp, 
Args args);

What is "run"?


Andrei