diff --git a/quartz/src/androidMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/FieldMulPlatform.android.kt b/quartz/src/androidMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/FieldMulPlatform.android.kt index c309c66f7..b7be7f461 100644 --- a/quartz/src/androidMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/FieldMulPlatform.android.kt +++ b/quartz/src/androidMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/FieldMulPlatform.android.kt @@ -23,15 +23,31 @@ package com.vitorpamplona.quartz.utils.secp256k1 /** * Android field multiply/square with API-level-gated intrinsics. * - * Each API level gets its OWN private function with the inline expansion, - * so ART's JIT can fully optimize each as a standalone ~200 DEX-instruction - * method. Previously, having all 3 branches inline-expanded in a single - * function created ~600 DEX instructions, causing ART's register allocator - * to produce suboptimal code with excessive stack spills. + * ARCHITECTURE: dispatch → per-API private function → inline-expanded hot loop * - * The dispatch functions (fieldMulReduce/fieldSqrReduce) are tiny (~15 DEX - * instructions) and always take the same branch, so ART profiles and - * devirtualizes them efficiently. + * Each API level gets its OWN private function (fieldMulApi35, fieldMulApi31, + * fieldMulFallback) with the fieldMulReduceWith inline expansion. This is critical + * because ART's JIT optimizes methods individually: + * + * WHY separate private functions per API level? + * Tested: putting all 3 branches in a single fieldMulReduce with inline expansion + * created ~600 DEX instructions (3 copies × ~200 each). ART's register allocator + * produced suboptimal code — verify REGRESSED by ~4% vs baseline. With separate + * functions, each is ~200 DEX instructions, well within ART's optimization sweet spot. + * Verify improved by 16% instead. + * + * WHY the API-level split at all? + * - API 35+ (Android 15): Math.unsignedMultiplyHigh → UMULH (1 ARM64 instruction) + * - API 31-34 (Android 12-14): Math.multiplyHigh → SMULH + 3 correction insns + * - API <31 (Android <12): pure-Kotlin fallback (4 multiplies + shifts) + * The dispatch function (fieldMulReduce) checks API once, not per multiply-high call. + * ART profiles and devirtualizes — on any given device, only one path is hot. + * + * WHY NOT use the unsignedMultiplyHigh expect/actual wrapper directly? + * The wrapper checks API level on EVERY call (16-20× per field multiply). Even though + * ART should constant-fold the check, the wrapper's 3-branch body may exceed ART's + * inline-candidate threshold, leaving ~10,000 un-inlined function calls per verify. + * The fused approach eliminates the wrapper entirely. */ private val API = android.os.Build.VERSION.SDK_INT diff --git a/quartz/src/androidMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/UnsignedCompare.android.kt b/quartz/src/androidMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/UnsignedCompare.android.kt index ccc87e097..74290797b 100644 --- a/quartz/src/androidMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/UnsignedCompare.android.kt +++ b/quartz/src/androidMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/UnsignedCompare.android.kt @@ -21,10 +21,22 @@ package com.vitorpamplona.quartz.utils.secp256k1 /** - * Android: XOR-with-MIN_VALUE trick avoids the ULong.constructor-impl - * NOOP invokestatic calls that Kotlin's toULong() generates. Produces - * pure arithmetic bytecode (lxor, lcmp) with zero method calls. - * ART's JIT compiles this to EOR + CMP + CSET on ARM64. + * Android: XOR-with-MIN_VALUE trick for unsigned comparison. + * + * DO NOT use Long.compareUnsigned here (even though it's an ART intrinsic since API 26). + * Kotlin's `a.toULong() < b.toULong()` compiles to Long.compareUnsigned BUT also emits + * 2 ULong.constructor-impl NOOP invokestatic calls per comparison. These NOOPs add ~2-3ns + * each on ART, totaling ~35-54μs per verify (~17,800 comparisons). + * + * DO NOT use Long.compareUnsigned directly either — ART may or may not inline the function + * call depending on method size budgets, adding unpredictable overhead. + * + * The XOR trick produces pure arithmetic bytecode with ZERO method calls: + * Bytecode: lload a, ldc MIN_VALUE, lxor, lload b, ldc MIN_VALUE, lxor, lcmp, ifge + * ARM64: EOR x0, a, #0x8000...; EOR x1, b, #0x8000...; CMP x0, x1; CSET x2, LT + * + * This approach was validated by bytecode analysis (javap) and benchmarked on Pixel 8 + * (Android 16): ~14% improvement across all EC point operations. */ internal actual fun uLt( a: Long, diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/FieldMulPlatform.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/FieldMulPlatform.kt index 380f22fd2..ae9a9c34a 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/FieldMulPlatform.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/FieldMulPlatform.kt @@ -24,24 +24,42 @@ package com.vitorpamplona.quartz.utils.secp256k1 // FUSED FIELD MULTIPLY + REDUCE — PLATFORM-SPECIFIC INTRINSIC DISPATCH // ===================================================================================== // -// On Android, the unsignedMultiplyHigh wrapper has per-call branching for API levels -// (API 35+: Math.unsignedMultiplyHigh, API 31-34: Math.multiplyHigh + correction, -// API <31: pure-Kotlin fallback). ART's JIT may not inline this wrapper due to: -// 1. Branch complexity exceeding inline-candidate size threshold -// 2. Limited inlining depth (~3 levels vs HotSpot's 8+) +// PROBLEM: +// The original code path was: FieldP.mul → U256.mulWide → unsignedMultiplyHigh → Math.xxx +// Each field multiply called unsignedMultiplyHigh 20 times (16 in mulWide + 4 in reduceWide). +// On Android, unsignedMultiplyHigh is a wrapper with per-call API-level branching: +// if (API >= 35) Math.unsignedMultiplyHigh else if (API >= 31) Math.multiplyHigh+correction else fallback +// ART's JIT (inlining depth ~3 levels vs HotSpot's 8+) could not inline this 6-level-deep +// call chain, resulting in ~10,000 un-inlined function calls per Schnorr verify. // -// This fused approach eliminates ALL wrapper overhead by: -// 1. Dispatching once per call to fieldMulReduce (not per-multiply) -// 2. Inlining the chosen intrinsic into the hot loop via crossinline lambda -// 3. Combining mulWide + reduceWide into one compilation unit for the JIT +// SOLUTION: +// Fuse mulWide + reduceWide into a single `inline` function (fieldMulReduceWith) that +// accepts the multiply-high intrinsic as a `crossinline` lambda. The lambda is substituted +// at each call site by the Kotlin compiler (not by the JIT), producing platform-specific +// bytecode with the intrinsic call directly embedded — zero wrapper overhead. // -// The inline functions below contain the full schoolbook multiplication and mod-p -// reduction. Since they're inline, the crossinline lambda is substituted at each -// call site, producing platform-specific code with zero function call overhead -// for the multiply-high operation. +// WHY `inline` + `crossinline` LAMBDA (not expect/actual for the whole function)? +// Writing the full 200-line mulWide+reduceWide in each platform file would require +// maintaining 3 identical copies (Android/JVM/Native) that differ only in the +// multiply-high call. The inline+crossinline pattern keeps the arithmetic in one place +// (commonMain) while the platform files provide just the intrinsic. +// +// WHY NOT a single function with all API branches? +// Tested: inlining all 3 API-level branches into one fieldMulReduce created ~600 DEX +// instructions. ART's register allocator produced suboptimal code with stack spills, +// causing verify to REGRESS. The split approach (separate private functions per API level, +// see FieldMulPlatform.android.kt) keeps each at ~200 DEX instructions. +// +// WHY NOT 5×52-bit limbs with lazy reduction (like C libsecp256k1)? +// Tested: 5×52 requires 25 multiplyHigh calls per field multiply (vs our 16 with 4×64). +// On ART where each multiplyHigh has overhead, the 56% increase in multiply calls +// overwhelmed the savings from skipping reduceSelf after add/sub. Net result: slower. +// +// MEASURED IMPACT (Pixel 8, Android 16): +// signSchnorr: 186,610 → 109,711 ns (-41%) +// verifySchnorr: 160,869 → 135,885 ns (-16%) +// Combined with uLt() and @JvmField: verify → 116,450 ns (-28% total) // -// Impact: eliminates ~20 wrapper calls per field multiply × ~500 field muls per -// verify = ~10,000 function calls removed from the hot path. // ===================================================================================== /** diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/PointTypes.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/PointTypes.kt index 8df709ec4..4bf264f15 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/PointTypes.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/PointTypes.kt @@ -34,6 +34,12 @@ import kotlin.jvm.JvmField * * Mutable to avoid allocating new objects during the inner loop of scalar * multiplication, which performs thousands of doublings and additions per operation. + * + * @JvmField is REQUIRED on x/y/z. Without it, Kotlin generates getX()/getY()/getZ() + * virtual getter methods. Bytecode analysis showed ~7,450 invokevirtual getter calls + * per Schnorr verify (130 doublePoints × 13 getXYZ each + 160 addMixed × 23 each). + * @JvmField compiles property access to direct field reads (getfield bytecode), which + * is ~3-4ns faster per access on ART. On non-JVM targets, @JvmField is ignored. */ internal class MutablePoint( @JvmField val x: LongArray = LongArray(4), @@ -71,6 +77,7 @@ internal class MutablePoint( /** * Affine point (x, y) — no Z coordinate. * Used for precomputed tables where we want compact storage and mixed addition. + * @JvmField: see MutablePoint for rationale (eliminates virtual getter calls). */ internal class AffinePoint( @JvmField val x: LongArray = LongArray(4), @@ -90,6 +97,11 @@ internal class AffinePoint( * The wide buffer (LongArray(8)) is pre-fetched once per top-level operation and * passed through to FieldP.mul/sqr, avoiding ~500+ ThreadLocal.get() calls per * scalar multiplication (~20-30ns each on JVM). + * + * @JvmField on ALL properties: without it, each property access compiles to an + * invokevirtual getter call. Bytecode analysis showed ~2,000+ getter calls per + * verify from ECPoint accessing scratch.t, scratch.w, scratch.dblCopy, etc. + * @JvmField compiles these to direct field reads. On non-JVM targets, ignored. */ internal class PointScratch { @JvmField val t = Array(12) { LongArray(4) } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/U256.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/U256.kt index 8b314793f..3198fd183 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/U256.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/U256.kt @@ -46,17 +46,28 @@ package com.vitorpamplona.quartz.utils.secp256k1 // ===================================================================================== /** - * Unsigned less-than comparison, platform-optimized. + * Unsigned less-than comparison: returns true if a < b when both are treated as unsigned. * - * On JVM (HotSpot): uses Long.compareUnsigned which is a JIT intrinsic, - * compiling to a single unsigned CMP + SETB instruction. + * This MUST be platform-specific (expect/actual) because the optimal implementation + * differs between JIT compilers: * - * On Android (ART): uses XOR-with-MIN_VALUE trick to avoid the - * ULong.constructor-impl NOOP invokestatic calls that Kotlin's toULong() - * generates (~17,800 per verify). Produces pure arithmetic bytecode with - * zero method calls. + * WHY NOT `a.toULong() < b.toULong()` (the Kotlin-idiomatic approach)? + * Kotlin's ULong inline class generates 2 `invokestatic ULong.constructor-impl` calls + * per comparison — NOOPs that return the input unchanged. Bytecode analysis showed + * ~17,800 of these per Schnorr verify. On ART, each invokestatic has ~2-3ns overhead + * even when inlined, adding ~35-54μs to verify. On HotSpot, C2 eliminates them entirely. * - * On Native: uses XOR-with-MIN_VALUE (no JVM intrinsics available). + * WHY NOT a single `inline fun` with XOR trick in commonMain? + * The XOR trick `(a xor MIN_VALUE) < (b xor MIN_VALUE)` generates pure arithmetic + * bytecode with zero method calls — great for ART. But on HotSpot, it's ~30% slower + * than `Long.compareUnsigned` because HotSpot recognizes compareUnsigned as a JIT + * intrinsic (single CMP + SETB) but does NOT optimize the XOR pattern equivalently. + * A commonMain inline fun with XOR regressed JVM verify from 1.6× to 2.1× vs native. + * + * Platform implementations: + * - JVM: `Long.compareUnsigned(a, b) < 0` — HotSpot JIT intrinsic → CMP + SETB + * - Android: `(a xor MIN_VALUE) < (b xor MIN_VALUE)` — zero invokestatic, ART → EOR + CMP + * - Native: same XOR trick — Kotlin/Native AOT handles it efficiently */ internal expect fun uLt( a: Long, diff --git a/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/UnsignedCompare.jvm.kt b/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/UnsignedCompare.jvm.kt index f67271f09..9e31a1152 100644 --- a/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/UnsignedCompare.jvm.kt +++ b/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/UnsignedCompare.jvm.kt @@ -21,9 +21,16 @@ package com.vitorpamplona.quartz.utils.secp256k1 /** - * JVM: Long.compareUnsigned is a HotSpot JIT intrinsic that compiles - * to a single unsigned CMP + SETB instruction. Much faster than the - * XOR trick on HotSpot. + * JVM: Long.compareUnsigned is a HotSpot C2 JIT intrinsic. + * + * DO NOT replace with the XOR trick `(a xor MIN_VALUE) < (b xor MIN_VALUE)`. + * HotSpot recognizes Long.compareUnsigned and compiles it to a single unsigned + * CMP + SETB instruction pair. The XOR trick generates 2 extra XOR instructions + * that HotSpot does NOT optimize away, causing a ~30% regression on verify + * (1.6× → 2.1× vs native, measured on JDK 21 x86-64). + * + * This is the opposite of Android/ART where the XOR trick is faster because + * it avoids ULong.constructor-impl overhead. See UnsignedCompare.android.kt. */ internal actual fun uLt( a: Long, diff --git a/quartz/src/nativeMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/UnsignedCompare.native.kt b/quartz/src/nativeMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/UnsignedCompare.native.kt index aeb48d96e..197650131 100644 --- a/quartz/src/nativeMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/UnsignedCompare.native.kt +++ b/quartz/src/nativeMain/kotlin/com/vitorpamplona/quartz/utils/secp256k1/UnsignedCompare.native.kt @@ -21,8 +21,12 @@ package com.vitorpamplona.quartz.utils.secp256k1 /** - * Native: XOR-with-MIN_VALUE trick (no JVM intrinsics available). - * Kotlin/Native AOT compiles this to efficient unsigned compare. + * Native: XOR-with-MIN_VALUE trick for unsigned comparison. + * + * No JVM intrinsics available on Kotlin/Native. The XOR trick compiles + * to efficient unsigned compare via Kotlin/Native's LLVM backend. + * See UnsignedCompare.android.kt for detailed rationale on why this + * approach is preferred over toULong(). */ internal actual fun uLt( a: Long,