-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Abort by default v2 #1765
Abort by default v2 #1765
Changes from all commits
b7359bd
6126805
b7653fa
624150b
162582d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
- Feature Name: abort_by_default | ||
- Start Date: 2016-10-01 | ||
- RFC PR: (leave this empty) | ||
- Rust Issue: (leave this empty) | ||
|
||
# Summary | ||
[summary]: #summary | ||
|
||
Specify abort-by-default in `Cargo.toml` when the user does `cargo new --bin`, as well as various other refinements to the panick strategy system. | ||
|
||
# Motivation | ||
[motivation]: #motivation | ||
|
||
## Performance | ||
|
||
Generally, the performance of bigger programs [improves by 10%](https://www.youtube.com/watch?v=mRGb4hoGuPs), which is no small amount. When overflow checking is enabled, it is as high as 2x, making overflowing checking plausible in production. | ||
|
||
The performance gains mainly originates in the fact that it is a lot easier for the compiler to optimize, due to the unwind path disappearing, and hence reasoning about the code becomes easier. | ||
|
||
## Binary size | ||
|
||
Binary size improves by 10% as well. This is due to the lack of landing pads which are injected in the stack when unwinding is enabled. | ||
|
||
## Compile time | ||
|
||
Compile time in debug mode improves by 20% on average. In release mode, the number is around 10%. | ||
|
||
## Runtime size | ||
|
||
Unwinding requires a fair amount of runtime code, which can be seen as conflicting with the goals of Rust. | ||
|
||
## Correctness | ||
|
||
You often see people abusing `std::panic::catch_unwind` for exception handling. Forcing the user to explicitly opt in to unwinding will make it harder to unintentionally misuse it. | ||
|
||
# Detailed design | ||
[design]: #detailed-design | ||
|
||
First of all, we add a new possible value to the `panic` field. We call this `any`. It specifies that the library or binary is compatible with any panicking strategy. `any` will default to the `abort` strategy, if compatible with all of its dependencies. If not, it will fall back to `unwind`, and leave a warning. | ||
|
||
When `cargo new` is invoked, it will generate `panic=any` in the `Cargo.toml`, both for new libraries and new binaries. | ||
|
||
Whenever the user inteacts with `cargo` (by using a command) in an existing (binary) crate and the `Cargo.toml` has no specified panicking strategy, it will add `panic=unwind` and leave a note to the user: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typo: s/inteacts/interacts/ |
||
|
||
Warning: No panic strategy was specified, added unwinding to the | ||
`Cargo.toml` file, please consider change it to your needs. If | ||
your crate's behavior does not depend on unwinding, please add | ||
`panic=any` instead. | ||
|
||
This will not happen to libraries, since they must only rely on unwind, if the specify it. Instead, it will add `panic=any` to libraries and give warning: | ||
|
||
Warning: No panic strategy was specified, so we default to aborting. If | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isn't this message misleading? If I understand correctly, its not that we default to aborting (at least from the POV of the library) -- its rather that we delay the decision to the client binary crate (and then there we would default to aborting). Perhaps you just mean "No panic strategy was specified, which means you may require support for aborting"?
|
||
your crate depends on unwinding, please put `panic=unwind` in | ||
`Cargo.toml`. | ||
|
||
## Libraries | ||
|
||
For libraries, the `Cargo.toml` is not modified, as they inherit the strategy of the binary. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This conflicts with the above, which says a default of |
||
|
||
### Relying on unwinding | ||
|
||
If a library specifies `panic=unwind`, it will stored in a rlib metadata field, `unwind_needed`. If this field does not match with the crate which is linking against the library (`abort`), `rustc` will produce an error. | ||
|
||
This is done in order to make sure that applications can rely on unwinding without leading to unsafety when being linked against by an aborting runtime. | ||
|
||
## Extensions | ||
|
||
After several release cycles, an extension could be added, which makes specifying the strategy mandatory. | ||
|
||
# Drawbacks | ||
[drawbacks]: #drawbacks | ||
|
||
It makes panics global by default (i.e., panicking in some thread will abort the whole program). | ||
|
||
It might make it tempting to ignore the existence of an unwind option and consequently shaping the code after panicking being aborting. | ||
|
||
## Unwinding is not bad per se | ||
|
||
Unwinding has numerous advantages. Especially for certain classes of applications. These includes better error handling and better cleanup. | ||
|
||
Unwinding is especially important to long-running applications. | ||
|
||
# Alternatives | ||
[alternatives]: #alternatives | ||
|
||
Keep unwinding as default. | ||
|
||
Make Cargo set `panic=abort` in all new binaries. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is no longer an alternative but the main proposal. |
||
|
||
Use unwind in `debug` and abort in `release`. | ||
|
||
Make use of more exotic instructions like `bound`, allowing e.g. overflow or bound checking without branches. This comes at the expense of error output. | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I remember seeing stack maps discussed as an alternative, can someone speak to that? I don't know much about whether those are beneficial here/related to this.... |
||
Use crate attributes instead. | ||
|
||
# Unresolved questions | ||
[unresolved]: #unresolved-questions | ||
|
||
Is there any way we can detect crates which do not rely on unwinding? Search for `catch_unwind`? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might not be true. I watched the video, and I believe that Alex Crichton may have misspoke. At 10:34, he says:
[emphasis added]
However, the slide behind him says:
[emphasis added]
I'm not sure how not emiting landing pads would have that significant of an impact on performance. This performance claim may be the result of a misunderstanding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, I believe I did indeed misspeak! I've never personally at least seen a runtime improvement from
panic=abort
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The folk lore around this (going back to @thestinger aka strcat) would suggest that the only runtime performance you would see is from optimizations that wouldn't fire with the landing pads there, mostly due to the landing pad accessing memory, i.e. unwind drop code preventing simplification of the non-unwind variable usage.
Another example would be inlining, where a function might be too large to inline due to landing pads.
It would be cool to have a tool that could automate comparing the two, but we don't atm.
We used to have
-Zno-landing-pads
while buildingstage0
libraries, but IIRC that was mostly to speed up compile times, as LLVM takes longer to optimize code with unwinding.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not a folklore. I can get a 10-15% improvement in [redox-os/ralloc].
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So what @alexcrichton mistakenly said is consistent with my experience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait and I'll make some benchmarks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://www.reddit.com/r/rust/comments/55ns2m/safe_and_efficient_bidirectional_trees/ had a 20% cpu time improvement by disabling landing pads. There were two ways to reach that goal. One, to use the -Z option, second to eliminate all panics in the actual code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's another landing pads related story. http://stackoverflow.com/questions/39333454/collatz-conjecture-in-rust-functional-v-imperative-approach