Various memory optimizations #94
Conversation
|
Oops, this failed to build on Rust <1.79 because the compiler cannot decide if If bumping MSRV to version this recent is not acceptable, keeping |
A lot of strings and arrays used in nftables structures are static, but currently we need to do allocations everywhere using owned String and Vec. Replace them with Cow so we can pass &'static str or &'static [_] whenever possible, while returning owned value when deserializing.
This evens out some of the large enums' sizes (e.g. Expression and Statement's) from over 200 bytes to <=144. Further reduction could be done, as this commit only changes the obvious ones for now. Also moves BinaryOperation's Box up one level so it only uses one Box.
Since we only expect two elements from Range, we can just use array instead.
This fails to build on 1.65:
```
error[E0277]: the trait bound `[SetItem]: ToOwned` is not satisfied in `Expression`
--> src/stmt.rs:245:14
|
245 | pub dev: Option<Expression>,
| ^^^^^^^^^^^^^^^^^^ within `Expression`, the trait `ToOwned` is not implemented for `[SetItem]`, which is required by `Expression: Sized`
|
= help: the trait `ToOwned` is implemented for `[T]`
note: required because it appears within the type `Expression`
--> src/expr.rs:10:10
|
10 | pub enum Expression {
| ^^^^^^^^^^
note: required by an implicit `Sized` bound in `std::option::Option`
--> /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/core/src/option.rs:572:1
...and many, many similar errors
```
It compiles successfully on Rust >=1.79, so this commit can be reverted once our MSRV reaches it.
|
Fixed MSRV build and clippy. Please have another look :) |
|
Amazing @hack3ric! Please allow us some more days to review it. |
| table: Cow::Borrowed("some_inet_table"), | ||
| chain: Cow::Borrowed("some_inet_chain"), |
There was a problem hiding this comment.
Would it also be possible to use .into() in places like these?
There was a problem hiding this comment.
impl<'a> From<&'a str> for Cow<'a, str> (and its fellows) is not const for now so .into() will result in a temporary value (as seen by rustc, even if it is 'static), therefore cannot be borrowed 'static. In this example both the outer slice and the inner str needs to be borrowed 'static, leading to the following error:
error[E0716]: temporary value dropped while borrowed
--> tests/json_tests.rs:57:33
|
57 | objects: Cow::Borrowed(&[
| ________________________________-^
| |_________________________________|
58 | || NfObject::ListObject(NfListObject::Table(Table {
59 | || family: NfFamily::INet,
60 | || name: "some_inet_table".into(),
... ||
106 | || })),
107 | || ]),
| || ^
| ||_________|
| |_________creates a temporary value which is freed while still in use
| cast requires that borrow lasts for `'static`
108 | };
| - temporary value is freed at the end of this statement
If all of the outer containers are Cow::Owned, we can use .into() inside. Otherwise we probably need to stick to manually use Cow::Borrowed. I've also pushed a commit that allows temporary arrays to be used by replacing 'static with 'a. It will then just need to move the array in Cow::Borrowed(&[..]) to another local variable to allow the usage of .into():
let objects = [
NfObject::ListObject(NfListObject::Table(Table {
family: NfFamily::INet,
name: "some_inet_table".into(), // here
handle: None,
})),
// ...
];
let expected = Nftables {
objects: objects.into(); // and here
};
|
Added one commit that lifts |
This adds a bunch of lifetimes, but it works with `.into()` when a borrowed value's array container is stored in another place, without the need of `Cow::Borrowed`:
```
let objects = [
NfObject::ListObject(NfListObject::Table(Table {
family: NfFamily::INet,
name: "some_inet_table".into(), // here
handle: None,
})),
// ...
];
let expected = Nftables {
objects: objects.into(); // and here
};
```
However, `Cow::{Borrowed, Owned}` still needs to be used when you want to write the above in one statement.
|
Clippy issue should be fixed now. |
|
FYI I have another patch ready for upstream, but it depends on this PR: 52902cd. I will submit another PR after this and I hope this too can get into v0.6.0 too :) |
See each commit message for details.
These are a lot of breaking changes, so please take your time to check if they are properly done :)