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

Simplify binary search using recursion. #21

Merged
merged 2 commits into from
Feb 4, 2018

Conversation

kareman
Copy link
Contributor

@kareman kareman commented Jan 15, 2018

Surprisingly, this is also faster in the performance tests.

@ole
Copy link
Owner

ole commented Jan 15, 2018

Thanks! Please give me some time to review this. I'm quite busy this week.

@ole
Copy link
Owner

ole commented Jan 31, 2018

Sorry I haven't gotten to this yet. Did you happen to test the performance with an array of objects? I came across this note on the Swift forums regarding recursion and performance:

The current sort is problematic because it uses recursion heavily, which defeats a number of optimizations relating to ARC and elimination of the overhead of passing in a closure for the comparative for the comparator.

It would be interesting to check if there are significant performance differences between recursion and non-recursion when ARC is involved.

@kareman
Copy link
Contributor Author

kareman commented Jan 31, 2018

Good point, I added some unit tests for objects, plus some tests with arrays without duplicate values, and a scheme for performance testing release builds. On my computer I get between 30 and 70% improvement in the performance tests with recursion, also when using objects. Could the compiler be optimising away the recursion?

Also, do you know if the sorted array is able to use generic specialisation with the class I created in the unit tests? Do we need to include SortedArray.swift in the unit test module to get more real-life performance testing?

@ole ole merged commit b1c8105 into ole:master Feb 4, 2018
@ole
Copy link
Owner

ole commented Feb 4, 2018

Thank you! To be honest, I'm still extremely puzzled why recursion is faster.

Also, do you know if the sorted array is able to use generic specialisation with the class I created in the unit tests? Do we need to include SortedArray.swift in the unit test module to get more real-life performance testing?

This is a good point. As far as I know, not even importing with @testable (which we don't do) would enable generics specialization for the SortedArray type within the test module. Whether including the file directly would be "more real-life" depends on your perspective. If clients add the file directly into their project, then yes, otherwise no.

Frankly, I don't care enough either way.

@ole
Copy link
Owner

ole commented Jun 18, 2018

I released version 0.7.0 that includes these changes.

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.

2 participants