Skip to content

Optimize pretty printing stack usage #21127

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

Merged
merged 3 commits into from
Jan 16, 2015
Merged

Optimize pretty printing stack usage #21127

merged 3 commits into from
Jan 16, 2015

Conversation

erickt
Copy link
Contributor

@erickt erickt commented Jan 14, 2015

libsyntax compiled without optimization uses a lot of stack, which can cause it to run out of stack space. This PR factors out some arm handlers from print_expr as well as converts advance_left into a loop. This helps to cut down on the stack usage.

@rust-highfive
Copy link
Contributor

r? @pcwalton

(rust_highfive has picked a reviewer for you, use r? to override)

@erickt
Copy link
Contributor Author

erickt commented Jan 14, 2015

To go into some more detail about why I made this change, I was testing pretty printing a structure with 10 fields with associated types with #[derive(PartialEq)], and print_expr with no optimizations according to lldb was taking up 28KB of stack space. This caused my printer to run out of stack space. It seems that breaking up that function into multiple functions was sufficient to cut down on the stack usage sufficient for my code to run successfully.

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Jan 15, 2015
libsyntax compiled without optimization uses a lot of stack, which can cause it to run out of stack space. This PR factors out some arm handlers from `print_expr` as well as converts `advance_left` into a loop. This helps to cut down on the stack usage.
@bors bors merged commit e14d053 into rust-lang:master Jan 16, 2015
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