In the old code, it was not possible to run the callbacks without incurring deadlocks. After the rewrite now, it's deadlock-free.
I tested with and without the call_out() construct.
In my testcases, I see: a. Comparable CPU usage for both cases. b. 6% wall-clock speed improvement when avoiding call_out().
I propose ripping out the call_out() method, so that callouts are ran directly in all cases except for the timeout-case.
Any objections?
Stephen R. van den Berg wrote:
In my testcases, I see: b. 6% wall-clock speed improvement when avoiding call_out().
I propose ripping out the call_out() method, so that callouts are ran directly in all cases except for the timeout-case.
Any objections?
One tradeoff here, BTW, is that you get deep stacks, at times, because callbacks nest. Using call_out() avoids that. Not quite sure what is best here.
Maybe we should leave it configurable after all.
In the old code, it was not possible to run the callbacks without incurring deadlocks. After the rewrite now, it's deadlock-free.
I tested with and without the call_out() construct.
In my testcases, I see: a. Comparable CPU usage for both cases. b. 6% wall-clock speed improvement when avoiding call_out().
I propose ripping out the call_out() method, so that callouts are ran directly in all cases except for the timeout-case.
Not a good idea:
* Unlimited stack use.
* Potential deadlocks/data inconsistency.
* No serialization of calls.
* No idea of in what context/thread your callback gets called.
/grubba
Henrik Grubbstr?m (Lysator) @ Pike (-) developers forum wrote:
I propose ripping out the call_out() method, so that callouts are ran directly in all cases except for the timeout-case.
Not a good idea:
- Unlimited stack use.
I realised that too. That is a definite downside.
- Potential deadlocks/data inconsistency.
This could be considered a plus. Properly written thread-safe code, should not be vulnerable to this. So turning off the call_out() intermediary could be considered a relatively robust way to check if your regular code is actually deadlock-free and thread-safe.
- No serialization of calls.
Same thing here. It could reorder the calls; then again, if your code is resistant against this, then it has been written in a robust manner.
- No idea of in what context/thread your callback gets called.
Yes, found this out the hard way. Turned out that all of a sudden I had database big_query() calls from within my pgsql local_backend (causing a deadlock, obviously).
Since the cost of having the option to use Promises without call_out() is close to zero this way, I propose to keep this option in. It allows for promises without backend, and for a proper stresstest of one's own code to verify robustness.
Henrik Grubbstr?m (Lysator) @ Pike (-) developers forum wrote:
I propose ripping out the call_out() method, so that callouts are ran directly in all cases except for the timeout-case.
Not a good idea:
- Unlimited stack use.
I realised that too. That is a definite downside.
- Potential deadlocks/data inconsistency.
This could be considered a plus. Properly written thread-safe code, should not be vulnerable to this. So turning off the call_out() intermediary could be considered a relatively robust way to check if your regular code is actually deadlock-free and thread-safe.
I'm not so sure; eg the following will hang:
Thread.Mutex mux = Thread.Mutex();
void producer() { Thread.MutexKey key = mux->lock();
...
promise->success(x);
...
key = 0; }
void got_sucess(mixed x) { Thread.Mutex key = mux->lock();
... }
- No serialization of calls.
Same thing here. It could reorder the calls; then again, if your code is resistant against this, then it has been written in a robust manner.
Reordering is fine; concurreny is not.
/grubba
Henrik Grubbstr?m (Lysator) @ Pike (-) developers forum wrote:
- Potential deadlocks/data inconsistency.
This could be considered a plus. Properly written thread-safe code, should not be vulnerable to this. So turning off the call_out() intermediary could be considered a relatively robust way to check if your regular code is actually deadlock-free and thread-safe.
I'm not so sure; eg the following will hang: Thread.Mutex mux = Thread.Mutex(); Thread.MutexKey key = mux->lock(); promise->success(x); key = 0;
void got_sucess(mixed x) Thread.Mutex key = mux->lock(); ...
Yes, it would hang. But, as you might have noticed, previously, the Promise module had some of those constructs, like you show above, in it. By optimising those parts, I found that the actual mutex lock should never be across the ->on_success() call, but usually only should extend over the mere setting/testing of a state variable.
By releasing that lock as early as possibly, I both increase parallelism and reduce lock contention, as well as make it impossible to get into the deadlock situation sketched above.
I think, as proficiency with multithreaded/parallel-running code increases, one learns that often using shorter locks is better, and often eliminates the possibility of a deadlock.
pike-devel@lists.lysator.liu.se
-
Henrik Grubbstr�m (Lysator) ＠ Pike (-) developers forum -
Stephen R. van den Berg