Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add boxed atomics #407

Closed
mgroth0 opened this issue Mar 4, 2024 · 6 comments
Closed

Add boxed atomics #407

mgroth0 opened this issue Mar 4, 2024 · 6 comments

Comments

@mgroth0
Copy link

mgroth0 commented Mar 4, 2024

I am new to this library, and right away I am confronted with unique compiler errors if I try to use atomic in an un-sanctioned way. It needs to be in a constructor or class initialization.

To get around this, I create a wrapper for myself:

class BoxedAtomicInt(
    value: Int
) {
    private val value = atomic(value)
    fun getAndIncrement() = value.getAndIncrement()
}

This is a suggestion to add classes such as BoxedAtomicInt directly into the atomicfu library itself.

As a newbie here, I am sure this is been brought up already. But searching through the issues and README, I couldn't find a discussion.

I understand that boxing has a performance cost. However, it seems unboxed atomic introduces novel restrictions on the compiler level that reduces the flexibility of their usage. If all of these restrictions are all in the name of a performance boost, I am not sure this is justified. After all, the performance boost will have practically zero benefit in many applications.

I understand that a user of this library can create their own boxes, as I will. But why should they have to? How does this benefit the kotlin world for every usage of this library to write their own set of wrapper classes? Especially since this library is called "the idiomatic way", I am surprised that it is considered idiomatic to have unboxed atomics with very complicated restrictions even in cases where performance doesn't matter.

@recursive-rat4
Copy link

Atomicfu has two components: library and plugin. Library provides boxed types, and the plugin rewrites code. You could try to add dependency on library, but skip the plugin.

@fzhinkin
Copy link
Contributor

However, it seems unboxed atomic introduces novel restrictions on the compiler level that reduces the flexibility of their usage.

@mgroth0 could you please elaborate on your use cases that currently suffer from atomic<T> being a primitive-typed field?

@mgroth0
Copy link
Author

mgroth0 commented Mar 15, 2024

@recursive-rat4 Thank you for the suggestion. Sorry, but I am confused and need to ask you for some explaination.

I have reviewed the README again, and I do not see any mention of the technique you are suggesting. I am confused how it would work.

Let me make sure I understand. You are suggesting I do the following:

  • add a regular dependency on org.jetbrains.kotlin:atomicfu
  • Do not apply the gradle plugin org.jetbrains.kotlinx:atomicfu-gradle-plugin
  • Create an instance of kotlinx.atomicfu.AtomicInt

Now on JVM, this is implemented with:

	private val FU = AtomicIntegerFieldUpdater.newUpdater(AtomicInt::class.java, "value")

The README here confuses me. It says:

To provide a user-friendly atomic API on the frontend and efficient usage of atomic values on the backend kotlinx-atomicfu library uses the compiler plugin to transform IR for all the target backends:

JVM: atomics are replaced with java.util.concurrent.atomic.AtomicXxxFieldUpdater

However, this is confusing because I already see AtomicIntegerFieldUpdater directly inside of the regular library. And if I use something such as getAndAdd, I can see in the implementation:

    /**
     * Maps to [AtomicIntegerFieldUpdater.getAndAdd].
     */
    public actual fun getAndAdd(delta: Int): Int {
        val oldValue = FU.getAndAdd(this, delta)
        if (trace !== None) trace { "getAndAdd($delta):$oldValue" }
        return oldValue
    }

So it seems to me that FU is used normally, without any compilation magic. Yet the README says that that plugin "replaces" normal implementation with FU-based implementation. What is this "default" behavior then that is being replaced? Because it seems to already be FU-based implementation?

@mgroth0
Copy link
Author

mgroth0 commented Mar 15, 2024

@fzhinkin here is my issue.

I start with this:

fun main() {
    val obj = MyObject()
    obj.next(1)
}

class MyObject {
    private val counters =
        List(10) {
            java.util.concurrent.atomic.AtomicInteger()
        }
    fun next(index: Int): Int = counters[index].getAndIncrement()
}

The buisness logic isn't really important (unless that is what you really are asking about?). But for whatever reason, I create boxed atomics like this in a jvm only object. My goal now is to make my object multiplatform in an idiomatic way. So as a newbie to this library my first attempt is:

class MyObjectNowMultiplatform {
    private val counters =
        List(10) {
            kotlinx.atomicfu.atomic(0)
        }
    fun next(index: Int): Int = counters[index].getAndIncrement()
}

But then I get a variety of errors. For example:

kotlinx.atomicfu.transformer.TransformerException: factory kotlinx.atomicfu.AtomicFU::atomic invocation must be followed by putfield

So then, based on the instructions on the explaination in the README that I need to strictly control how atomics from this library are defined, I try creating my own box:

class MyObjectNowMultiplatform {
    private val counters =
        List(10) {
            BoxedAtomicInt(0)
        }
    fun next(index: Int): Int = counters[index].getAndIncrement()
}


class BoxedAtomicInt(
    value: Int
) {
    private val value = kotlinx.atomicfu.atomic(value)
    fun getAndIncrement() = value.getAndIncrement()
}

And this works perfectly.

The reason I created this issue then, is specifically because I don't really understand why this library doesn't provide classes like BoxedAtomicInt itself so that we can just directly replace things like java.util.concurrent.atomic.AtomicInteger without having to make major changes or write our own box classes.

And now after reading the comment from @recursive-rat4 and taking another look at the readme and the implementation of kotlinx.atomicfu.AtomicInt, I am beginning to understand that kotlinx.atomicfu.AtomicInt is essentially already boxed, but that it is the plugin that un-boxes them and causes users to have these restrictions for the sake of better performance. Am I understanding this correctly?

I tried doing what @recursive-rat4 suggested, and depended on org.jetbrains.kotlin:atomicfu directly but I am not seeing any classes in there without applying the gradle plugin.

@fzhinkin
Copy link
Contributor

@mgroth0 thank you for elaborating!
Speaking of your particular case with counters, I believe the "arrays of atomic values" feature may cover your needs: https://github.com/Kotlin/kotlinx-atomicfu?tab=readme-ov-file#arrays-of-atomic-values:

import kotlinx.atomicfu.AtomicIntArray

class MyObjectNowMultiplatform {
    private val counters = AtomicIntArray(10)
    fun next(index: Int): Int = counters[index].getAndIncrement()
}

And now after reading the comment from @recursive-rat4 and taking another look at the readme and the implementation of kotlinx.atomicfu.AtomicInt, I am beginning to understand that kotlinx.atomicfu.AtomicInt is essentially already boxed, but that it is the plugin that un-boxes them and causes users to have these restrictions for the sake of better performance. Am I understanding this correctly?

That's correct.

I tried doing what @recursive-rat4 suggested, and depended on org.jetbrains.kotlin:atomicfu directly but I am not seeing any classes in there without applying the gradle plugin.

Something like that should indeed do the trick:

//build.gradle.kts
kotlin {
    sourceSets {
        val commonMain by getting {
            dependencies {
                implementation("org.jetbrains.kotlinx:atomicfu:0.23.2")
            }
        }
   }
}
// some.source.file.kt
import kotlinx.atomicfu.AtomicInt
import kotlinx.atomicfu.atomic

val x: AtomicInt = atomic(0) 

Could you please share your build config to check why classes are not available to you?

@mgroth0
Copy link
Author

mgroth0 commented Mar 15, 2024

Ok, I just got it to work the way I expect finally.

The issue ended up being that I used org.jetbrains.kotlin:atomicfu when what I should have been using was org.jetbrains.kotlinx:atomicfu.

Thank you both for the help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants