feat: refactor UserList and UserListImpl for enhanced performance and thread safety

- Added `uuidSnapshot` method to `UserList` for generating point-in-time UUID snapshots.
- Refactored `UserListImpl` to leverage `ConcurrentHashMap` for lock-free, weakly-consistent iteration.
- Introduced caching mechanism with `modSeq` for efficient snapshot reuse in `UserListImpl`.
- Updated `CloudPlayerManagerImpl` to use updated `UserListImpl.of` structure.
- Improved efficiency and scalability of iteration, snapshotting, and mutation operations.

Author: twisti-dev

surf-cloud-api/surf-cloud-api-common/src/main/kotlin/dev/slne/surf/cloud/api/common/server/UserList.kt

@@ -1,34 +1,78 @@
 package dev.slne.surf.cloud.api.common.server
 
 import dev.slne.surf.cloud.api.common.player.CloudPlayer
-import it.unimi.dsi.fastutil.objects.ObjectSet
+import it.unimi.dsi.fastutil.objects.ObjectArrayList
+import org.jetbrains.annotations.ApiStatus
 import java.util.*
 
 /**
- * Represents a list of users currently connected to a server.
+ * Represents the *current* set of users connected to a server.
  *
- * This list provides a view of the player data and allows snapshots to be created.
- * The list itself does not directly modify the state of the server.
+ * ### Thread-safety and iteration semantics
+ * Implementations of this interface are designed to be used concurrently.
+ * The default implementation ([UserListImpl]) is backed by a
+ * lock-free, weakly-consistent iterator (via a `ConcurrentHashMap`-based key set),
+ * which means iteration does not block writes and will not throw [java.util.ConcurrentModificationException].
+ * However, a running iteration may or may not reflect concurrent additions/removals.
+ * If you require a stable, point-in-time view, call {@link #uuidSnapshot()} or {@link #snapshot()}.
+ *
+ * ### Mutability
+ * This is a read-only view. To obtain a mutable copy that you can modify freely
+ * (without affecting the underlying server state), use [snapshot] which returns
+ * a [MutableUserList].
  */
+@ApiStatus.NonExtendable
 interface UserList : Collection<CloudPlayer> {
 
-    val references: ObjectSet<UUID>
-
     /**
-     * Creates a mutable snapshot of the current user list.
+     * Creates a mutable, point-in-time copy of the current user list.
      *
-     * The snapshot is independent of the original list and can be modified
-     * without affecting the server or the original user list.
+     * The returned snapshot is independent from the live list; modifying the snapshot
+     * has no effect on the server or the original list.
      *
-     * @return A [MutableUserList] containing a copy of the current user list.
+     * @return a new [MutableUserList] containing a copy of the current users.
      */
     fun snapshot(): MutableUserList
+
+    /**
+     * Returns a snapshot of all player UUIDs as a contiguous array-backed list.
+     *
+     * The returned list is safe to iterate without blocking writers and remains
+     * stable even if the underlying live set changes after the call.
+     *
+     *
+     * **Performance:** This method allocates a new list and copies all
+     * UUIDs (O(n)). Prefer this when you need a consistent view across a longer
+     * operation or when you plan to iterate multiple times.
+     *
+     * @return an [ObjectArrayList] containing the UUIDs at the time of the call.
+     */
+    fun uuidSnapshot(): ObjectArrayList<UUID>
 }
 
 /**
- * Represents a mutable list of users on a server.
+ * A mutable user list.
+ *
+ * This type is intended for working with detached snapshots obtained via
+ * [UserList.snapshot].
+ * Mutating a [MutableUserList] does not affect
+ * the server or the original live list.
+ */
+@ApiStatus.NonExtendable
+interface MutableUserList : UserList, MutableCollection<CloudPlayer>
+
+/**
+ * Performs a fast, lock-free traversal over the UUIDs of this list using the
+ * default implementation's weakly-consistent iterator.
+ *
+ * **Important:** This extension relies on the instance being backed by
+ * [UserListImpl]. If you pass a custom implementation, a [ClassCastException]
+ * may occur. Use [UserList.uuidSnapshot] for a portable, implementation-agnostic
+ * traversal.
  *
- * This interface allows modifications to the list, but any changes
- * will not affect the actual server or its state.
+ * @receiver the user list to traverse (must be [UserListImpl]).
+ * @param action callback invoked for each UUID visible to the iterator at traversal time.
  */
