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

Factoring bigint and rational out of libextra. #12141

Conversation

pnkfelix
Copy link
Member

@pnkfelix pnkfelix commented Feb 9, 2014

(Left libextra/num/complex in place; real goal is to enable quick
iteration on bigint improvements.)

@pnkfelix
Copy link
Member Author

pnkfelix commented Feb 9, 2014

To see what sort of bigint improvements I am talking about, see e.g. pnkfelix/rust@d669822

@alexcrichton
Copy link
Member

Would it be possible to remove feature(globs)?

@pnkfelix
Copy link
Member Author

pnkfelix commented Feb 9, 2014

@alexcrichton I'm willing to do it, but can we make that a follow-on patch, and land this first?

@pnkfelix
Copy link
Member Author

pnkfelix commented Feb 9, 2014

oh wait, this can't land as is; the current tutorial uses rational and I hadn't updated that yet.

Closing temporarily, will reopen after I get make check working.

@pnkfelix pnkfelix closed this Feb 9, 2014
@huonw
Copy link
Member

huonw commented Feb 9, 2014

Can extra::complex go out too? (Maybe these can all go into a libnum?)

@thestinger
Copy link
Contributor

I think complex and rational should be provided in separate libraries, but the big integer implementation is just going to make Rust look bad...

http://www.reddit.com/r/rust/comments/1xcq1q/c_gcc_vs_rust_wtf/

If someone needs big integers, they're going to use the implementation that's provided by default and have a bad experience with the language.

@pnkfelix
Copy link
Member Author

pnkfelix commented Feb 9, 2014

@thestinger ... It is precisely that reddit post that led me to try to pull this out.

From my point-of-view, making the bigint a separate crate should make it easier for an end-developer to swap-in a GMP based implementation.

More importantly, the separate crate makes it far far easier to quickly iterate through performance improvements, like the one I linked above on pidigits. (And I'm sure there's a lot of more low-hanging fruit here -- but its only low-hanging if we can iterate quickly.)

@thestinger
Copy link
Contributor

If it's provided in the official repository and built by default, then it's not different. It's what people are going to use by default and makes the language look like rubbish. It may or may not make the builds faster, but it doesn't make it any less official or harder to use. If anything, it makes it more discoverable because there's a whole crate for it rather than it being tucked away in extra. I doubt it will have competitive performance for years, so why present it as the best Rust can do by having it part of the official Rust distribution?

It's easier to iterate on it in an independent repository no tied at all to Rust's build system, and not requiring a whole clone of the Rust repository to avoid unnecessary rebuilds when switching between branches. It's currently not a very a useful library...

@pnkfelix
Copy link
Member Author

pnkfelix commented Feb 9, 2014

@thestinger I'm not trying to make the builds of the standard library faster. I'm trying to make it easier for everyone to collectively improve the bigint library.

As for the question of how easy we can/should make it to swap in the GMP library (the limit of this line-of-thinking being that GMP becomes the default, and one would have to opt-out of using it), that seems totally orthogonal to the change suggested by this ticket. Am I missing something?

(update: this comment was in response to an earlier draft of thestinger's previous comment that made no mention of separate repositories. Github based dialogues are such fun...)

@thestinger
Copy link
Contributor

It's far easier to iterate on the big integer library in a separate repository without the need to go through the serialized bors queue and deal with unnecessary rebuilds whenever something like LLVM is upgraded.

It's even more prominent than it is now if it's included in the official repository in a more focused crate. If we don't want people to infer their opinion of Rust from this library, then it shouldn't be part of the official Rust distribution before the embarrassing performance issues are fixed. No one is going to lose out by this being out of the official repository because it's not yet a useful library.

@pnkfelix
Copy link
Member Author

pnkfelix commented Feb 9, 2014

@thestinger okay, now I understand what you are suggesting. Thank you for stating that clearly.

@pnkfelix
Copy link
Member Author

pnkfelix commented Feb 9, 2014

@huonw I considered moving all three to a single libnum crate, but I didn't have much reason to go one way or the other, and decided that a libbigint crate of minimal size would be best for initial performance hacking.

@pnkfelix pnkfelix reopened this Feb 10, 2014
@pnkfelix
Copy link
Member Author

okay this passes make check now.

The suggestion from thestinger to remove bigint entirely is certainly an option, but for now I'd like to see how far we might get without having to go that far.

Fix tutorial.md to reflect `librational` factoring out of `libextra`.

Removed use of globs present in earlier `bigint` and `rational` modules.

(Left libextra/num/complex in place; real goal is to enable quick
iteration on bigint improvements.)
@liigo
Copy link
Contributor

liigo commented Feb 10, 2014

+1 for libnum, don't want to see too many crates. And bigint::BigInt VS.
num::BigInt, the later looks well.
2014年2月10日 上午7:30于 "Felix S Klock II" notifications@github.com写道:

@huonw https://github.com/huonw I considered moving all three to a
single libnum crate, but I didn't have much reason to go one way or the
other, and decided that a libbigint crate of minimal size would be best
for initial performance hacking.


Reply to this email directly or view it on GitHubhttps://github.com//pull/12141#issuecomment-34591257
.

@pnkfelix
Copy link
Member Author

Okay I will close this and open a fresh PR that puts bigint and rational (and complex) into a single libnum crate.

@pnkfelix pnkfelix closed this Feb 10, 2014
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jan 25, 2024
from_over_into: suggest a correct conversion to ()

changelog: [`from_over_into`]: suggest a correct conversion to `()`

Fix rust-lang#12138
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.

5 participants