search

google / libnop

libnop: C++ Native Object Protocols
Other
575 stars 59 forks source link

Feature request: Support for zero-copy views like std::span #12

Open heiner opened 4 years ago

heiner commented 4 years ago

I have a situation where at deserialization time I'd like a view-only object like std::span (which I would give to another object's constructor where the data will be copied out). I'd like to implement my own nop::Encoding<std::span<uint8_t>> specialization for that. Unfortunately, it cannot be done on that level of abstractions since I would need to access, say, buffer_[index_] of BufferReader (https://github.com/google/libnop/blob/master/include/nop/utility/buffer_reader.h#L80).

I can instead read the prefix_byte and length at the place where I would ideally just call deserializer.Read(&myspan), get the offset via deserializer.reader().capacity() - deserializer.reader().remaining(), call deserializer.reader().Skip(length_bytes) and use the offset to index into my buffer. However, it would be nicer to encapsulate this logic.

One option would be to add a Status<void*> Ptr() { return buffer_[index_]; } method to BufferReader, and perhaps similar methods returning a failure status for other readers.

eieio commented 4 years ago

I would be open to adding some form of support for view-like objects to the Reader interface. Let me consider the consequences of various options.

Hopefully you don't mind maintaining your own specialization for std::span until libnop officially adds support for C++17 standard library types?

heiner commented 4 years ago

Thanks for the quick response!

And also thanks for your library, we (Facebook AI Research) are planning to use it in a number of projects.

I would be open to adding some form of support for view-like objects to the Reader interface. Let me consider the consequences of various options.

Awesome, looking forward to it!

Hopefully you don't mind maintaining your own specialization for std::span until libnop officially adds support for C++17 standard library types?

Yes definitely. I'm not actually using std::span but instead a similar view-only array that ships with PyTorch (at::ArrayRef<T>). I'm happy to write my own Encoding<at::ArrayRef<T>> for those (the documentation doesn't seem to mention this, but adding support for more types is easy enough that way).