Increase the default stack size to 16 MiB and remove ensure_sufficient_stack - #160535
Increase the default stack size to 16 MiB and remove ensure_sufficient_stack#160535ChrisDenton wants to merge 5 commits into
ensure_sufficient_stack#160535Conversation
|
Some changes occurred in match checking cc @Nadrieril Some changes occurred in match lowering cc @Nadrieril changes to the core type system cc @lcnr These commits modify the If this was unintentional then you should revert the changes before this PR is merged. The parser was modified, potentially altering the grammar of (stable) Rust cc @fmease Some changes occurred to the CTFE machinery Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt Some changes occurred in exhaustiveness checking cc @Nadrieril |
|
rustbot has assigned @petrochenkov. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
Oh that pinged everyone, sorry. It does touch a lot of files but most of the +/- diff is whitespace changes. |
ensure_sufficient_stack
This comment has been minimized.
This comment has been minimized.
6f4f307 to
39e551e
Compare
This comment has been minimized.
This comment has been minimized.
39e551e to
5b3365e
Compare
|
The list of allowed third-party dependencies may have been modified! You must ensure that any new dependencies have compatible licenses before merging. cc @davidtwco, @BoxyUwU |
This comment has been minimized.
This comment has been minimized.
4b5f459 to
8f4397d
Compare
There was a problem hiding this comment.
This test is fairly recent but I'm not sure that this it is still useful. It's basically testing the use of stacker, which is obsolete now.
There was a problem hiding this comment.
I suspect that this test might start failing again with increased scale of the test (e.g. if N=8000 fails at 3MiB, then at N=45000 it would fail at 16MiB.) But maybe making the scale here large enough makes the code itself impractical in other ways? 🤷
There was a problem hiding this comment.
One thing I'm finding with these high recursion cases is that they start becoming very slow even before they approach the (new) stack limit. So we could increase the stack size further if there's a real world use case (which I don't see atm but not ruling out) but I think that might be sweeping the problem under the rug. I do feel that at the very least we should be warning about it if not treating it as a bug to be fixed.
8f4397d to
a4e65d7
Compare
This comment has been minimized.
This comment has been minimized.
| "windows_i686_msvc", | ||
| "windows_x86_64_gnu", | ||
| "windows_x86_64_gnullvm", | ||
| "windows_x86_64_msvc", |
|
This PR changes a file inside |
|
@bors try jobs=i686-gnu-nopt-1 |
This comment has been minimized.
This comment has been minimized.
…=<try> Increase the default stack size to 16 MiB and remove `ensure_sufficient_stack` try-job: i686-gnu-nopt-1
|
@bors try jobs=various,i686,arm,apple,android |
This comment has been minimized.
This comment has been minimized.
…=<try> Increase the default stack size to 16 MiB and remove `ensure_sufficient_stack` try-job: *various* try-job: *i686* try-job: *arm* try-job: *apple* try-job: *android*
|
💔 Test for 271f298 failed: CI. Failed job:
|
This comment has been minimized.
This comment has been minimized.
|
@bors try jobs=various,i686*,arm,android |
This comment has been minimized.
This comment has been minimized.
…=<try> Increase the default stack size to 16 MiB and remove `ensure_sufficient_stack` try-job: *various* try-job: i686* try-job: *arm* try-job: *android*
This comment has been minimized.
This comment has been minimized.
On 32-bit systems, 255 threads will use up the available address space.
b11010b to
12d204f
Compare
|
@bors try jobs=various,i686*,arm,android |
This comment has been minimized.
This comment has been minimized.
…=<try> Increase the default stack size to 16 MiB and remove `ensure_sufficient_stack` try-job: *various* try-job: i686* try-job: *arm* try-job: *android*
|
Ok, I think I tested all the 32-bit runners so hopefully there are no more surprises. @bors r=petrochenkov,oli-obk |
This comment has been minimized.
This comment has been minimized.
|
The job Click to see the possible cause of the failure (guessed by this bot)Important For more information how to resolve CI failures of this job, visit this link. |
|
💔 Test for 9791cec failed: CI. Failed jobs:
|
View all comments
Tracking issue: #160275
Fixes #93237
Fixes #108499