kotlinx-atomicfu icon indicating copy to clipboard operation
kotlinx-atomicfu copied to clipboard

Consider getting rid of common locks and posix interop

Open qwwdfsad opened this issue 2 years ago • 5 comments

Currently, atomicfu provides access to two [on first glance] distinct API sets that are off overall atomicfu direction and are rather legacy of the past that obstruct the reasoning, maintenance and performance work of other libraries:

  1. Interoperability API. https://github.com/Kotlin/kotlinx-atomicfu/blob/master/atomicfu/src/nativeInterop/cinterop/interop.def

This one leaks to public API (due to how interop is implemented in K/N) and creates additional difficulties in train/aggregate/overall maintenance. This interop part can be completely removed and replaced with the corresponding calls from posix.platform package (e.g. platform.posix.pthread_mutex_init()) . We consider this step fully backwards compatible.

  1. kotlinx.atomicfu.locks package. With the rising popularity of multiplatform, atomicfu starts to be [transitively] present in various projects, including other foundational libraries (link, e.g. Ktor), meaning that our quite ad-hoc and tailored for a very specific need API (SyncrhonizedObject) is leaking into public surface. Effectively it means that soon we won't be able to do anything with kotlinx.atomicfu.locks package, sealing its public API.

I propose to start decommissioning it -- starting from warning-level deprecation, eventually raising it to error; in kotlinx.coroutines we can just roll our own, tailored SyncrhonizedObject/ReentrantLock

qwwdfsad avatar Jul 19 '23 08:07 qwwdfsad

It would be nice to do it in 1.9.20 scope, but taking into account, it's a library-level change, it worth returning to it after all Kotlin-repo-related changes are merged. Feel free to split the issue into multiple ones as you see fit

qwwdfsad avatar Jul 19 '23 08:07 qwwdfsad

Note, that other libraries (like Compose multiplatform) also need locks and, AFAIU, currently depend on atomicfu's locks. Thus, deprecating them without providing a replacement will raise questions. Atomicfu locks need to be replaced with the proper (and fast) locks that should be integrated into the Kotlin standard library. They'll definitely have to come to stdlib as a part of wider support for threads in stdlib, but, in reality, locks don't have to wait for threads and can appear earlier.

P.S. Atomicfu locks are slow (which is another reason to redo them anyway) and that is actually causing concerns in Compose multiplatform.

elizarov avatar Jul 19 '23 09:07 elizarov

Indeed. The moment we deprecate it, we'll see the proper demand and will act accordingly, depending on our constraints.

The poor man's [initial] replacement is to copy-paste SyncrhonizedObject.kt into your codebase and call it a day. Even though it sounds terrible, it's still a better option than locking the ecosystem on the ad-hoc underdesigned intrusive (i.e. encourages subclassing) solution.

should be integrated into the Kotlin standard library.

Sure thing. But first we both have to design proper threading abstraction and gather at least the initial requirements for proper locking

qwwdfsad avatar Jul 19 '23 09:07 qwwdfsad

Note about 1: removing previously published cinterop could be incompatible change in dependency resolution: https://github.com/icerockdev/moko-units/issues/81 (not sure, that there is YouTrack issue about this)

whyoleg avatar Jul 19 '23 10:07 whyoleg

Just speculating and for sure don't know all details, but considering that some day in future:

  • locks could be implemented in stdlib
  • there could be some threads support in sdtlib (JVM and Native?)
  • atomicfu-runtime will become almost just simple no-op artifact where all it declarations will be replaces by compiler plugin (where plugin is coupled with compiler version)
  • there is already internal K/N atomic API which is used by atomicfu compiler plugin

May be in that case it make sense to merge atomic(X) declarations from atomicfu into stdlib? If no, it will be a little strange, that stdlib has volatile, locks and threads, but no atomics.

Or another idea to not clutter stdlib with things needed for specific use-cases (threads, locks, etc) may be it's possible to introduce some new kotlinx.concurrent (name just TBD) with all this, plus may be things like ThreadLocal and some concurrent data structures if needed? (similar to what Stately is/was)

whyoleg avatar Jul 20 '23 11:07 whyoleg