-interface MutableUserList : UserList, MutableCollection<CloudPlayer>
+inline fun UserList.forEachWeakUuid(action: (UUID) -> Unit) {
+    (this as UserListImpl).forEachWeakUuid(action)
+}

surf-cloud-api/surf-cloud-api-common/src/main/kotlin/dev/slne/surf/cloud/api/common/server/UserListImpl.kt

@@ -4,108 +4,248 @@ import dev.slne.surf.cloud.api.common.netty.network.codec.streamCodec
 import dev.slne.surf.cloud.api.common.netty.protocol.buffer.SurfByteBuf
 import dev.slne.surf.cloud.api.common.player.CloudPlayer
 import dev.slne.surf.cloud.api.common.player.CloudPlayerManager
+import dev.slne.surf.cloud.api.common.server.UserListImpl.Companion.STREAM_CODEC
 import dev.slne.surf.cloud.api.common.util.annotation.InternalApi
-import dev.slne.surf.cloud.api.common.util.mutableObjectSetOf
-import dev.slne.surf.cloud.api.common.util.synchronize
-import dev.slne.surf.cloud.api.common.util.toObjectSet
-import it.unimi.dsi.fastutil.objects.ObjectSet
+import it.unimi.dsi.fastutil.objects.ObjectArrayList
+import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet
 import java.util.*
