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

Run cargo with a large stack #1908

Merged
merged 2 commits into from Aug 17, 2015

Conversation

Projects
None yet
5 participants
@alexcrichton
Copy link
Member

alexcrichton commented Aug 16, 2015

There have been a number of reports of Cargo triggering a stack overflow in the
algorithm implemented in cargo::core::resolve, and although many attempts have
been made to reduce the stack space of the two relevant recursive functions it
seems likely that this will not always be enough. For now, before moving the
recursion to the heap manually, spawn the main thread with a large stack (e.g.
mirror what the compiler does) to ensure that the same scenarios happen across
platforms at least.

Currently on my machine I get a 2MB stack on Linux and a 512K stack on OSX, so
bumping this up to 8MB should be more than enough for the recursion in this
algorithm. I also hope that with nonzeroing drop a few of the recursive calls
will be able to become tail recursive, which should also help with stack space!

Closes #1897

alexcrichton added some commits Aug 15, 2015

Run cargo with a large stack
There have been a number of reports of Cargo triggering a stack overflow in the
algorithm implemented in `cargo::core::resolve`, and although many attempts have
been made to reduce the stack space of the two relevant recursive functions it
seems likely that this will not always be enough. For now, before moving the
recursion to the heap manually, spawn the main thread with a large stack (e.g.
mirror what the compiler does) to ensure that the same scenarios happen across
platforms at least.

Currently on my machine I get a 2MB stack on Linux and a 512K stack on OSX, so
bumping this up to 8MB should be more than enough for the recursion in this
algorithm. I also hope that with nonzeroing drop a few of the recursive calls
will be able to become tail recursive, which should also help with stack space!

Closes #1897
@rust-highfive

This comment has been minimized.

Copy link

rust-highfive commented Aug 16, 2015

r? @huonw

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

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Aug 16, 2015

I built Cargo on a mac with this PR and managed to build Servo with it. Thanks Alex!

@huonw

This comment has been minimized.

Copy link
Member

huonw commented Aug 17, 2015

Travis & appveyor both failed? r=me if they're spurious.

@alexcrichton

This comment has been minimized.

Copy link
Member Author

alexcrichton commented Aug 17, 2015

@bors: r=huonw

Yeah those were fixed in the "update deps" PR

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Aug 17, 2015

📌 Commit 0139707 has been approved by huonw

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Aug 17, 2015

⌛️ Testing commit 0139707 with merge d48f89d...

bors added a commit that referenced this pull request Aug 17, 2015

Auto merge of #1908 - alexcrichton:moar-stack, r=huonw
There have been a number of reports of Cargo triggering a stack overflow in the
algorithm implemented in `cargo::core::resolve`, and although many attempts have
been made to reduce the stack space of the two relevant recursive functions it
seems likely that this will not always be enough. For now, before moving the
recursion to the heap manually, spawn the main thread with a large stack (e.g.
mirror what the compiler does) to ensure that the same scenarios happen across
platforms at least.

Currently on my machine I get a 2MB stack on Linux and a 512K stack on OSX, so
bumping this up to 8MB should be more than enough for the recursion in this
algorithm. I also hope that with nonzeroing drop a few of the recursive calls
will be able to become tail recursive, which should also help with stack space!

Closes #1897
@bors

This comment has been minimized.

@bors bors merged commit 0139707 into rust-lang:master Aug 17, 2015

1 of 3 checks passed

continuous-integration/appveyor AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
homu Test successful
Details

@alexcrichton alexcrichton deleted the alexcrichton:moar-stack branch Aug 17, 2015

SimonSapin added a commit to servo/servo that referenced this pull request Aug 20, 2015

josiahdaniels added a commit to josiahdaniels/servo that referenced this pull request Sep 28, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.