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

Specialize Intersperse::fold #348

Merged
merged 1 commit into from
Jul 10, 2019

Conversation

jswrenn
Copy link
Member

@jswrenn jswrenn commented Jun 10, 2019

Overriding Intersperse::fold's default implementation makes iteration with for_each approximately 10× faster than external iteration, and approximately 20× faster than for_each with an un-specialized fold:

$ cargo bench -- specialization
specialization::intersperse::internal_specialized          64 ns/iter (+/- 1)
specialization::intersperse::external                     638 ns/iter (+/- 45)
specialization::intersperse::internal_unspecialized     1,274 ns/iter (+/- 131)

@jswrenn
Copy link
Member Author

jswrenn commented Jun 12, 2019

@bluss I'd be interested in helping maintain itertools if you're still looking for someone. I'm a member of the rustsim organization, a regular contributor to nalgebra, and a drive-by contributor of API improvements to many libraries.

@bluss bluss self-requested a review July 10, 2019 18:16
@bluss
Copy link
Member

bluss commented Jul 10, 2019

bors: r+

Thanks!

bors bot added a commit that referenced this pull request Jul 10, 2019
348: Specialize Intersperse::fold r=bluss a=jswrenn

Overriding `Intersperse::fold`'s default implementation makes iteration with `for_each` approximately 10× faster than external iteration, and approximately 20× faster than `for_each` with an un-specialized `fold`:
```
$ cargo bench -- specialization
specialization::intersperse::internal_specialized          64 ns/iter (+/- 1)
specialization::intersperse::external                     638 ns/iter (+/- 45)
specialization::intersperse::internal_unspecialized     1,274 ns/iter (+/- 131)
```

Co-authored-by: Jack Wrenn <[email protected]>
@bluss
Copy link
Member

bluss commented Jul 10, 2019

Yes, maintenance is definitely needed here, it would be great to have you if you are willing to take ownership!

@bors
Copy link
Contributor

bors bot commented Jul 10, 2019

Build succeeded

@bors bors bot merged commit 91c421c into rust-itertools:master Jul 10, 2019
@jswrenn
Copy link
Member Author

jswrenn commented Jul 11, 2019

Yes, maintenance is definitely needed here, it would be great to have you if you are willing to take ownership!

I am!

@bluss
Copy link
Member

bluss commented Jul 13, 2019

Great, you're added as a contributor. crates.io invite sent. Nothing more is technically needed, but we could move this repo to an organization?

Thank you! I was supposed to give some maintenance advice too. Remember that a maintainer needs to make decisions and should make decisions, so don't feel bad for doing exactly that.

@jswrenn
Copy link
Member Author

jswrenn commented Jul 14, 2019

Thanks, @bluss! Life is unusually busy for me at the moment (all good things!), but I think I can generally give itertools at least a few hours each week, and I'll aim for (at the very least) monthly minor releases.

I'm thrilled to have the opportunity to give back to this library. I've used itertools in nearly every data analysis pipeline I've built so far in my PhD.

I've sent you an invitation to the rust-itertools organization. At your convenience, could you transfer this repository to that organization? To avoid breaking any cargo git dependencies, it's probably a good idea if you then create a fork of itertools under your account.

@bluss
Copy link
Member

bluss commented Jul 14, 2019

Interesting conundrum since afaik if I create a fork, it breaks a lot of the redirects that github sets up. Will transfer after having done some research on that

@jswrenn
Copy link
Member Author

jswrenn commented Jul 14, 2019

Ah, I wasn't aware github creates redirects. If so, no need to create a fork!

@bluss
Copy link
Member

bluss commented Jul 15, 2019

New home done. 🎉 I've renamed the repo to just itertools since I think that's what we would use inside this organisation. All the automatic redirects should mean it all works.

The old github pages site was just full of redirects to docs.rs. I removed github pages in the new location, because right now it dosen't seem useful to establish a new url for a lot of redirects. The redirects are kept at http://bluss.github.io/rust-itertools/ (by moving the github pages content from blus/rust-itertools to blus/bluss.github.io.

@bluss
Copy link
Member

bluss commented Jul 16, 2019

Also, thrilled that you are coming on! If you feel like it, we can see if others want to join the team. I haven't had much time for Rust, and uncertain about the future. Near future is vacation and not Rust at least :)

@jswrenn
Copy link
Member Author

jswrenn commented Jul 16, 2019

I'll be able to start giving itertools attention this thursday, and also next week. :)

Enjoy your vacation!

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