search

Kotlin / kotlinx.coroutines

Library support for Kotlin coroutines
Apache License 2.0
12.77k stars 1.82k forks source link

Rewrite the implementation of the linked list for `JobSupport` #4095

Open dkhalanskyjb opened 1 month ago

dkhalanskyjb commented 1 month ago

Fixes #3886

One of the steps for #3887

A draft because

dkhalanskyjb commented 1 month ago

@qwwdfsad, could you help with optimizing the memory consumption? Not requesting a proper review yet because I know already the code is messy, I just expect it to get even messier during memory consumption optimization.

dkhalanskyjb commented 1 week ago

For some reason, tests hang after a rebase, exclusively on Wasm/JS.

dkhalanskyjb commented 1 week ago

After some discussions with @igoriakovlev, we pinpointed this to the following behavior that occurs on Wasm/JS with Kotlin 2.0.0 but not with other platforms or Wasm/JS with Kotlin 1.9.24: https://pl.kotl.in/OtO03tnT- The issue occurs on https://github.com/Kotlin/kotlinx.coroutines/blob/70c992653d1a97b594ebf6995d2e46f0620cdf65/kotlinx-coroutines-core/common/src/internal/LockFreeLinkedList.common.kt#L157 Here, CAS never succeeds on Wasm/JS, since identity comparisons on value classes are prohibited and may return undefined results. The straightforward "fix" is to make BrokenForSomeElements a non-value class, but we're currently negotiating about having value classes work with CAS consistently.

This shouldn't block further work on this PR, we can work around this at the last moment if needed.

dkhalanskyjb commented 1 week ago

It's not just value classes; apparently, all types that can be boxed aren't guaranteed to do something sensible with === in multiplatform Kotlin, including primitive types.