Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jun 23, 2024

Conversation

ivmarkov
Copy link
Contributor

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
Copy link
Contributor Author

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 kedars merged commit eb7c152 into project-chip:main Jun 23, 2024
12 checks passed
@kedars
Copy link
Collaborator

kedars commented Jun 23, 2024

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

@ivmarkov ivmarkov deleted the replace-mdns branch September 14, 2024 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants