feat: Bytes::from_owner by amunra · Pull Request #742 · tokio-rs/bytes

<opinionated>

A custom trait is how I had originally started implementing this, but gave up early on for a few reasons:

• Worse ergonomics for custom trait:
  • Many things already implement AsRef<[u8]>, so Bytes::from_owner(mmap); is easy and straight-forward.
  • Conversely, a custom trait would require Bytes::from_custom(MyMmapWrapper::new(mmap)).
• Safety:
  • AsRef<[u8]> can be called during construction and only once. It's a trick that makes the from_owner API safe.
  • Any additional APIs would - in practice - need to be called during the lifetime of bytes. As an example trait CustomOwner { fn try_into_mut(self) -> Result<impl CustomOwnerMut, Self>; } (unless I'm wrong) would make it into an unsafe trait.
  • What's missing from the current approach is zero-copy conversion to BytesMut.
    The implementor of a trait that that exposed this would need to be very careful here to respect the Vec<u8> semantics. Context: If Bytes is not unique, the existing code creates a newly allocated Vec<u8> copy of the referenced memory. Unless due care is taken - in the specific case of an Mmap - this would likely lead an implementor to erroneously assume that they should create an MmapMut. This would be wrong, especially if the mmapped section is backed by a file as the resulting behaviour would sometimes update the contents of a file and sometimes not, depending on Bytes's internal refcount.
  • In general, also allowing any other calls to the object during its lifetime also allows for scope for the object to invalidate the slice returned from .as_ref() rendering that initial call unsafe too, since there's no borrow checker here to police usage.
• Feature creep:
  • By all means, I think a custom trait approach might work in the future for a fn from_custom <T: CustomOwner> and as a good way of exposing more of the vtable functionality, but it is not where the current pain point in the API is and there are many hurdles to get past that as has been shown in previous efforts to tackle meta: Expose Bytes vtable #437.

The full-fat custom use case would mostly cover - as far as I can tell - mixing Bytes created and managed via multiple custom memory allocators (QuestDB would certainly care!), but I suspect there's a more elegant way to handle those anyway.

</opinionated>

.. that said, if you guys prefer that route I'm happy to amend my PR in that direction.