[PR 2/5] Introduce unsafe API for bulk read/write ops #334
Conversation
|
CC @e5l |
|
Previously, newly introduced API was published as a snapshot build from https://github.com/Kotlin/kotlinx-io/tree/snapshot/unsafe-api branch. |
|
This change sits on top of #332. kotlinx-io-benchmarking-result-2024-05-29-dev.json UPD:
The results look legit: direct access to the buffer's tail improved performance on the write path. |
| @@ -36,46 +37,52 @@ import kotlin.jvm.JvmField | |||
| * `limit` and beyond. There is a single owning segment for each byte array. Positions, | |||
| * limits, prev, and next references are not shared. | |||
| */ | |||
| internal class Segment { | |||
| public class Segment { | |||
There was a problem hiding this comment.
Could you tell me why the Segment is public? I can't remember any usage of it.
if it still needs to be public, it would be good to annotate it with @InternalAPI and make useful extensions public as well
There was a problem hiding this comment.
An API to iterate over Buffer's segments (see #135 (comment)) will be implemented in subsequent PRs. That API would expose Segments to make iteration allocation free.
fzhinkin
force-pushed
the
organize-segments-as-list
branch
from
e3f76f7 to
c128d21
Compare
fzhinkin
changed the title
The API aimed to facilitate integration with other frameworks and libraries. Implemented API was initially described in the "Bulk API" subsection of #135 (comment)
fzhinkin
force-pushed
the
bulk-api-part-1
branch
from
d105268 to
edd4461
Compare
| @PublishedApi | ||
| @JvmSynthetic | ||
| @Suppress("UNUSED_PARAMETER") | ||
| internal fun dataAsByteArray(readOnly: Boolean): ByteArray = data |
There was a problem hiding this comment.
readOnly parameter created for the future?
| @@ -295,3 +341,6 @@ internal fun Segment.indexOfBytesOutbound(bytes: ByteArray, startOffset: Int): I | |||
| } | |||
| return -1 | |||
| } | |||
|
|
|||
| @PublishedApi | |||
| internal fun Segment.isEmpty(): Boolean = size == 0 | |||
There was a problem hiding this comment.
Why is it not a member of the Segment?
There was a problem hiding this comment.
While the size is a core property of a Segment that could not be easily implemented outside the Segment, isEmpty is just a convince function. So I decided to implement it as an extension.
| } | ||
|
|
||
| /** | ||
| * Provides read-only access to [buffer]'s data by filling provided [iovec] array with [ByteBuffer]'s representing |
There was a problem hiding this comment.
It is not entirely clear from the documentation what the size of each ByteBuffer in the array will be and, in general, on the basis of which the total number of buffers is determined, is it predictable or not, which array size is better to use
There was a problem hiding this comment.
The documentation is indeed require some improvements.
However, the API itself was designed with the idea that iovec-array will be allocated once and cached by the caller (for instance, as a thread-local variable) to reduce excessive allocations on the write path.
The number of ByteBuffers to be created corresponds to the number of segments a buffer has. Subsequent patches provides an API allowing to find that value, but in general, it's hard to give an advice on the array size (for example, a buffer holding 8000 bytes may have either 1 segment, or multiple).
- got rid of duplicate checks - started verifying segment's content for bulk reads
This is a first portion of unsafe API described in #135.
This particular part aimed to facilitate integration with other frameworks and libraries.
Implemented API was initially described in the "Bulk API" subsection of #135 (comment)