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 creating Relations from Vecs #10

Closed
wants to merge 1 commit into from

Conversation

lqd
Copy link
Member

@lqd lqd commented Jul 13, 2018

While this makes 🐸 rely on an unstable feature, it can help avoiding consuming Vecs and collecting them immediately afterwards.

For example, it should prevent 81k of such cases in Polonius' clap benchmark, for the datafrogopt variant. (It's not a huge time saving in this case, but still)

Thoughts ? (especially if we'd want to add a rust-toolchain file, or still support stable by having different functions for IntoInter and Vec)

@frankmcsherry
Copy link
Contributor

I think the code used to have a from_vec in it, if you want to avoid specialization (I would, because I've been bit by "nightly isn't stable" enough). It is less fabulous, but perhaps clearer?

@lqd
Copy link
Member Author

lqd commented Jul 13, 2018

I don't mind either way :) That's why I was interested in y'all's opinion, and will update this PR accordingly!

Sticking to stable is worth it, so I'll pubthe from_vec fn which is still here of course, and use that instead.

@lqd
Copy link
Member Author

lqd commented Jul 13, 2018

Although, if I do this now it'll break the treefrog PR, as the leapjoin creates non-empty Relations (and so will benefit from bypassing the Vec consumption) — so it might be interesting to wait a bit, say for #11 to possibly land, in order to update both this PR and then Polonius.

@nikomatsakis
Copy link
Contributor

I too think it would be wise to stick to stable if possible — since this lives outside the rust-lang repo, it will make coordinating breaking changes much easier.

@lqd
Copy link
Member Author

lqd commented Sep 27, 2018

agreed, I'll reopen this PR once #11 lands so that I can switch from specialization to using from_vec everywhere in one fell swoop in datafrog and then in Polonius.

@lqd lqd closed this Sep 27, 2018
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

3 participants