Remove Request Batching in Merge

[benjchristensen]

Removing the batching until we can find a correct way to do it as per discussion at #1941 (comment)

The performance impact of this change is seen here:

Benchmark                                          (size)   Mode   Samples          1.x    No Request Batching
r.o.OperatorMergePerf.merge1SyncStreamOfN               1  thrpt         5  4585554.607    4666745.314 102%
r.o.OperatorMergePerf.merge1SyncStreamOfN            1000  thrpt         5    51273.033      39922.246 78%
r.o.OperatorMergePerf.merge1SyncStreamOfN         1000000  thrpt         5       47.515         37.634 79%
r.o.OperatorMergePerf.mergeNAsyncStreamsOfN             1  thrpt         5    90901.735      93454.726 103%
r.o.OperatorMergePerf.mergeNAsyncStreamsOfN          1000  thrpt         5        5.407          4.910 91%
r.o.OperatorMergePerf.mergeNSyncStreamsOf1              1  thrpt         5  4181618.767    4173322.551 100%
r.o.OperatorMergePerf.mergeNSyncStreamsOf1            100  thrpt         5   422193.599     408972.130 97%
r.o.OperatorMergePerf.mergeNSyncStreamsOf1           1000  thrpt         5    36886.812      36448.978 99%
r.o.OperatorMergePerf.mergeNSyncStreamsOfN              1  thrpt         5  4815945.720    4887943.643 101%
r.o.OperatorMergePerf.mergeNSyncStreamsOfN           1000  thrpt         5       43.926         39.027 89%
r.o.OperatorMergePerf.mergeTwoAsyncStreamsOfN           1  thrpt         5    72578.046      70412.656 97%
r.o.OperatorMergePerf.mergeTwoAsyncStreamsOfN        1000  thrpt         5     3260.024       3064.403 94%
r.o.OperatorMergePerf.oneStreamOfNthatMergesIn1         1  thrpt         5  4678858.201    4808504.588 103%
r.o.OperatorMergePerf.oneStreamOfNthatMergesIn1      1000  thrpt         5    34407.547      36364.476 106%
r.o.OperatorMergePerf.oneStreamOfNthatMergesIn1   1000000  thrpt         5       31.312         32.261 103%

[benjchristensen]

Should I proceed with this and release 1.0.3 or does someone have a better recommendation of what to do as an immediate fix?

Major changes in how merge is implemented to try and improve performance should wait for 1.1 which we can do release candidates for.

[akarnokd]

The perf isn't that bad. The merge1SyncStreamOfN seems to be bad because emitting just 1 item needs a lot of orchestration on the iterable/range source. I'm not claiming I understand all aspects of merge optimizations done, but perhaps just these synchronous sources could override the request(1) sent to them and produce N elements, emitting them one-by-one from a cheap buffer.

[akarnokd]

Maybe this is unrelated, but this code gives me ints dominated results, meaning that merge prefers synchronous sources over asynchronous even though they theoretically can produce at the same rate. Using observeOn on both inputs seems to balance out things much nicer.

[benjchristensen]

merge prefers synchronous sources

merge becomes concat with synchronous sources. That is always how it will behave unless concurrency is added. Are you saying something different than that?

[benjchristensen]

The perf isn't that bad.

I agree it is not bad enough to not move forward. It it still good perf, just not as good as we could be if we figure this batching out.

I'm going to move forward with this change for 1.0.3 as the correctness is more important and the perf hit is not significant enough (in my opinion) to not get a fix out.