Amb + Backpressure #1533
Amb + Backpressure #1533
Conversation
|
RxJava-pull-requests #1456 ABORTED |
|
I can not yet replicate the hang that's happening on the builds. It has happened around this area twice: |
|
RxJava-pull-requests #1457 SUCCESS |
|
Forcing the build again shows that this change is successful and that unfortunately it is indeed a non-deterministic hang somewhere. |
|
Amb (http://www-formal.stanford.edu/jmc/basis1/node7.html#SECTION00025000000000000000) with back pressure is intrinsically hard. If you would do it on iterable you'd need to add a thread. Glancing at the code, I would request just a single item from each source initially. |
| if (other != notThis) { | ||
| other.unsubscribe(); | ||
| } | ||
| ambSubscribers.clear(); |
There was a problem hiding this comment.
ambSubscribers.clear(); should be out of the loop.
There was a problem hiding this comment.
Ha! Yes it should :-)
|
@headinthebox I don't understand the complexity of backpressure on this. It is pretty straight forward to implement here. Whatever the child requests, we propagate to each of the parent |
|
@zsxwing can you please review the various changes I've made to prevent race conditions and memory leaks? |
|
RxJava-pull-requests #1459 FAILURE |
|
Ha ... I accidentally didn't commit the chunk of the file including the imports ... re-committing :-) |
|
RxJava-pull-requests #1460 SUCCESS |
| return true; | ||
| } else { | ||
| // we lost so unsubscribe ... and force cleanup again due to possible race conditions | ||
| unsubscribe(); |
There was a problem hiding this comment.
unsubscribe(); is redundant because unsubscribeLosers will call it. LGTM if removing this line.
... and then keep propagating to the winner.
|
Rebased to collapse my 3 commits. Forced commit. Merging. Thanks @zsxwing for the reviews and your initial work on this. |
|
RxJava-pull-requests #1461 ABORTED |
Building on top of #1516