Kotlin / kotlinx.collections.immutable

Immutable persistent collections for Kotlin
Apache License 2.0
1.12k stars 56 forks source link

Questionable PersistentList time complexity specification #125

Open mcpiroman opened 2 years ago

mcpiroman commented 2 years ago

As far as I see the current implementation of collections, specifically PersistentList, is that of described in the proposal, which said that the complexity of PersistentList is:

To my understanding, an implementation by simple delegation to an array that is copied on each operation would have the following:

This is, the current tire-based approach is only better at appending elements, which is very common operation for sure, but otherwise is either worse or unnecessary complicated compared to naive coping.

Do I understand it correctly? I assume the actual performance, including other aspects, like GC-pressure or data locality, does overweight raw complexity metrics, am I right?

mcpiroman commented 2 years ago

OK, I have read more about bit-mapped trie structure and now I understand that it is, generally speaking, optimal. (Funny enough it appears that I have invented this struct myself, before I read about it).

I suggest (better) documenting the current implementations' performance characteristic, e.g. that O(log32N) is in practice very little, but middle insert and remove are actually worse than O(N) - so much that it's the reason why .NET initially rejected moving from AVL tree to tire implementation.

For the letter I have some ideas how to improve the implementation, however I have a question: Why are all buffers always preallocated with 32 size? My only guess is that it helps runtime reuse the memory region in case it can prove that some buffer is not referenced anymore. Would it be a problem if I sometimes used 33 element buffers?

vlsi commented 1 year ago

set(index, element) is O(N) in case you copy the array on modifications, so it is way worse than the trie