mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-06 03:38:23 +00:00
fix(quartz): stop asserting the outbox cost curve where the GC dominates it
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F4Z1E5JJkYXqZznqVsYex6
This commit is contained in:
Claude
+73
@@ -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")
|
||||
}
|
||||
}
|
||||
+34
-20
@@ -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")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user