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

Make take_boxed() create messages directly on the heap #281

Open
nnmm opened this issue Oct 8, 2022 · 3 comments
Open

Make take_boxed() create messages directly on the heap #281

nnmm opened this issue Oct 8, 2022 · 3 comments

Comments

@nnmm
Copy link
Contributor

nnmm commented Oct 8, 2022

Currently, the implementation of take_boxed() will place the message on the stack during intermediate steps, even though the intent of this function is to avoid that, since large messages might overflow the stack.

This requires avoiding any Box::new() calls, see rust-lang/rust#53827

@Nizerlak
Copy link
Contributor

Nizerlak commented Nov 7, 2022

Hi, I'm considering contribution to this issue. However, I need some hints how to start. AFAIK this and this Box::new functions have to be replaced with another one (which? from_raw?). Also I've found the following comment in take_boxed()

        // TODO: This will still use the stack in general. Change signature of
        // from_rmw_message to allow placing the result in a Box directly.

How should the signature look like? I assume that it should return a pointer, but how to achieve that?
It'd be great, if you could explain that to me.
Regards

@asterycs
Copy link

asterycs commented Nov 7, 2022

Hi! I've wanted to make a PR for this one as well but I've been having some issues with setting up the environment.

As far as I understand from the discussion in rust-lang/rust#53827 the safest way would be to use vec! together with into_boxed_slice since vec! apparently avoids the stack allocation. I'm not sure if this still requires the signature change that is mentioned in the comment.

@Nizerlak will probably have a PR before me anyway :)

@nnmm
Copy link
Contributor Author

nnmm commented Nov 12, 2022

I've actually already looked into this a while back, so I pushed my work-in-progress branch in case one of you wants to take over: main...take_boxed

Let me know if that helps. The main problem I was facing was that the msg_idiomatic.rs.em template code becomes very large with this change, perhaps there's a way to avoid it though.

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

No branches or pull requests

3 participants