On 3/19/2011 12:03 PM, Andrei Alexandrescu wrote:
On 03/19/2011 02:32 AM, dsimcha wrote:
Ok, thanks again for clarifying **how** the docs could be improved. I've
implemented the suggestions and generally given the docs a good reading
over and clean up. The new docs are at:
http://cis.jhu.edu/~dsimcha/d/phobos/std_parallelism.html
* Still no synopsis example that illustrates in a catchy way the most
attractive artifacts.
I don't see what I could put here that isn't totally redundant with the
rest of the documentation. Anything I could think of would basically
just involve concatentating all the examples. Furthermore, none of the
other Phobos modules have this, so I don't know what one should look
like. In general I feel like std.parallelism is being held to a
ridiculously high standard that none of the other Phobos modules
currently meet.
* "After creation, Task objects are submitted to a TaskPool for
execution." I understand it's possible to use Task straight as a
promise/future, so s/are/may be/.
No. The only way Task is useful is by submitting it to a pool to be
executed. (Though this may change, see below.)
Also it is my understanding that
TaskPool efficiently adapts the concrete number of CPUs to an arbitrary
number of tasks. If that's the case, it's worth mentioning.
Isn't this kind of obvious from the examples, etc.?
* "If a Task has been submitted to a TaskPool instance, is being stored
in a stack frame, and has not yet finished, the destructor for this
struct will automatically call yieldWait() so that the task can finish
and the stack frame can be destroyed safely." At this point in the doc
the reader doesn't understand that at all because TaskPool has not been
seen yet. The reader gets worried that she'll be essentially serializing
the entire process by mistake. Either move this explanation down or
provide an example.
This is getting ridiculous. There are too many circular dependencies
between Task and TaskPool that are impossible to remove here that I'm
not even going to try. One or the other has to be introduced first, but
neither can be explained without mentioning the other. This is why I
explain the relationship briefly in the module level summary, so that
the user has at least some idea. I think this is about the best I can do.
* Is done() a property?
Yes. DDoc sucks.
* The example that reads two files at the same time should NOT use
taskPool. It's just one task, why would the pool ever be needed? If you
also provided an example that reads n files in memory at the same time
using a pool, that would illustrate nicely why you need it. If a Task
can't be launched without being put in a pool, there should be a
possibility to do so. At my work we have a simple function called
callInNewThread that does what's needed to launch a function in a new
thread.
I guess I could add something like this to Task. Under the hood it
would (for implementation simplicity, to reuse a bunch of code from
TaskPool) fire up a new single-thread pool, submit the task, call
TaskPool.finish(), and then return. Since you're already creating a new
thread, the extra overhead of creating a new TaskPool for the thread
would be negligible and it would massively simplify the implementation.
My only concern is that, when combined with scoped versus non-scoped
tasks (which are definitely here to stay, see below) this small
convenience function would add way more API complexity than it's worth.
Besides, task pools don't need to be created explicitly anyhow.
That's what the default instantiation is for. I don't see how
callInNewThread would really solve much.
Secondly, I think you're reading **WAY** too much into what was meant to
be a simple example to illustrate usage mechanics. This is another case
where I can't think of a small, cute example of where you'd really need
the pool. There are plenty of larger examples, but the smallest/most
self-contained one I can think of is a parallel sort. I decided to use
file reading because it was good enough to illustrate the mechanics of
usage, even if it didn't illustrate a particularly good use case.
* The note below that example gets me thinking: it is an artificial
limitation to force users of Task to worry about scope and such. One
should be able to create a Future object (Task I think in your
terminology), pass it around like a normal value, and ultimately force
it. This is the case for all other languages that implement futures. I
suspect the "scope" parameter associated with the delegate a couple of
definitions below plays a role here, but I think we need to work for
providing the smoothest interface possible (possibly prompting
improvements in the language along the way).
This is what TaskPool.task is for. Maybe this should be moved to the
top of the definition of TaskPool and emphasized, and the scoped/stack
allocated versions should be moved below TaskPool and de-emphasized?
At any rate, though, anyone who's playing with parallelism should be an
advanced enough programmer that concepts like scope and destructors are
second nature.
* I'm not sure how to interpret the docs for
ReturnType!(F) run(F, Args...)(F fpOrDelegate, ref Args args);
So it's documented but I'm not supposed to care. Why not just remove?
Surely there must be higher-level examples that clarify that I can use
delegates etc.
Yes, and there are such examples. It's just that I want to explicitly
state what type is returned in this case rather than returning auto. If
I omitted the docs for run(), you'd ask what run() is.
* "If you want to escape the Task object from the function in which it
was created or prefer to heap allocate and automatically submit to the
pool, see TaskPool.task()." I'm uncomfortable that I need to remember a
subtle distinction of the same primitive name ("task") depending on
membership in a TaskPool or not, which is a tenuous relationship to
remember. How about "scopedTask()" vs. "task()"? Also, it's worth asking
ourselves what's the relative overhead of closure allocation vs. task
switching etc. Maybe we get to simplify things a lot at a small cost in
efficiency.
We absolutely **need** scope delegates for calling stuff where a closure
can't be allocated due to objects on the stack frame having scoped
destruction. This is not going anywhere. Also, I **much** prefer to
have everything that does conceptually the same thing have the same
name. I think this is clearer, not less clear.
* As I mentioned, in the definition:
Task!(run,TypeTuple!(F,Args)) task(F, Args...)(F fun, Args args);
I can't tell what "run" is.
run() is just the adapter function described right above this code. I
cannot for the life of me understand how this could possibly be unclear.
* "A goto from inside the parallel foreach loop to a label outside the
loop will result in undefined behavior." Would this be a bug in dmd?
No, it's because a goto of this form has no reasonable, useful
semantics. I should probably mention in the docs that the same applies
to labeled break and continue.
I have no idea what semantics these should have, and even if I did,
given the long odds that even one person would actually need them, I
think they'd be more trouble than they're worth to implement. For
example, once you break out of a parallel foreach loop to some arbitrary
address (and different threads can goto different labels, etc.), well,
it's no longer a parallel foreach loop. It's just a bunch of completely
unstructured threading doing god-knows-what.
Therefore, I slapped undefined behavior on it as a big sign that says,
"Just don't do it." This also has the advantage that, if anyone ever
thinks of any good, clearly useful semantics, these will be
implementable without breaking code later.
* Again: speed of e.g. parallel min/max vs. serial, pi computation etc.
on a usual machine?
I **STRONGLY** believe this does not belong in API documentation because
it's too machine specific, compiler specific, stack alignment specific,
etc. and almost any benchmark worth doing takes up more space than an
example should. Furthermore, anyone who wants to know this can easily
time it themselves. I have absolutely no intention of including this.
While in general I appreciate and have tried to accommodate your
suggestions, this is one I'll be standing firm on.
* The join example is fine, but the repetitive code suggests that loops
should be better:
import std.file;
void main() {
auto pool = new TaskPool();
foreach (filename; ["foo.txt", "bar.txt", "baz.txt"]) {
pool.put(task!read(filename));
}
// Call join() to guarantee that all tasks are done running, the worker
// threads have terminated and that the results of all of the tasks can
// be accessed without any synchronization primitives.
pool.join();
ubyte[][] data; // void[], meh
// Use spinWait() since the results are guaranteed to have been computed
// and spinWait() is the cheapest of the wait functions.
foreach (task; pool.howDoIEnumerateTasks()) {
data ~= task1.spinWait();
}
You can't enumerate the tasks like this, but there's a good reason for
this limitation. The design requires the pool not hold onto any
references to the tasks after they're out of the queue, so that they can
be safely destroyed as soon as the caller wants to destroy them. If you
need to enumerate over them like this, then:
1. You're probably doing something wrong and parallel foreach would be
a better choice.
2. If you insist on doing this, you need to explicitly store them in an
array or something.
As a more general comment, I know the join() example sucks. join() is
actually a seldom-used primitive, not a frequently used one as you
suggest. Most of the time you wait for individual tasks (explicitly or
implicitly through the higher level functions), not the whole pool, to
finish executing. I feel like it's obligatory that join() and finish()
exist, but I never use them myself. They were much more useful in
pre-release versions of this lib, when parallel foreach didn't exist and
it made sense to have explicit arrays of Tasks. The difficulty I've had
in coming up with a decent example for them makes me halfway tempted to
just rip those $#)*% functions out entirely.
