-
Notifications
You must be signed in to change notification settings - Fork 636
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
Further improve stream decoding performance #2101
Conversation
e04b839
to
7245379
Compare
7245379
to
0dea861
Compare
Rebased & ready for review |
formats/json/jvmMain/src/kotlinx/serialization/json/internal/ArrayPools.kt
Outdated
Show resolved
Hide resolved
formats/json/jvmMain/src/kotlinx/serialization/json/internal/ArrayPools.kt
Outdated
Show resolved
Hide resolved
formats/json/jvmMain/src/kotlinx/serialization/json/internal/CharsetReader.kt
Show resolved
Hide resolved
Logic have hidden issue. If you take 128b buffer then release (buffer added to pool), then call take (512) you get last 128 buffer. And i think buffer sizes very small. Most systems are configured to use block sizes of 4096 or 8192(up to 64k). Ideal look to system memory page size. |
|
||
// Byte array pool | ||
|
||
internal open class ByteArrayPoolBase { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe Rewrite this code to remove many synchronized calls and speed up code?
internal class ByteArrayPoolBase(val size: Int) {
// new pool instance in every kotlin object. Not need super.take and protected overload or change to glocal static array / kotlin object
private val arrays = ArrayDeque<kotlin.ByteArray>()
private val bufferSize =size / 2
// bytesCount is private and not used in code
fun take(): ByteArray =
if (array.isEmpty) { // try check for empty without synchronized
ByteArray(size)
} else {
synchronized(this) {
arrays.removeLastOrNull()
} ?: ByteArray(size)
}
}
fun release(array: ByteArray): Unit {
// arrays.size atomic. But If will be data race we temporary add +1 element to poll. synchronized not needed. MAX_CHARS_IN_POOL pool size for chars. is for bytebuffer is same?
if (arrays.size * bufferSize < MAX_CHARS_IN_POOL)
synchronized(this) {
arrays.addLast(array)
}
}
}
internal object ByteArrayPool8k : ByteArrayPoolBase(8196)
internal object ByteArrayPool : ByteArrayPoolBase(512)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also ByteArray, CharArray can replace with template T :)
Current dev:
This branch: