Shrink the size of all Error types#225
Shrink the size of all Error types#225alexcrichton wants to merge 1 commit intorust-lang-deprecated:masterfrom
Error types#225Conversation
| #[doc(hidden)] | ||
| pub $crate::State, | ||
| ); | ||
| pub struct $error_name(pub Box<($error_kind_name, $crate::State)>); |
There was a problem hiding this comment.
Since we can't pattern match on the kind (box if nightly only), maybe we could remove pub and put everything into $crate as
struct ErrorInternals<K> {
kind: K,
backtrace: Backtrace,
next: ...
}I tried that at one time, but it was rejected because it prohibited the use of pattern matching. But if we can't use it anyway...
There was a problem hiding this comment.
At least in the usage I've seen I think it's quite important to match on ErrorKind, but is matching on the entire error itself that important?
There was a problem hiding this comment.
Yes : https://github.com/rust-lang-nursery/error-chain/blob/master/src/lib.rs#L348-L353
With the new method, you can't do that anymore. You have to match on Result, then on error.kind().
There was a problem hiding this comment.
That's true, yes.
|
Nice! You just have to update the tests which use pattern matching on the kind. It's also a breaking change so could you update the CHANGELOG? |
alexcrichton
force-pushed
the
optimize-for-size
branch
from
26e98c6 to
f9c26a0
Compare
|
Updated |
alexcrichton
force-pushed
the
optimize-for-size
branch
from
f9c26a0 to
23586d7
Compare
|
Could you rebase please? |
This commit improves the in-memory size of `Error` from 7 pointers to 1 pointer. Errors are in general relatively rare in applications and having a huge error type ends up generating lots of instructions for moves and such, so this PR optimizes for size and passing around errors rather than creating errors.
alexcrichton
force-pushed
the
optimize-for-size
branch
from
23586d7 to
f3020b8
Compare
|
Sure thing, done now |
| ErrorKind::Msg(_) => {} | ||
| _ => {} | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm not very satisfied with this new behaviour, it's less straightforward. Not sure how to improve it though...
There was a problem hiding this comment.
box_patterns will be stabilized some day, no?
There was a problem hiding this comment.
@Yamakaky do you have a preferred method of how we might land this? I'd personally like to see this as at least an option, it's what I'd want for libraries 99% of the time I believe.
|
When might this pull request be land/be merged? |
4 participants
This commit improves the in-memory size of
Errorfrom 7 pointers to 1 pointer.Errors are in general relatively rare in applications and having a huge error
type ends up generating lots of instructions for moves and such, so this PR
optimizes for size and passing around errors rather than creating errors.