pjfanning opened a new pull request, #863: URL: https://github.com/apache/pekko-grpc/pull/863
### Motivation `GrpcMarshalling.unmarshal` on a strict entity, `unmarshalStream`, the negotiation failure path, `GrpcExceptionHandler.from` and the `ServiceHandler` `notFound` / `unsupportedMediaType` responses all produce futures that are already completed by the time they are returned, but they were built with `Future.successful` / `Future.failed` / `Future.fromTry`. Every transform a handler chains onto one of those is then scheduled on an `ExecutionContext`, even though the value is already available. Pekko HTTP's `FastFuture` exists for exactly this: `FulfilledFuture` / `ErrorFuture` override `transform` and `transformWith`, so plain `.map` / `.flatMap` / `.recoverWith` on them run directly on the calling thread and return another already-completed future, rather than going through the dispatcher. This is the `FastFuture` part of #862, split out on its own so it can be reviewed and merged independently of the larger unary handler rework in that PR. ### Modification Cherry-picked `Use FastFuture for completed futures.` from #862 (authored by @timw) unchanged — `FastFuture.apply` / `successful` / `failed` in place of the `Future` equivalents in `GrpcMarshalling`, `GrpcExceptionHandler`, `ServiceHandler`, `PekkoHttpClientUtils`, `RequestBuilderImpl` and `HardcodedServiceDiscovery`. On top of that: - New `runtime/src/test/scala/org/apache/pekko/grpc/scaladsl/GrpcMarshallingSpec.scala`. Each test asserts both that the future is already completed (`.value` is defined) and that a transform chained onto it still completes when given an `ExecutionContext` that throws if anything is scheduled on it. Covers `unmarshal` of a strict entity (success and parse failure), `unmarshalStream`, `GrpcExceptionHandler.from`, and `ServiceHandler.notFound` / `unsupportedMediaType`. - New `GrpcMarshallingBenchmark.unmarshallStrictChained`, which measures `unmarshal(strict).flatMap(...).map(...)` — the chain shape a generated unary handler builds — rather than `unmarshal` alone. - New `ScalaUnaryHandlerBenchmark.oldStyleUnaryStrictRequestProcessingFastImplementation`. `BenchmarkGreeterService` now takes the response `Future`, so the same handler can be benchmarked against a service returning an ordinary `Future.successful` (what user code normally does) and one returning `FastFuture.successful`. No public API, binary shape or wire format changes; `mimaReportBinaryIssues` is clean. ### Result The measurable effect depends on what the service implementation returns, and the two benchmark variants are there to make that explicit rather than report one flattering number. A chain stays direct only while every step returns an already-completed fast future. `unmarshal(...).flatMap(impl)` now runs `impl` inline, but if `impl` hands back a plain `Future.successful` — a `KeptPromise` — the following `.map(marshal)` and `.recoverWith(...)` are scheduled as before. So a typical service saves one dispatch out of three; a service that returns `FastFuture.successful` keeps the whole chain inline. Intel Core i7-4770HQ, macOS 12.7.6, Temurin 17.0.19, `-f 2 -wi 5 -i 5`. Both columns run with the benchmark sources from this PR, differing only in the runtime sources. | Benchmark | before (ops/s) | after (ops/s) | | --- | --- | --- | | `GrpcMarshallingBenchmark.unmarshallStrict` | 13,765,653 ± 2,221,933 | 16,054,190 ± 944,574 | | `GrpcMarshallingBenchmark.unmarshallStrictChained` | 91,705 ± 17,593 | 97,672 ± 4,849 | | `ScalaUnaryHandlerBenchmark.generatedUnaryStrictRequestProcessing` | 109,880 ± 28,583 | 112,235 ± 7,581 | | `ScalaUnaryHandlerBenchmark.oldStyleUnaryStrictRequestProcessing` | 47,765 ± 11,038 | 48,860 ± 12,315 | | `ScalaUnaryHandlerBenchmark.oldStyleUnaryStrictRequestProcessingFastImplementation` | 51,448 ± 4,540 | 103,650 ± 24,119 | Reading that honestly: - With a service returning `FastFuture.successful`, the old-style unary handler roughly doubles (51k -> 104k ops/s). That is the real win, and it applies to existing generated code without regeneration. - With a service returning a plain `Future.successful`, the improvement is inside the noise on this hardware. One saved dispatch is small next to the `Await` park in the benchmark harness. - Nothing regresses. `unmarshallStrict` and `unmarshallStrictChained` also become noticeably more stable run to run (the error bars shrink), which is consistent with dispatches being removed. Worth noting for anyone comparing against the numbers in #862: the `ScalaHandlerBenchmark` there changes `BenchmarkGreeterService.sayHello` to return `FastFuture.successful(response)`, so its old-style handler figures are measuring the second row of that list, not the first. ### Tests - `sbt runtime/test` - 139 succeeded, 0 failed - New `scaladsl/GrpcMarshallingSpec` is directional: 5/5 tests fail against `main`'s runtime sources, 5/5 pass with this change - `sbt checkCodeStyle` - success - `sbt +headerCheckAll` - success (via `headerCreateAll`, no changes) - `sbt grpcVersionSyncCheck googleProtobufVersionSyncCheck mimaReportBinaryIssues` - success, no binary compatibility problems - `sbt "benchmarks/Jmh/run -f 2 -wi 5 -i 5 org.apache.pekko.grpc.ScalaUnaryHandlerBenchmark org.apache.pekko.grpc.GrpcMarshallingBenchmark.unmarshallStrict"` - results above - Not run: Gradle, Maven and sbt scripted suites - no plugin or codegen changes ### References Refs #862 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
