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

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

Open
heiner opened this issue Jul 21, 2020 · 2 comments
Open

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

heiner opened this issue Jul 21, 2020 · 2 comments

Comments

@heiner
Copy link
Contributor

heiner commented Jul 21, 2020

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
Copy link
Collaborator

eieio commented Jul 23, 2020

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
Copy link
Contributor Author

heiner commented Jul 23, 2020

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).

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

2 participants