Fix ksrpc-bench compile rot and guard it in CI (#242) - #244
Conversation
JniCommunicationBenchmark called a stale 3-arg JniConnection constructor plus NativeHost.createEnv()/registerService() — externals with no native backing, left over from before the JNI host path was reworked (#204/#209). This broke `:ksrpc-bench` compilation, unnoticed because no CI job compiles the module. Fix the benchmark to use the current API — KsrpcNativeHost.connect(scope, TestNativeHost::initialize, env) — exactly as the ksrpc-test JNI tests do. This registers the default TestJniImpl service, so the benchmarked `/rpc` round-trip is unchanged (only the untimed @setup path moves to the supported entry point). Replace the dead NativeHost scaffolding with a TestNativeHost binding. Verified: the JMH benchmark builds and runs green against the linuxX64 JNI library (~92-94 ops/s across payload sizes, dispatch-bound, no errors). Also add a compile-only `:ksrpc-bench:compileKotlinJvm` step to the jvm-tests CI job so the benchmarks can't silently rot again (JMH is still never run in CI). Fixes #242. Co-Authored-By: Claude Opus 4.8 <[email protected]>
monkopedia-reviewer
left a comment
There was a problem hiding this comment.
Reviewed and approved.
Fix is correct: KsrpcNativeHost.connect(scope, TestNativeHost::initialize, env) is the right API — confirmed against KsrpcNativeHost.kt (3-arg form; environment has a default). The @CName("Java_com_monkopedia_ksrpc_TestNativeHost_initialize") in ksrpc-test/src/nativeMain/kotlin/Test.kt resolves the new object TestNativeHost binding correctly (package com.monkopedia.ksrpc, name mangling matches).
NativeHost deletion verified: No remaining references in ksrpc-bench/src. The NativeHost that still exists in ksrpc-test is a separate class with distinct native bindings for low-level JNI tests — unaffected.
Benchmark semantics unchanged: Only the untimed @Setup path changed; the @Benchmark fun jniRpcRoundTrip() is untouched.
CI guard is the right minimal prevention: :ksrpc-bench:compileKotlinJvm in jvm-tests catches future drift without running JMH.
All 5 CI checks green. Code is idiomatic and consistent with surrounding patterns.
Fixes #242.
Problem
:ksrpc-benchdidn't compile.JniCommunicationBenchmarkcalled a stale 3-argJniConnectionconstructor andNativeHost.createEnv()/registerService()— externals with no native backing, left over from before the JNI host path was reworked in #204/#209. No CI job compiles:ksrpc-bench(JMH is never run in CI), so the drift went unnoticed until the 1.1.4 release build compiled the whole tree.Fix
KsrpcNativeHost.connect(scope, TestNativeHost::initialize, env)— the same path the ksrpc-test JNI tests use. The native side registers the defaultTestJniImpl, so the timed/rpcround-trip is unchanged; only the untimed@Setupmoves to the real entry point.NativeHostscaffolding (itsserializeDeserialize/createContinuations/createContinuationRelayexternals were unused by any benchmark) with a smallTestNativeHostbinding.:ksrpc-bench:compileKotlinJvmstep to the jvm-tests CI job so the module can't silently rot again (JMH still not run in CI).Verification
:ksrpc-benchcompiles on all targets (jvm/js/linuxX64); ktlint clean.Built the JMH jar + linuxX64 JNI library and ran the benchmark green (JDK 21):
Quick config (1 fork / 1 warmup / 3 iters). Flat across payload sizes — the round-trip is dispatch/
runBlocking-bound, not payload-bound — with no errors (-foe true). It measures the same operation as before, so no benchmark-semantics regression.Non-published module; no API/ABI impact.