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

Support for std::sync::Arc as indirection #429

Open
segeljakt opened this issue Jan 19, 2021 · 9 comments · May be fixed by #705
Open

Support for std::sync::Arc as indirection #429

segeljakt opened this issue Jan 19, 2021 · 9 comments · May be fixed by #705

Comments

@segeljakt
Copy link

segeljakt commented Jan 19, 2021

Hi, I wonder, would be possible to allow prost-types to be wrapped inside std::sync::Arc? The reason is that I want my messages to be sendable across both processes and threads. When sent across processes, my messages should be serialized/deserialized with prost. When sent across threads, I would like to use reference counting.

#[derive(prost::Message)]
struct MyMessage {
    #[prost(string, tag = "1")]
    label: Arc<String>,
    #[prost(message, tag = "2")]
    payload: Arc<Payload>,
}

#[derive(prost::Message)]
struct Payload {
    #[prost(i32, repeated, tag = "1")]
    stuff: Vec<i32>,
}

Thanks for the great crate.

@wchargin
Copy link
Contributor

wchargin commented Mar 4, 2021

This would also be nice in conjunction with Tonic servers. If we had
something analogous to bytes or btree_map, like:

Config::new()
    .arc(&[".my_service.FooRequest.my_big_field"])
    .compile_protos(...)

…then we could get away with fewer wasted clones in RPC handlers.

I think that this might be nice, but am not strongly lobbying for it,
and wouldn’t be too upset if it were wontfixed.

@danburkert
Copy link
Collaborator

Sounds like a good addition! We should probably also support Box and Rc similarly.

@bjchambers
Copy link

This could also be helpful (I think) for cases where a large object is received in the request and needs to outlive it. Allowing that object to be an Arc would allow it to be taken outside the request/response lifetime without copying.

@name1e5s
Copy link

name1e5s commented Apr 1, 2022

🤔Seems it's possible to implement the feature, I'll try this.

@The-Mr-L
Copy link

The-Mr-L commented May 1, 2022

will this be on the road map?

@LucioFranco LucioFranco mentioned this issue May 2, 2022
6 tasks
@ldm0 ldm0 linked a pull request Aug 13, 2022 that will close this issue
@ldm0
Copy link
Contributor

ldm0 commented Oct 17, 2022

Sounds like a good addition! We should probably also support Box and Rc similarly.

Rc cannot be supported since prost::Message requires Send + Sync

@jasongoodwin
Copy link

Yeah I can here just to comment/upvote this.
I'm trying to use a watch with tonic and I need to clone the message that hits each receiver because arc isn't supported. I can probably modify some of the generated code.

@AshleySchaeffer
Copy link

I can probably modify some of the generated code.

@jasongoodwin do you happen to have an implementation for this? I'm in a similar position, only the data I'm sending can get quite large.

@morrisonlevi
Copy link

morrisonlevi commented Jan 31, 2024

Rc cannot be supported since prost::Message requires Send + Sync

Honestly... why does Message require Send + Sync? I peeked at the associated functions and there didn't seem any reason for it.

It's actually blocking me for another reason: I tried implementing Message for Cow<str>. I can logically encode and decode a Cow<str> into proto string:

  1. Encoding is easy, it can borrow basically all of string encoding infrastructure.
  2. Decoding is not bad either:
    • If it's an empty string, then decode it as Cow::Borrowed("").
    • If it's not an empty string, then decode it as Cow::Owned String.

But due to the requirement of Message to be Send + Sync, it doesn't matter.

The requirement of Send is particularly odd, because I would think serializing to protobuf is one way you can send non-Send types to other threads...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.