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

Remove usage of @ast::Crate in favor of ownership #9598

Closed
wants to merge 2 commits into from

Conversation

alexcrichton
Copy link
Member

This patch exposes actual ownership of an ast::Crate structure so it's not implicitly copied and reference counted via @.

The main purpose for this patch was to get rid of the massive spike in memory during the start of the compiler (this can be seen on isrustfastyet). The reason that this spike exists is that during phase_2 we're creating many copies of the crate by folding. Because these are reference counted, all instances of the old crates aren't dropped until the end of the function, which is why so much memory is accumulated.

This patch exposes true ownership of the crate, meaning that it will be destroyed ASAP when requested. There are no code changes except for dealing with actual ownership of the crate. The large spike is then avoided: http://i.imgur.com/IO3NENy.png

This shouldn't help our overall memory usage (that still is the highest at the end), but if we ever manage to bring that down it should help us not have a 1GB spike at the beginning of compilation.

This help enable some later refactorings.
@@ -4986,40 +4982,6 @@ impl Resolver {
}
}

pub fn name_exists_in_scope_struct(&mut self, name: &str) -> bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take it this was redundant/over-complicated and could be folded into the code below?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That, and that I didn't see any actual use cases for this because it only checked structs defined at the top level of a crate. I believe that we have code that already does this in a more robust fashion.

bors added a commit that referenced this pull request Sep 29, 2013
This patch exposes actual ownership of an `ast::Crate` structure so it's not implicitly copied and reference counted via `@`.

The main purpose for this patch was to get rid of the massive spike in memory during the start of the compiler (this can be seen on isrustfastyet). The reason that this spike exists is that during `phase_2` we're creating many copies of the crate by folding. Because these are reference counted, all instances of the old crates aren't dropped until the end of the function, which is why so much memory is accumulated.

This patch exposes true ownership of the crate, meaning that it will be destroyed ASAP when requested. There are no code changes except for dealing with actual ownership of the crate. The large spike is then avoided: http://i.imgur.com/IO3NENy.png

This shouldn't help our overall memory usage (that still is the highest at the end), but if we ever manage to bring that down it should help us not have a 1GB spike at the beginning of compilation.
@alexcrichton
Copy link
Member Author

Closing to un-stuck bors, will re-open.

flip1995 pushed a commit to flip1995/rust that referenced this pull request Oct 6, 2022
lint: fix a few comments

minor cleanup per `@Alexendoo` [comment](rust-lang/rust-clippy#9586 (comment))

changelog: none
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.

3 participants