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

Free intermediate translation results as soon as possible #8092

Closed
wants to merge 1 commit into from

Conversation

dotdash
Copy link
Contributor

@dotdash dotdash commented Jul 28, 2013

This fixes the recently introduced peak memory usage regression by
freeing the intermediate results as soon as they're not required
anymore instead of keeping them around for the whole compilation
process.

Refs #8077

@alexcrichton
Copy link
Member

Could you add a comment above it to explain why it looks a bit silly? Otherwise r+!

This fixes the recently introduced peak memory usage regression by
freeing the intermediate results as soon as they're not required
anymore instead of keeping them around for the whole compilation
process.

Refs rust-lang#8077
bors added a commit that referenced this pull request Jul 28, 2013
This fixes the recently introduced peak memory usage regression by
freeing the intermediate results as soon as they're not required
anymore instead of keeping them around for the whole compilation
process.

Refs #8077
@thestinger
Copy link
Contributor

@dotdash: it might be clearer to use util::ignore as an early destructor call

@dotdash
Copy link
Contributor Author

dotdash commented Jul 28, 2013

Does that work with @-ptrs? Looks like it would just cause an inc + dec on the refcount.

@alexcrichton
Copy link
Member

Sadly I think it does refcount business with util::ignore:

$ cat foo.rs
struct A;
impl Drop for A {
    fn drop(&self) {
        println("drop");
    }
}

fn main() {
    let a = @A;
    std::util::ignore(a);
    println("here");
}
$ rust run foo.rs
warning: no debug symbols in executable (-arch x86_64)
here
drop

@thestinger
Copy link
Contributor

Ah, didn't realize these were managed pointers. Although, these should really be Rc rather than managed pointers which aren't going to be freed immediately when the garbage collector lands. The Rc type does have move semantics, rather than implicit copies, as any reference-counted type should.

@bors bors closed this Jul 28, 2013
@graydon
Copy link
Contributor

graydon commented Jul 29, 2013

Doh! I actually had it written this way but in a later commit in the wip branch I was working on this, because I was trying (and failing, due to too many @ boxes in the session) to put the earlier phases in a task and dump it (freeing the whole AST) before running LLVM passes. Thanks.

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.

7 participants