Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ReactiveSwift tests #3

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add ReactiveSwift tests #3

wants to merge 4 commits into from

Conversation

olejnjak
Copy link

@olejnjak olejnjak commented Aug 8, 2019

Hi,

as I usually use ReactiveSwift I thought it would be great to add it to the comparison as well.

@grafele
Copy link
Collaborator

grafele commented Aug 8, 2019

Wow, amazing! I will take a look and also update the statistics before I merge it. Will then also include the numbers file, so that everyone can update the graphics as well.

@grafele grafele self-assigned this Aug 8, 2019
@grafele
Copy link
Collaborator

grafele commented Aug 8, 2019

I've added one comment and ran the tests locally. The crazy thing is, that the tests for ReactiveSwift are much much slower. Do you know any reason for this?

Bildschirmfoto 2019-08-08 um 17 31 56

@olejnjak
Copy link
Author

olejnjak commented Aug 8, 2019

Not sure, I was as surprised as you are. I've opened a discussion on ReactiveSwift Slack if it is really that slow or the tests don't do what thay should (but I honestly doubt that)

@grafele
Copy link
Collaborator

grafele commented Aug 8, 2019

Hmm ok. I would be ready to merge it, but I would prefer waiting for a response to your question in the slack channel. Maybe you can also ask in an issue on github as well.

@olejnjak
Copy link
Author

olejnjak commented Aug 9, 2019

Well the only answer I got so far is link to Twitter thread you are part of 🤷‍♂️ I’ll file an issue

@andersio
Copy link

andersio commented Sep 8, 2019

Judging by the benchmark, I see a logical pattern here:

  1. The MapFilter test cases involve only SignalProducer and two single-stream transform operators, which have been heavily optimised to reduce overhead (e.g. eliminate unnecessary locks ) whenever possible.

  2. The rest of the test cases all either use the RAS thread-safe, multicast Signal primitive, or use operators that are backed by it (basically all flattening operators).

With (2), I can see how it can snowball into a significant deficit. Nonetheless, it would still be an interesting performance case to look into.

andersio added a commit to ReactiveCocoa/ReactiveSwift that referenced this pull request Sep 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants