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

Stop using @str in the compiler #10516

Closed
emberian opened this issue Nov 16, 2013 · 12 comments
Closed

Stop using @str in the compiler #10516

emberian opened this issue Nov 16, 2013 · 12 comments
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. I-slow Issue: Problems and improvements with respect to performance of generated code.

Comments

@emberian
Copy link
Member

We almost never want it, and when we do, we're going to want to replace it with a custom interned string type rather than some GC type. We do a lot of pointless copies right now because we get ~str from almost everything and we constantly do str.to_managed().

For now, convert to &str where we can, but replace every use of str.to_managed() with ~str and mark with a comment like /* should be interned */.

@kaseyc
Copy link
Contributor

kaseyc commented Nov 16, 2013

Can I take this?

@emberian
Copy link
Member Author

Go for it! Don't sweat too hard trying to remove all of them; the compiler can get real messy.

@kaseyc
Copy link
Contributor

kaseyc commented Nov 17, 2013

Quick question: For functions that are currently returning @str using str.to_managed, should they return&str or ~str?

@emberian
Copy link
Member Author

They should return ~str. In general you can't return &str since it
needs to point at something.

On Sat, Nov 16, 2013 at 7:16 PM, Kasey Carrothers
[email protected]:

Quick question: For functions that are currently returning @str using
str.to_managed, should they return&str or ~str?


Reply to this email directly or view it on GitHubhttps://github.com//issues/10516#issuecomment-28639366
.

@dguenther
Copy link
Contributor

Is this still in progress? I've been making a decent foray into this area, but I don't want to steal someone else's ticket.

@kaseyc
Copy link
Contributor

kaseyc commented Jan 13, 2014

Go for it. I've been struggling with it, and haven't had as much time as I hoped to work on it.

@pnkfelix
Copy link
Member

@dguenther I think pcwalton was also working a bit on this (or something related to it) last week, so you may want to ping him (perhaps on IRC).

@emberian
Copy link
Member Author

I have an in-progress patch somewhere, can't find the patch atm.

On Mon, Jan 13, 2014 at 11:45 AM, Felix S Klock II <[email protected]

wrote:

@dguenther https://github.com/dguenther I think pcwalton was also
working a bit on this (or something related to it) last week, so you may
want to ping him (perhaps on IRC).


Reply to this email directly or view it on GitHubhttps://github.com//issues/10516#issuecomment-32186778
.

@dguenther
Copy link
Contributor

Cool, I'll chat up pcwalton and see what progress he's made. I've got a working build with a significant portion of ~str usages removed (tests still need to be updated), but still more to go, and I'll definitely need some CR help to clean up and check my code. @cmr If you happen to run into that patch, let me know. It'd probably be a more solid base than what I've got so far.

edit: pcwalton is currently working on this, so I'm going to hunt down another ticket to work on.

@emberian
Copy link
Member Author

Found it: https://github.com/cmr/rust/tree/issue-2243

On Mon, Jan 13, 2014 at 1:01 PM, Derek Guenther [email protected]:

Cool, I'll chat up pcwalton and see what progress he's made. I've got a
working build with a significant portion of ~str usages removed (tests
still need to be updated), but still more to go, and I'll definitely need
some CR help to clean up and check my code. @cmr https://github.com/cmrIf you happen to run into that patch, let me know. It'd probably be a more
solid base than what I've got so far.


Reply to this email directly or view it on GitHubhttps://github.com//issues/10516#issuecomment-32193904
.

@fhahn
Copy link
Contributor

fhahn commented Feb 5, 2014

This issue seems kind of fixed, greping for "@str" in src/librustc does not give any results and in src/librustc @str is only used in a comment (I think this one line should be changed) and in two functions that are commented out.

I could create a tiny pull request, changing the comment if this would help to close this issue

@huonw
Copy link
Member

huonw commented Feb 5, 2014

@str is now completely removed from the language, not just the compiler. Closing.

(Feel free to submit that PR though.)

@huonw huonw closed this as completed Feb 5, 2014
bors added a commit that referenced this issue Feb 6, 2014
This tiny pull request updates a comment referring to `@str` which was replaced by `(InternedString,StrStyle)` .

related to #10516
flip1995 pushed a commit to flip1995/rust that referenced this issue Apr 6, 2023
Use `split-debuginfo = "unpacked"` for debug builds

On Windows this has no effect as it's unsupported. On macOS the default set by cargo is already unpacked so no effect there either

For Linux it shaves a bit off the rebuild time, for me in the case of a simple `touch` + `cargo build` it goes from 12s to 10s

It saves a good amount of disk space too, on `aarch64-unknown-linux-gnu` it saves 1.2GB for a plain `cargo build`, 3GB when also running `cargo dev` and `cargo test --no-run -F internal`

r? `@flip1995`

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. I-slow Issue: Problems and improvements with respect to performance of generated code.
Projects
None yet
Development

No branches or pull requests

6 participants