Performance degradation due to Kotlin value class checks

[koo-taejin]

Affects: : >= 6.1.2
Related issue : #31698

Since spring-boot 3.2.1, we have noticed a significant performance drop in our application.
A colleague of mine (@koisyu) have tried to figure out the cause.
He had analyzed that loading the Java class every time while invoking getClassifier() from KType was the reason.
I think that if cache these parts, spring-framework can use this feature with similar performance as before.

Steps to reproduce

This is a very simple Kotlin-based controller method that makes it easy to test.

@GetMapping("/message")
fun message(message1: String, message2: String, message3: String): String {
    return "$message1 $message2 $message3"
}

benchmark

• spring-boot 3.2.0

[~/Workspaces/spring]$ hey -n 100000 -c 1 http://localhost:8080/message\?message1\=hello\&message2\=hi\&message3\=thanks

Summary:
  Total:	13.5588 secs
  Slowest:	0.0098 secs
  Fastest:	0.0001 secs
  Average:	0.0001 secs
  Requests/sec:	7375.3008

• spring-boot 3.2.1

[~/Workspaces/spring]$ hey -n 100000 -c 1 http://localhost:8080/message\?message1\=hello\&message2\=hi\&message3\=thanks

Summary:
  Total:	14.9566 secs
  Slowest:	0.1087 secs
  Fastest:	0.0001 secs
  Average:	0.0001 secs
  Requests/sec:	6686.0123

async-profiler

• spring-boot 3.2.0
  

• spring-boot 3.2.1
  

[sdeleuze]

The analysis of the flame graphs confirmed this issue, and I have been able to check on my side that the fix I just pushed bring back the performances to the pre-regression level. Feel free to double check with 6.1.5-SNAPSHOT (once the CI build is finished) and provide a feedback here.

[koo-taejin]

I have checked a lot of improvements.
However, the resource usage of the `getClassifier()' method remains the same.

Because it is registered as AOP, it has a performance impact because it is executed on every call to the suspend method registered with the bean.

• spring-boot 3.2.0
  

• spring-boot 3.2.1 with 6.1.5-SNAPSHOT
  

[sdeleuze]

For more performance optimizations, #21546 will likely be needed so feel free to vote for this issue that I am actively reviewing.

[koo-taejin]

I left it in because I thought it was a change since that issue, rather than an optimization.
This is a very resource intensive part of especially where coroutines are heavily used.
I hope you will consider it someday.

Thank you always for creating a framework that is easy to use. 👍

[sdeleuze]

I have refined the optimizations in order to leverage KTypesJvm#getJvmErasure instead since it seems faster and also refactored a bit the code to be cleaner and more efficient.

@koo-taejin @efemoney Please check 6.1.5-SNAPSHOT against your user cases once this build is finished.

Notice I will likely do shortly another pass of optimization for Coroutines.

[koo-taejin]

Unfortunately, I don't think the situation has changed much.
It would be accurate to put it into production and test it, but I apologize for not being able to do this.

Method of getClassifier() has been replaced by method of getJvmErasure().
I had checked that getClassifier() is called from getJvmErasure().
(in org.springframework.core.CoroutinesUtils.lambda$invokeSuspendingFunction$2(Object[], KFunction, Object, CoroutineScope, Continuation))

• spring-boot 3.2.0
  

• spring-boot 3.2.1 with 6.1.5-SNAPSHOT-p1
  

• spring-boot 3.2.1 with 6.1.5-SNAPSHOT-p2
  

Also, in the latest version, the isSubTypeOf() method uses resources.
(in org.springframework.core.CoroutinesUtils#invokeSuspendingFunction(kotlin.coroutines.CoroutineContext, java.lang.reflect.Method, java.lang.Object, java.lang.Object...))

• spring-boot 3.2.1 with 6.1.5-SNAPSHOT-p1
  

• spring-boot 3.2.1 with 6.1.5-SNAPSHOT-p2
  

This is called for every suspend function, so more use Coroutines, the bigger the impact seems to be.

If there is anything I can do to help, please let me know.

Thank you so much. 🙇

[sdeleuze]

I had another look but I can't really optimize much more without using a cache. I discussed with the Kolin team the possibility to use KType#javaType in order to avoid retrieving the classifier, but that does not work as the unboxed type is returned.

Concerning getJvmErasure you are right, so I have restored getClassifier usage, and have removed 1 or the 3 isSubTypeOf for the most common code path.

I would suggest that we don't go further for 6.1.5 as with #32390 optimizations should limit the performance regression we see with value type checks, or even provide better performances (this is what I see in my benchmarks). If that's not enough, I guess we will need to explore if a cache is worth to introduce, but that if we go that path, that will probably have to wait for 6.2 as some refactoring will be needed to have a single cache for the various use cases.

[koo-taejin]

Thank you for your thoughtful consideration.
I totally agree with you :) 👍