vitaly-t / iter-ops

Basic operations on iterables
https://vitaly-t.github.io/iter-ops
MIT License
136 stars 5 forks source link

Benchmark Readme statement about RXJS subscription is incorrect. #223

Closed caleb-vear closed 2 months ago

caleb-vear commented 1 year ago

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.

vitaly-t commented 1 year ago

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.

caleb-vear commented 1 year ago

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.

vitaly-t commented 1 year ago

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

vitaly-t commented 2 months ago

@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 commented 2 months ago

@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.