Use CompositeDecoder::decodeCollectionSize to pre-allocate collections by fzhinkin · Pull Request #3192 · Kotlin/kotlinx.serialization

fzhinkin · 2026-05-04T21:24:06Z

Reimplemented AbstractCollectionSerializer::merge to read expected collection size first and pass it as an initial capacity to a corresponding builder. Builder were updated to take the capacity into account.

The change is quite intrusive, but for formats providing information about the collection size before reading it (it's only CBOR for now, though) it reduces the footprint and slightly improve decoding performance.

Fixes #3153

Fixes #3154

Fixes #3153

…city straight away

…ed capacity straight away

fzhinkin · 2026-05-05T21:15:56Z

Raw benchmarking results:

Comparison: https://jmh.morethan.io/?sources=https://gist.githubusercontent.com/fzhinkin/5e63a49d551118a7eba58620b60c618c/raw/b0e9331a1fe133aa94ee8419a9a2b996eb9514d4/baseline.json,https://gist.githubusercontent.com/fzhinkin/5e63a49d551118a7eba58620b60c618c/raw/b0e9331a1fe133aa94ee8419a9a2b996eb9514d4/opt.json

fzhinkin · 2026-05-06T13:42:07Z

        buffer[position++] = c
    }

    override fun build() = buffer.copyOf(position)


To discuss: we can avoid a copy here if position == buffer.size, but we need to explicitly invalidate the builder, JIC.

builders do not live outside of merge() IIRC

pdvrieze · 2026-05-06T20:06:40Z

        val compositeDecoder = decoder.beginStructure(descriptor)
+        val expectedSize = compositeDecoder.decodeCollectionSize(descriptor)
+        val builder = if (previous != null) {
+            previous.toBuilder().also { it.checkCapacity(expectedSize) }


This should add the new new "expectedSize" to the current size when calling checkCapacity. The common case for this branch is for incremental parsing of list elements.

It also makes sense to pass the new needed size to toBuilder to avoid an extra copy in the case that a new builder is actually created (not default behaviour).

Nice catch, thanks for pointing to it!

Not only the code does not work as expected, it can also fail in runtime if checkCapacity can't handle negative expectedSize (which is the case for all non-empty implementations, I guess).

fzhinkin · 2026-05-12T19:47:25Z

Updated benchmarking results:

sandwwraith · 2026-06-23T13:34:35Z

+    override fun Array<Element>.toBuilder(expectedAdditionalSize: Int): ArrayList<Element> {
+        checkBuildersInitialCapacity(expectedAdditionalSize)
+        val expectedCapacity = size + expectedAdditionalSize.coerceAtLeast(0)
+        return ArrayList<Element>(expectedAdditionalSize).also { it.addAll(this) }


Should be ArrayList<Element>(expectedCapacity), right?

sandwwraith · 2026-06-23T13:35:06Z

-        val builder = previous?.toBuilder() ?: builder()
-        val startIndex = builder.builderSize()
        val compositeDecoder = decoder.beginStructure(descriptor)
+        val expectedSize = compositeDecoder.decodeCollectionSize(descriptor)


I suggest checking that it is not less than -1 right here instead of calling checkBuildersInitialCapacity(initialCapacityHint) in every implementation

Apparently, I was in the "let's write over 9000 LOC" mood. :/

sandwwraith · 2026-06-23T13:41:59Z

        val compositeDecoder = decoder.beginStructure(descriptor)
+        val expectedSize = compositeDecoder.decodeCollectionSize(descriptor)
+        val builder = if (previous != null) {
+            previous.toBuilder(expectedSize)


I think passing here previous.toBuider(previous.collectionSize() + expectedSize.coerceAtLeast(0)) would also significantly simplify implementations

Maybe the coerceAtLeast should be 1 (the idea that this would not be called if there is not at least one element added, and the exception would be much less likely than this case).

Incoming collection can be empty, there's no need to re-allocate existing one in this case

sandwwraith · 2026-06-23T13:48:26Z

        buffer[position++] = c
    }

    override fun build() = buffer.copyOf(position)


builders do not live outside of merge() IIRC

pdvrieze · 2026-05-12T11:04:41Z

-    override fun Array<Element>.toBuilder(): ArrayList<Element> = ArrayList(this.asList())
+    override fun Array<Element>.toBuilder(expectedAdditionalSize: Int): ArrayList<Element> {
+        checkBuildersInitialCapacity(expectedAdditionalSize)
+        val expectedCapacity = size + expectedAdditionalSize.coerceAtLeast(0)


It may be more optimal to coerce it to at least 1. The use case for extending lists kind of implies to add at least one new element.

pdvrieze · 2026-06-23T15:52:20Z

        val compositeDecoder = decoder.beginStructure(descriptor)
+        val expectedSize = compositeDecoder.decodeCollectionSize(descriptor)
+        val builder = if (previous != null) {
+            previous.toBuilder(expectedSize)


Maybe the coerceAtLeast should be 1 (the idea that this would not be called if there is not at least one element added, and the exception would be much less likely than this case).

hfhbd · 2026-07-12T19:53:11Z

Nice! BTW it's not only CBOR but any 3rd party format too, like my CSV format that returns a List.

fzhinkin added 3 commits May 4, 2026 11:18

Cbor: Implement decodeCollectionSize for map and list reader

3b65d6c

Fixes #3154

Use CompositeDecoder::decodeCollectionSize to pre-allocate collections

d7b619d

Fixes #3153

Reimplmenent array serializers to allocate builder with required capa…

cc88753

…city straight away

Base automatically changed from fzhinkin/cbor-collections-size to dev May 5, 2026 12:00

fzhinkin added 4 commits May 5, 2026 09:09

Added a benchmark

0f68ae5

fixup! Reimplmenent array serializers to allocate builder with requir…

bbac6af

…ed capacity straight away

Add test covering fixed-length collection decoding

50468c0

fixup! Added a benchmark

c042815

fzhinkin force-pushed the fzhinkin/collections-initial-capacity branch from 1a41b4d to c042815 Compare May 5, 2026 15:59

Correctly initialize map's capacity wrt its load factor

3e2c9d5

fzhinkin commented May 6, 2026

View reviewed changes

fzhinkin marked this pull request as ready for review May 6, 2026 13:42

fzhinkin requested a review from Copilot May 6, 2026 13:42

Copilot started reviewing on behalf of fzhinkin May 6, 2026 13:43 View session

This comment was marked as resolved.

Sign in to view

fixup! Correctly initialize map's capacity wrt its load factor

83800fe

fzhinkin requested review from sandwwraith and shanshin May 6, 2026 14:38

fzhinkin linked an issue May 6, 2026 that may be closed by this pull request

Pre-allocate Map and Collection capacities using CBOR definite-length headers #3153

Open

pdvrieze reviewed May 6, 2026

View reviewed changes

fzhinkin marked this pull request as draft May 6, 2026 21:11

Support pre-sizing builders in toBuilder

a984865

fzhinkin marked this pull request as ready for review May 12, 2026 19:45

sandwwraith requested changes Jun 23, 2026

View reviewed changes

pdvrieze reviewed Jun 24, 2026

View reviewed changes

fzhinkin added 2 commits July 10, 2026 10:13

Simplify parameters validation

706dad5

Simplify toBuilder implementations

9d73ff1

fzhinkin requested a review from sandwwraith July 10, 2026 14:37

Uh oh!

Conversation

fzhinkin commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fzhinkin commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fzhinkin commented May 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hfhbd commented Jul 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

fzhinkin commented May 4, 2026 •

edited

Loading

fzhinkin commented May 5, 2026 •

edited

Loading