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

How to return a reader? #259

Open
vbakc opened this issue Feb 18, 2022 · 6 comments
Open

How to return a reader? #259

vbakc opened this issue Feb 18, 2022 · 6 comments

Comments

@vbakc
Copy link

vbakc commented Feb 18, 2022

I want to store a Reader inside of a struct but it only uses a reference a does not implement Clone.

use super::capnp_error::CapnpError;
use capnp::{message::ReaderOptions, serialize_packed};
use std::io::Cursor;

pub struct ReaderWrapper<'a> {
    reader: &'a Reader<'a>,
}

impl<'a> TryFrom<Vec<u8>> for ReaderWrapper<'a> {
    type Error = CapnpError;

    fn try_from(buffer: Vec<u8>) -> Result<ReaderWrapper<'a>, CapnpError> {
        let reader =
            &serialize_packed::read_message(&mut Cursor::new(buffer), ReaderOptions::new())?;
        let typed_reader = reader.get_root::<Reader>()?;

        Ok(ReaderWrapper {
            reader: &typed_reader,
        })
    }
}
@vbakc
Copy link
Author

vbakc commented Feb 18, 2022

cannot return value referencing temporary value
returns a value referencing data owned by the current function

@OliverEvans96
Copy link
Contributor

Okay, so my understanding is that you're trying to keep the original Vec<u8> around, and also create a ReaderWrapper which reads directly from the Vec<u8> without modifying it or storing an intermediate representation.

If that's correct, there are a few issues with your initial attempt:

  • You can't impl TryFrom<Vec<u8>>, because that will consume the Vec<u8> that you're trying to reference. You can impl TryFrom<&'a [u8]>, which has two advantages:
    • doesn't consume the buffer
    • properly relates 'a to the buffer's lifetime.
  • You're using serialize_packed::read_message, which can't work. The packed representation has to be un-packed before it can be read, so it's only possible to read directly from the Vec<u8> without copying if it's in the standard binary representation to begin with. In this case, you can use capnp::serialize::read_message_from_flat_slice to read directly from a slice.

Here's my first attempt at something like what you're asking (full repo), based on the Point example in the capn-proto-rust repo readme:

use anyhow::Result;
use capnp::{message::ReaderOptions, serialize::SliceSegments};
use points_capnp::point;

pub struct ReaderWrapper<'a> {
    reader: capnp::message::Reader<SliceSegments<'a>>,
}

impl<'a> TryFrom<&'a [u8]> for ReaderWrapper<'a> {
    type Error = anyhow::Error;

    fn try_from(mut buffer: &'a [u8]) -> Result<ReaderWrapper<'a>> {
        let reader =
            capnp::serialize::read_message_from_flat_slice(&mut buffer, ReaderOptions::new())?;

        Ok(ReaderWrapper { reader })
    }
}

impl<'a> ReaderWrapper<'a> {
    fn print_point(&self) -> Result<()> {
        let point_reader = self.reader.get_root::<point::Reader>()?;
        println!("x = {:.2}", point_reader.get_x());
        println!("y = {:.2}", point_reader.get_y());

        Ok(())
    }
}

Note that your ReaderWrapper stores the "typed" reader (would be points_capnp::point::Reader in my example), whereas mine just stores the "untyped" capnp::message::Reader, which is suboptimal because as you can see in the print_point function, reading from the reader is unnecessarily fallible (the failure should happen once when the ReaderWrapper is created, not every time it's read from).

I thought this would be a good starting point - I haven't tried storing the "typed" version yet, although I think it might be possible with something like the owning_ref crate, although I'm not 100% sure.

@alexbleotu
Copy link

@OliverEvans96 I do realise this is an old issue but I am trying something similar and can't get it working either - I feel like I am missing something basic so any help is welcome

This feels like basic usage to me but I am trying cache the underlying bytes as well as the specific capnp reader for efficiency (like you pointed out in the previous comment it is not efficient to get_root on each access. I wasn't able to do that because:

I tried several combinations, the problem with simply storing the point_reader in your exanple is that reader would become out of scope after try_from.

Caching both data structures on self is tricky (at least I wasn't able to) because point_reader holds a borrow of reader and also needs a lifetime, which means
a. you can't create them in the same method because reader needs to be stored on self to get the correct lifetime 'a
b. even after you create reader and store it on self, you can't then create point_reader and store it on self because it will hold a borrow to reader, so it will hold a borrow to self and it will lock it

It feels like I am missing something, but not sure what

@dwrensha
Copy link
Member

dwrensha commented Jun 2, 2023

@alexbleotu does capnp::message::TypedReader work for your use case? You'll need to call get() to access the message, but that should be fairly cheap.

@ariel-miculas
Copy link
Contributor

ariel-miculas commented Jun 16, 2023

read_message_from_flat_slice returns a Reader<SliceSegments<'a>>, is there a way to completely avoid referencing the buffer? My use case is that I have a memory mapped buffer and I want to store it in a structure, together with the reader, so I only call read_message_from_flat_slice once, in the constructor. But if the reader contains a reference to my buffer, than of course I cannot store it alongside the buffer in the same structure.
Edit: note to self, see #121
Edit 2: seems like this is the answer to my use case: #243

@tv42
Copy link

tv42 commented Feb 17, 2024

Here's a trick for making a capnp Reader hold on to e.g. a Box<[u8]> or vec, and thus make it returnable (no outliving a borrow):

                let bytes: Box<[u8]> = foo();
                let capnp_reader_options = capnp::message::ReaderOptions::new();
                let segments = capnp::serialize::BufferSegments::new(bytes, capnp_reader_options)?;
                let reader = capnp::message::Reader::new(segments, capnp_reader_options)
                            .into_typed::<foo::Owned>();

Now, if you wanted to also hold on to your data, I think you can use Rc<[u8]> or Arc<[u8]> as the owner of the bytes, and let reference count manage the lifetime.

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

6 participants