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 Vec's IntoIter's methods #57246

Closed
wants to merge 1 commit into from

Conversation

czipperz
Copy link
Contributor

@czipperz czipperz commented Jan 1, 2019

In particular nth and last. To do: implement nth_back.

@rust-highfive
Copy link
Collaborator

r? @bluss

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 1, 2019
@rust-highfive

This comment has been minimized.

@rust-highfive

This comment has been minimized.

@czipperz czipperz force-pushed the optimize_vec_iterator branch 3 times, most recently from 067e9ba to 59755d6 Compare January 1, 2019 08:24
src/liballoc/vec.rs Outdated Show resolved Hide resolved
@varkor
Copy link
Member

varkor commented Jan 1, 2019

Have you benchmarked these changes?

@czipperz
Copy link
Contributor Author

czipperz commented Jan 2, 2019

Related to #57245 (comment), I think this change would cause side effects in destructors to trigger in different orders and thus not be backwards compatible.

@czipperz czipperz closed this Jan 2, 2019
@czipperz
Copy link
Contributor Author

czipperz commented Jan 2, 2019

To directly address your comment @varkor, these changes would take O(n) operations and reduce them to O(1) operations. But because IntoIter owns the element it loops over, it must do O(n) destructions in any case and by changing these methods in the given way, the elements will be dropped in a different order.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants