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

Common Atomics and Atomic Arrays #398

Open
mvicsokolova opened this issue Nov 7, 2024 · 12 comments
Open

Common Atomics and Atomic Arrays #398

mvicsokolova opened this issue Nov 7, 2024 · 12 comments

Comments

@mvicsokolova
Copy link

mvicsokolova commented Nov 7, 2024

This issue is for discussion of the proposal to introduce Common Atomic and Atomic Array types in the standard library.
The full text of the proposal is here: proposals/common-atomics.md.

The KEEP presents the proposed API for Common Atomics and Atomic Arrays and focuses on the implementation options for the JVM backend.

PR: #397

@kyay10
Copy link

kyay10 commented Nov 7, 2024

I want to expand a little more on "why not value class". You mention that the objects will have no identity, but I don't see the issue with that. Couldn't we just have the value class's equals method check equality of underlying Java atomic reference? I think that equality for atomics only makes sense as reference equality. In other words, I'd be very surprised if AtomicInt(0) == AtomicInt(0)
I'm also unclear as to what this means:

Loosely defined different value semantics on different platforms

and for the point:

Value classes are not interoperable with Java, e.g., signatures of functions with inline classes get mangled

There's 2 solutions. One can use @JvmName to stop the mangling (which makes sense in an interop situation. #393 might be a potential solution as well.

@kyay10
Copy link

kyay10 commented Nov 7, 2024

I think the atomics should also come with an assign method by default that works with the assign plugin

@rnett
Copy link

rnett commented Nov 7, 2024

Streamlining the kotlinx-atomicfu machinery to Kotlin is an explicit next step of this proposal and a subject of the next KEEP after atomics stabilization.

Does this mean that atomicfu would become part of the Kotlin compiler and stdlib (with one of the solutions mentioned) rather than an external plugin?

In a similar vein, are there longer term plans for multiplatform Kotlin concurrent collections (i.e. ConcurrentHashMap, maybe even ConcurrentSkipListMap like classes)?

@mvicsokolova
Copy link
Author

@kyay10 thank you for your comment!

On implementing JVM atomics as value classes:

  • Referential equality operator (===) is not defined for value classes by design, comparing AtomicIntInline(0) === AtomicIntInline(0) would lead to a compilation error. While for other platforms (Native/Js/Wasm) atomics can be compared by reference.

  • On function mangling:

@JvmInline
value class AtomicIntInline(val a: AtomicInteger)

fun foo(x: AtomicIntInline) {...} // calling this function from Java results in an resolution error, since it's name is mangled by the compiler

Marking foo with a @JvmName annotation would allow calling foo from Java, though a user would have to add this annotation manually, which does not look like a good solution.

@JvmExposeBoxed described in #393 could become a solution for this issue, it would expose a boxed version of foo:
public foo(other: AtomicIntInline)

@kyay10
Copy link

kyay10 commented Nov 7, 2024

My point about referential equality is that it's unnecessary since normal equality will suffice (it'll compare the underlying references in case of JVM). In other words, I'd expect that AtomicIntInline(0) == AtomicIntInline(0) is false, and hence that I can use regular equality on all platforms (so that Native and JS aren't "losing out" on reference comparisons, they just have to use == in place of ===)

WRT function mangling, maybe we want a variant of #393 that exposes the underlying type instead of a boxed type, so that one can basically say "my value class is morally the same as its underlying type, I'm just trying to have a different API than its underlying type, but Java users don't need to know about any of that".
Exposing the boxed types is fine IMO too, Java users will just have to deal with converting it.

@mvicsokolova
Copy link
Author

mvicsokolova commented Nov 8, 2024

@rnett yes, the plan is to turn kotlinx-atomicfu into a compiler plugin, which would provide optimization by inlining stdlib atomics marked with some special @InlineAtomic annotation or e.g. created using a factory function atomic(value: Int) (the final solution has not yet been designed).
At that stage the library will be removed.

@mvicsokolova
Copy link
Author

Some more thoughts on implementing atomics as value classes on JVM:

  • On JVM, a value class wrapping a Java atomic would introduce an extra layer of boxing when e.g. a list of atomics is created.
  • If we implement atomics as value classes on JVM, atomics should be value classes on all other backends as well (currently, expected and actual declarations do not allow this distinction, and changing this would require some hacks in the compiler). And for K/N value classes would not work, because an atomic could only be passed to a function as a copy of the wrapped value, not the reference, which would allow to update the value atomically.

@lppedd
Copy link

lppedd commented Dec 5, 2024

Re. JS, the KEEP says "implementation will be single threaded", which is fine, but will the atomic usages be inlined?
For example, wrapping a number into an AtomicInt class would mean an unnecessary level of indirection.

@lppedd
Copy link

lppedd commented Dec 6, 2024

Introduce a special annotation (e.g. @InlineAtomic) to mark the atomics, which a user wants to inline. Then the atomicfu compiler plugin checks constraints for those atomics and inlines them.

And from Kotlin/kotlinx-atomicfu#493

Users will have control over which atomics to inline by applying a dedicated annotation, such as @InlineAtomic

Wouldn't it be better from a DX standpoint to always try to inline, but allow a consumer to opt-out in case the compiler reports a constraint violation? E.g. with a @NoInline annotation.

@mvicsokolova
Copy link
Author

Wouldn't it be better from a DX standpoint to always try to inline, but allow a consumer to opt-out in case the compiler reports a constraint violation? E.g. with a @NoInline annotation.

@lppedd that's actually a good point for discussion, thank you!
A little bit later I'll create an issue in kotlinx-atomicfu repo with the description of this new optimization compiler plugin in more details: the scope of features, support of JS, and with the options of the user interface (@InlineAtomic or @NoInline annotation) etc.

@mvicsokolova
Copy link
Author

Re. JS, the KEEP says "implementation will be single threaded", which is fine, but will the atomic usages be inlined?

Yes, inlining of kotlin.concurrent atomics for JS will be considered in the new light atomicfu compiler plugin.

@lukellmann
Copy link
Contributor

The documentation (and implementation) for compareAndSet and compareAndExchange of AtomicReference and compareAndSetAt and compareAndExchangeAt of AtomicArray seems to be inconsistent across platforms.

The documentation for Common and Native says this:

Comparison of values is done by reference.

But the documentation for JVM, JS and Wasm says this:

Comparison of values is done by value.

For JVM this seems to be wrong, the API specification mentions == (the reference equality operator in Java).

For JS and Wasm, these methods are indeed implemented using ==/!= (the value equality operators in Kotlin, which calls equals). Is this intended?

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

6 participants
@kyay10 @lppedd @rnett @lukellmann @mvicsokolova and others