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

Expose boxed inline value classes in JVM #394

Open
serras opened this issue Sep 12, 2024 · 9 comments
Open

Expose boxed inline value classes in JVM #394

serras opened this issue Sep 12, 2024 · 9 comments

Comments

@serras
Copy link
Contributor

serras commented Sep 12, 2024

This is an issue to discuss exposing boxed inline value classes in JVM. The current full text of the proposal can be found here.

We propose modifications to how value classes are exposed in JVM, with the goal of easier consumption from Java. This includes exposing the constructor publicly and giving the ability to expose boxed variants of operations.

@daniel-rusu
Copy link

daniel-rusu commented Sep 12, 2024

Exposing Kotlin inline classes to Java prior to the release of Valhalla value classes is dangerous as it will expose a new category of defects regarding their identity. While Kotlin marks these usages as errors preventing compilation, the Java world only does the same for Valhalla value classes as they also don't have object identity.

If inline classes are exposed before Valhalla value classes then the following new categories of defects will be introduced (assume PositiveInt is a Kotlin inline class):

Equality Checks

// Kotlin
class Person(val name: String, val id: PositiveInt)

// Java
if (person1.getId() === person2.getId()) // fails when person1 === person2!

The above fails to compile in Kotlin but this would be allowed in Java (unless it's exposed as a Valhalla value class). Java would auto-box each access so that the reference equality fails even though they come from the same value. Even worse is that if the reference to a Kotlin inline class instance is passed around in the Java world then reference equality checks will pass but will start to fail when crossing the Kotlin-Java boundary such as when passing the reference to a Kotlin function that stores it in a local unboxed variable before passing it to some other Java code that auto-boxes it again. While reference equality checks aren't too common in Kotlin, they are quite common in Java code.

Identity hashCode

// Kotlin
class Person(val name: String, val id: PositiveInt)

// Java
val bob = Person("Bob", 123)

// Use the identity hashCode of an inline class reference for some operation
identityHashCode(bob.getId())

// Use the identity hashCode for some other operation
identityHashCode(bob.getId()) // different identity hashCode even though it's coming from the same `id` "instance".

Synchronization
Similarly to identity hashCode, we can end up with multiple boxed instances in the Java world that all come from the same inline class value so we incorrectly think we're synchronizing on the same object.

// Kotlin
class Person(val name: String, val bankId: PositiveInt, val governmentId: PositiveInt)

// Java
val bob = Person("Bob", 123, 9999)

// synchronize on the bankId for some banking operation
synchronize(bob.getBankId()) { ... }

// perform some other banking operation
synchronize(bob.getBankId()) // different identity hashCode even though it's coming from the same `id` "instance".

For this hypothetical scenario, we want to allow concurrent access when performing unrelated operations so we only synchronize on the bankId when performing banking operations etc.

Conclusion
There are additional categories of defects from other operations that rely on the object identity such as wait / notify, weak references, collections that rely on identity like IdentityHashMap, etc. Java programmers won't always be aware which classes are Kotlin Inline value classes. Even if they did, they likely won't be aware of these new gotchas. If we wait for Valhalla value classes to be released first, Kotlin inline classes could be exposed as value classes in Java preventing Java developers from attempting to use the object identity of these instances.

@JakeWharton
Copy link

I didn't have time to dig in deeply today, but I'll just echo that the timing of this in relation to the forthcoming changes around Valhalla as presented at JVMLS recently is risky. Surely little harm would come from waiting to see what they ship first?

@serras
Copy link
Contributor Author

serras commented Sep 13, 2024

Thanks for the fast feedback! We're indeed monitoring the upcoming changes in Valhalla, but there are a few comments.

The goal of this KEEP is to expose a Kotlin API better to the JVM world, something in which we were lacking. Since right now Kotlin inlines the uses of value class, I think there is a need for this KEEP in ensuring that a "non-inlined" version is exposed.

Once value classes land (and the corresponding JVM version is somehow common, which could take years), this problem does not go away completely, if the inlining mechanism is still in place. One possible future in that case is:

  1. value class without @JvmInline is compiled into a JVM value class;
  2. value class with @JvmInline (which is mandatory now) are inlined, as currently.

The good news is that we can turn the "boxed variants" of the inlined classes also into a value class in a backward compatible way (or so they hinted in JVMLS). In that case would avoid the problems mentioned by @daniel-rusu, which are unfortunately out of reach with the current JVM. I think it's worth adding this note to the KEEP, in fact.

In summary: this KEEP moves the needle a bit into improved compatibility with Java. The best solution would indeed involve value classes, but there is a risk on waiting for that JVM to be common; so I think a good approximation is to expose the boxed variant with the same caveats that exist now, but mark then as value class for newer versions.

@daniel-rusu
Copy link

daniel-rusu commented Sep 14, 2024

I think it's important to mention that the improved performance of inline classes is the only reason they are used since regular classes or data classes provide a better experience in pretty much every way when performance isn't the primary concern. As users, we need to be extra careful with inline classes to avoid auto-boxing otherwise the performance can be worse than regular classes negating the only purpose for their use. Inline classes provide fewer capabilities than regular classes, they add surprises such as JVM conflicts (eg. constructor that accepts inline class vs constructor that accepts underlying value etc.), and introduce restrictions such as avoiding operations that rely on their missing identity. So when inline classes are chosen, companies are choosing to pay the price of increased complexity with the only benefit of gaining performance.

The need to wait for Valhalla value classes to become common seems to be a misconception because that refers to a different target audience. The type of places that use bleeding-edge features like inline classes are the type of places that upgrade to the latest JVM release. After all, inline classes are strictly a performance choice, and upgrading the JVM improves performance aligning with the target audience of inline classes since performance is so important for this audience. The decision should be based on when Valhalla value classes are released on the JVM instead of when that version of the JVM becomes common.

From a design perspective, it doesn't make sense to choose inline classes if we want to expose these to the Java world without project Valhalla. That's because using these values from Java would auto-box them and negate any performance benefit of inline classes likely resulting in worse performance than regular classes (as that would have just passed a reference) and thus nullifying our reason for choosing inline classes in the first place. However, if these were exposed as Valhalla value classes, they could take advantage of Valhalla JVM optimizations so they should continue to have a performance benefit and reason to exist.

Regarding backward compatibility, unlike inline classes, it's my understanding that the Java team plans to enumerate the classes that are intended to be converted to Valhalla value classes (such as the primitive wrapper classes) and add warnings whenever they are used in a future incompatible way like performing operations that rely on their identity. The behavior of some of these operations will change. For example, == will switch from checking referential equality to checking value equality in Java when operating on Valhalla value classes, changing the behavior of existing Java code that performs this operation on these classes. Additionally, some operations will no longer compile when performed on Valhalla value classes. The warnings will steer developers away from performing these operations with value-class candidates. So the Valhalla changes don't appear to be backward compatible and Kotlin will be in a worse state. Luckily for Java, the value class candidates are rarely used in a future-unsafe way because it's rare to compare reference equality of primitive wrapper classes or synchronize on them etc.. A common use-case of inline classes is for storing validated content such as EmailAddress which wraps a String. Given existing patterns of how inline classes are used in Kotlin, using inline classes from Java in a future-unsafe manner will be much more likely as developers won't think of these as they do with primitive wrapper classes but instead treat them as normal classes and rely on their identity. Java developers don't have the concept of inline classes so they likely won't think about these usages in a special way resulting in a poor Kotlin-Java experience.

Regarding compatibility with Java, exposing inline classes to Java without Project Valhalla would provide a worse compatibility experience as it would introduce the large swath of defect categories that I talked about in my first comment. When we choose inline classes, we know that they will meet our performance reasons for choosing them and are fairly safe since the Kotlin compiler helps reduce dangerous usage patterns but these implicit requirements will no longer be valid if they are exposed in Java without the safety and performance of Project Valhalla.

@daniel-rusu
Copy link

daniel-rusu commented Sep 14, 2024

Just a quick note that the fun PositiveInt.duplicate() example in the KEEP appears to fall for the identity traps outlined above. The only reason to duplicate an immutable object is if its identity matters. However, the identity of inline/value classes should never be used or relied on as that will introduce many defects.

On a similar note regarding the first 2 design goals:

The following are some scenarios that we would like to support:

  1. Creating boxed values: for example, creating an actual instance of PositiveInt.
  2. Calling operations that take or return value classes using their boxed representations.

Hopefully these new abilities won't be accessible from Kotlin code. While they are needed by Java to maintain its real type, any usages from Kotlin would only encourage dangerous identity-related defects and also introduce performance issues since the boxed variants will be automatically unboxed by the Kotlin compiler at the first opportunity such as when passing the boxed value to a function that operates on the inline class type etc.

@serras
Copy link
Contributor Author

serras commented Sep 16, 2024

Let me reiterate why I think that, regardless of the upcoming value class support in the JVM, exposing a "non-inlined" version of Kotlin's @JvmInline value class is useful.

People want to have a way to use the best representation they can get in Kotlin, while keeping "good enough" compatibility with other languages in the JVM, especially Java. This can be seen by the many comments in the corresponding YouTrack ticket and the ones related to it.

Right now, if you want to expose your API with a wrapper type and it being accessible in Java, you can use a data class (this always brings risks with forwards compatibility, but in this case this is no worse than a value class).

data class StudentId(val id: Long)
class Student(val id: StudentId, val name: String, ...)

This KEEP amounts to proposing that if you decide to use a value class for StudentId, your API for Java is equivalent to the one above, whereas in Kotlin you get the benefits of inlining. For me that is a net gain.

As I mentioned above, it is possible for us to generate a "real" JVM value class, which shall keep the identity-based behavior on JVMs which do not support the feature. The design document specifies that value classes are distinguished by setting the ACC_VALUE flag, and the JVM spec states that:

All bits of the access_flags item not assigned in Table 4.1 are reserved for future use. They should be set to zero in generated class files and should be ignored by Java Virtual Machine implementations.

@serras
Copy link
Contributor Author

serras commented Sep 16, 2024

On this discussion, I would like to distinguish two different positions which I feel are getting mixed up:

  1. Kotlin should stop doing inline as soon as possible, and switch to JVM value classes,
  2. Kotlin should not have the ability to expose the boxed variant of an inline value class.

On that note, this KEEP gives you the option to expose boxed variants (well, in all truth, exposing the constructor is not optional in the current design, but any operations are). If your target with a library, or your application, uses only Kotlin, you may not want to do this. But again, there are library authors which want this boxed variant to the available, knowing the risks that it entails with respect to identity.

@JakeWharton
Copy link

I was more coming from the angle of that boxing of a value class is an already problematic concern in just Kotlin (as mentioned above), that this is only going to increase that problem since they're generally thought of as a performance optimization but can actually be the opposite.

But if we're just going to say that @JvmInline and its boxing is mostly a short-term hack that we don't care about that much, and at some point a non-@JvmInline value class will become a Valhalla value type available to both languages, then this KEEP doesn't really change anything.

@serras
Copy link
Contributor Author

serras commented Sep 19, 2024

Just to clarify: this KEEP changes nothing about the view Kotlin has of an inline value class. All of the “boxed variants” of callables are only available to Java, the Kotlin compiler hides them in the same way that it hides the boxed class it currently creates.

Update: clarification added in the KEEP text.

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