diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relays/EOSERelayList.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relays/EOSERelayList.kt index dc831f3db6..ca22c91182 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relays/EOSERelayList.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relays/EOSERelayList.kt @@ -23,6 +23,7 @@ package com.vitorpamplona.amethyst.commons.relays import com.vitorpamplona.amethyst.commons.util.KmpLock import com.vitorpamplona.amethyst.commons.util.withLock import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl +import kotlin.concurrent.Volatile typealias SincePerRelayMap = MutableMap @@ -30,29 +31,51 @@ typealias SincePerRelayMap = MutableMap * Tracks EOSE (End Of Stored Events) timestamps for relay subscriptions. * Used by UserCardsCache and similar classes to manage relay subscription state. */ -class EOSERelayList { - var relayList: SincePerRelayMap = mutableMapOf() +class EOSERelayList { /** - * Writers are serialized because they are not all on one thread: [addOrUpdate] runs on each - * relay's own socket-reader thread as EOSE frames land, so a client holding a few hundred relays - * has that many potential writers to one plain map. [EOSEAccountFast] wraps its lists in a lock - * for the same reason; a bare list handed to [SingleSubEoseManager] had none. + * Copy-on-write: replaced wholesale under [lock], never mutated in place after publication, so + * readers need no synchronization at all. * - * Reads still go through the map returned by [since] — deliberately live rather than a snapshot, - * since callers clear a relay and then re-read it inside one assembly pass. + * The access pattern makes this nearly free. [addOrUpdate] runs on **every live event** — hundreds + * per second across a few hundred relays — but an event from a relay already in the map only bumps + * the `Long` inside its own [MutableTime]. The map itself is only ever written on the *first* frame + * from a relay, plus [remove] and [clear]: a couple of hundred writes for the lifetime of the + * process. Locking the common path would have put every one of those events through one monitor + * for no structural change at all. */ + @Volatile + var relayList: SincePerRelayMap = mutableMapOf() + private set + + /** Guards the rare structural writes only. Readers and the per-event bump never take it. */ private val lock = KmpLock() fun addOrUpdate( relayUrl: NormalizedRelayUrl, time: Long, - ) = lock.withLock { - val eose = relayList[relayUrl] - if (eose == null) { - relayList[relayUrl] = MutableTime(time) - } else { - eose.updateIfNewer(time) + ) { + // Hot path: the relay is already known, so nothing about the map changes. + // + // The bump itself is unsynchronized. Two socket threads racing it can leave the older of two + // timestamps, because `updateIfNewer` reads-compares-writes without a lock — which is harmless + // here: this value is a floor for `since`, so losing a millisecond re-asks for a couple of + // events rather than skipping any. + val existing = relayList[relayUrl] + if (existing != null) { + existing.updateIfNewer(time) + return + } + + // Rare: first frame from this relay. Re-check inside the lock, since another thread may have + // inserted it between the read above and here. + lock.withLock { + val current = relayList[relayUrl] + if (current != null) { + current.updateIfNewer(time) + } else { + relayList = relayList.toMutableMap().apply { put(relayUrl, MutableTime(time)) } + } } } @@ -70,7 +93,9 @@ class EOSERelayList { */ fun remove(relayUrl: NormalizedRelayUrl) = lock.withLock { - relayList.remove(relayUrl) + if (relayList.containsKey(relayUrl)) { + relayList = relayList.toMutableMap().apply { remove(relayUrl) } + } } fun since() = relayList