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

Allow compareLength to shortcircuit in both directions #76

Merged
merged 5 commits into from
Apr 28, 2014

Conversation

ekmett
Copy link
Member

@ekmett ekmett commented Apr 4, 2014

This is a more invasive alternative to the patch I offered up in #75.

This permits early termination in many inexact cases (like, you know, stream!) by tracking both minimum as well as maximum sizes.

This means that examples like

>>> compareLength "hello" 6
>>> compareLength "hello" 2

can both execute in O(1), spite the fact that stream has to give an inexact bound on its size. This is because it can give both an upper bound (as usual) and lower bound (words / 2) for use in the test.

In theory more combinators could be extended to make use of the lower bounds.

I tried to exhibit care to preserve sharing like the original code, but this version could definitely use a pair of eyeballs.

If it is too much trouble then by all means just take the other version. =)

@bos
Copy link
Contributor

bos commented Apr 28, 2014

This seems to have a significant negative impact on at least some fusion-heavy functions.

                         kmett            HEAD
Pure/toUpper/Text+tiny   449.0195   ns    357.8654   ns
Pure/toUpper/Text+ascii    4.573443 ms      4.435177 ms

Other numbers are variable in favour of your changes, so I'm not entirely sure what's going on, and am a bit inclined to just merge this.

@bos bos merged commit 0fc03a9 into haskell:master Apr 28, 2014
@ekmett
Copy link
Member Author

ekmett commented Apr 28, 2014

Thanks! Between these changes I should be able to viably switch trifecta over to text shortly.

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.

None yet

2 participants