project-chip / rs-matter

Rust implementation of the Matter protocol. Status: Experimental
Apache License 2.0
323 stars 45 forks source link

Option to replace initial mDNS and DevAtt post-construction #193

Closed ivmarkov closed 3 months ago

ivmarkov commented 3 months ago

There is a big comment above the newly-introduced replace_mdns utility method that explains the issue at hand.

I have this problem in actual real-life code, hence pushing for it. :) Not a fictitious problem.

ivmarkov commented 3 months ago

Let me elaborate a bit on the background, as it might still be not so obvious as to what exactly is going on here:

In my opinion, Rust currently has two main challenges when trying to fit it on devices which have small amount of RAM (like MCUs):

Autogenerated futures end up a bit too large

The futures generated by the async syntax tend to take 2x as much space as necessary, and the problem becomes exponential with a lot of nested futures. This problem (as well as our workarounds) is described in details in the Transport rework RFC so I won't cover it here

No placement-new C++-like construct

Rust does not have constructors in the C++ sense. It just uses "constructor functions" which is merely a convention for any associated function which contains the new word inside its name, and returns Self (or Option<Self>, or Result<Self, ...> and so on).

The problem with construction functions is that - putting aside any optimizations - the value is first constructed on-stack, and only after that it is moved to its final location (be it still on-stack, or on-heap or others).

The problem with these moves induced by Rust construction-functions (and in fact with any function in Rust that returns a large result, but this is usually the constructor functions) is that - with large objects - you are risking a stack blowup, if the stack of the thread is smaller than the object you are truing to return (or to place elsewhere, like in a Box or in a static cell context).

The above situation is typical on MCUs where threads are usually running with just a few kilobytes of RAM (or max being a few tens of kilobytes).

There are three workarounds w.r.t. stack blowups I'm aware of:

Allocate statically

This is a very simple method which works 100% of the time. To statically allocate the object using only safe code, i.e. static FOO: Foo = Foo::new() you DO need the new constructor function to be const. If the new constructor function is const, then what rustc does is roughly the following desugaring:

// This `const` is generated at compile-time and is put by the compiler 
// in the `rodata` segment; `rodata` on MCUs means flash memory. 
// I.e. the constant takes up firmware space, but no SRAM memory space
const FOO_CONST: Foo = Foo::new();

/// What the compiler does here is - it instructs the loader - 
// when the `rwdata` segment is formed - to take the memory of 
// `FOO_CONST` and `memcpy` it into RAM. I.e. no moves at all, 
// and the static comes up pre-initialized even before the program 
// had started.
static FOO: Foo = FOO_CONST; 

(There are variations of this scheme which are useful when e.g. Foo is not thread-safe, i.e. it is not Sync. Then you can use stuff like StaticConstCell from the static_cell crate.)

Anyway, the key to this approach is that as long as the object is const-new-able, and we are OK to trade-in flash size for a warranty that there will be no stack blow-ups, it works.

Make sure that rustc optimizes away the move

This is not a real workaround in that it simply cannot be guaranteed in all circumstances and under all opt settings, except when the returned object is of type MaybeUninit<T> - i.e. the code does not return a concrete "data" but just an "indication" of what the object layout is. I believe for this, the opt triggers always, including in debug / opt=0 builds.

Something which also triggers almost always is this:

const FOO: Foo = Foo::new();

let mut foo = Box::new_uninit(); // Or any other function that returns `MaybeUninit`. As per above, this is guaranteed to be optimized

let foo = unsafe {
    obj.as_mut_ptr().write($val);
    obj.assume_init()
};

... but the above ^^^ requires that Foo is const-newable in the first place! Which is just like the requirement from the previous section!

Invent placement-new in Rust

When your data is not const-newable, OR when you are not willing to trade-in flash space for no-stack-blockups, you don't have many options, but to re-invent "placement-new" in Rust.

You can kind of do placement-new in Rust even today, simply because MaybeUninit<T> is always guaranteed to not involve moves. So you can allocate a chunk of memory (say, a structure with a ton of small fields) as MaybeUninit<T> and then laboriously - field by field - using lots of unsafe - code - (possibly recursively) initialize each field and then "proclaim" the structure is initialized by using one of the MaybeUninit::assume_init* variations that unsafely trasmute MaytbeUninit<T> into T, but this is a lot of unsafe code.

(By the way, the const solution from the "Make sure that rustc optimizes away the move" section does exactly that, and hopes that - in light of the fact that the rvalue is a const - rustc will just memcpy it from flash. But the lvalue is exactly a MaybeUninit location, which is unsafely initialized, and unsafely assumed to be initialized at the end.)

The one solution I'm aware of that does a placement-new invention (by - oversimplifying - but just hiding the unsafe constructs for initializing MaybeUninit memory under a bunch of Rust macros) is the pinned-init crate from the RfL project (also a blog here). Which is itself based on ideas from the moveit crate, but stripping out the topic of Rust-C++ interop, which I think inspired moveit.

======================================================

But what does the above elaboration have to do with this small PR?

Simple. Assuming you already do have

impl Matter {
    const fn new(my_ref: &'static Mdns) -> Self {
        // ...
    }

... and you plan to use the "Allocate statically" or "Make sure that rustc optimizes away the move" tricks... you can't because this is currently not possible:

static MDNS: Mdns = Mdns::new();

const MATTER: Matter = Matter::new(&MDNS);

... even though &MDNS is 'static. Rust simply does not allow references to static objects in consts. And even if it started allowing these, it would still not allow static refs to interior-mutable objects. And our Mdns struct needs to have interior mutability, or else it is impossible to implement.

The workaround in this PR simply allows to replace - post Matter construction - and as long as you have mutable access to the Matter object - the original Mdns implementation with a reference.

So we start with

static MATTER: StaticConstCell<Matter> = StaticConstCell::new(Matter::new(MdnsType::Disabled)); // `MdnsType::Disabled` is not a reference but an enum member and hence can be used to form a `const`

and then, at runtime:


let matter = MATTER.take(); // Can only be done once, and returns `&mut Matter`
matter.replace_mdns(&my_mdns_reference);
kedars commented 3 months ago

Great pointers to RFCs and efforts for some form of ways to address this in Rust