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

Optimize take, takeRight, drop, dropRight #1910

Closed
danieldietrich opened this issue Mar 11, 2017 · 3 comments
Closed

Optimize take, takeRight, drop, dropRight #1910

danieldietrich opened this issue Mar 11, 2017 · 3 comments

Comments

@danieldietrich
Copy link
Contributor

danieldietrich commented Mar 11, 2017

Note: This should have been done already in #1800.

Catalyst:

List.drop(int) is usually O(n):

@Override
default List<T> drop(int n) {
    List<T> list = this;
    for (long i = n; i > 0 && !list.isEmpty(); i--) {
        list = list.tail();
    }
    return list;
}

For finite-sized collections like List we store the size, so we can do better:

@Override
default List<T> drop(int n) {
    if (n >= size()) {
        return empty();
    } else {
        List<T> list = this;
        for (long i = n; i > 0 && !list.isEmpty(); i--) {
            list = list.tail();
        }
        return list;
    }
}

In other words, when we drop all elements, we can take a shortcut in O(1).

Possible Optimizations

Op Opt (empty) Opt (no-op)
drop, dropRight n >= size() -> empty() n <= 0 -> this
take, takeRight n >= size() -> this n <= 0 -> empty()

Note: First check n <= 0 before n >= size() to omit method call if possible.

@danieldietrich danieldietrich added this to the 2.1.0 milestone Mar 11, 2017
danieldietrich added a commit that referenced this issue Mar 12, 2017
* Fixes memory leak of grouped iterator

* Improved by throwing away unnecessary elements and by cleaning buffer before adding new elements.

* Used Iterator instead of AbstractIterator

* List.take/drop optimizations. See also #1910

* Added Iterator.sliding() benchmark

* Based benchmark Iterators of element array

* Fixed a bug/uncovered case

* low-level optimization
@danieldietrich danieldietrich self-assigned this Mar 12, 2017
@danieldietrich danieldietrich modified the milestones: vavr-1.0.0, vavr-0.9.0 Apr 20, 2017
@danieldietrich danieldietrich modified the milestones: vavr-1.0.0, vavr-1.1.0 Nov 26, 2017
@NataliiaPrivezentseva
Copy link
Contributor

@danieldietrich hi! Why this issue is still open? Maybe we need to make sure that all methods take, takeRight, drop, dropRight were optimized?

@danieldietrich
Copy link
Contributor Author

@NataliiaPrivezentseva Hi! I don't know exactly. Maybe we only have to take a look at all implementations. I think then we can close this issue.

@danieldietrich
Copy link
Contributor Author

Will close this now. We will check benchmarks before 1.0 final.

@danieldietrich danieldietrich removed this from the next milestone Jan 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants