Kotlin / kotlinx.collections.immutable

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

PeristentHashMap.values creates a new instance of `PersistentHashMapValues` on each call #153

Open develar opened 8 months ago

develar commented 8 months ago

I don't know if it is an oversight or not, but fastutil/JDK caches computed values.

It is important for IntelliJ IDEA — PersistentMap.values called quite often.

I realize that modern JDK can optimize it, but it is still unclear to me whether it was benchmarked / by intention.

Screenshot 2023-10-24 at 18 03 19
qurbonzoda commented 7 months ago

Hi, This is done intentionally. The persistent collections are also immutable, and their internal state never changes after initialization. Therefor, caching is not possible. Since PersistentMap.values/keys/entries returns a lightweight object, it was decided not to store it within the PersistentMap.

Thanks for the feedback! We really do appreciate it.

qwwdfsad commented 5 months ago

I would suggest re-opening the issue.

It might be the case that values reading is performance-sensitive and expected to be GC-free. If we see that it's actually the case (e.g. any empirical evidence or a benchmark that it's a visible overhead), we'll reconsider that.

qurbonzoda commented 4 months ago

When considering the introduction of backing fields for the entries, keys, and values properties, an important factor should be taken into account. A persistent map is recreated with each mutation, and it is a common practice to "modify" an initial map repeatedly until reaching its final version. This process results in numerous intermediate, temporary maps, each incurring additional memory overhead. Therefore, special attention must be paid to minimizing this overhead.