clippy: ignore result_large_err clippy lint for Tablet::from_raw_tablet

[muzarski]

With recent update of rust toolchain, new clippy lints were introduced - result_large_err being one of them.

See: https://rust-lang.github.io/rust-clippy/master/index.html#result_large_err

I decided to ignore the lint in Tablet::from_raw_tablet.
I justified this decision in the code:

// Ignore clippy lints here. Clippy suggests to
// Box<> `Err`` variant, because it's too large. It does not
// make much sense to do so, looking at the caller of this function.
// Tablet returned in `Err` variant is used as if no error appeared.
// The only difference is that we use node ids to emit some debug logs.

Pre-review checklist

• I have split my patch into logically separate commits.
• All commit messages clearly explain what they change and why.
• [ ] I added relevant tests for new features and bug fixes.
• All commits compile, pass static checks and pass test.
• PR description sums up the changes and reasons why they should be introduced.
• [ ] I have provided docstrings for the public items that I want to introduce.
• [ ] I have adjusted the documentation in ./docs/source/.
• [ ] I added appropriate Fixes: annotations to PR description.

[github-actions]

cargo semver-checks found no API-breaking changes in this PR! 🎉🥳
Checked commit: cebcb0c

[Lorak-mmk]

I agree that it should be ignored - Ok and Err variants differ only by 8 bytes I think.
Perhaps that it what the lint should look for: big size of Err compared to Ok.

[wprzytula]

I agree that it should be ignored - Ok and Err variants differ only by 8 bytes I think. Perhaps that it what the lint should look for: big size of Err compared to Ok.

I think the reason why big Err variants should be avoided is that Err are most commonly propagated, possibly through many layers. On each layer, stack variables must be copied, which implies O(depth*size), where depth is number of propagation layers and size is size of the Err variant.

[Lorak-mmk]

I agree that it should be ignored - Ok and Err variants differ only by 8 bytes I think. Perhaps that it what the lint should look for: big size of Err compared to Ok.

I think the reason why big Err variants should be avoided is that Err are most commonly propagated, possibly through many layers. On each layer, stack variables must be copied, which implies O(depth*size), where depth is number of propagation layers and size is size of the Err variant.

Sure, but if your Ok variant is big as well, then the whole type will be big, so propagation will still require this much memory moving.
Big Err is worth reducing if Ok is small, in order to reduce stack usage and memory moves.

[wprzytula]

I see your point now, but your argument is only valid for one layer of propagation: that with big Ok variant. I think (but I'm not convinced) that Err values tend to accumulate stack size more than Ok values. Big Errs may, in general, flow deeper through layers, accumulating overhead.

Now I also understood that our way of error handling, with error enums nested deep, can bring significant overhead. Stack memory is not that bad in terms of space waste, but a worse thing is reduced memory locality on stack due to large slots for errors in each function frame.

[Lorak-mmk]

I see your point now, but your argument is only valid for one layer of propagation: that with big Ok variant. I think (but I'm not convinced) that Err values tend to accumulate stack size more than Ok values. Big Errs may, in general, flow deeper through layers, accumulating overhead.

Sure, but then in those higher layers:

• You will either return small Ok and big Err - which lint should catch
• You will return another big Ok or big Err - so reducing Err still makes no sense.

Now I also understood that our way of error handling, with error enums nested deep, can bring significant overhead. Stack memory is not that bad in terms of space waste, but a worse thing is reduced memory locality on stack due to large slots for errors in each function frame.

We have nested enums, but we don't store a lot of data in them.
Rust is really good at optimizing nested enums, so they usually don't take space and the size will be ~determined by size of largest variant.

Take QueryError for example. It's size is 112 bytes, which is exactly the size of one of its variants - CqlResultParseError, which is again the size of its largest variant PreparedParseError.
If you have nested enums then Rust will try hard to still have only a single discriminant for it all.

[wprzytula]

If you have nested enums then Rust will try hard to still have only a single discriminant for it all.

I didn't know that! Awesome!

[wprzytula]

Sure, but then in those higher layers:

• You will either return small Ok and big Err - which lint should catch
• You will return another big Ok or big Err - so reducing Err still makes no sense.

Correct, I'm convinced now.