flatMap: fixed scalar-merging. #2878
Conversation
|
Is this a replacement for your significant rewrite of |
|
No, this is still incremental. The full rewrite will only happen for 2.0. |
|
Cool, I like that approach better. That context helps me review this. Don't spend too much time on the 2.0 rewrite quite yet though until we figure out some of the foundational abstractions and contracts. |
| if (moreToDrain) { | ||
| drainQueuesIfNeeded(); | ||
| } | ||
| request(1); |
There was a problem hiding this comment.
In a future rewrite we can optimize this because it could drain queues and request(n) and then immediately request(1) again.
|
I'm not ready to proceed with this PR. I've added some comments above. The key part that concerns me is that we are changing to always route through a queue for scalar values when request(n) < Long.MAX_VALUE is happening. |
|
Apparently a full rewrite can't be avoided. |
|
If you're going to pursue a full rewrite, make sure to pay attention not just to the JMH throughput numbers, but also the object allocation rates. I expect a rewrite of merge to take some non-trivial time to write, review and test considering the history and attempts made. It is a far more important operator performance-wise than most. |
FlatMapping over a sequence of scalar observable values while observing them from a different thread did not work: items in the scalar queue where ignored by some terminal checks.