From cd344ac07dac9ca2bf67096d177699e1fd8e853c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 16:29:11 +0000 Subject: [PATCH] fix(quartz): stop asserting the outbox cost curve where the GC dominates it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PoolEventOutboxScaleTest failed on iosSimulatorArm64. The outbox is not at fault: on Apple targets LargeCache wraps charlietap's CacheMap, whose LeftRight `mutate` applies each write to both of its two maps under a lock. That is O(1) per put with no copying, so the quadratic this test exists to catch cannot occur there. What the test actually measures on Kotlin/Native is the GC. It keeps 60k entries alive on purpose, so the late window runs against a heap ~30x larger than the early one. A generational collector does not rescan that old generation on a young collection and the growth stays invisible; Kotlin/ Native's non-generational tracing GC does rescan it, and the ratio reports the collector instead of the outbox. Measured on Kotlin/Native (linuxX64, -opt), publishing the same 60k events into structures that are O(1) per put by construction: retains nothing ratio 0.51 - 0.80 one HashMap, 60k live ratio 1.93 - 3.39 two HashMaps per put, 60k live ratio 2.28 - 5.21 The last row is the Apple path's actual work, and it crosses the test's 5.0 threshold on a loaded machine — which is how a shared CI runner turns a healthy implementation red. The first row is the control: same allocations, nothing retained, curve flat. So move the timing assertion to jvmAndroidTest, where LargeCache is a ConcurrentHashMap and a wall-clock ratio is a valid instrument. The guard it provides is unchanged: reintroducing a copy-on-write map or a per-publish full scan in this class still fails it. The test body is untouched; only its source set and its KDoc change. The relay-set bookkeeping half was platform-independent logic, not a measurement, so it stays in commonTest as PoolEventOutboxRelaySetTest and keeps running on every target. Worth a separate look: linuxX64's LargeCache actual is genuinely copy-on-write (LinkedHashMap(mapRef.value) per mutation), so it really is O(N) per put. No CI job runs linuxX64Test today, and the numbers above show a wall-clock ratio cannot report that reliably anyway. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F4Z1E5JJkYXqZznqVsYex6 --- .../pool/PoolEventOutboxRelaySetTest.kt | 73 +++++++++++++++++++ .../client/pool/PoolEventOutboxScaleTest.kt | 54 +++++++++----- 2 files changed, 107 insertions(+), 20 deletions(-) create mode 100644 quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/pool/PoolEventOutboxRelaySetTest.kt rename quartz/src/{commonTest => jvmAndroidTest}/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/pool/PoolEventOutboxScaleTest.kt (70%) diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/pool/PoolEventOutboxRelaySetTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/pool/PoolEventOutboxRelaySetTest.kt new file mode 100644 index 0000000000..5f2e30538f --- /dev/null +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/pool/PoolEventOutboxRelaySetTest.kt @@ -0,0 +1,73 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.quartz.nip01Core.relay.client.pool + +import com.vitorpamplona.quartz.nip01Core.core.Event +import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl +import kotlin.test.Test +import kotlin.test.assertEquals + +/** + * The relay set the pool keeps open must track what is actually pending. + * + * Additions are exact and immediate — a publish unions in its own relays. + * Removals are swept in batches (see `PoolEventOutbox.pendingSweep`), because + * retiring a relay means asking whether ANY remaining entry still wants it, + * which is O(outbox); holding a connection slightly too long is cheaper than + * scanning the whole backlog on every ack. Emptying the outbox forces the + * sweep, so a finished push never strands a connection open. + * + * Pure bookkeeping, identical on every target — the per-publish cost curve + * that motivated the batching is pinned separately in PoolEventOutboxScaleTest + * (jvmAndroidTest, where a wall-clock ratio is a valid instrument). + */ +class PoolEventOutboxRelaySetTest { + private fun event(i: Int) = + Event( + id = i.toString(16).padStart(64, '0'), + pubKey = "00".repeat(32), + createdAt = 1_700_000_000L, + kind = 1, + tags = emptyArray(), + content = "hello", + sig = "00".repeat(64), + ) + + @Test + fun `the relay set still reflects what is pending`() { + val outbox = PoolEventOutbox() + val a = NormalizedRelayUrl("wss://a.relay.test") + val b = NormalizedRelayUrl("wss://b.relay.test") + + outbox.markAsSending(event(1), setOf(a)) + assertEquals(setOf(a), outbox.relays.value, "a publish adds its relay immediately") + + outbox.markAsSending(event(2), setOf(b)) + assertEquals(setOf(a, b), outbox.relays.value, "a second relay joins without a rebuild") + + // Draining every entry must clear the set — the sweep is batched, but + // emptying the outbox forces it, so a finished push does not strand a + // connection open forever. + outbox.newResponse(event(1).id, a, true, "") + outbox.newResponse(event(2).id, b, true, "") + assertEquals(emptySet(), outbox.relays.value, "an empty outbox wants no relays") + } +} diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/pool/PoolEventOutboxScaleTest.kt b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/pool/PoolEventOutboxScaleTest.kt similarity index 70% rename from quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/pool/PoolEventOutboxScaleTest.kt rename to quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/pool/PoolEventOutboxScaleTest.kt index 03058c51ff..d4335574d6 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/pool/PoolEventOutboxScaleTest.kt +++ b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/pool/PoolEventOutboxScaleTest.kt @@ -41,6 +41,40 @@ import kotlin.time.TimeSource * quadratic makes the second half of a run dramatically slower than the first, * whatever the machine. A constant factor cannot be pinned in a unit test, but * a growth curve can. + * + * ## Why this lives in jvmAndroidTest and not in commonTest + * + * The instrument only works on a runtime whose GC cost does not scale with the + * retained set. This test deliberately keeps 60k entries alive, so the late + * window measures a heap ~30x larger than the early one. A generational + * collector (HotSpot, ART) does not rescan that old generation on a young + * collection, so the growth stays invisible and the ratio reflects the outbox. + * Kotlin/Native's non-generational tracing GC does rescan it, and the ratio + * then reflects the GC instead. + * + * Measured on Kotlin/Native (linuxX64, `-opt`), publishing the same 60k events + * into structures that are O(1) per put by construction: + * + * ``` + * retains nothing ratio 0.51 - 0.80 + * one HashMap, 60k live ratio 1.93 - 3.39 + * two HashMaps per put, 60k live ratio 2.28 - 5.21 + * ``` + * + * The last row is what Apple targets actually run: LargeCache there wraps + * charlietap's CacheMap, whose LeftRight `mutate` applies each write to both + * of its two maps under a lock — O(1), no copying. It still crossed the 5.0 + * threshold on a loaded machine, which is how this failed on the iOS simulator + * without anything being wrong with the outbox. The `retains nothing` row is + * the control: same allocations, nothing kept alive, ratio flat. + * + * So on Apple the O(1) guarantee comes from the data structure by + * construction and does not need pinning here; on JVM/Android it comes from + * LargeCache being a ConcurrentHashMap, and a regression in this class + * (someone reintroducing a copy-on-write map, or a per-publish full scan) + * shows up cleanly. Note that Kotlin/Native's linuxX64 LargeCache IS + * copy-on-write today — that is a real cost, but not one a wall-clock ratio + * can report reliably, as the numbers above show. */ class PoolEventOutboxScaleTest { private val relay = NormalizedRelayUrl("wss://scale.relay.test") @@ -98,24 +132,4 @@ class PoolEventOutboxScaleTest { "~$sample entries, last $sample took ${late.inWholeMilliseconds}ms at ~$total entries (ratio $ratio)", ) } - - @Test - fun `the relay set still reflects what is pending`() { - val outbox = PoolEventOutbox() - val a = NormalizedRelayUrl("wss://a.relay.test") - val b = NormalizedRelayUrl("wss://b.relay.test") - - outbox.markAsSending(event(1), setOf(a)) - assertEquals(setOf(a), outbox.relays.value, "a publish adds its relay immediately") - - outbox.markAsSending(event(2), setOf(b)) - assertEquals(setOf(a, b), outbox.relays.value, "a second relay joins without a rebuild") - - // Draining every entry must clear the set — the sweep is batched, but - // emptying the outbox forces it, so a finished push does not strand a - // connection open forever. - outbox.newResponse(event(1).id, a, true, "") - outbox.newResponse(event(2).id, b, true, "") - assertEquals(emptySet(), outbox.relays.value, "an empty outbox wants no relays") - } }