Tikv: coprocessor::Result<T> is at least 176 bytes..

[breeswish]

Bug Report

Found during an investigation of performance regression after applying some new optimizations to the Coprocessor.

    println!("{}", std::mem::size_of::<tikv::coprocessor::Result<i64>>());
    println!("{}", std::mem::size_of::<tikv::coprocessor::Error>());

Output:

184
176

This means that, any frequent interface using coprocessor::Result as the return value will be very ineffective. This should be one of the root case of so many memcpy in the profiling result.

Follow up:

println!("{}", std::mem::size_of::<codec::Error>());
// => 16

println!("{}", std::mem::size_of::<tikv_util::codec::Error>());
// => 16

println!("{}", std::mem::size_of::<tikv::storage::Error>());
// => 200

println!("{}", std::mem::size_of::<tikv::storage::mvcc::Error>());
// => 184

println!("{}", std::mem::size_of::<tikv::storage::txn::Error>());
// => 192

[siddontang]

oh, seem we need to use Box to wrap the Error, right?

[breeswish]

@siddontang Maybe yes, maybe no:

Yes: -- This can solve the large memory problem.
No: -- This will simply make ? operator fail everywhere.

[kennytm]

This sort of thing is supposed to be warned by the large_enum_variant lint. Unfortunately the default setting is allow up to 200 bytes (inclusive), i.e. your tikv::storage::txn::Error is 16 bytes too small from triggering the warning 🙃.

[breeswish]

@kennytm Maybe we can adjust the throttle, to discover what is missing in the list. I believe there are a lot :( For those really unimportant & large ones we can whitelist them explicitly in #[allow].

[siddontang]

@breeswish

Can we file an issue to let contributors help us do the large enum check?

[siddontang]

Not only the large enum, I also think we should find a way to detect large structure move.