Skip to content

Allow early release of rvalues in assignments #8123

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

Closed
wants to merge 1 commit into from

Conversation

dotdash
Copy link
Contributor

@dotdash dotdash commented Jul 30, 2013

The rvalues in assignments might be temporaries (like function return
values) that could be freed immediately after the assignment is done. But
currently, they live to the end of the enclosing scope. For managed
box, this currently means that there is an extra ref count that keeps
the value alive. This leads to a huge memory usage peak in the
expansion phase of compilation with rustc, because all intermediate
expansion results are kept alive till the end of phase 2, instead of
being freed immediately after their last "usable" ref is dropped.

As a stop-gap solution, we can add a scope around the assignment during
translation, which means that the temporary value is dropped right away.
It's only a stop-gap solution because once we switch from refcounting to
garbage collection, it won't help anymore and the code should switch to
either RC or ~ for Crate data, but for now this reduces the peak memory
usage when compiling librustc by about 100MB on my x86_64 Linux box.

@graydon
Copy link
Contributor

graydon commented Jul 30, 2013

I stopped the build, apologies. This shouldn't have got r+, I don't think.

I believe this patch contradicts the direction @nikomatsakis was suggesting in #3511 though it's difficult to tell. That bug needs a real resolution in any case; randomly changing the lifetimes like this is going to change a lot of subtle semantics (destructor order, for example!)

@huonw
Copy link
Member

huonw commented Jul 30, 2013

My apologies.

(I was under the impression that this solely affects trans & LLVM, not the higher-level concept of Rust's lifetimes.)

@graydon
Copy link
Contributor

graydon commented Jul 30, 2013

I think the two are related. It might be that the higher level concept was not correctly being translated (and you were fixing that bug) or it could be that we were translating the right thing (and you were introducing a bug) or there could be some third option that we haven't encoded in either high level rules or translation rules yet. I just wanted to suggest holding any further changes to this area until the semantics are worked out and fully documented, so we can be sure we're doing the right thing.

The rvalues in assignments might be temporaries (like function return
values) that could be freed immediately after the assignment is done. But
currently, they live to the end of the enclosing scope. For managed
box, this currently means that there is an extra ref count that keeps
the value alive. This leads to a huge memory usage peak in the
expansion phase of compilation with rustc, because all intermediate
expansion results are kept alive till the end of phase 2, instead of
being freed immediately after their last "usable" ref is dropped.

As a stop-gap solution, we can add a scope around the assignment during
translation, which means that the temporary value is dropped right away.
It's only a stop-gap solution because once we switch from refcounting to
garbage collection, it won't help anymore and the code should switch to
either RC or ~ for Crate data, but for now this reduces the peak memory
usage when compiling librustc by about 100MB on my x86_64 Linux box.
@dotdash
Copy link
Contributor Author

dotdash commented Aug 4, 2013

Only pushed the new version to stop bors from picking this up, there was an accidental(?) retry on the old version. Nothing to see here

@thestinger
Copy link
Contributor

@dotdash: ah, whoops - I went through and restarted the pulls screwed up by the GitHub DoS by looking at whether they were interrupted

@catamorphism
Copy link
Contributor

Closing due to lack of activity. Reopen if needed.

@dotdash dotdash deleted the temporaries branch February 6, 2018 08:45
flip1995 pushed a commit to flip1995/rust that referenced this pull request Apr 7, 2022
… r=llogiq

Handle relative paths in module_files lints

The problem being that when clippy is run in the project's directory `lp` would be a relative path, this wasn't caught by the tests as there `lp` is an absolute path. Being a relative path it did not start with `trim_src_path` and so was ignored

Also allowed the removal of some `.to_os_string`/`.to_owned`s

changelog: Fixes [`self_named_module_files`] and [`mod_module_files`] not linting

Fixes rust-lang#8123, cc `@DevinR528`
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