-
+import java.util.concurrent.ConcurrentHashMap
+import java.util.concurrent.atomic.AtomicLong
+
+/**
+ * Default, high-throughput implementation of [UserList] optimized for
+ * concurrent reads and frequent iteration in large Minecraft networks.
+ *
+ * ### Design rationale
+ *
+ * - **Concurrency**: Backed by [ConcurrentHashMap.newKeySet], providing
+ * fine-grained striping/CAS and weakly-consistent iterators. Readers never block writers, which
+ * is crucial when plugins may traverse thousands of players while joins/leaves occur.
+ *
+ * - **Stable wire format**: Serialization via [STREAM_CODEC] uses a
+ * snapshot array to ensure a consistent view on the wire even during concurrent updates.
+ *
+ * - **Snapshot cache**: A lightweight modification counter ([modSeq]) invalidates a
+ * cached array snapshot to avoid repeated allocations when the set is unchanged (e.g., multiple
+ * serializations per tick).
+ *
+ * ### Iteration semantics
+ * Iteration is weakly consistent: it never throws [java.util.ConcurrentModificationException],
+ * but may omit or not yet include elements added/removed concurrently. Use [uuidSnapshot] for
+ * a stable, point-in-time collection.
+ */
 @InternalApi
 open class UserListImpl : UserList {
     companion object {
+
+        /**
+         * Binary codec for streaming a [UserListImpl] over [SurfByteBuf].
+         *
+         * **Consistency:** The codec writes a snapshot array captured at the start of encoding,
+         * ensuring readers see a coherent list of UUIDs regardless of concurrent modifications.
+         */
         val STREAM_CODEC = streamCodec<SurfByteBuf, UserListImpl>({ buf, list ->
-            buf.writeCollection(list.playerReferences) { buffer, uuid -> buffer.writeUuid(uuid) }
+            val snapshot = list.snapshotArray()
+            buf.writeArray(snapshot) { buffer, uuid -> buffer.writeUuid(uuid) }
         }, { buf ->
-            UserListImpl(buf.readCollection({ mutableObjectSetOf(it) }, { it.readUuid() }))
+            val uuids = buf.readArray { buf.readUuid() }
+            UserListImpl(ObjectArrayList(uuids))
         })
 
-        fun of(players: Iterable<CloudPlayer>): UserListImpl {
-            return UserListImpl(players.mapTo(mutableObjectSetOf()) { it.uuid })
+
+        /**
+         * Creates a [UserListImpl] from a preexisting set of UUIDs.
+         *
+         * @param uuids UUIDs to initialize the list with.
+         * @return a new [UserListImpl] containing the given UUIDs.
+         */
+        fun of(uuids: Set<UUID>): UserListImpl {
+            return UserListImpl(ObjectArrayList(uuids))
         }
     }
 
-    internal val playerReferences: ObjectSet<UUID>
+    /** Backing set: scalable, lock-free key set. */
+    @PublishedApi
+    internal val uuids: ConcurrentHashMap.KeySetView<UUID, Boolean>
 
-    override val size get() = playerReferences.size
-    override val references: ObjectSet<UUID>
-        get() = playerReferences.toObjectSet()
+    /** Modification counter used to invalidate the cached snapshot array. */
+    protected val modSeq = AtomicLong(0)
 
+    @Volatile
+    private var cachedSnapshot: Array<UUID> = emptyArray()
+
+    @Volatile
+    private var cachedModSeq: Long = -1
+
+    /** Creates an empty user list. */
     constructor() {
-        playerReferences = mutableObjectSetOf<UUID>().synchronize()
+        uuids = ConcurrentHashMap.newKeySet<UUID>()
     }
 
-    internal constructor(playerReferences: ObjectSet<UUID>) {
-        this.playerReferences = playerReferences.synchronize()
+    /**
+     * Creates a user list preloaded with UUIDs.
+     *
+     * @param initial UUIDs to preload into the set.
+     */
+    internal constructor(initial: ObjectArrayList<UUID>) {
+        uuids = ConcurrentHashMap.newKeySet<UUID>(initial.size)
+        uuids.addAll(initial)
     }
 
-    override fun contains(element: CloudPlayer) = playerReferences.contains(element.uuid)
-    override fun containsAll(elements: Collection<CloudPlayer>) =
-        elements.all { playerReferences.contains(it.uuid) }
+    override val size
+        get() = uuids.size
 
-    override fun isEmpty() = playerReferences.isEmpty()
-    override fun iterator(): Iterator<CloudPlayer> {
-        return object : Iterator<CloudPlayer> {
-            private val iterator = playerReferences.iterator()
-            override fun hasNext() = iterator.hasNext()
-            override fun next() = CloudPlayerManager.getPlayer(iterator.next())
-                ?: throw NoSuchElementException("Player not found")
-        }
+    override fun isEmpty() = uuids.isEmpty()
+    override fun contains(element: CloudPlayer) = uuids.contains(element.uuid)
+    override fun containsAll(elements: Collection<CloudPlayer>): Boolean {
+        for (e in elements) if (!uuids.contains(e.uuid)) return false
+        return true
     }
 
-    fun add(playerUuid: UUID): Boolean {
-        return playerReferences.add(playerUuid)
+    /**
+     * Returns an iterator over players resolved on demand from [CloudPlayerManager].
+     *
+     * **Note:** If a UUID in the set cannot be resolved to a [CloudPlayer]
+     * (e.g., the player disconnected between reading the UUID and resolution),
+     * a [NoSuchElementException] is thrown from [next] to signal the race.
+     *
+     * @throws NoSuchElementException if a UUID cannot be resolved to a player at iteration time.
+     */
+    override fun iterator() = object : Iterator<CloudPlayer> {
+        private val iterator = uuids.iterator()
+        override fun hasNext() = iterator.hasNext()
+        override fun next() = CloudPlayerManager.getPlayer(iterator.next())
+            ?: throw NoSuchElementException("Player not found")
     }
 
-    fun remove(playerUuid: UUID): Boolean {
-        return playerReferences.remove(playerUuid)
+    /**
+     * Implementation-specific fast traversal over UUIDs using the weakly-consistent iterator.
+     *
+     * @param action callback invoked for each UUID visible to the iterator.
+     */
+    inline fun forEachWeakUuid(action: (UUID) -> Unit) {
+        for (u in uuids) action(u)
     }
 
     override fun snapshot(): MutableUserList {
-        return MutableUserListImpl().also { it.addAll(this) }
+        return MutableUserListImpl(uuidSnapshot())
+    }
+
+    override fun uuidSnapshot() = ObjectArrayList(snapshotArray())
+
+    /**
+     * Returns a cached snapshot array of UUIDs, invalidated whenever the set changes.
+     *
+     * **Performance:** Avoids repeated allocations between unchanged serializations.
+     * The snapshot is coherent at the time of creation and unaffected by later mutations.
+     *
+     * @return array of UUIDs representing a point-in-time view.
+     */
+    fun snapshotArray(): Array<UUID> {
+        val m = modSeq.get()
+        if (m == cachedModSeq) return cachedSnapshot
+        val snap = uuids.toTypedArray()
+        cachedSnapshot = snap
+        cachedModSeq = m
+        return snap
+    }
+
+
+    /**
+     * Adds a player UUID to the live set.
+     *
+     * @param playerUuid the UUID to add.
+     * @return `true` if the UUID was not present and has been added; `false` otherwise.
+     */
+    fun add(playerUuid: UUID): Boolean {
+        val added = uuids.add(playerUuid)
+        if (added) {
+            modSeq.incrementAndGet()
+        }
+        return added
+    }
+
+    /**
+     * Removes a player UUID from the live set.
+     *
+     * @param playerUuid the UUID to remove.
+     * @return `true` if the UUID was present and has been removed; `false` otherwise.
+     */
+    fun remove(playerUuid: UUID): Boolean {
+        val removed = uuids.remove(playerUuid)
+        if (removed) {
+            modSeq.incrementAndGet()
+        }
+        return removed
     }
 
     override fun toString(): String {
-        return "UserListImpl(playerReferences=$playerReferences, size=$size)"
+        return "UserListImpl(size=$size)"
     }
 }
 
+
+/**
+ * Mutable snapshot implementation that can be freely modified without impacting the live set.
+ *
+ * ### Notes
+ *
+ * - [clear] increments [modSeq] to invalidate the internal snapshot cache.
+ * - Mutating via the iterator's [MutableIterator.remove] also bumps [modSeq]
+ * to ensure consistent snapshot invalidation.
+ * - [retainAll] builds a temporary [ObjectOpenHashSet] of UUIDs to perform the set
+ * operation efficiently.
+ */
 @InternalApi
-class MutableUserListImpl : UserListImpl(), MutableUserList {
+class MutableUserListImpl(initial: ObjectArrayList<UUID>) : UserListImpl(initial), MutableUserList {
     override fun add(element: CloudPlayer): Boolean {
         return add(element.uuid)
     }
 
     override fun addAll(elements: Collection<CloudPlayer>): Boolean {
-        return elements.all { playerReferences.add(it.uuid) }
+        var changed = false
+        for (e in elements) if (add(e.uuid)) changed = true
+        return changed
     }
 
     override fun clear() {
-        playerReferences.clear()
+        if (uuids.isNotEmpty()) {
+            uuids.clear()
+            modSeq.incrementAndGet()
+        }
     }
 
-    override fun iterator(): MutableIterator<CloudPlayer> {
-        return object : MutableIterator<CloudPlayer> {
-            private val iterator = playerReferences.iterator()
-            override fun hasNext() = iterator.hasNext()
-            override fun next() = CloudPlayerManager.getPlayer(iterator.next())
-                ?: throw NoSuchElementException("Player not found")
-
-            override fun remove() {
-                iterator.remove()
-            }
+    /**
+     * Iterator over players resolved on demand. Removal through the iterator
+     * invalidates the snapshot cache to keep [snapshotArray] fresh.
+     */
+    override fun iterator() = object : MutableIterator<CloudPlayer> {
+        private val iterator = uuids.iterator()
+        override fun hasNext() = iterator.hasNext()
+        override fun next() = CloudPlayerManager.getPlayer(iterator.next())
+            ?: throw NoSuchElementException("Player not found")
+
+        override fun remove() {
+            iterator.remove()
+            modSeq.incrementAndGet()
         }
     }
 
+
     override fun remove(element: CloudPlayer): Boolean {
         return remove(element.uuid)
     }
 
     override fun removeAll(elements: Collection<CloudPlayer>): Boolean {
-        return elements.all { playerReferences.remove(it.uuid) }
+        var changed = false
+        for (e in elements) if (remove(e.uuid)) changed = true
+        return changed
     }
 
     override fun retainAll(elements: Collection<CloudPlayer>): Boolean {
-        return playerReferences.retainAll(elements.map { it.uuid })
+        val keep = ObjectOpenHashSet<UUID>(elements.size)
+        for (e in elements) keep.add(e.uuid)
+        val changed = uuids.retainAll(keep)
+        return changed
     }
 }