support jdk9 forkjoinpool maximum-pool-size

[pjfanning]

• relates to support creating ForkJoinPools with maximumPoolSize #483
• see maximumPoolSize description in ForkJoinPool javadocs
• this fork-join-executor.maximum-pool-size config has no effect if you use Java 8 but has an effect if you use a newer JDFK.
• the default value works like the Java default (a large value is treated as saying that you want a max pool size of max-parallelism + 256
• you can override this if you want a lower max

[He-Pin]

Maybe add a util to read the current jdk version instead, try pekko.util.JavaVersion

[pjfanning]

because of the way Pekko builds the Java9+ classes, even the unit tests seem not to have this class available. It does eventually get built and appears in the jar.

[He-Pin]

private lazy val pekkoJdk9ForkJoinPoolHandleOpt: Option[MethodHandle] = {
     if (JavaVersion.majorVersion >= 9) {
      pekkoJdk9ForkJoinPoolClassOpt.map { clz =>
        val methodHandleLookup = MethodHandles.lookup()
        val mt = MethodType.methodType(classOf[Unit], classOf[Int],
          classOf[ForkJoinPool.ForkJoinWorkerThreadFactory],
          classOf[Int], classOf[Thread.UncaughtExceptionHandler], classOf[Boolean])
        methodHandleLookup.findConstructor(clz, mt)
      }} else None
    }

how about this?

[pjfanning]

Could you try this yourself? I explained above that the build is weird - adding the java 9+ classes very late. After the unit tests run. Meaning we need to not rely on the classes being there. This change will fail multiple tests.

[pjfanning]

Looking at this again on my phone - it will probably be ok. Looks the same as existing code with an additional jdk version check. The existing code works - it just relies n checking if a class loads ok.. In the full lifetime of the JVM we will try to class load once.. Java 8 users who only ever create one dispatcher will see a small perf boost. Everyone else will see a perf decrease because of the extra if check on every dispatcher create call.

[pjfanning]

I added the java-version check because it happens inside the lazy val calculation - see we won't check this over and over.

[He-Pin]

We are using Pekko now, and all of our nodes are running on Java 21, I would like to see this feature be shipped in 1.1.x.

[mdedetrich]

We are using Pekko now, and all of our nodes are running on Java 21, I would like to see this feature be shipped in 1.1.x.

I don't think this is possible to do properly without multi-release-jar which is hitting problems

[He-Pin]

We are using Pekko now, and all of our nodes are running on Java 21, I would like to see this feature be shipped in 1.1.x.

I don't think this is possible to do properly without multi-release-jar which is hitting problems

Why? Netty can support multi jdk just with some feature tags.

If that method is not called, no problem, we can built a release with jdk 11.

[pjfanning]

This code can be written using Java reflection. The PR as it stands uses Java reflection.

[He-Pin]

I think we should go with methodHandle, this can be done whiteout the multi release jar support.

I did something like that in #666

This feature will make our 1.1.0 release shiny

[He-Pin]

This code can be written using Java reflection. The PR as it stands uses Java reflection.

Better with method handle, on Java 17+, we need an addition add opens, the less reflections the better,we should move on prepare for future Java releases.

[pjfanning]

This code can be written using Java reflection. The PR as it stands uses Java reflection.

Better with method handle, on Java 17+, we need an addition add opens, the less reflections the better,we should move on prepare for future Java releases.

I've changed the PR to use a MethodHandle.

[pjfanning]

@mdedetrich He Pin wants this in v1.1.0. I'm neutral on this. Is it ok to use MethodHandles like this and rewrite later when we have Multi-Release jar support?

[He-Pin]

@pjfanning I want this because it will address production runtime issue especially when user is blocking the actors.

We have something like this in our production code with Virtual Thread too, If you want to defer this to 1.2.x/ 2.0.x that's fine, but this will be a killer feature in 1.1.x.

[He-Pin]

lgtm at the first round , just a little nitpick , I think we should enable run test on JDK 21 too, if the code is target to higher JDK version.

[pjfanning]

lgtm at the first round , just a little nitpick , I think we should enable run test on JDK 21 too, if the code is target to higher JDK version.

We do test with java 21. Check the nightly builds

[He-Pin]

The code is well done, and the last part should be update the document, that can come up later too.

[mdedetrich]

I would be more comfortable putting this in 1.1.0-M2 or maybe 1.2.0 as we are already overloaded with changes and we have just created a regression with JDK

[pjfanning]

@He-Pin out of interest, is there a reason why you can't create a custom dispatcher that supports java9+ fork-join-pool? Such an approach would work today, no reason to wait for a Pekko 1.1 release.

[He-Pin]

@He-Pin out of interest, is there a reason why you can't create a custom dispatcher that supports java9+ fork-join-pool? Such an approach would work today, no reason to wait for a Pekko 1.1 release.

1. Custom dispatcher is much change for most crud available Java boies/girls.
2. Out of box solution is better, my systems is full async, but not all.
3. This is a demand from the pekko community, not by me.

I'm fine to postpone this to later version eg 1.1.0-m2 or m3 , and I can help to test it.

@mdedetrich @pjfanning feel free change it to 1.1.0-m2

[mdedetrich]

@He-Pin Argument would be a lot more convincing if creating a JDK 9 ForkJoinPool would be impossible which is not the case. Although having it default to the better version of JDK9+ would be nice, its also not a deal breaker and we can also document this as a stop gap.

The reason I am hesitant is that the current Pekko build with all of the JDK9 classes, multi-jvm etc etc is a bit of a mess, note that I am NOT blaming anyone specifically here, there are legitimate reasons for this, i.e. Akka was a really old sbt project that had to implement various workarounds which are no longer needed but those workarounds were still left in the code and then there is the case of working around even current day sbt limitations which is what is causing the sbt-osgi/sbt-multi-jar problems.

In short I would like to start devoting actual effort to cleaning up the sbt build which is something that I have already initiated rather than dumping additional features that involve non trivial changes to the sbt build which then end up causing regressions (either at the start or later down the track due to how the sbt build interactions work). sbt-osgi is a good example of this, I took initiate/responsibility to get it working and as you can see we are still getting problems 3-4 months down the track.

We need to be more strategic/intelligent about what features we push and for this feature specifically I would prefer if we actually put it on hold until I figure out and then stabilize all of the sbt-multi-jar/sbt-multi-jvm/sbt-osgi interactions.

I know that the feature is important to others, but we need to prioritize the stability of the project and Pekko 1.1.x doesn't need to be perfect/have every feature we want, no one wins if we burn OS developers in trying to achieve that. To put things into perspective, this build complexity/maintenance burden was actually one of the reasons why Lightbend ended up making Akka commercial, their engineers couldn't handle all of the effort that was required so we need to be wary of that.

I am doing my best to simplify the Pekko build so we don't have these same issues that Akka maintainers were dealing with but I can't really do that while we keep on dumping more high priority features that require significant changes to the sbt build.

[He-Pin]

Yes, I want to polish the toolchain too. WDYT, a document still needs to be done to ship this. but the code should be mostly ready.
Let's delay it to 1.2.x. That will make our life easier because we must fix the toolchain first.

[mdedetrich]

If 1.1.0-M2 happens and it takes a long time because of other reasons then it can still land for 1.1.0 but to make it clear, we shouldn't delay 1.1.0 just for this feature.

I would also prefer to implement this properly using multi-jar-release rather than reflection.

[He-Pin]

That's true, I expect 1.1.0-M1 to be released soon, and I will not submit any feature pr before the 1.1.x branch cut.

[pjfanning]

@He-Pin would you support having this in v1.1.0 ?

[He-Pin]

@He-Pin would you support having this in v1.1.0 ?

Yes, I would like to have it ,even in m1.

[pjfanning]

@mdedetrich can we merge this? I can rewrite this in another release to use Multi-Release jar support. The impl is private and controlled solely by config so it is easy to just completely rewrite it. We can document that the current support is experimental.

[mdedetrich]

lgtm, as long as we can improve on it later without breaking backwards compatibility

[Roiocam]

LGTM