Make use of CompletionStage#handle instead of whenComplete.
#3475
Conversation
There was a problem hiding this comment.
I've checked the implementation of uniWhenCompleteStage and it seems like the issue described in Akka/armeria is only applicable for *Async methods, not to regular ones.
I've also benchmarked the simplest asCompletableFuture and future{} builders and found no difference.
Taking into account that handle is semantically heavier than when* as it returns new value and potentially can change the result of the computation when used carelessly, it would be nice to see a benchmark that shows that this change yields a performance improvement prior to merge.
Code-wise changes looks good
|
@qwwdfsad thanks for the reviewing, you can take a look at this link line/armeria#1444, I will submit an jmh bench later。 |
|
jmh:run -i 5 -wi 5 -f1 -t1 .CompletionStageBenchmark. package bench
import org.openjdk.jmh.annotations._
import java.util.concurrent.CompletableFuture
import java.util.concurrent.CountDownLatch
import java.util.concurrent.TimeUnit
@State(Scope.Benchmark)
@OutputTimeUnit(TimeUnit.SECONDS)
@BenchmarkMode(Array(Mode.Throughput))
class CompletionStageBenchmark {
val ex = new RuntimeException("ex")
@Benchmark
def benchWhenCompleteFailedSync(): Unit = {
val future = new CompletableFuture[Int]()
future.completeExceptionally(ex)
val latch = new CountDownLatch(1)
future.whenComplete((_, _) => latch.countDown())
latch.await()
}
@Benchmark
def benchWhenCompleteSuccessSync(): Unit = {
val future = new CompletableFuture[Int]()
future.complete(1)
val latch = new CountDownLatch(1)
future.whenComplete((_, _) => latch.countDown())
latch.await()
}
@Benchmark
def benchHandleFailedSync(): Unit = {
val future = new CompletableFuture[Int]()
future.completeExceptionally(ex)
val latch = new CountDownLatch(1)
future.handle((_, _) => latch.countDown())
latch.await()
}
@Benchmark
def benchHandleSuccessSync(): Unit = {
val future = new CompletableFuture[Int]()
future.complete(1)
val latch = new CountDownLatch(1)
future.handle((_, _) => latch.countDown())
latch.await()
}
}
|
|
Thanks! For the history, here is the coroutines-related benchmark and the results that are much less noisy: |
|
Please rebase your PR on |
|
Have rebased on to the |
Which leads a better performance.
line/armeria#1440
akka/akka#31283