Skip to content

Implement advance_by, advance_back_by for slice::{Iter, IterMut}#87387

Merged
bors merged 3 commits intorust-lang:masterfrom
the8472:slice-iter-advance_by
Jul 31, 2021
Merged

Implement advance_by, advance_back_by for slice::{Iter, IterMut}#87387
bors merged 3 commits intorust-lang:masterfrom
the8472:slice-iter-advance_by

Conversation

@the8472
Copy link
Member

@the8472 the8472 commented Jul 22, 2021 •
edited
Loading

Part of #77404.

Picking up where #77633 was closed.

I have addressed #77633 (comment) by restoring nth and nth_back. So according to that comment this should already be r=m-ou-se, but it has been sitting for a while.

timvermeulen and others added 3 commits July 22, 2021 22:05
@timvermeulen @the8472
Implement slice::{Iter, IterMut}::{advance_by, advance_back_by}
2b89914 
Co-authored-by: The8472 <git@infinite-source.de>
@timvermeulen @the8472
Add tests
5e90909
@timvermeulen @pickfire @the8472
Fix whitespace
5485f8a 
Co-authored-by: Ivan Tham <pickfire@riseup.net>
@rust-highfive
Copy link
Contributor

rust-highfive commented Jul 22, 2021

r? @scottmcm

(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 Jul 22, 2021
@the8472 the8472 added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jul 25, 2021
@scottmcm
Copy link
Member

scottmcm commented Jul 31, 2021

Looks good; thanks!

@bors r+ rollup=iffy

@bors
Copy link
Collaborator

bors commented Jul 31, 2021

📌 Commit 5485f8a has been approved by scottmcm

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 31, 2021
@bors
Copy link
Collaborator

bors commented Jul 31, 2021

⌛ Testing commit 5485f8a with merge 6b0b07d...

@bors
Copy link
Collaborator

bors commented Jul 31, 2021

☀️ Test successful - checks-actions
Approved by: scottmcm
Pushing 6b0b07d to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 31, 2021
@bors bors merged commit 6b0b07d into rust-lang:master Jul 31, 2021
@rustbot rustbot added this to the 1.56.0 milestone Jul 31, 2021
@Mark-Simulacrum Mark-Simulacrum added the perf-regression Performance regression. label Aug 3, 2021
@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented Aug 3, 2021

Seems to have been a consistent regression on the regex-opt benchmark in particular, amongst a few others.

Marking as perf-regression. Seems likely that we just accept this regression, though.

@the8472
Copy link
Member Author

the8472 commented Aug 3, 2021 •
edited
Loading

@Mark-Simulacrum it looks like I forgot an #[inline] there, that might perturb things a bit if anything in regex uses iter.nth() somewhere which delegates to advance_by.

I can do a perf run in another PR to see if it makes any difference.

@the8472 the8472 mentioned this pull request Aug 3, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 4, 2021
@bors
Auto merge of rust-lang#87736 - the8472:inline-advance-by, r=Mark-Sim…
6fe0886 
…ulacrum

#[inline] slice::Iter::advance_by

rust-lang#87387 (comment) was marked as a regression. One of the methods in the PR was missing an inline annotation unlike all the other methods on slice iterators.

Let's see if that makes a difference.
@pnkfelix
Copy link
Contributor

pnkfelix commented Aug 11, 2021

Perf-regression was addressed by PR #87736, so removing perf-regression label.

@pnkfelix pnkfelix added perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. and removed perf-regression Performance regression. labels Aug 11, 2021
@the8472 the8472 mentioned this pull request Oct 15, 2021
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@scottmcm scottmcm

Labels

merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.56.0

Development

Successfully merging this pull request may close these issues.

8 participants

@the8472 @rust-highfive @scottmcm @bors @Mark-Simulacrum @pnkfelix @rustbot @timvermeulen