Getting offset of Decoder

[julianmarmier]

Hello, is it an explicit implementation choice to abstract the decoder's offset field from the user? I'm currently working with decoding multiple messages over a fixed-length buffer (which is smaller than the stream of messages being read), and it would be useful to have some Decoder.offset function that returns the value of offset in Decoder.t so that the program knows when to reset the buffer with the remaining data.

Perhaps I'm not imagining the right way to decode over a buffer? Otherwise I'll gladly submit a small PR.

[c-cube]

You don't have an external framing? How do you know where one message ends and where the next one begins? I'm not opposed to exposing an offset function in theory, but decoder is designed to hold one message.

[julianmarmier]

I'm using Protobuf to stream data between two processes via a pipe on a single host which originally communicated using a text-based format. I don't have external framing to avoid overhead but taking inspiration from the documentation on streaming multiple messages I prepend the varint-encoded length of each message in the stream.

My messages correspond to a single line of text (generally between 10 and 60 bytes each) and the files can have up to a couple million lines, hence why I figure copying into a separate buffer each time might cause overhead. I could group these into multiple "super-messages" with chunks of messages but I still get the same problem of having to read multiple of these. Up until now I've been reusing the same decoder, as it returns the first message it's able to parse and advances its buffer. However from what I understand this is an ad-hoc implementation choice that isn't guaranteed in the original Protocol Buffers spec, hence why I'm trying to switch to offsets.

Some other implementations (Java, C++, Rust) have a CodedInputStream that reads exactly the given length to avoid extra copying into a buffer. I was looking into using Decoder.of_subbytes so that only one message is in the decoder at a time, but because I also read a varint without knowing its length, I still need either some notion of offset or to the size of the varint in the buffer. I could either switch to using fixed-size unsigned integers (which I might end up doing at the cost of using more space) or calculate the varint's length in an external function, but it would be easier to either read from the Decoder offset or otherwise have access to the internal varint_size function.

It might be better to just use fixed-size integers, in which case I wouldn't need access to the offset...

[c-cube]

I don't have time to implement this right now (will try to soon) but I think a of_subbytes is fine, and exposing offsets as well. You'd still be creating a decoder for every message, but borrowing from your main buffer, is that correct?

[julianmarmier]

Precisely! Would look something like:

(* off stores the current offset of buf from 0 *)
let decoder = Pbrt.Decoder.of_subbytes buf off varint_max in (* e.g. varint_max := 5 *)
let msg_sz = Pbrt.Decoder.int_as_varint decoder in
(* ... check if the buffer is still large enough ... *)
let decoder = Pbrt.Decoder.of_subbytes buf decoder.offset msg_sz in
let new_off = decoder.offset in 
let message = Message_line.decode_pb_message_line decoder  (* Message_line.ml is the protoc generated code *)
(* process message *)

For comparison, here is similar code I have in Rust using the protobuf crate which implements a translator from the binary format to the text format:

// buf_reader is a Reader on the protobuf binary stream
let mut is = CodedInputStream::new(&mut buf_reader);
let mut get_next = || -> protobuf::Result<ProofStep> {
    let i = match is.read_unknown(Varint)? {
        UnknownValue::Varint(i) => i,
        _ => panic!("invalid proof step: expected varint length prefix"),
    };
    let prev_lim = is.push_limit(i)?;
    let res = MessageLine::parse_from(&mut is);   // MessageLine is my schema-based message type
    is.pop_limit(prev_lim);
    res
};

while let Ok(message) = get_next() {
    // process message
}

Edit: false offset calculation in of_subbytes call

[c-cube]

take a look at #266 (sorry for the unrelated json changes), tell me if it's good enough for you :)

[julianmarmier]

Looks amazing, thank you very much!

Cheers :)

[c-cube]

Done. Cheers!