<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div>On Aug 31, 2010, at 12:41 PM, David Simcha wrote:</div><blockquote type="cite"><br><div class="gmail_quote">On Tue, Aug 31, 2010 at 3:07 PM, Sean Kelly <span dir="ltr"><<a href="mailto:sean@invisibleduck.org">sean@invisibleduck.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
A few general comments:<br>
<br>
I'm wondering whether the use of a TaskPool should always be explicit. Perhaps a default pool should be considered to exist, and calling parallel() instead of pool.parallel() will use that? It's rare for apps to have multiple distinct pools anyway, so this may be an easy way to help obscure a tiny bit more of the implementation.<br>
</blockquote><div><br>I could create a <b>default</b> lazily initialized Singleton for the TaskPool (a few other people I've showed this code to have requested this, too, so maybe it's a trend), but I absolutely insist on being able to manually initialize it. First of all, this gives the user the ability to manually decide how many threads to use. Secondly, I've had use cases where all threads in a pool are blocked waiting for the same really expensive computation to complete, so I fire up a new pool to do that really expensive computation in parallel.<br></div></div></blockquote><div><br></div>Right. I've added some fancy behavior to the TaskPool itself so the thread count is a bit more dynamic, but that tends to overcomplicate the code. Your approach is probably better.</div><div><br><blockquote type="cite"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin-top: 0pt; margin-right: 0pt; margin-bottom: 0pt; margin-left: 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex; position: static; z-index: auto; ">
Rather than making the use of 'break' inside parallel foreach undefined, why not say that it aborts the processing of the current block?<br></blockquote><blockquote class="gmail_quote" style="margin-top: 0pt; margin-right: 0pt; margin-bottom: 0pt; margin-left: 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex; position: static; z-index: auto; "><br>
<br>
When waiting on condition variables, there's a remote chance that wait() will return when there isn't actually something to process. Instead of:<br>
<br>
if (!condition)<br>
wait();<br>
<br>
use:<br>
<br>
while (!condition)<br>
wait();<br>
<br>
I saw the "if" approach in "AbstractTask* pop()"... I think that's the only one.<br></blockquote><div><br>Thanks, will do. How does this happen, though?<br></div></div></blockquote><div><br></div>Theoretical mumbo-jumbo, mostly. Here's the wiki entry: <a href="http://en.wikipedia.org/wiki/Spurious_wakeup">http://en.wikipedia.org/wiki/Spurious_wakeup</a> For the most part, it's just good programming practice to double-check the invariant on wakeup.</div><div><br><blockquote type="cite"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<br>
In "ReturnType spinWait()" you throw "exception" if it exists but don't clear the variable. Will this be a problem?<br></blockquote><div><br>I can't see why it would be a problem. Also, if for some crazy reason the user handles the exception and then tries to get at the return value again, IMHO the correct behavior is to throw it again. If there's something I'm missing, please let me know.<br></div></div></blockquote><br></div><div>I hadn't spent enough time with the code to know what the correct behavior should be, so I asked.</div></body></html>