iter-ops icon indicating copy to clipboard operation
iter-ops copied to clipboard

Benchmark Readme statement about RXJS subscription is incorrect.

Open caleb-vear opened this issue 1 year ago • 3 comments

The benchmark readme states:

This library performs about 2.5x faster than rxjs synchronous pipeline. However, just as you add a single subscription in rxjs ( which is inevitable with rxjs), then iter-ops performance is about 5x times better. So ultimately, this library can process synchronous iterables about 5x times faster than synchronous rxjs.

However upon examining the benchmark code I think this is a misunderstanding.

image

image

The firstValueFrom source can be found here: https://github.com/ReactiveX/rxjs/blob/7.8.1/src/internal/firstValueFrom.ts#L11-L75

So there is actually no need to do a separate subscription. If you want to simulate where there are multiple subscribers you would want to convert the sequence over to a shared observable using the share operator.

caleb-vear avatar Jun 27 '23 11:06 caleb-vear

Thanks for pointing this out. That original document was written and tests were done at the time when RxJS v6 was the latest. Some of them can be no longer valid, because numerous performance improvements were done in RxJS since then.

I did some re-testing a few months back, and it performed pretty much on par with the latest RxJS v7, which is very good. I will try to make time later to update the tests.

Still, this library has something unique to offer here - simpler syntax, plus some unique operators, to be considered as an alternative for handling iterables.

vitaly-t avatar Jul 07 '23 16:07 vitaly-t

I agree that iter-ops has something to offer over rxjs. We are using it on my current project because we have a lot of code that synchronously processes iterable data. We often want to do multiple steps, e.g. filter, window and map and it is great to not need to loop through multiple times or create unnecessary copies of everything for each step.

caleb-vear avatar Jul 09 '23 06:07 caleb-vear

Yep, and there's plenty more in iter-ops-extras ;)

vitaly-t avatar Jul 09 '23 14:07 vitaly-t

@caleb-vear Would you be interested to PR the necessary change for this?

I have updated the documentation + dependencies + test results, but not the test logic per your report. If you can make that change, then we can close it conclusively ;)

P.S. I am not convinced that the test is incorrect. The test was designed to simulate how clients add an extra subscription, by using subscribe, which is exactly what the test does, and not share in the pipeline.

vitaly-t avatar Aug 04 '24 16:08 vitaly-t

@caleb-vear If you still think that the above is incorrect, then please follow up, and perhaps PR the necessary change. But for the time being, after reviewing it, I see no issue in the current test results. We can re-open the issue later, if you follow up on this, but for the time being, I am closing it.

vitaly-t avatar Aug 04 '24 17:08 vitaly-t