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

Size of failure::Error is too big #9

Open
dtolnay opened this issue Nov 3, 2017 · 14 comments
Open

Size of failure::Error is too big #9

dtolnay opened this issue Nov 3, 2017 · 14 comments

Comments

@dtolnay
Copy link

dtolnay commented Nov 3, 2017

extern crate failure;

fn main() {
    println!("{}", std::mem::size_of::<failure::Error>());
}

Currently 2 words but I would expect this to be 1 word. In my experience a return type like Result<(), Error> can be faster if 1 word than 2 words. (Let me know if this is controversial and I can try to recreate some benchmarks.)

Would it be possible to double-Box or use a C++like layout for the DST so Error can be a thin pointer?

@withoutboats
Copy link
Contributor

Obviously 1 word should be better than 2, I'd like benchmarks to know exactly how much better though.

Ideally I think we'd store the vtable or a pointer to it inline in the heap with the Error data, but I'm not sure how, on stable, to get a DST without automatically opting into a wide pointer representation.

@withoutboats
Copy link
Contributor

To be clearer:

  • If we can store the vtable directly in the heap with the data on stable Rust, compatible back to 1.18.0, I would accept that PR without any further argument.
  • I'd like some sense of the trade off (through benchmarks etc) to be convinced that we should double box this if we can't store the vtable in the heap.

@dtolnay
Copy link
Author

dtolnay commented Nov 3, 2017

I grabbed some code from here and put a Result in it. Playground

  • Intel(R) Core(TM) i7-6600U CPU @ 2.60GHz
  • rustc 1.23.0-nightly (e340996ff 2017-11-02)

For k=26 in release mode, a 1-word error takes 3.6 seconds and a 2-word error takes 5.0 seconds. The whole program is 40% slower.

@dtolnay
Copy link
Author

dtolnay commented Nov 3, 2017

With Result<(), Error> as the return types the difference is 80%, 2.3 seconds vs 4.2 seconds. Playground

@withoutboats
Copy link
Contributor

Can you expand on why the "man-or-boy test" is a good benchmark? I admit I don't fully understand the algorithm, but it doesn't seem like it imitates normal Rust code patterns to me.

Just intuitively (and handwavingly), there's some sort of curve when the Ok value is 1 or 0 words, where we reach a % of error cases at which it becomes more expensive to double allocate than the impact on the happy path is. Then there are, of course, cases where the Ok value is more than 2 words (like returning a vector for example), in which case it has no impact.

So to me the decision is a factor of the portion of code returning small success variants and the portion at which errors occur on average.

Not necessarily asking you to come up with some great, definite proof benchmark, just trying to talk through the trade offs.

@bluss
Copy link

bluss commented Nov 3, 2017

A different experiment I'd wonder about -- does it make a difference if the error has a destructor or not, keeping the size constant? Basically just testing Box<> vs *const T here.

@withoutboats
Copy link
Contributor

@bluss We have to have a destructor though, don't we? We don't want to leak the error data or the backtrace.

@bluss
Copy link

bluss commented Nov 3, 2017

I don't know this crate, sorry! 😄, I'm just curious if there are drawbacks to having such allocated data in errors or not. In my trial... looks like that benchmark speeds up slows down if one uses usize instead of Box<String>.

@dtolnay
Copy link
Author

dtolnay commented Nov 3, 2017

You are right, that example is a worst case and not an average case.

In my heavily biased experience, I have always optimized for the happy path only. If some client were sending enough malformed requests to affect the server, they would be throttled. Even in the failure case, when deserialization fails there might be 10,000 Ok method calls before we arrive at the error and 10 Err method calls to propagate it to the caller. When serializing, practically every Ok value is 0 words -- in Formatter and Serializer. I guess that means I can't argue convincingly for which path to optimize. Would be interested to hear the opposite point of view where errors are frequent and Ok values are big.

@withoutboats
Copy link
Contributor

Your argument is pretty convincing. I think I'd like to hear other positions, so I'll leave this open, but right now I'm open to double boxing Error.

@U007D
Copy link
Contributor

U007D commented Nov 4, 2017

+1 for double-boxing from my perspective.

As a user of the library who is concerned about performance, optimizing for small Ok by double-boxing seems like the right tradeoff. I am generally less concerned about the double-deref performance impact once I have entered into an error state (and can work around this in the rare case it is a concern).

@withoutboats Failure is a fantastic contribution to the community--thank you! Please let me know if I can help with any development in any way (beginner, with 1y of Rust experience)

And remember, all success begins with Failure! ;)

@bluss
Copy link

bluss commented Nov 4, 2017

It seems a thin pointer to dst would be ideal, but it's a bit too experimental now.

@arielb1
Copy link

arielb1 commented Nov 7, 2017

@bluss

It's not like we are going to expose a crate like thin as public API, so I don't think it being experimental matters that much.

@withoutboats
Copy link
Contributor

I think I'm inclined to leave it how it is (including not doing #20, which would be contradictory to this proposal) for 0.1, and then based on use cases try to get more hard number for what further optimizations to perform.

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

No branches or pull requests

5 participants