Skip to content
This repository has been archived by the owner on Aug 16, 2021. It is now read-only.

Memory bloat #44

Open
est31 opened this issue Sep 16, 2016 · 23 comments
Open

Memory bloat #44

est31 opened this issue Sep 16, 2016 · 23 comments
Milestone

Comments

@est31
Copy link

est31 commented Sep 16, 2016

The code (basing on the example from the readme)

#![recursion_limit = "1024"]
#[macro_use]
extern crate error_chain;

mod errors {
    error_chain! { }
}

fn main () {
    use errors::Result as ChainResult;
    use std::mem::size_of;
    println!("size of vanilla result {}", size_of::<Result<(), ()>>());
    println!("size of error_chain result {}", size_of::<ChainResult<()>>());
}

outputs:

size of vanilla result 1
size of error_chain result 56

I might be wrong, but isn't ChainResult just "yet another enum", which means those 55 extra bytes have to be copied over when returning, even in the success event? That would mean quite a performance overhead over normal Result in hot code, especially where only the Ok case which may be minimal is encountered, no?

Maybe the situation can be improved by making the backtrace functionality optional, as in possible to opt out of at compile time?

@est31
Copy link
Author

est31 commented Oct 1, 2016

@kali has made a very interesting overview over the origin of the 56 bytes: #45 (comment)

@withoutboats
Copy link

withoutboats commented Oct 1, 2016

This doesn't seem like a great comparison - Result<(), ()> carries no data aside from the discriminant. No one is using Result<(), ()> (if they are, they should probably be using bool or a new enum). How does an error_chain generated result compare to say io::Result, carrying the same information?

@est31
Copy link
Author

est31 commented Oct 2, 2016

if they are, they should probably be using bool or a new enum

Result<(), ()> is very useful as it can be used together with the try macro, and it carries the information for the reader with it that the operation is failable. Bool can mean anything, you have to read the docs to find out what it means. Documenting your API over the type system is one of the things Rust is about. Also, it gives the user a warning that it needs to be used, unlike bool.

Regardless of whether Result<(),()> is considered something to avoid or not, which can be surely discussed about, I only wanted to measure what the additional overhead was of the error-chain error.

And about io::Result: io::Result<()> uses 24 bytes, which is still less than half of what the error-chain Result is carrying.

@withoutboats
Copy link

Regardless of whether Result<(),()> is considered something to avoid or not, which can be surely discussed about, I only wanted to measure what the additional overhead was of the error-chain error.

OK but you've used a type which has 0 bytes of data for the non-discriminant portion. All you've documented is that the largest variant of the null error chain is 55 bytes. That doesn't mean that the equivalent of io::Result is 79 bytes, it might still be 55 bytes large.

@Yamakaky
Copy link
Contributor

Not really an issue, closing.

@est31
Copy link
Author

est31 commented Nov 17, 2016

This is a pretty major issue of this crate, as its against the "performance speed ease of use pick three" motto of Rust, by improving ease of use by heavily impairing performance.

@Yamakaky Yamakaky reopened this Nov 19, 2016
@Yamakaky
Copy link
Contributor

The solution would be to make the Msg variant optional. Any idea about the macro syntax?

@Yamakaky
Copy link
Contributor

2d34f22

@colin-kiegel
Copy link

If we assume that errors are less likely than the happy path, we can optimize the memory footprint for the happy path if we heap-allocate most of the error payload.

If we look at Result<T, E>, then T might be as small as (). So it might be desirable to get E as small as a pointer, i.e. boxing everything E = Box<SomeError> without erasing the type information (i.e. no trait object). In situations where errors are unlikely, the error-path may be expensive as long as the happy-path is optimized.

But on the other hand there may be situations where such heap allocations are not desirable, e.g. (i) when errors happen frequently or (ii) when compiling with #[no_std] and heap allocations are not possible or (iii) error handling in oom situations.

So maybe error_chain could offer different memory layouts which implement the same interfaces and are therefore completely exchangeable (when looked at from the outside).

@Yamakaky
Copy link
Contributor

Hum, interesting. Any idea for a the syntax?

@withoutboats
Copy link

I think the first question is: do you want this behavior to be controlled by syntax in the macro invocation, or by a crate feature? A crate feature would be easier to implement & IMO probably easier to use (less new syntax to learn), but wouldn't allow you to scope the behavior to an individual invocation. I'm not sure how often people are defining multiple error_chains in their crate, much less they have a need to different performance properties between them.

@est31
Copy link
Author

est31 commented Nov 22, 2016

A crate feature would be enough for me, too. Anything that makes the memory footprint smaller :)

@colin-kiegel
Copy link

I think you could start with a crate feature. That should be sufficient in most cases. A syntax could still be introduced later if there is demand and the crate feature would then only set the default behaviour.

@withoutboats
Copy link

withoutboats commented Nov 22, 2016

Someone correct me if we don't guarantee this yet, but I think if you box the error, Result<()>s will be subject to the NPO, and be 1 pointer in size. The Ok case will be represented by a null pointer. This seems pretty 👍.

@Yamakaky
Copy link
Contributor

Yamakaky commented Nov 22, 2016

I started the implementation with the feature, but I get errors like Box<Error>: std::convert::From<std::io::Error> not satisfied. I guess I have to use a tuple struct?

@withoutboats
Copy link

withoutboats commented Nov 22, 2016

@Yamakaky Have you tried implementing From<E: std::error::Error> for Box<Error>? Box is exempted from some of the normal orphan rules and so it might work. (Seems to work in playpen: https://is.gd/fjbjZS)

Edit: Looking at all the impls for the error type, you probably will need a tuple struct, because some of them like Deref for example will conflict with std impls.

@Yamakaky
Copy link
Contributor

Oh, cool!

Yamakaky added a commit that referenced this issue Nov 22, 2016
@Yamakaky
Copy link
Contributor

Please review #69

@Yamakaky
Copy link
Contributor

Result<()>: 8
  (): 0
  Error: 8

@est31
Copy link
Author

est31 commented Nov 23, 2016

Awesome!

Yamakaky added a commit that referenced this issue Nov 23, 2016
Yamakaky added a commit that referenced this issue Nov 23, 2016
@Yamakaky Yamakaky added this to the 1.0 milestone Jul 25, 2017
@cgm616
Copy link

cgm616 commented Nov 4, 2017

Is there still work to be done on this issue?

@willir
Copy link

willir commented Jan 25, 2018

Was this feature implemented? If not, why it was abandoned?

@Yamakaky
Copy link
Contributor

Honestly, I don't really work on a Rust project anymore, so I haven't been working on error-chain for some time...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants