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

Introduce ByteString based on an existing ByteArray #428

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Laxystem
Copy link

I implemented this using an existing class and an extendable boolean as I didn't want to break binary compatibility with existing usage of the ByteStringBuilder constructor, but it is possible to rewrite using an abstract class (should be ever-so-slightly-faster).

My use case: I have a game engine, and I'd like to avoid copying data around more than necessary. Usually, this would be a micro-optimization; But re-meshing everything every frame adds up quickly. This way, I can pass my existing vertex buffer, and an offset (that I can record using the buffer's own size), and simply override the changed vertex's data.

I did not add a buildByteString function as this is a lower-level use-case and should be treated as such; I expect most to not need the resulting ByteString at all, and simply use apply instead.

I would add a DelicateIoApi annotation, but I didn't seem to find one.

@lppedd
Copy link
Contributor

lppedd commented Jan 12, 2025

Maybe OT, but while we're talking about an easy way to initialize a ByteString with an existing array, I still can't find the same for a Buffer.

@JakeWharton
Copy link
Contributor

JakeWharton commented Jan 13, 2025

You can already wrap an existing ByteArray into a ByteString with UnsafeByteStringOperations.wrapUnsafe.

Maybe OT, but while we're talking about an easy way to initialize a ByteString with an existing array, I still can't find the same for a Buffer.

There is no way to give a ByteArray to a Buffer directly. A Buffer is comprised of a series of Segment instances which own their backing ByteArray, and those segments are pooled for efficiency. It's not impossible to design a system where you give a ByteArray which creates a Segment that is also never pooled, but I think it is unclear whether this is useful enough in contrast to some of the other APIs. We haven't needed such an API in Okio, for example. You can move a ByteArray into a Buffer! See next comment...

You can ask a Buffer for the ByteArrays which back its Segments to directly write to using UnsafeBufferOperations.writeToTail. This is similar in capability to Okio's UnsafeCursor API.

@fzhinkin
Copy link
Collaborator

@lppedd in addition to what Jake wrote, you can also use UnsafeBufferOperations.moveToTail to gave ownership over an array to the buffer, which, to some extent, should be the same as initializing a buffer with an array.

@fzhinkin fzhinkin self-assigned this Jan 13, 2025
@fzhinkin
Copy link
Collaborator

@Laxystem one of the most crucial parts of the ByteString's contract is its immutability. The approach proposed in this PR let users to accidentaly break the contract much easily.

There is existing API that raises the same concern, however, it is ringing all the bells to ensure that nobody will break the contract unintentionally (and, hopefully, won't break it at all):

You can already wrap an existing ByteArray into a ByteString with UnsafeByteStringOperations.wrapUnsafe.

If some "unsafe" API is required, (most likely) the only place for it would be UnsafeByteStringOperations, not the regular public API.

I would love to read more details about a problem you're trying to solve. Perhaps, we'll figure out how to solve your problem without making the builder less "safe".

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

Successfully merging this pull request may close these issues.

4 participants