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

[4.x] Atomic reference types should use new Java features, if possible #1560

Open
6 tasks
alexandru opened this issue May 9, 2022 · 4 comments
Open
6 tasks

Comments

@alexandru
Copy link
Member

alexandru commented May 9, 2022

Monix exposes Atomic references, as an alternative to using java.util.concurrent.atomic directly. There are several advantages for doing this, such as … reliable cross-platform compilation, a more idiomatic Scala API, and support for all primitive types.

The implementation is primarily based on unsage of sun.misc.Unsafe. Usage of sun.misc.Unsafe is deprecated in newer Java versions and it might go away soon. Monix relies on atomic references for its concurrency (that, plus the queues implemented in JCTools), so if Unsafe goes away, we might be in trouble. Also, Monix goes out of its way to make it easier to avoid the problem of false sharing, which is why we have a PaddingStrategy when initializing atomic references.

There are 2 new additions to the standard library that need to be investigated, and possibly used by default in the future:

  1. VarHandle, as an alternative to using sun.misc.Unsafe or AtomicReferenceFieldUpdater (Since Java 9) ;
  2. The jdk.internal.vm.annotation.Contended annotation, see paper (Since Java 8, but we might need Java 9 or later, due to JEP 261);

Prior art in Monix

The low-level JVM-specific code that Monix uses is in monix-execution/atomic/jvm/src/main/java/. This directory/package has a lot of code repetition b/c these internal classes are specified per Java version and PaddingStrategy.

For example:

  • Right64Java7BoxedLong is meant for Java 7, which does not have getAndSet platform intrinsics, but is based on sun.misc.Unsafe. It also adds 64 bytes of padding to the right of the volatile variable;
  • Left128Java7BoxedLong is the same as above, except it applies 128 bytes of padding to the left;
  • Left128Java8BoxedLong is based on sun.misc.Unsafe AND uses getAndSet platform intrinsics, available since Java 8;
  • Left128JavaXBoxedLong applies 128 bytes of padding to the left + it works with an AtomicLongFieldUpdater (no sun.misc.Unsafe), which makes it compatible with older Android versions or future Java versions that disallow access to sun.misc.Unsafe;

NOTE — knowing what to build happens in the Factory, which makes use of runtime reflection in UnsafeAccess to evaluate if sun.misc.Unsafe or its getAndSet platform intrinsic are available.

Making use of reflection is a standard practice for supporting multiple standard libraries from multiple JVM versions. The alternative would be JEP 238: Multi-release JARs, but this never took off (yet).

TODO

  • The versions meant for Java 7 might be completely irrelevant since we dropped support for Scala 2.11, since we can assume support for Java 8 at least; so maybe drop these implementations entirely;
  • Add a PaddingStrategy.Auto that chooses the best padding strategy, depending on the capabilities of the underlying platform;
  • Use VarHandle if possible and where applicable;
  • Use @Contended if possible and where applicable;
  • Make sure the CI workflows in GitHub Actions tests everything, preferably on top of Java 8, too;
  • Do some benchmarks on Java 11 (via sbt-jmh, which is already in plugins.sbt) — it would be interesting to see if the new implementations are slower than the sun.misc.Unsafe ones, and if we don't have a performance hit, maybe they should be preferred by default; note however that benchmarking is tricky, but it would be a fun exercise regardless;

NOTE — the difficulty of this issue is that VarHandle is available since Java 9. But Monix needs compatibility to start with Java 8.

So the question is — in our CI workflows, how do we compile AND test for Java 8, but also have the ability to access a Java 9's standard library when possible, preferably with tests in CI too?

@alexandru alexandru changed the title [4.x] monix.execution.atomic should be able to use Java's VarHandle [4.x] monix.execution.atomic should be able to use Java's VarHandle + @Contended May 23, 2022
@alexandru alexandru changed the title [4.x] monix.execution.atomic should be able to use Java's VarHandle + @Contended [4.x] monix.execution.atomic should use new OpenJDK features May 23, 2022
@alexandru alexandru changed the title [4.x] monix.execution.atomic should use new OpenJDK features [4.x] Atomic reference types should use new OpenJDK features, if possible May 23, 2022
@alexandru alexandru changed the title [4.x] Atomic reference types should use new OpenJDK features, if possible [4.x] Atomic reference types should use new Java features, if possible May 23, 2022
@hugo-vrijswijk
Copy link

🙋‍♂️ I don't have much experience with Monix, but I saw this on Twitter. One option might be multi-release JARs where different code would be run by different JDK's. It's sort of meant for this kind of issue. I think sbt-missinglink has support for it in sbt, but I've never used it myself.

@alexandru
Copy link
Member Author

@hugo-vrijswijk 👋 I already mentioned the multi-release JARs:

Making use of reflection is a standard practice for supporting multiple standard libraries from multiple JVM versions. The alternative would be JEP 238: Multi-release JARs, but this never took off (yet).

I once saw a plugin for it, but it's not maintained, and it isn't clear to me what it could handle: sbt-multi-release-jar. If multi-release JARs is desirable, the code could be extracted into a pure Java/Maven project, then Monix could depend on it.

Another use-case for multi-release JARs is in supporting newer APIs, such as Java 9's Flow. Not sure if possible, at all, and that's not a concern at the moment.

The easier option, however, and the de facto standard, is to use Java's runtime reflection mechanisms. We already do that, since once upon a time we supported Scala 2.11 with Java 6 (AFAIR), and so we are querying sun.misc.Unsafe to see if it has the Java 8 specific platform intrinsics (getAndSet, incrementAndGet).

In addition to Monix's code that I already highlighted in the description (in particular Unsafe.java and Factory.java), which relies on the standard library being there, an alternative is to use MethodHandle for doing reflection. So, not just for checking if the type exists (like we currently do), but for initiating it and calling methods on it. This would have the virtue that you don't need the standard library of Java 9 to be available at compile time. Like in this PR: typelevel/cats-effect#1782

Reflection may not solve the @Contended issue, though. It's not clear to me if we can include, say, Java 11 classes in JARs compiled with the class-file format of Java 8. I don't know how Java's -target works. And if we can, the other problem would be to test in CI and ensure the project is still usable from Java 8, which might be problematic if Java 11 is used for compilation (multiple Java versions required in a build task).

So there are definitely 2 ways to go forward, but that need research and PoC:

  1. Java reflection (possibly the least painful, but has the most limitations, e.g., I'd be sad if we wouldn't be able to use @Contended);
  2. Depending on a multi-release JAR, built via a pure Java project, if Maven is more mature;
  3. Not doing anything, if we discover that VarHandle doesn't have benefits over AtomicReferenceFieldUpdater, although I'd still like @Contended;

Given @Contended, I'd be interested in exploring the possibility of going with multi-release JAR. But it's definitely not possible with sbt, so pure Java it must be 🤷‍♂️

@CoolDalek
Copy link

CoolDalek commented Jul 13, 2022

If I understand everything correctly - currently monix use longs for padding.
With recent updates of JVM layouter - bytes probably should be preferred for this purpose.
Source: https://shipilev.net/jvm/objects-inside-out/#_field_packing

Also, Contended annotation can't be used because it isn't publicly accessible.

@alexandru
Copy link
Member Author

With recent updates of JVM layouter - bytes probably should be preferred for this purpose.

Yikes. This is a very good observation. Memory layout for that padding should be checked, and possibly corrected, for the latest LTS.

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

No branches or pull requests

3 participants