From ee296f3bfa00b17a59adcd2dadb94137e703d166 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Fri, 21 Aug 2026 17:01:01 -0400 Subject: [PATCH 1/4] fix(scanner): take the luminance fast path for packed camera frames `getLuminancePlaneData()` guarded the Y-plane unpadding with `pixelStride != -1`. A YUV_420_888 Y plane always has a pixel stride of 1 (the format guarantees the Y plane is never interleaved), so that term was vacuously true and the `||` short-circuited the whole condition to always-true. The `return data` fast path was unreachable, and the O(width*height) per-pixel Kotlin copy ran on every analyzed frame even when the buffer was already tightly packed. The referenced upstream (Aegis fb58c877) uses `pixelStride != 1`; this was a `-1` vs `1` typo. Measured on a Pixel 10 emulator, per frame at the analysis resolutions the app requests: 640x480 233us -> 0.05us 1280x720 699us -> 0.05us 1920x1080 1470us -> 0.06us At the 1920x1080 target the analyzer requests, that is ~1.5ms of pure overhead per frame on the analysis thread, which is a plausible contributor to the reports of occasionally-slow scanning. The unpadding logic moves into `unpadLuminancePlane` so it can be covered without an `ImageProxy`; behaviour is otherwise unchanged, and the native scanner only ever reads the first `width * height` bytes (`kikCodeScan` memcpy's exactly that), so handing back the longer backing array on the fast path is safe. Adds a JVM test pinning fast-path/slow-path behaviour and asserting the output stays byte-identical to the pre-fix implementation, plus an instrumented sweep that renders real codes through the production encode -> geometry -> scan pipeline across resolutions, code scales and row strides (18/18 decode; padded and packed planes decode identically). --- vendor/kik/scanner/build.gradle.kts | 6 + .../kotlin/com/kik/scan/KikCodeScanTest.kt | 263 ++++++++++++++++++ .../kotlin/com/getcode/util/ImageProxy.kt | 35 ++- .../com/getcode/util/LuminancePlaneTest.kt | 152 ++++++++++ 4 files changed, 450 insertions(+), 6 deletions(-) create mode 100644 vendor/kik/scanner/src/androidTest/kotlin/com/kik/scan/KikCodeScanTest.kt create mode 100644 vendor/kik/scanner/src/test/kotlin/com/getcode/util/LuminancePlaneTest.kt diff --git a/vendor/kik/scanner/build.gradle.kts b/vendor/kik/scanner/build.gradle.kts index 657540956c..ab1cbd9ea3 100644 --- a/vendor/kik/scanner/build.gradle.kts +++ b/vendor/kik/scanner/build.gradle.kts @@ -36,4 +36,10 @@ dependencies { api(project(":libs:codes:kikcode")) implementation(project(":libs:encryption:ed25519")) implementation(project(":vendor:opencv:sdk")) + + androidTestImplementation(libs.junit) + androidTestImplementation(libs.androidx.junit) + androidTestImplementation(libs.androidx.test.runner) + androidTestImplementation(libs.kotlin.test.junit) + androidTestImplementation(libs.kotlinx.coroutines.core) } diff --git a/vendor/kik/scanner/src/androidTest/kotlin/com/kik/scan/KikCodeScanTest.kt b/vendor/kik/scanner/src/androidTest/kotlin/com/kik/scan/KikCodeScanTest.kt new file mode 100644 index 0000000000..5ab247a13d --- /dev/null +++ b/vendor/kik/scanner/src/androidTest/kotlin/com/kik/scan/KikCodeScanTest.kt @@ -0,0 +1,263 @@ +package com.kik.scan + +import android.graphics.Bitmap +import android.graphics.Canvas +import android.graphics.Color +import android.graphics.drawable.ShapeDrawable +import android.graphics.drawable.shapes.OvalShape +import android.util.Log +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.getcode.util.unpadLuminancePlane +import com.kik.kikx.kikcodes.ScanQuality +import com.kik.kikx.kikcodes.implementation.KikCodeScannerImpl +import com.kik.kikx.kincodes.KikCodeContentRendererImpl +import com.kik.kikx.models.ScannableKikCode +import kotlinx.coroutines.runBlocking +import org.junit.Test +import org.junit.runner.RunWith +import kotlin.system.measureNanoTime +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * End-to-end sweep over rendered Kik codes. + * + * Follows the production pipeline exactly: [Scanner.encode] produces the same encoded bytes the + * backend hands the bill UI, [KikCodeContentRendererImpl] draws them through the shared geometry, + * the result is packed into a synthetic YUV_420_888 Y plane, run through the same + * [unpadLuminancePlane] conversion the camera analyzer uses, and handed to the native scanner. + * + * Results are logged under [TAG]. + */ +@RunWith(AndroidJUnit4::class) +class KikCodeScanTest { + + /** + * The detector locates a code by its centre ellipse, so the badge well must be filled — in the + * app that is the round logo drawable. An empty well is simply not scannable. + */ + private val renderer = KikCodeContentRendererImpl().apply { + badge = ShapeDrawable(OvalShape()).apply { paint.color = Color.WHITE } + } + private val scanner = KikCodeScannerImpl() + + /** Analysis resolutions the app requests, plus common fallbacks. */ + private val resolutions = listOf( + 640 to 480, + 1280 to 720, + 1920 to 1080, + ) + + /** Fraction of the frame's short side the code graphic occupies. */ + private val codeScales = listOf(0.5f, 0.7f, 0.9f) + + private data class Frame( + val data: ByteArray, + val width: Int, + val height: Int, + val rowStride: Int, + ) + + /** A remote code payload is 20 bytes; encode it the way the backend does. */ + private fun encodeRemoteCode(seed: Int): Pair { + val payload = ByteArray(REMOTE_PAYLOAD_BYTES) { ((it * 7 + seed) and 0xFF).toByte() } + val encoded = requireNotNull(Scanner.encode(payload)) { "native encode returned null" } + return payload to encoded + } + + /** + * Renders [encoded] centred in a `width x height` frame and returns it as a Y plane with + * [rowPadding] bytes of stride padding per row — i.e. the shape the camera hands us. + */ + private fun renderFrame( + encoded: ByteArray, + width: Int, + height: Int, + scale: Float, + rowPadding: Int, + ): Frame { + val bitmap = Bitmap.createBitmap(width, height, Bitmap.Config.ARGB_8888) + val canvas = Canvas(bitmap) + canvas.drawColor(Color.BLACK) + + val codeSize = (minOf(width, height) * scale).toInt() + canvas.save() + canvas.translate((width - codeSize) / 2f, (height - codeSize) / 2f) + renderer.render(encoded, codeSize, canvas) + canvas.restore() + + val pixels = IntArray(width * height) + bitmap.getPixels(pixels, 0, width, 0, 0, width, height) + bitmap.recycle() + + val rowStride = width + rowPadding + val plane = ByteArray(rowStride * height) + for (y in 0 until height) { + val rowStart = y * rowStride + val pixelRow = y * width + for (x in 0 until width) { + val p = pixels[pixelRow + x] + // BT.601 luma, matching what the camera produces for the Y plane. + val luma = ( + 77 * ((p shr 16) and 0xFF) + + 150 * ((p shr 8) and 0xFF) + + 29 * (p and 0xFF) + ) shr 8 + plane[rowStart + x] = luma.toByte() + } + } + return Frame(plane, width, height, rowStride) + } + + private fun scan(frame: Frame): ScannableKikCode? { + val converted = unpadLuminancePlane( + data = frame.data, + width = frame.width, + height = frame.height, + rowStride = frame.rowStride, + pixelStride = 1, + ) + return runBlocking { + scanner.scanKikCode(converted, frame.width, frame.height, ScanQuality.Best).getOrNull() + } + } + + @Test + fun sweepRenderedCodesAcrossResolutionsAndStrides() { + val (payload, encoded) = encodeRemoteCode(seed = 3) + + var attempts = 0 + var decoded = 0 + val failures = mutableListOf() + + for ((width, height) in resolutions) { + for (scale in codeScales) { + // rowPadding 0 exercises the fast path; 64 exercises the unpadding copy. + for (rowPadding in listOf(0, 64)) { + val frame = renderFrame(encoded, width, height, scale, rowPadding) + val label = "${width}x$height scale=$scale rowStride=${frame.rowStride}" + attempts++ + + val result = scan(frame) + if (result is ScannableKikCode.RemoteKikCode && + result.payloadId.contentEquals(payload) + ) { + decoded++ + Log.i(TAG, "DECODED $label") + } else { + failures += label + Log.w(TAG, "MISSED $label -> $result") + } + } + } + } + + Log.i(TAG, "sweep: $decoded/$attempts decoded") + assertTrue( + failures.isEmpty(), + "scanner failed to decode rendered code at: ${failures.joinToString()}", + ) + } + + /** + * The padded and packed representations of the same frame must decode identically — this is what + * proves the fast path is a pure speedup and not a behaviour change. + */ + @Test + fun paddedAndPackedPlanesDecodeIdentically() { + val (payload, encoded) = encodeRemoteCode(seed = 11) + + for ((width, height) in resolutions) { + val packed = renderFrame(encoded, width, height, 0.7f, rowPadding = 0) + val padded = renderFrame(encoded, width, height, 0.7f, rowPadding = 128) + + // Sanity: the fast path really is taken for the packed frame and not for the padded one. + assertTrue( + unpadLuminancePlane(packed.data, width, height, packed.rowStride, 1) === packed.data, + "${width}x$height packed frame should take the fast path", + ) + assertTrue( + unpadLuminancePlane(padded.data, width, height, padded.rowStride, 1) !== padded.data, + "${width}x$height padded frame should be unpadded", + ) + + val fromPacked = scan(packed) + val fromPadded = scan(padded) + Log.i(TAG, "stride parity ${width}x$height: packed=$fromPacked padded=$fromPadded") + + assertTrue( + fromPacked is ScannableKikCode.RemoteKikCode && + fromPacked.payloadId.contentEquals(payload), + "packed frame did not decode at ${width}x$height", + ) + // RemoteKikCode is a data class over a ByteArray, so its generated equals() compares + // array identity -- compare contents explicitly. + assertTrue( + fromPadded is ScannableKikCode.RemoteKikCode && + fromPadded.payloadId.contentEquals(payload), + "padded frame did not decode at ${width}x$height", + ) + assertEquals( + (fromPacked as ScannableKikCode.RemoteKikCode).colorIndex, + fromPadded.colorIndex, + "colour drift at ${width}x$height", + ) + } + } + + /** + * Measures the per-frame Y-plane conversion cost on-device: the fast path vs. the per-pixel copy + * the old `pixelStride != -1` guard forced on every frame. + */ + @Test + fun benchmarkPerFrameConversion() { + for ((width, height) in resolutions) { + val data = ByteArray(width * height) { (it % 251).toByte() } + + repeat(5) { + unpadLuminancePlane(data, width, height, width, 1) + legacyUnpad(data, width, height, width, 1) + } + + val iterations = 30 + val fastNanos = measureNanoTime { + repeat(iterations) { unpadLuminancePlane(data, width, height, width, 1) } + } / iterations + val legacyNanos = measureNanoTime { + repeat(iterations) { legacyUnpad(data, width, height, width, 1) } + } / iterations + + Log.i( + TAG, + "conversion ${width}x$height packed: fast=${fastNanos / 1000.0}us " + + "legacy=${legacyNanos / 1000.0}us " + + "saved=${(legacyNanos - fastNanos) / 1000.0}us/frame", + ) + } + } + + /** The pre-fix guard, reproduced so the benchmark compares like for like. */ + private fun legacyUnpad( + data: ByteArray, + width: Int, + height: Int, + rowStride: Int, + pixelStride: Int, + ): ByteArray { + if (width != rowStride || pixelStride != -1) { + val cleanData = ByteArray(width * height) + for (y in 0 until height) { + for (x in 0 until width) { + cleanData[y * width + x] = data[y * rowStride + x * pixelStride] + } + } + return cleanData + } + return data + } + + private companion object { + const val TAG = "KikCodeScanSweep" + const val REMOTE_PAYLOAD_BYTES = 20 + } +} diff --git a/vendor/kik/scanner/src/main/kotlin/com/getcode/util/ImageProxy.kt b/vendor/kik/scanner/src/main/kotlin/com/getcode/util/ImageProxy.kt index e14334f934..3997c95293 100644 --- a/vendor/kik/scanner/src/main/kotlin/com/getcode/util/ImageProxy.kt +++ b/vendor/kik/scanner/src/main/kotlin/com/getcode/util/ImageProxy.kt @@ -15,12 +15,35 @@ private fun ImageProxy.getLuminancePlaneData(): ByteArray { buffer.get(data) buffer.rewind() - val width = width - val height = height - val rowStride = plane.rowStride - val pixelStride = plane.pixelStride + return unpadLuminancePlane( + data = data, + width = width, + height = height, + rowStride = plane.rowStride, + pixelStride = plane.pixelStride, + ) +} - if (width != rowStride || pixelStride != -1) { +/** + * Strips row padding (and any pixel interleaving) from a YUV_420_888 Y plane so the result is a + * tightly packed `width * height` luminance matrix. + * + * When the plane is already tightly packed — `rowStride == width` and `pixelStride == 1`, which is + * the common case for analysis resolutions whose width is a multiple of the hardware alignment — + * [data] is returned as-is, skipping the O(width * height) per-pixel copy. + * + * Note that YUV_420_888 guarantees a Y-plane pixel stride of 1, so in practice only + * the row-stride check can send us down the copying path; the pixel-stride term is kept to match + * the upstream implementation and to stay correct if that guarantee ever loosens. + */ +internal fun unpadLuminancePlane( + data: ByteArray, + width: Int, + height: Int, + rowStride: Int, + pixelStride: Int, +): ByteArray { + if (width != rowStride || pixelStride != 1) { // remove padding from the Y plane data val cleanData = ByteArray(width * height) for (y in 0 until height) { @@ -33,4 +56,4 @@ private fun ImageProxy.getLuminancePlaneData(): ByteArray { } return data -} \ No newline at end of file +} diff --git a/vendor/kik/scanner/src/test/kotlin/com/getcode/util/LuminancePlaneTest.kt b/vendor/kik/scanner/src/test/kotlin/com/getcode/util/LuminancePlaneTest.kt new file mode 100644 index 0000000000..d2e2a02cb5 --- /dev/null +++ b/vendor/kik/scanner/src/test/kotlin/com/getcode/util/LuminancePlaneTest.kt @@ -0,0 +1,152 @@ +package com.getcode.util + +import kotlin.system.measureNanoTime +import kotlin.test.Test +import kotlin.test.assertContentEquals +import kotlin.test.assertEquals +import kotlin.test.assertSame +import kotlin.test.assertTrue + +/** + * Covers the Y-plane unpadding used by every analyzed camera frame. + * + * The guard used to read `pixelStride != -1`, which is vacuously true for a YUV_420_888 Y plane + * (whose pixel stride is always 1), so the `||` short-circuited to always-true and the per-pixel + * copy ran on every frame even for tightly packed buffers. + */ +class LuminancePlaneTest { + + /** The pre-fix guard, kept verbatim so we can assert the two agree on output. */ + private fun legacyUnpad( + data: ByteArray, + width: Int, + height: Int, + rowStride: Int, + pixelStride: Int, + ): ByteArray { + if (width != rowStride || pixelStride != -1) { + val cleanData = ByteArray(width * height) + for (y in 0 until height) { + for (x in 0 until width) { + cleanData[y * width + x] = data[y * rowStride + x * pixelStride] + } + } + return cleanData + } + return data + } + + private fun plane(width: Int, height: Int, rowStride: Int): ByteArray = + ByteArray(rowStride * height) { (it % 251).toByte() } + + @Test + fun `tightly packed plane takes the fast path and avoids a copy`() { + val width = 640 + val height = 480 + val data = plane(width, height, rowStride = width) + + val result = unpadLuminancePlane(data, width, height, rowStride = width, pixelStride = 1) + + assertSame(data, result, "tightly packed plane should be returned without copying") + } + + @Test + fun `padded plane still strips row padding`() { + val width = 640 + val height = 480 + val rowStride = 768 // 128 bytes of row padding + val data = plane(width, height, rowStride) + + val result = unpadLuminancePlane(data, width, height, rowStride, pixelStride = 1) + + assertEquals(width * height, result.size) + for (y in 0 until height) { + for (x in 0 until width) { + assertEquals(data[y * rowStride + x], result[y * width + x], "mismatch at ($x,$y)") + } + } + } + + @Test + fun `interleaved plane still honours pixel stride`() { + val width = 32 + val height = 16 + val pixelStride = 2 + val rowStride = width * pixelStride + val data = plane(width, height, rowStride) + + val result = unpadLuminancePlane(data, width, height, rowStride, pixelStride) + + assertEquals(width * height, result.size) + for (y in 0 until height) { + for (x in 0 until width) { + assertEquals(data[y * rowStride + x * pixelStride], result[y * width + x]) + } + } + } + + /** + * The fix is a pure speedup: for every plane geometry the camera can hand us, the bytes the + * scanner sees must be byte-identical to what the old code produced. + */ + @Test + fun `output is byte-identical to the pre-fix implementation`() { + val geometries = listOf( + Triple(640, 480, 640), + Triple(640, 480, 768), + Triple(1280, 720, 1280), + Triple(1280, 720, 1408), + Triple(1920, 1080, 1920), + Triple(1920, 1080, 2048), + ) + + for ((width, height, rowStride) in geometries) { + val data = plane(width, height, rowStride) + val fixed = unpadLuminancePlane(data, width, height, rowStride, pixelStride = 1) + val legacy = legacyUnpad(data, width, height, rowStride, pixelStride = 1) + + // The fast path may hand back the backing array, which is longer than width*height + // when the buffer is over-allocated; the scanner only reads the first width*height. + assertTrue(fixed.size >= width * height, "${width}x$height/$rowStride too small") + assertContentEquals( + legacy.copyOf(width * height), + fixed.copyOf(width * height), + "content drift at ${width}x$height rowStride=$rowStride", + ) + } + } + + /** + * Not a strict benchmark, but a regression tripwire: at 1080p the fast path should be orders of + * magnitude cheaper than the per-pixel copy the old guard forced on every frame. + */ + @Test + fun `fast path is dramatically cheaper than the per-pixel copy`() { + val width = 1920 + val height = 1080 + val data = plane(width, height, rowStride = width) + + repeat(3) { + unpadLuminancePlane(data, width, height, width, 1) + legacyUnpad(data, width, height, width, 1) + } + + val iterations = 20 + val fixedNanos = measureNanoTime { + repeat(iterations) { unpadLuminancePlane(data, width, height, width, 1) } + } / iterations + val legacyNanos = measureNanoTime { + repeat(iterations) { legacyUnpad(data, width, height, width, 1) } + } / iterations + + println( + "LuminancePlane 1920x1080 packed: fixed=${fixedNanos / 1000}us " + + "legacy=${legacyNanos / 1000}us speedup=${legacyNanos.toDouble() / fixedNanos.coerceAtLeast(1)}x" + ) + + assertTrue( + fixedNanos * 10 < legacyNanos, + "expected fast path to be >10x cheaper, got fixed=${fixedNanos}ns legacy=${legacyNanos}ns", + ) + } +} From 44504b1fc64de102af3e0b02156bcd5dfe42730a Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Fri, 21 Aug 2026 17:14:33 -0400 Subject: [PATCH 2/4] refactor(scanner): share the luminance packing rule with iOS The Y-plane packing rule now lives in :libs:codes:kikcode commonMain, which SharedCore already exports, so iOS applies the same rule instead of its own. Both platforms had a bug here, in opposite directions. Android guarded the unpadding with `pixelStride != -1` -- vacuously true for a YUV_420_888 Y plane -- and ran the per-pixel copy on every frame. iOS never unpadded at all, and read its stride with the whole-buffer CVPixelBufferGetBytesPerRow rather than the plane-level API; it survives only because the 1080p capture width happens to be 64-aligned. LuminancePlane shares the decision, not the bytes: unpad() stays a JVM-side detail and iOS asks isTightlyPacked() then moves the plane in its own native code. Handing a frame across the Kotlin/Native bridge would convert Data to ByteArray and copy the whole ~2MB plane -- worse than the copy this avoids. The pure-logic tests move to commonTest so they run on iOS targets too (6/6 on both testAndroidHostTest and iosSimulatorArm64Test). Measured on Pixel_10 AVD, packed 1080p frames: the cross-module call costs 4.3ns/frame vs 1.9ns for a module-local copy, against 1565us/frame saved by taking the fast path at all. The instrumented sweep still decodes 18/18 and packed/padded planes decode identically. --- .../getcode/codes/kikcode/LuminancePlane.kt | 74 +++++++++ .../codes/kikcode/LuminancePlaneTest.kt | 135 ++++++++++++++++ .../kotlin/com/kik/scan/KikCodeScanTest.kt | 97 +++++++++-- .../kotlin/com/getcode/util/ImageProxy.kt | 48 ++---- .../com/getcode/util/LuminancePlaneTest.kt | 152 ------------------ 5 files changed, 307 insertions(+), 199 deletions(-) create mode 100644 libs/codes/kikcode/src/commonMain/kotlin/com/getcode/codes/kikcode/LuminancePlane.kt create mode 100644 libs/codes/kikcode/src/commonTest/kotlin/com/getcode/codes/kikcode/LuminancePlaneTest.kt delete mode 100644 vendor/kik/scanner/src/test/kotlin/com/getcode/util/LuminancePlaneTest.kt diff --git a/libs/codes/kikcode/src/commonMain/kotlin/com/getcode/codes/kikcode/LuminancePlane.kt b/libs/codes/kikcode/src/commonMain/kotlin/com/getcode/codes/kikcode/LuminancePlane.kt new file mode 100644 index 0000000000..556a8f5c78 --- /dev/null +++ b/libs/codes/kikcode/src/commonMain/kotlin/com/getcode/codes/kikcode/LuminancePlane.kt @@ -0,0 +1,74 @@ +package com.getcode.codes.kikcode + +/** + * The contract between a camera frame's luminance (Y) plane and the native code scanner. + * + * `kikCodeScan` does `memcpy(greyscale.data, image, height * width)` — it reads exactly + * [scannedByteCount] bytes from the front of whatever buffer it is handed, and assumes those bytes + * are a tightly packed `width * height` greyscale image. Cameras do not always hand us that: rows + * are commonly padded out to a hardware alignment, so a plane's row stride can exceed its width. + * + * Both platforms got this wrong in different directions, which is why the rule lives here: + * - Android guarded the unpadding with `pixelStride != -1`, which is vacuously true for a + * YUV_420_888 Y plane, so it ran an O(width * height) per-pixel copy on *every* frame — ~1.5 ms + * at 1080p — even when the plane was already packed. + * - iOS never unpadded at all, and read its stride with `CVPixelBufferGetBytesPerRow` (which + * reports a whole-buffer value for planar formats) instead of + * `CVPixelBufferGetBytesPerRowOfPlane(_, 0)`. It survives only because the 1080p capture width + * happens to be 64-aligned and therefore unpadded. + * + * ## Why this shares the decision and not the bytes + * + * [unpad] is deliberately *not* part of the iOS-facing surface. Handing a plane across the + * Kotlin/Native bridge would convert `Data` to `ByteArray`, copying the whole ~2 MB frame — far + * worse than the copy this is meant to avoid. Callers ask [isTightlyPacked] whether a copy is + * needed and then move the bytes in their own native code, so the shared piece stays branch-only + * and allocation-free. + */ +object LuminancePlane { + + /** + * Whether the plane can be handed to the scanner as-is. + * + * When true the first [scannedByteCount] bytes are already the image the scanner expects, so the + * buffer can be passed through with no copy. When false the caller must repack it — see [unpad] + * for the reference implementation. + * + * A YUV_420_888 Y plane always reports a [pixelStride] of 1 (the format guarantees the Y plane + * is never interleaved), so in practice only [rowStride] decides this; the pixel-stride term is + * kept so the rule stays correct if that guarantee ever loosens. + */ + fun isTightlyPacked(width: Int, rowStride: Int, pixelStride: Int): Boolean = + rowStride == width && pixelStride == 1 + + /** How many bytes the scanner reads for a `width x height` frame. */ + fun scannedByteCount(width: Int, height: Int): Int = width * height + + /** + * Repacks a padded or interleaved plane into a tightly packed `width * height` buffer. + * + * Returns [data] untouched when [isTightlyPacked] already holds, so the common case costs + * nothing. Note the returned array may be *longer* than [scannedByteCount] — the scanner only + * reads the front of it. + * + * This is the JVM/Android path. iOS repacks in Swift over the raw plane pointer rather than + * calling this, to keep the frame out of the Kotlin/Native bridge. + */ + fun unpad( + data: ByteArray, + width: Int, + height: Int, + rowStride: Int, + pixelStride: Int, + ): ByteArray { + if (isTightlyPacked(width, rowStride, pixelStride)) return data + + val cleanData = ByteArray(scannedByteCount(width, height)) + for (y in 0 until height) { + for (x in 0 until width) { + cleanData[y * width + x] = data[y * rowStride + x * pixelStride] + } + } + return cleanData + } +} diff --git a/libs/codes/kikcode/src/commonTest/kotlin/com/getcode/codes/kikcode/LuminancePlaneTest.kt b/libs/codes/kikcode/src/commonTest/kotlin/com/getcode/codes/kikcode/LuminancePlaneTest.kt new file mode 100644 index 0000000000..2bd5dfe0a3 --- /dev/null +++ b/libs/codes/kikcode/src/commonTest/kotlin/com/getcode/codes/kikcode/LuminancePlaneTest.kt @@ -0,0 +1,135 @@ +package com.getcode.codes.kikcode + +import kotlin.test.Test +import kotlin.test.assertContentEquals +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertSame +import kotlin.test.assertTrue + +/** + * Covers the packing rule every analyzed camera frame goes through, on both platforms. + * + * Android's guard used to read `pixelStride != -1`, which is vacuously true for a YUV_420_888 Y + * plane (whose pixel stride is always 1), so the `||` short-circuited to always-true and the + * per-pixel copy ran on every frame even for tightly packed buffers. iOS had the opposite bug: it + * never unpadded at all. These tests run on both targets so the rule cannot drift again. + */ +class LuminancePlaneTest { + + /** Android's pre-fix guard, kept verbatim so we can assert the two agree on output. */ + private fun legacyUnpad( + data: ByteArray, + width: Int, + height: Int, + rowStride: Int, + pixelStride: Int, + ): ByteArray { + if (width != rowStride || pixelStride != -1) { + val cleanData = ByteArray(width * height) + for (y in 0 until height) { + for (x in 0 until width) { + cleanData[y * width + x] = data[y * rowStride + x * pixelStride] + } + } + return cleanData + } + return data + } + + private fun plane(height: Int, rowStride: Int): ByteArray = + ByteArray(rowStride * height) { (it % 251).toByte() } + + @Test + fun `a plane is tightly packed only when the row stride matches the width`() { + assertTrue(LuminancePlane.isTightlyPacked(width = 1920, rowStride = 1920, pixelStride = 1)) + // 64-byte row alignment, the usual source of padding on both platforms + assertFalse(LuminancePlane.isTightlyPacked(width = 1440, rowStride = 1472, pixelStride = 1)) + assertFalse(LuminancePlane.isTightlyPacked(width = 1000, rowStride = 1024, pixelStride = 1)) + // interleaved planes are never packed, even when the arithmetic happens to line up + assertFalse(LuminancePlane.isTightlyPacked(width = 64, rowStride = 64, pixelStride = 2)) + } + + @Test + fun `the scanner reads exactly width times height bytes`() { + assertEquals(1920 * 1080, LuminancePlane.scannedByteCount(1920, 1080)) + } + + @Test + fun `tightly packed plane takes the fast path and avoids a copy`() { + val width = 640 + val height = 480 + val data = plane(height, rowStride = width) + + val result = LuminancePlane.unpad(data, width, height, rowStride = width, pixelStride = 1) + + assertSame(data, result, "tightly packed plane should be returned without copying") + } + + @Test + fun `padded plane still strips row padding`() { + val width = 640 + val height = 480 + val rowStride = 768 // 128 bytes of row padding + val data = plane(height, rowStride) + + val result = LuminancePlane.unpad(data, width, height, rowStride, pixelStride = 1) + + assertEquals(width * height, result.size) + for (y in 0 until height) { + for (x in 0 until width) { + assertEquals(data[y * rowStride + x], result[y * width + x], "mismatch at ($x,$y)") + } + } + } + + @Test + fun `interleaved plane still honours pixel stride`() { + val width = 32 + val height = 16 + val pixelStride = 2 + val rowStride = width * pixelStride + val data = plane(height, rowStride) + + val result = LuminancePlane.unpad(data, width, height, rowStride, pixelStride) + + assertEquals(width * height, result.size) + for (y in 0 until height) { + for (x in 0 until width) { + assertEquals(data[y * rowStride + x * pixelStride], result[y * width + x]) + } + } + } + + /** + * The Android fix is a pure speedup: for every plane geometry the camera can hand us, the bytes + * the scanner sees must be byte-identical to what the old code produced. + */ + @Test + fun `output is byte-identical to the pre-fix implementation`() { + val geometries = listOf( + Triple(640, 480, 640), + Triple(640, 480, 768), + Triple(1280, 720, 1280), + Triple(1280, 720, 1408), + Triple(1920, 1080, 1920), + Triple(1920, 1080, 2048), + ) + + for ((width, height, rowStride) in geometries) { + val data = plane(height, rowStride) + val fixed = LuminancePlane.unpad(data, width, height, rowStride, pixelStride = 1) + val legacy = legacyUnpad(data, width, height, rowStride, pixelStride = 1) + + // The fast path hands back the backing array, which is longer than width*height when + // the buffer is over-allocated; the scanner only reads the first width*height. + val scanned = LuminancePlane.scannedByteCount(width, height) + assertTrue(fixed.size >= scanned, "${width}x$height/$rowStride too small") + assertContentEquals( + legacy.copyOf(scanned), + fixed.copyOf(scanned), + "content drift at ${width}x$height rowStride=$rowStride", + ) + } + } +} diff --git a/vendor/kik/scanner/src/androidTest/kotlin/com/kik/scan/KikCodeScanTest.kt b/vendor/kik/scanner/src/androidTest/kotlin/com/kik/scan/KikCodeScanTest.kt index 5ab247a13d..7f9ad99559 100644 --- a/vendor/kik/scanner/src/androidTest/kotlin/com/kik/scan/KikCodeScanTest.kt +++ b/vendor/kik/scanner/src/androidTest/kotlin/com/kik/scan/KikCodeScanTest.kt @@ -7,7 +7,7 @@ import android.graphics.drawable.ShapeDrawable import android.graphics.drawable.shapes.OvalShape import android.util.Log import androidx.test.ext.junit.runners.AndroidJUnit4 -import com.getcode.util.unpadLuminancePlane +import com.getcode.codes.kikcode.LuminancePlane import com.kik.kikx.kikcodes.ScanQuality import com.kik.kikx.kikcodes.implementation.KikCodeScannerImpl import com.kik.kikx.kincodes.KikCodeContentRendererImpl @@ -25,7 +25,7 @@ import kotlin.test.assertTrue * Follows the production pipeline exactly: [Scanner.encode] produces the same encoded bytes the * backend hands the bill UI, [KikCodeContentRendererImpl] draws them through the shared geometry, * the result is packed into a synthetic YUV_420_888 Y plane, run through the same - * [unpadLuminancePlane] conversion the camera analyzer uses, and handed to the native scanner. + * [LuminancePlane.unpad] conversion the camera analyzer uses, and handed to the native scanner. * * Results are logged under [TAG]. */ @@ -110,7 +110,7 @@ class KikCodeScanTest { } private fun scan(frame: Frame): ScannableKikCode? { - val converted = unpadLuminancePlane( + val converted = LuminancePlane.unpad( data = frame.data, width = frame.width, height = frame.height, @@ -173,11 +173,11 @@ class KikCodeScanTest { // Sanity: the fast path really is taken for the packed frame and not for the padded one. assertTrue( - unpadLuminancePlane(packed.data, width, height, packed.rowStride, 1) === packed.data, + LuminancePlane.unpad(packed.data, width, height, packed.rowStride, 1) === packed.data, "${width}x$height packed frame should take the fast path", ) assertTrue( - unpadLuminancePlane(padded.data, width, height, padded.rowStride, 1) !== padded.data, + LuminancePlane.unpad(padded.data, width, height, padded.rowStride, 1) !== padded.data, "${width}x$height padded frame should be unpadded", ) @@ -215,27 +215,104 @@ class KikCodeScanTest { val data = ByteArray(width * height) { (it % 251).toByte() } repeat(5) { - unpadLuminancePlane(data, width, height, width, 1) + LuminancePlane.unpad(data, width, height, width, 1) legacyUnpad(data, width, height, width, 1) } val iterations = 30 - val fastNanos = measureNanoTime { - repeat(iterations) { unpadLuminancePlane(data, width, height, width, 1) } - } / iterations val legacyNanos = measureNanoTime { repeat(iterations) { legacyUnpad(data, width, height, width, 1) } } / iterations + // The fast path returns in a few instructions, so 30 iterations sits at the + // System.nanoTime measurement floor -- run it enough times to actually resolve. + val fastIterations = 200_000 + val fastNanos = measureNanoTime { + repeat(fastIterations) { LuminancePlane.unpad(data, width, height, width, 1) } + }.toDouble() / fastIterations + Log.i( TAG, "conversion ${width}x$height packed: fast=${fastNanos / 1000.0}us " + "legacy=${legacyNanos / 1000.0}us " + - "saved=${(legacyNanos - fastNanos) / 1000.0}us/frame", + "saved=${legacyNanos / 1000.0 - fastNanos / 1000.0}us/frame", ) } } + /** + * The shared packing rule lives in `:libs:codes:kikcode` so iOS applies the same one, which puts + * a cross-module call on the hot path where there used to be a module-local function. This + * A/Bs it against a module-local copy to confirm that indirection costs nothing. + * + * Measured over several alternating rounds taking the best of each: a single round is dominated + * by whichever loop the JIT compiled first, which is enough to invent a double-digit-nanosecond + * "difference" that reverses if you swap the order. + */ + @Test + fun sharedFastPathCostsNoMoreThanAModuleLocalOne() { + val (width, height) = resolutions.last() + val data = ByteArray(width * height) { (it % 251).toByte() } + val iterations = 200_000 + + repeat(50_000) { + LuminancePlane.unpad(data, width, height, width, 1) + localUnpad(data, width, height, width, 1) + } + + fun timeShared(): Double = measureNanoTime { + repeat(iterations) { LuminancePlane.unpad(data, width, height, width, 1) } + }.toDouble() / iterations + + fun timeLocal(): Double = measureNanoTime { + repeat(iterations) { localUnpad(data, width, height, width, 1) } + }.toDouble() / iterations + + var shared = Double.MAX_VALUE + var local = Double.MAX_VALUE + repeat(5) { round -> + // alternate which runs first so neither systematically pays for the other's warmup + if (round % 2 == 0) { + shared = minOf(shared, timeShared()) + local = minOf(local, timeLocal()) + } else { + local = minOf(local, timeLocal()) + shared = minOf(shared, timeShared()) + } + } + + Log.i( + TAG, + "fast path ${width}x$height: shared=${shared}ns local=${local}ns " + + "delta=${shared - local}ns/frame", + ) + + // Both are a comparison and a return. A cross-module call that is not being optimized away + // would show up as a consistent multiple, not a fraction of a nanosecond. + assertTrue( + shared < local + 5.0, + "shared fast path regressed: shared=${shared}ns local=${local}ns", + ) + } + + /** A module-local copy of the fast path, used only as the A/B baseline above. */ + private fun localUnpad( + data: ByteArray, + width: Int, + height: Int, + rowStride: Int, + pixelStride: Int, + ): ByteArray { + if (rowStride == width && pixelStride == 1) return data + val cleanData = ByteArray(width * height) + for (y in 0 until height) { + for (x in 0 until width) { + cleanData[y * width + x] = data[y * rowStride + x * pixelStride] + } + } + return cleanData + } + /** The pre-fix guard, reproduced so the benchmark compares like for like. */ private fun legacyUnpad( data: ByteArray, diff --git a/vendor/kik/scanner/src/main/kotlin/com/getcode/util/ImageProxy.kt b/vendor/kik/scanner/src/main/kotlin/com/getcode/util/ImageProxy.kt index 3997c95293..d6eb9edd8b 100644 --- a/vendor/kik/scanner/src/main/kotlin/com/getcode/util/ImageProxy.kt +++ b/vendor/kik/scanner/src/main/kotlin/com/getcode/util/ImageProxy.kt @@ -1,13 +1,21 @@ package com.getcode.util import androidx.camera.core.ImageProxy +import com.getcode.codes.kikcode.LuminancePlane fun ImageProxy.toByteArray(): ByteArray { - // Remove padding from Y plane data before passing it to ZXing + // Remove padding from Y plane data before passing it to the scanner // @see https://github.com/beemdevelopment/Aegis/commit/fb58c877d1b305b1c66db497880da5651dda78d7 - return getLuminancePlaneData() + return getLuminancePlaneData() } +/** + * Reads the Y plane of an analyzed frame into the tightly packed `width * height` buffer the native + * scanner expects. + * + * The packing rule lives in [LuminancePlane] because iOS has to apply the same one — see that file + * for why the shared piece is the decision rather than the bytes. + */ private fun ImageProxy.getLuminancePlaneData(): ByteArray { val plane = planes[0] val buffer = plane.buffer @@ -15,7 +23,7 @@ private fun ImageProxy.getLuminancePlaneData(): ByteArray { buffer.get(data) buffer.rewind() - return unpadLuminancePlane( + return LuminancePlane.unpad( data = data, width = width, height = height, @@ -23,37 +31,3 @@ private fun ImageProxy.getLuminancePlaneData(): ByteArray { pixelStride = plane.pixelStride, ) } - -/** - * Strips row padding (and any pixel interleaving) from a YUV_420_888 Y plane so the result is a - * tightly packed `width * height` luminance matrix. - * - * When the plane is already tightly packed — `rowStride == width` and `pixelStride == 1`, which is - * the common case for analysis resolutions whose width is a multiple of the hardware alignment — - * [data] is returned as-is, skipping the O(width * height) per-pixel copy. - * - * Note that YUV_420_888 guarantees a Y-plane pixel stride of 1, so in practice only - * the row-stride check can send us down the copying path; the pixel-stride term is kept to match - * the upstream implementation and to stay correct if that guarantee ever loosens. - */ -internal fun unpadLuminancePlane( - data: ByteArray, - width: Int, - height: Int, - rowStride: Int, - pixelStride: Int, -): ByteArray { - if (width != rowStride || pixelStride != 1) { - // remove padding from the Y plane data - val cleanData = ByteArray(width * height) - for (y in 0 until height) { - for (x in 0 until width) { - cleanData[y * width + x] = data[y * rowStride + x * pixelStride] - } - } - - return cleanData - } - - return data -} diff --git a/vendor/kik/scanner/src/test/kotlin/com/getcode/util/LuminancePlaneTest.kt b/vendor/kik/scanner/src/test/kotlin/com/getcode/util/LuminancePlaneTest.kt deleted file mode 100644 index d2e2a02cb5..0000000000 --- a/vendor/kik/scanner/src/test/kotlin/com/getcode/util/LuminancePlaneTest.kt +++ /dev/null @@ -1,152 +0,0 @@ -package com.getcode.util - -import kotlin.system.measureNanoTime -import kotlin.test.Test -import kotlin.test.assertContentEquals -import kotlin.test.assertEquals -import kotlin.test.assertSame -import kotlin.test.assertTrue - -/** - * Covers the Y-plane unpadding used by every analyzed camera frame. - * - * The guard used to read `pixelStride != -1`, which is vacuously true for a YUV_420_888 Y plane - * (whose pixel stride is always 1), so the `||` short-circuited to always-true and the per-pixel - * copy ran on every frame even for tightly packed buffers. - */ -class LuminancePlaneTest { - - /** The pre-fix guard, kept verbatim so we can assert the two agree on output. */ - private fun legacyUnpad( - data: ByteArray, - width: Int, - height: Int, - rowStride: Int, - pixelStride: Int, - ): ByteArray { - if (width != rowStride || pixelStride != -1) { - val cleanData = ByteArray(width * height) - for (y in 0 until height) { - for (x in 0 until width) { - cleanData[y * width + x] = data[y * rowStride + x * pixelStride] - } - } - return cleanData - } - return data - } - - private fun plane(width: Int, height: Int, rowStride: Int): ByteArray = - ByteArray(rowStride * height) { (it % 251).toByte() } - - @Test - fun `tightly packed plane takes the fast path and avoids a copy`() { - val width = 640 - val height = 480 - val data = plane(width, height, rowStride = width) - - val result = unpadLuminancePlane(data, width, height, rowStride = width, pixelStride = 1) - - assertSame(data, result, "tightly packed plane should be returned without copying") - } - - @Test - fun `padded plane still strips row padding`() { - val width = 640 - val height = 480 - val rowStride = 768 // 128 bytes of row padding - val data = plane(width, height, rowStride) - - val result = unpadLuminancePlane(data, width, height, rowStride, pixelStride = 1) - - assertEquals(width * height, result.size) - for (y in 0 until height) { - for (x in 0 until width) { - assertEquals(data[y * rowStride + x], result[y * width + x], "mismatch at ($x,$y)") - } - } - } - - @Test - fun `interleaved plane still honours pixel stride`() { - val width = 32 - val height = 16 - val pixelStride = 2 - val rowStride = width * pixelStride - val data = plane(width, height, rowStride) - - val result = unpadLuminancePlane(data, width, height, rowStride, pixelStride) - - assertEquals(width * height, result.size) - for (y in 0 until height) { - for (x in 0 until width) { - assertEquals(data[y * rowStride + x * pixelStride], result[y * width + x]) - } - } - } - - /** - * The fix is a pure speedup: for every plane geometry the camera can hand us, the bytes the - * scanner sees must be byte-identical to what the old code produced. - */ - @Test - fun `output is byte-identical to the pre-fix implementation`() { - val geometries = listOf( - Triple(640, 480, 640), - Triple(640, 480, 768), - Triple(1280, 720, 1280), - Triple(1280, 720, 1408), - Triple(1920, 1080, 1920), - Triple(1920, 1080, 2048), - ) - - for ((width, height, rowStride) in geometries) { - val data = plane(width, height, rowStride) - val fixed = unpadLuminancePlane(data, width, height, rowStride, pixelStride = 1) - val legacy = legacyUnpad(data, width, height, rowStride, pixelStride = 1) - - // The fast path may hand back the backing array, which is longer than width*height - // when the buffer is over-allocated; the scanner only reads the first width*height. - assertTrue(fixed.size >= width * height, "${width}x$height/$rowStride too small") - assertContentEquals( - legacy.copyOf(width * height), - fixed.copyOf(width * height), - "content drift at ${width}x$height rowStride=$rowStride", - ) - } - } - - /** - * Not a strict benchmark, but a regression tripwire: at 1080p the fast path should be orders of - * magnitude cheaper than the per-pixel copy the old guard forced on every frame. - */ - @Test - fun `fast path is dramatically cheaper than the per-pixel copy`() { - val width = 1920 - val height = 1080 - val data = plane(width, height, rowStride = width) - - repeat(3) { - unpadLuminancePlane(data, width, height, width, 1) - legacyUnpad(data, width, height, width, 1) - } - - val iterations = 20 - val fixedNanos = measureNanoTime { - repeat(iterations) { unpadLuminancePlane(data, width, height, width, 1) } - } / iterations - val legacyNanos = measureNanoTime { - repeat(iterations) { legacyUnpad(data, width, height, width, 1) } - } / iterations - - println( - "LuminancePlane 1920x1080 packed: fixed=${fixedNanos / 1000}us " + - "legacy=${legacyNanos / 1000}us speedup=${legacyNanos.toDouble() / fixedNanos.coerceAtLeast(1)}x" - ) - - assertTrue( - fixedNanos * 10 < legacyNanos, - "expected fast path to be >10x cheaper, got fixed=${fixedNanos}ns legacy=${legacyNanos}ns", - ) - } -} From da971d0eb1dd23c2bef00b8707f4616829ab0df0 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Fri, 21 Aug 2026 18:55:26 -0400 Subject: [PATCH 3/4] test(scanner): gate the shared fast path on the copy it replaces, not a fixed budget The A/B assertion was `shared < local + 5.0`, a threshold picked on an emulator. It encodes the speed of whatever CPU it happens to run on rather than a property of the code, and it duly failed on an S25 Ultra (shared=12.5ns local=5.1ns) while passing on the emulator (4.3 / 1.9). The cross-module hop really does cost ~2.4x a module-local call in a debug build, consistently across both machines. But that is not what the test should guard. A fast path that stops being one moves from nanoseconds to milliseconds -- measured at 1:446,036 on device -- so the gate is now the ratio against the legacy copy timed on the same device, which is meaningful everywhere. The A/B delta is still logged as an observation. Worth recording: R8 closes the gap almost entirely. On a minified release build the same A/B measures 3.77ns shared vs 3.43ns local -- 0.34ns -- since the rule is a two-int comparison and a return. The debug figure is an artifact of the build type, not the cost of sharing. --- .../kotlin/com/kik/scan/KikCodeScanTest.kt | 44 ++++++++++++++----- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/vendor/kik/scanner/src/androidTest/kotlin/com/kik/scan/KikCodeScanTest.kt b/vendor/kik/scanner/src/androidTest/kotlin/com/kik/scan/KikCodeScanTest.kt index 7f9ad99559..bd3414b4ad 100644 --- a/vendor/kik/scanner/src/androidTest/kotlin/com/kik/scan/KikCodeScanTest.kt +++ b/vendor/kik/scanner/src/androidTest/kotlin/com/kik/scan/KikCodeScanTest.kt @@ -242,15 +242,28 @@ class KikCodeScanTest { /** * The shared packing rule lives in `:libs:codes:kikcode` so iOS applies the same one, which puts - * a cross-module call on the hot path where there used to be a module-local function. This - * A/Bs it against a module-local copy to confirm that indirection costs nothing. + * a cross-module call on the hot path where there used to be a module-local function. * - * Measured over several alternating rounds taking the best of each: a single round is dominated - * by whichever loop the JIT compiled first, which is enough to invent a double-digit-nanosecond - * "difference" that reverses if you swap the order. + * Unoptimized, that hop is not free: in a debug build it costs a consistent ~2.4x a module-local + * call (2.5ns on an emulator, 7.4ns on an S25 Ultra). R8 all but erases it -- the same A/B on a + * minified release build measures 3.77ns shared vs 3.43ns local, a 0.34ns difference, because + * the function is a two-int comparison and a return and gets inlined. Users run the optimized + * build, so the honest figure for sharing the rule is ~0.3ns/frame. + * + * Either way this does NOT assert an absolute nanosecond budget: that would encode the speed of + * whatever hardware and build type it last ran on, and flip-flops between them. + * + * What actually matters is that the fast path stays orders of magnitude below the copy it + * replaces. A real regression -- someone making the packed case copy again -- moves it from + * nanoseconds to milliseconds, a ~350,000x jump, not a few nanoseconds. So the gate is measured + * against the legacy cost on the same device, and the A/B delta is logged as an observation. + * + * Both timings are taken over several alternating rounds keeping the best of each: a single + * round is dominated by whichever loop the JIT compiled first, which is enough to invent a + * double-digit-nanosecond "difference" that reverses if you swap the order. */ @Test - fun sharedFastPathCostsNoMoreThanAModuleLocalOne() { + fun sharedFastPathStaysOrdersOfMagnitudeBelowTheCopyItReplaces() { val (width, height) = resolutions.last() val data = ByteArray(width * height) { (it % 251).toByte() } val iterations = 200_000 @@ -281,17 +294,26 @@ class KikCodeScanTest { } } + // The copy the fast path exists to avoid, on this same device, as the yardstick. + val legacyIterations = 30 + val legacy = measureNanoTime { + repeat(legacyIterations) { legacyUnpad(data, width, height, width, 1) } + }.toDouble() / legacyIterations + Log.i( TAG, "fast path ${width}x$height: shared=${shared}ns local=${local}ns " + - "delta=${shared - local}ns/frame", + "delta=${shared - local}ns/frame legacy=${legacy / 1_000}us " + + "ratio=1:${(legacy / shared).toLong()}", ) - // Both are a comparison and a return. A cross-module call that is not being optimized away - // would show up as a consistent multiple, not a fraction of a nanosecond. + // A fast path that stopped being one shows up as a four-to-five-order-of-magnitude move, + // not a few nanoseconds. 1000x is far below the ~350,000x actually observed and far above + // any plausible cross-module dispatch cost, on any device. assertTrue( - shared < local + 5.0, - "shared fast path regressed: shared=${shared}ns local=${local}ns", + shared * 1_000 < legacy, + "shared fast path regressed: shared=${shared}ns is not <1/1000th of the " + + "${legacy / 1_000}us copy it replaces (local baseline=${local}ns)", ) } From 95ca1190f1aeefdeb9e51d44ba5747aaca60d766 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Fri, 21 Aug 2026 20:23:46 -0400 Subject: [PATCH 4/4] test(scanner): measure what sustained scanning costs the collector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The wall-clock benchmarks measure a frame in isolation, which says nothing about the shape of the original report — scanning that is occasionally slow rather than uniformly slow. Adds a 300-frame variant comparing the pre-fix path, the current fast path, and a hypothetical reusable frame buffer, so the remaining per-frame allocation can be sized before anyone builds it away. --- .../kotlin/com/kik/scan/KikCodeScanTest.kt | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/vendor/kik/scanner/src/androidTest/kotlin/com/kik/scan/KikCodeScanTest.kt b/vendor/kik/scanner/src/androidTest/kotlin/com/kik/scan/KikCodeScanTest.kt index bd3414b4ad..66e0e96a6a 100644 --- a/vendor/kik/scanner/src/androidTest/kotlin/com/kik/scan/KikCodeScanTest.kt +++ b/vendor/kik/scanner/src/androidTest/kotlin/com/kik/scan/KikCodeScanTest.kt @@ -5,6 +5,7 @@ import android.graphics.Canvas import android.graphics.Color import android.graphics.drawable.ShapeDrawable import android.graphics.drawable.shapes.OvalShape +import android.os.Debug import android.util.Log import androidx.test.ext.junit.runners.AndroidJUnit4 import com.getcode.codes.kikcode.LuminancePlane @@ -317,6 +318,72 @@ class KikCodeScanTest { ) } + /** + * What sustained scanning costs the collector. + * + * The wall-clock benchmarks above measure one frame in isolation, which says nothing about the + * *shape* of the original complaint: scanning that is occasionally slow rather than uniformly + * slow. A steady per-frame tax reads as the latter. Blocking GC reads as the former. + * + * The pre-fix path allocated twice per frame at 1080p — once to read the plane out of the + * `ByteBuffer`, once more for the unpadding copy — roughly 4MB/frame, ~120MB/s at 30fps. The fix + * removes the second. A reusable frame buffer would remove the first as well, which is the only + * reason variant C is here: to size that remaining opportunity before anyone builds it. + * + * Reported as blocking GC count and time, since that is the part a user actually feels. + * + * Read the raw GC *counts* with care: the variants differ by two orders of magnitude in wall + * time, which gives the concurrent collector correspondingly more opportunity to run during the + * slow one. The quantity that compares cleanly across variants is allocations per frame — two, + * one, none. + */ + @Test + fun sustainedScanningGcCost() { + val (width, height) = resolutions.last() + val frames = 300 // ten seconds of scanning at 30fps + val bufferBytes = width * height // packed: the common case, and the one the fix targets + + fun gcStat(name: String): Long = Debug.getRuntimeStat(name)?.toLongOrNull() ?: -1L + + fun measure(label: String, frame: (Int) -> ByteArray) { + Runtime.getRuntime().gc() + Thread.sleep(SETTLE_MS) + val gcBefore = gcStat("art.gc.gc-count") + val blockingBefore = gcStat("art.gc.blocking-gc-count") + val blockingTimeBefore = gcStat("art.gc.blocking-gc-time") + + var sink = 0L + val elapsed = measureNanoTime { + repeat(frames) { i -> sink += frame(i)[0].toLong() } + } + + Log.i( + TAG, + "gc $label ${width}x$height over $frames frames: " + + "gc=${gcStat("art.gc.gc-count") - gcBefore} " + + "blockingGc=${gcStat("art.gc.blocking-gc-count") - blockingBefore} " + + "blockingGcTime=${gcStat("art.gc.blocking-gc-time") - blockingTimeBefore}ms " + + "wall=${elapsed / 1_000_000}ms sink=$sink", + ) + } + + // Pre-fix: a fresh plane read plus the unpadding copy, every frame. + measure("legacy") { + legacyUnpad(ByteArray(bufferBytes), width, height, width, 1) + } + + // Current: the plane read still allocates; the fast path adds nothing. + measure("fastPath") { + LuminancePlane.unpad(ByteArray(bufferBytes), width, height, width, 1) + } + + // Hypothetical: ImageAnalysis delivers frames serially, so one buffer could be reused. + val reusable = ByteArray(bufferBytes) + measure("reusedBuffer") { + LuminancePlane.unpad(reusable, width, height, width, 1) + } + } + /** A module-local copy of the fast path, used only as the A/B baseline above. */ private fun localUnpad( data: ByteArray, @@ -358,5 +425,6 @@ class KikCodeScanTest { private companion object { const val TAG = "KikCodeScanSweep" const val REMOTE_PAYLOAD_BYTES = 20 + const val SETTLE_MS = 200L } }