From 993ef8e8f30092749c381cdce0e00ca196041f42 Mon Sep 17 00:00:00 2001 From: GeekOfWires Date: Wed, 22 Jul 2026 05:15:39 -0400 Subject: [PATCH 01/18] Restore the missing Default import in LocalActionTest LocalActionTest referenced Default.GUID0 but imported only ExplosiveDeployable and GlobalDefinitions from net.psforever.objects, so Test/compile failed with "not found: value Default". This broke test compilation for the entire project, not just this suite -- no test could be run on master until it was fixed. Add Default to the existing import list. No test logic changes. Co-Authored-By: Claude Opus 4.8 --- src/test/scala/service/local/LocalActionTest.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/scala/service/local/LocalActionTest.scala b/src/test/scala/service/local/LocalActionTest.scala index 87d055260..9f187eb80 100644 --- a/src/test/scala/service/local/LocalActionTest.scala +++ b/src/test/scala/service/local/LocalActionTest.scala @@ -1,7 +1,7 @@ // Copyright (c) 2026 PSForever package service.local -import net.psforever.objects.{ExplosiveDeployable, GlobalDefinitions} +import net.psforever.objects.{Default, ExplosiveDeployable, GlobalDefinitions} import net.psforever.packet.game.{ObjectCreateMessage, TriggeredEffect, TriggeredEffectLocation} import net.psforever.services.Service import net.psforever.services.base.message.SendResponse From e6400edc264180dd936f2d70ba80a150090a6468 Mon Sep 17 00:00:00 2001 From: GeekOfWires Date: Wed, 22 Jul 2026 05:15:51 -0400 Subject: [PATCH 02/18] Index blockmap sectors directly instead of copying the whole zone BlockMap.blocks was a ListBuffer[Sector], and both sector lookup helpers resolved indices through structure.toSeq. On a mutable ListBuffer, toSeq builds a fresh immutable.List -- a full copy of every sector in the zone -- and List.apply(index) is then an O(index) linked-list walk. For a standard 8192-square map at the default span size of 100 that is ~6,700 sectors copied per call, followed by an average ~3,300 pointer hops per index resolved. sectorsOnlyWithinBlockStructure resolves every index in the query, and sectorOnlyWithinBlockStructure is called once per changed sector index by move(), so a boundary crossing paid the copy repeatedly. Every blockmap entry point routes through these helpers -- sector(), addTo(), move(), actuallyRemoveFrom() -- and the per-player movement path hits them twice per upstream packet: once via SessionData.updateBlockMap and again via InteractsWithZone.zoneInteractions. This was therefore a steady, density-scaled source of CPU and young-generation garbage, and young-gen collections stall every session at once. blocks is only ever read after construction, never structurally modified, so it becomes an IndexedSeq and both helpers index it directly. The Sector objects themselves remain mutable as before; only the container changes. Co-Authored-By: Claude Opus 4.8 --- .../objects/zones/blockmap/BlockMap.scala | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/main/scala/net/psforever/objects/zones/blockmap/BlockMap.scala b/src/main/scala/net/psforever/objects/zones/blockmap/BlockMap.scala index 27c7f8631..f6bff91d1 100644 --- a/src/main/scala/net/psforever/objects/zones/blockmap/BlockMap.scala +++ b/src/main/scala/net/psforever/objects/zones/blockmap/BlockMap.scala @@ -7,8 +7,6 @@ import net.psforever.objects.serverobject.structures.Building import net.psforever.objects.zones.MapScale import net.psforever.types.Vector3 -import scala.collection.mutable.ListBuffer - /** * A data structure which divides coordinate space into buckets or coordinate spans. * The function of the blockmap is to organize the instantiated game objects (entities) @@ -40,16 +38,14 @@ class BlockMap(fullMapWidth: Int, fullMapHeight: Int, desiredSpanSize: Int) { /** the sectors / blocks / buckets into which entities that submit themselves are divided; * while the represented region need not be square, the sectors are defined as squares */ - val blocks: ListBuffer[Sector] = { + val blocks: IndexedSeq[Sector] = { val horizontal: List[Int] = List.range(0, fullMapWidth, spanSize) val vertical: List[Int] = List.range(0, fullMapHeight, spanSize) - ListBuffer.newBuilder[Sector].addAll( - vertical.flatMap { latitude => - horizontal.map { longitude => - new Sector(longitude, latitude, spanSize) - } + vertical.flatMap { latitude => + horizontal.map { longitude => + new Sector(longitude, latitude, spanSize) } - ).result() + }.toIndexedSeq } /** @@ -507,10 +503,10 @@ object BlockMap { */ private def sectorOnlyWithinBlockStructure( index: Int, - structure: Iterable[Sector] + structure: IndexedSeq[Sector] ): Sector = { if (index < structure.size) { - structure.toSeq(index) + structure(index) } else { Sector.Empty } @@ -525,11 +521,10 @@ object BlockMap { */ private def sectorsOnlyWithinBlockStructure( list: Iterable[Int], - structure: Iterable[Sector] + structure: IndexedSeq[Sector] ): Iterable[Sector] = { if (list.nonEmpty && list.max < structure.size) { - val structureSeq = structure.toSeq - list.toSet.map { structureSeq } + list.toSet.map(structure.apply) } else { List[Sector]() } From 2cd05fa5bd0b5e7f661c046ed2f05d76904d73c9 Mon Sep 17 00:00:00 2001 From: GeekOfWires Date: Wed, 22 Jul 2026 05:16:00 -0400 Subject: [PATCH 03/18] Condense sector group entity lists on demand SectorGroup eagerly materialised all ten of its entity categories at construction. For a multi-sector group each category ran flatMap -> toList -> distinct across every covered sector, and each sector accessor itself copied its backing ListBuffer via SectorListOf.list. Callers almost never want all ten. The per-movement-packet refresh in SessionData.updateBlockMap covers ~144 sectors at the default 550m draw range and the result is consumed almost entirely for one scalar, localSector.livePlayerList.size, so nine categories were condensed and discarded on every upstream packet from every player. The lists are now by-name constructor parameters retained as lazy vals, so each category is condensed on first access and memoised thereafter. This does move when a category is snapshotted, from construction to first read. The backing sector lists are mutable and updated by other actors, so that read was already unsynchronised; this shifts the window rather than introducing one, and removes it entirely for categories that are never consulted. Co-Authored-By: Claude Opus 4.8 --- .../objects/zones/blockmap/Sector.scala | 40 ++++++++++++++----- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/src/main/scala/net/psforever/objects/zones/blockmap/Sector.scala b/src/main/scala/net/psforever/objects/zones/blockmap/Sector.scala index 8e8b8ba95..78de9d6ee 100644 --- a/src/main/scala/net/psforever/objects/zones/blockmap/Sector.scala +++ b/src/main/scala/net/psforever/objects/zones/blockmap/Sector.scala @@ -276,21 +276,39 @@ object Sector { * @param amenityList the structures within the structures * @param environmentList fields that represent the game world environment */ +/** + * The entity lists are declared by-name and retained as `lazy val`s. + * A sector group frequently spans a great many sectors, and callers usually consult only one or two + * of the ten categories, so materializing all of them eagerly does a large amount of discarded work + * on hot paths such as the per-movement-packet local sector refresh. + * Each list is instead condensed on first access and memoized thereafter. + */ class SectorGroup( val rangeX: Float, val rangeY: Float, - val livePlayerList: List[Player], - val botList: List[AvatarBot], - val corpseList: List[Player], - val vehicleList: List[Vehicle], - val equipmentOnGroundList: List[Equipment], - val deployableList: List[Deployable], - val buildingList: List[Building], - val amenityList: List[Amenity], - val environmentList: List[PieceOfEnvironment], - val projectileList: List[Projectile] + livePlayers: => List[Player], + bots: => List[AvatarBot], + corpses: => List[Player], + vehicles: => List[Vehicle], + equipmentOnGround: => List[Equipment], + deployables: => List[Deployable], + buildings: => List[Building], + amenities: => List[Amenity], + environment: => List[PieceOfEnvironment], + projectiles: => List[Projectile] ) - extends SectorPopulation + extends SectorPopulation { + lazy val livePlayerList: List[Player] = livePlayers + lazy val botList: List[AvatarBot] = bots + lazy val corpseList: List[Player] = corpses + lazy val vehicleList: List[Vehicle] = vehicles + lazy val equipmentOnGroundList: List[Equipment] = equipmentOnGround + lazy val deployableList: List[Deployable] = deployables + lazy val buildingList: List[Building] = buildings + lazy val amenityList: List[Amenity] = amenities + lazy val environmentList: List[PieceOfEnvironment] = environment + lazy val projectileList: List[Projectile] = projectiles +} object SectorGroup { /** cached sector of no range and no entity coverage */ From 692fe494c0c3ca5ceb41812f64a3c42db3b31b4a Mon Sep 17 00:00:00 2001 From: GeekOfWires Date: Wed, 22 Jul 2026 05:18:06 -0400 Subject: [PATCH 04/18] Reuse the sector query when recalculating a facility's SOI UpdateSOI called zone.blockMap.sector(facility) twice per facility, once for livePlayerList and again for vehicleList, discarding the first result. The query spans every sector within the facility's SOIRadius, so the whole lookup and sector-group construction was performed twice for no benefit. This runs for every facility on the continent in a single message, every five seconds, which gives it a periodic-stutter shape. Hoist the result into a local. Co-Authored-By: Claude Opus 4.8 --- .../net/psforever/objects/zones/SphereOfInfluenceActor.scala | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/scala/net/psforever/objects/zones/SphereOfInfluenceActor.scala b/src/main/scala/net/psforever/objects/zones/SphereOfInfluenceActor.scala index 4c5a4171a..62ad407a7 100644 --- a/src/main/scala/net/psforever/objects/zones/SphereOfInfluenceActor.scala +++ b/src/main/scala/net/psforever/objects/zones/SphereOfInfluenceActor.scala @@ -55,11 +55,12 @@ class SphereOfInfluenceActor(zone: Zone) extends Actor { sois.foreach { case (facility, radius) => val facilityXY = facility.Position.xy - val playersOnFoot = zone.blockMap.sector(facility) + val sector = zone.blockMap.sector(facility) + val playersOnFoot = sector .livePlayerList .filter(p => Vector3.DistanceSquared(facilityXY, p.Position.xy) < radius) - val vehicleOccupants = zone.blockMap.sector(facility) + val vehicleOccupants = sector .vehicleList .filter(v => Vector3.DistanceSquared(facilityXY, v.Position.xy) < radius) .flatMap(_.Seats.values.flatMap(_.occupants)) From c4a84a93807cdf17566d7268b254290d1bea7d69 Mon Sep 17 00:00:00 2001 From: GeekOfWires Date: Wed, 22 Jul 2026 05:18:06 -0400 Subject: [PATCH 05/18] Unwind bundled envelopes in linear time unwind accumulated with out :+ first. Appending to a List is O(length), making the whole traversal O(n^2). Bundles are built per recipient over the zone population -- deconstruction, emotes, deployable updates -- so the cost scaled with player count on paths that already fan out. The constructor assertion then called unwind a second time purely to compare sizes, doubling that work. assert is not elided here; the build sets no -Xelide-below. Accumulate by prepending and reverse once at the end, and replace the assertion with a direct O(n) test for a nested bundle. The size comparison was also unsound: a nested BundledEnvelope holding exactly one element unwound to the same size and passed. Co-Authored-By: Claude Opus 4.8 --- .../psforever/services/base/envelope/BundledEnvelope.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/scala/net/psforever/services/base/envelope/BundledEnvelope.scala b/src/main/scala/net/psforever/services/base/envelope/BundledEnvelope.scala index 9c831b014..e61d977e9 100644 --- a/src/main/scala/net/psforever/services/base/envelope/BundledEnvelope.scala +++ b/src/main/scala/net/psforever/services/base/envelope/BundledEnvelope.scala @@ -35,7 +35,7 @@ case object NoResponse extends GenericResponseEnvelope { */ final case class BundledEnvelope(msgs: Iterable[GenericMessageEnvelope]) extends GenericMessageEnvelope { - assert(msgs.size == BundledEnvelope.unwind(msgs).size, "do not nest bundled event system envelopes") + assert(!msgs.exists(_.isInstanceOf[BundledEnvelope]), "do not nest bundled event system envelopes") override def msg: EventMessage = NoMessage override def response(stamp: EventSystemStamp): GenericResponseEnvelope = NoResponse @@ -69,13 +69,13 @@ object BundledEnvelope { @tailrec private def unwind(in: Iterable[GenericMessageEnvelope], out: List[GenericMessageEnvelope] = Nil): List[GenericMessageEnvelope] = { if (in.isEmpty) { - out + out.reverse } else { in.head match { case bundle: BundledEnvelope => unwind(bundle.msgs ++ in.tail, out) case first => - unwind(in.tail, out :+ first) + unwind(in.tail, first :: out) } } } From 13dfbc189434506d6b1320d7df77727b0525b18d Mon Sep 17 00:00:00 2001 From: GeekOfWires Date: Wed, 22 Jul 2026 05:18:16 -0400 Subject: [PATCH 06/18] Stop rebuilding codecs and error strings on the per-packet path Two allocation sources on the hottest path in the server: MultiPacketEx.sizeCodec built its composed codecs inside encode and allocated its sizeTypes and guards lists inside decode. sizeCodec runs once per sub-packet of every outbound bundle and every inbound multi-packet, so each sub-packet paid for a fresh constant :: uintNL codec tree and two list allocations with hex literals parsed each time. scodec codecs are immutable and safe to share, so these are hoisted to private vals; the lists become Vectors for constant-time indexing. noDecoder in both opcode tables rendered the entire payload as hex into the failure message. Err takes its message by value, so that string -- twice the packet's length -- was constructed eagerly for every packet arriving on an unimplemented opcode, and many opcodes are still stubs. The opcode by itself identifies which marshaller is missing, so the hex dump is dropped. Co-Authored-By: Claude Opus 4.8 --- .../net/psforever/packet/ControlPacketOpcode.scala | 3 ++- .../net/psforever/packet/GamePacketOpcode.scala | 6 +++++- .../psforever/packet/control/MultiPacketEx.scala | 14 +++++++++----- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/main/scala/net/psforever/packet/ControlPacketOpcode.scala b/src/main/scala/net/psforever/packet/ControlPacketOpcode.scala index 3e8286dee..52d8771d0 100644 --- a/src/main/scala/net/psforever/packet/ControlPacketOpcode.scala +++ b/src/main/scala/net/psforever/packet/ControlPacketOpcode.scala @@ -48,8 +48,9 @@ object ControlPacketOpcode extends Enumeration { ClientHotStart // Probably a more lightweight variant of ClientStart, containing only the client nonce = Value + /* see the note on GamePacketOpcode.noDecoder -- Err builds its message eagerly */ private def noDecoder(opcode: ControlPacketOpcode.Type) = - (bits: BitVector) => Attempt.failure(Err(s"Could not find a marshaller for control packet $opcode (${bits.toHex})")) + (_: BitVector) => Attempt.failure(Err(s"Could not find a marshaller for control packet $opcode")) def getPacketDecoder( opcode: ControlPacketOpcode.Type diff --git a/src/main/scala/net/psforever/packet/GamePacketOpcode.scala b/src/main/scala/net/psforever/packet/GamePacketOpcode.scala index 54c4e3352..249e21c71 100644 --- a/src/main/scala/net/psforever/packet/GamePacketOpcode.scala +++ b/src/main/scala/net/psforever/packet/GamePacketOpcode.scala @@ -294,8 +294,12 @@ object GamePacketOpcode extends Enumeration { ClientCheatedMessage // last known message type (243, 0xf3) = Value + /* Err takes its message by value, so any interpolation here is built eagerly on every + packet that lands on an unimplemented opcode -- and a great many opcodes are still + stubs. Rendering the payload as hex allocated a string twice the packet's length each + time; the opcode alone identifies the gap. */ private def noDecoder(opcode: GamePacketOpcode.Type) = - (bits: BitVector) => Attempt.failure(Err(s"Could not find a marshaller for game packet $opcode (${bits.toHex})")) + (_: BitVector) => Attempt.failure(Err(s"Could not find a marshaller for game packet $opcode")) /// Mapping of packet IDs to decoders. Notice that we are using the @switch annotation which ensures that the Scala /// compiler will be able to optimize this as a lookup table (switch statement). Microbenchmarks show a nearly 400x diff --git a/src/main/scala/net/psforever/packet/control/MultiPacketEx.scala b/src/main/scala/net/psforever/packet/control/MultiPacketEx.scala index 1848025eb..5d2bcf142 100644 --- a/src/main/scala/net/psforever/packet/control/MultiPacketEx.scala +++ b/src/main/scala/net/psforever/packet/control/MultiPacketEx.scala @@ -21,6 +21,13 @@ object MultiPacketEx extends Marshallable[MultiPacketEx] { val MaxValue = (1L << 31) - 1 val MinValue = 0 + /* hoisted out of encode/decode; these run once per sub-packet of every inbound + and outbound bundle, and scodec codecs are immutable and safe to share */ + private val mediumCodec = (constant(hex"ff") :: uint16L).dropUnits + private val largeCodec = (constant(hex"ffffff") :: uint32L).dropUnits + private val sizeTypes = Vector(8, 16, 32) + private val guards = Vector(hex"ff".bits, hex"ffff".bits) + override def encode(i: Long) = { if (i > MaxValue) { Attempt.failure(Err(s"$i is greater than maximum value $MaxValue for $description")) @@ -30,17 +37,14 @@ object MultiPacketEx extends Marshallable[MultiPacketEx] { if (i < 0xff) { uint8L.encode(i.toInt) } else if (i < 0xffff) { - (constant(hex"ff") :: uint16L).dropUnits.encode(i.toInt :: HNil) + mediumCodec.encode(i.toInt :: HNil) } else { - (constant(hex"ffffff") :: uint32L).dropUnits.encode(i :: HNil) + largeCodec.encode(i :: HNil) } } } override def decode(buffer: BitVector): Attempt[DecodeResult[Long]] = { - val sizeTypes = List(8, 16, 32) - val guards = List(hex"ff".bits, hex"ffff".bits) - var buf = buffer for (i <- sizeTypes.indices) { From e2fa78a7d33517623a60ba862a99214a02e1395c Mon Sep 17 00:00:00 2001 From: GeekOfWires Date: Wed, 22 Jul 2026 05:21:07 -0400 Subject: [PATCH 07/18] Give each task bundle its own completion monitor composeTaskAndSubs synchronised on a local var of type Boolean. Scala boxes a primitive to call synchronized, and java.lang.Boolean.valueOf returns the cached TRUE and FALSE singletons, so every task bundle in every zone contended one of the same two JVM-wide monitors. Registering an avatar fans out across holsters, locker and inventory -- routinely 30-50 subtasks -- and each completion re-entered that monitor and ran an O(n) forall inside it, so a squad respawning serialised against a vehicle spawning and a corpse being looted. It was also not a correct mutex. The monitor is the boxed value, so its identity changed the moment the flag flipped from true to false, and two threads could be inside the critical section simultaneously and complete the promise twice. Use a dedicated lock object. The O(n) completion scan is left alone; it wants the barrier rewritten in terms of Future.sequence, which is a larger change than this fix. Co-Authored-By: Claude Opus 4.8 --- .../net/psforever/objects/guid/TaskWorkflow.scala | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/main/scala/net/psforever/objects/guid/TaskWorkflow.scala b/src/main/scala/net/psforever/objects/guid/TaskWorkflow.scala index 85b13429d..507e10885 100644 --- a/src/main/scala/net/psforever/objects/guid/TaskWorkflow.scala +++ b/src/main/scala/net/psforever/objects/guid/TaskWorkflow.scala @@ -145,11 +145,17 @@ object TaskWorkflow { //no subtasks; just execute the main task promise.completeWith(task.action()) case list => - var unassignedCompletion: Boolean = true //shared mutex + /* The monitor must be a dedicated object. Synchronising on a Boolean synchronises on + the boxed java.lang.Boolean, and valueOf returns the cached TRUE/FALSE singletons, + so every task bundle in the server contended the same two JVM-wide monitors. + It was also not a valid mutex: the monitor identity changed when the flag flipped, + so two threads could hold "the lock" at once and complete the promise twice. */ + val completionLock = new Object + var unassignedCompletion: Boolean = true //wait for subtasks to complete list.foreach { result => result.onComplete { _ => - unassignedCompletion.synchronized { + completionLock.synchronized { if (unassignedCompletion && composedSubs.forall(matchOnFutureCompletion)) { unassignedCompletion = false if (composedSubs.forall(matchOnFutureSuccess)) { From dcb05c4258799c3bd180295da0507be54342b9a3 Mon Sep 17 00:00:00 2001 From: GeekOfWires Date: Wed, 22 Jul 2026 05:21:07 -0400 Subject: [PATCH 08/18] Release scheduled work and thread pools when they are finished with Two leaks of java.util.concurrent resources. HackCaptureActor.processBuildingsWithDelay scheduled a fixed-rate task and never cancelled it, so once its building iterator was exhausted the task kept waking every delayMillis, forever, doing nothing. The near identical loop in ChatOperations.processBuildingsWithDelay already captures its handle and cancels; this applies the same pattern. ChatOperations holds its own Executors.newScheduledThreadPool(2), and the class is constructed once per session. Nothing shut it down, so the threads outlived every session that used it. It already has a stop() that SessionData.stop() invokes for teardown, so the shutdown goes there. Co-Authored-By: Claude Opus 4.8 --- .../actors/session/support/ChatOperations.scala | 2 ++ .../services/local/support/HackCaptureActor.scala | 11 +++++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/main/scala/net/psforever/actors/session/support/ChatOperations.scala b/src/main/scala/net/psforever/actors/session/support/ChatOperations.scala index 0294b7531..2e671a451 100644 --- a/src/main/scala/net/psforever/actors/session/support/ChatOperations.scala +++ b/src/main/scala/net/psforever/actors/session/support/ChatOperations.scala @@ -1487,5 +1487,7 @@ class ChatOperations( override protected[session] def stop(): Unit = { silenceTimer.cancel() chatService ! ChatService.LeaveAllChannels(chatServiceAdapter) + /* this pool is per-session; without this its threads outlive the session that made them */ + scheduler.shutdown() } } diff --git a/src/main/scala/net/psforever/services/local/support/HackCaptureActor.scala b/src/main/scala/net/psforever/services/local/support/HackCaptureActor.scala index dc579b915..568025829 100644 --- a/src/main/scala/net/psforever/services/local/support/HackCaptureActor.scala +++ b/src/main/scala/net/psforever/services/local/support/HackCaptureActor.scala @@ -20,7 +20,7 @@ import net.psforever.services.local.support.HackCaptureActor.GetHackingFaction import net.psforever.services.local.LocalAction import net.psforever.types.{ChatMessageType, PlanetSideEmpire} -import java.util.concurrent.{Executors, TimeUnit} +import java.util.concurrent.{Executors, ScheduledFuture, TimeUnit} import scala.collection.Seq import scala.collection.mutable.ArrayBuffer import scala.concurrent.duration.{FiniteDuration, _} @@ -342,7 +342,11 @@ class HackCaptureActor extends Actor { delayMillis: Long ): Unit = { val buildingIterator = buildings.iterator - scheduler.scheduleAtFixedRate( + /* the handle must be cancelled once the iterator is spent, or the task keeps waking + every delayMillis for the lifetime of the pool doing nothing; compare the equivalent + loop in ChatOperations.processBuildingsWithDelay, which already cancels */ + var handle: ScheduledFuture[_] = null + handle = scheduler.scheduleAtFixedRate( () => { if (buildingIterator.hasNext) { val building = buildingIterator.next() @@ -352,6 +356,9 @@ class HackCaptureActor extends Actor { buildingActor ! BuildingActor.AmenityStateChange(terminal, Some(false)) buildingActor ! BuildingActor.MapUpdate() } + else { + handle.cancel(false) + } }, 0, delayMillis, From 4c1b69e7f2be829ef12d0655834aea6813b50be7 Mon Sep 17 00:00:00 2001 From: GeekOfWires Date: Wed, 22 Jul 2026 05:21:21 -0400 Subject: [PATCH 09/18] Send exactly one Resume for each container Wait The cross-container move path sent Wait() to the destination and then answered it with: moveItemOver .recover { case _: AskTimeoutException => ... Resume() } .onComplete { _ => ... Resume() } recover produces a successful future, so on an ask timeout the recover sent a Resume and the onComplete immediately sent a second one. The surplus Resume releases the guard belonging to an unrelated concurrent move, allowing insertions to interleave with a move that is still in flight -- while the item is removed from its source. onComplete alone fires for both success and failure, so the recover clause is redundant as well as harmful. Co-Authored-By: Claude Opus 4.8 --- .../containable/ContainableBehavior.scala | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/main/scala/net/psforever/objects/serverobject/containable/ContainableBehavior.scala b/src/main/scala/net/psforever/objects/serverobject/containable/ContainableBehavior.scala index 77bf4c960..6a206c5b7 100644 --- a/src/main/scala/net/psforever/objects/serverobject/containable/ContainableBehavior.scala +++ b/src/main/scala/net/psforever/objects/serverobject/containable/ContainableBehavior.scala @@ -2,7 +2,7 @@ package net.psforever.objects.serverobject.containable import akka.actor.{Actor, ActorRef} -import akka.pattern.{AskTimeoutException, ask} +import akka.pattern.ask import akka.util.Timeout import net.psforever.objects.equipment.{Equipment, EquipmentSize} import net.psforever.objects.inventory.{Container, InventoryItem} @@ -196,10 +196,12 @@ trait ContainableBehavior { case _ => ; //TODO what? } - //always do this - moveItemOver - .recover { case _: AskTimeoutException => destination.Actor ! ContainableBehavior.Resume() } - .onComplete { _ => destination.Actor ! ContainableBehavior.Resume() } + /* exactly one Resume must answer the Wait() above. The previous form recovered an + AskTimeoutException by sending a Resume and, because recover yields a successful + future, the following onComplete sent a second one -- releasing an unrelated + concurrent move's guard and letting interleaved insertions through. + onComplete alone already covers both success and failure. */ + moveItemOver.onComplete { _ => destination.Actor ! ContainableBehavior.Resume() } } case _ => ; //we could not find the item to be moved in the source location; trying to act on old data? From 6df975806206e64baf586f3b7bbcafc0845babfe Mon Sep 17 00:00:00 2001 From: GeekOfWires Date: Wed, 22 Jul 2026 05:25:00 -0400 Subject: [PATCH 10/18] Configure the dispatcher everything actually runs on akka.conf assigns dispatchers to more than thirty actor paths, and dispatchers.conf defines all of them carefully. None of it takes effect. No actor is created at any of those paths: the socket actors are named world-socket- beneath socketPane, there is no world-session-router, and zone actors are named zone- rather than -actor. Grepping the source for world-udp-endpoint or session-router returns nothing. Config-based deployment also only applies to classic actors, and the network and session actors are spawned through the typed API, so these assignments could not apply even with corrected names. Consequently socket I/O, session logic and zone logic all share akka.actor.default-dispatcher, which was never configured and so ran on Akka's stock throughput of 5 -- against the 50 the intended world-session dispatcher specifies. The pinned thread that the comments call "extremely performance critical" was never allocated. Configure default-dispatcher to suit the load it is actually carrying, and enlarge the scheduler wheel, which holds a per-session outbound packet timer plus per-player and per-entity ticks and collides heavily at 512 buckets. This does not replace assigning dispatchers explicitly at each spawn site, which is the real fix and needs verifying against a thread dump on a running server. Co-Authored-By: Claude Opus 4.8 --- src/main/resources/akka.conf | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/main/resources/akka.conf b/src/main/resources/akka.conf index 5116a8221..32d1eafe1 100644 --- a/src/main/resources/akka.conf +++ b/src/main/resources/akka.conf @@ -246,3 +246,36 @@ akka.actor.deployment { dispatcher = tzshvs-zone-dispatcher } } + +# Safety net for the default dispatcher. +# +# The path-based assignments above do not correspond to any actor this server actually +# creates: there is no /world-udp-endpoint (the socket actors are named world-socket- +# under socketPane), no world-session-router, and the zone actors are named zone- rather +# than -actor. Akka's config-based deployment also only applies to classic actors, while +# the network and session actors are spawned through the typed API, so assigning their +# dispatchers by path cannot work regardless of the names. +# +# The practical result is that socket I/O, session logic and zone logic all share +# akka.actor.default-dispatcher with its stock throughput of 5, which produces frequent +# thread hand-off on the packet path. Until each actor selects its dispatcher explicitly at +# spawn, give the pool everything actually runs on a configuration suited to that load. +akka.actor.default-dispatcher { + throughput = 20 + + fork-join-executor { + parallelism-min = 8 + parallelism-factor = 1.0 + parallelism-max = 64 + } +} + +# The scheduler carries a large number of short-lived tasks: a per-session outbound packet +# timer, per-player stamina and keep-alive ticks, and per-entity ticks for painboxes, +# siphons, turrets and proximity terminals. The stock 512 buckets collide heavily at those +# volumes, and a cancelled task stays in its bucket until its tick comes round, so the tick +# thread walks long chains. +akka.scheduler { + tick-duration = 10ms + ticks-per-wheel = 2048 +} From c7a7680db3f037aaa1723b0f8539a03050a8a8d8 Mon Sep 17 00:00:00 2001 From: GeekOfWires Date: Wed, 22 Jul 2026 05:25:00 -0400 Subject: [PATCH 11/18] Size the connection pool, heap and collector for the running server Three deployment settings that were left at defaults unsuited to the workload. maxActiveConnections was 5 for the whole server. It caps concurrent SQL statements, not player connections -- clients never touch it -- and the driver queues excess queries rather than rejecting them, so the effect is latency. Character login alone issues around a dozen sequential queries, so a login wave and in-game writes starve each other. The packaged launcher passed no memory or collector options whatsoever, leaving the server on JVM defaults; in a container that means a fraction of host RAM. The -Xmx in .jvmopts governs sbt, not the packed script. Set an explicit heap and select G1 with a pause target, since stop-the-world pauses stall every connected session at once. docker-compose ran the server under JDK 8 while the Dockerfile builds with 11. JDK 8 defaults to ParallelGC, whose full collections are stop-the-world and can reach seconds. Align it with the Dockerfile. Co-Authored-By: Claude Opus 4.8 --- build.sbt | 13 ++++++++++++- docker-compose.yml | 5 ++++- src/main/resources/application.conf | 9 ++++++++- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/build.sbt b/build.sbt index 6b8fb632a..5518b52e9 100644 --- a/build.sbt +++ b/build.sbt @@ -110,7 +110,18 @@ lazy val server = (project in file("server")) inConfig(QuietTest)(Defaults.testTasks), packMain := Map("psforever-server" -> "net.psforever.server.Server"), packArchivePrefix := "psforever-server", - packJvmOpts := Map("psforever-server" -> Seq("-Dstacktrace.app.packages=net.psforever")), + // The packaged launcher previously passed no memory or collector settings at all, so the + // server ran on whatever the JVM defaulted to -- in a container, a fraction of host RAM. + // The -Xmx in .jvmopts applies to sbt, not to this script. G1 with an explicit pause + // target keeps collections short; the server's hitching is sensitive to stop-the-world + // pauses because they stall every connected session simultaneously. + packJvmOpts := Map("psforever-server" -> Seq( + "-Dstacktrace.app.packages=net.psforever", + "-Xms2g", + "-Xmx4g", + "-XX:+UseG1GC", + "-XX:MaxGCPauseMillis=50" + )), packExtraClasspath := Map("psforever-server" -> Seq("${PROG_HOME}/config")), packResourceDir += ((psforever / baseDirectory).value / "config" -> "config") ) diff --git a/docker-compose.yml b/docker-compose.yml index a49ed2236..322ace407 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,6 +1,9 @@ services: loginserver: - image: sbtscala/scala-sbt:eclipse-temurin-jammy-8u352-b08_1.8.3_2.13.10 + # Matches the JDK the Dockerfile builds with. This previously ran JDK 8, whose default + # collector is ParallelGC -- full collections there are stop-the-world and can run into + # seconds, stalling every connected client at once. + image: sbtscala/scala-sbt:eclipse-temurin-focal-11.0.17_8_1.8.2_2.13.10 volumes: - .:/PSF-Loginserver:z working_dir: /PSF-Loginserver diff --git a/src/main/resources/application.conf b/src/main/resources/application.conf index 765c800fe..168fe0155 100644 --- a/src/main/resources/application.conf +++ b/src/main/resources/application.conf @@ -63,7 +63,14 @@ database { sslmode = prefer # The maximum number of active connections. - maxActiveConnections = 5 + # + # This is the ceiling on concurrent SQL statements for the entire server, not a player + # limit -- game clients never touch it. Every session, zone actor and kill report shares + # this one pool, and the driver queues excess queries rather than rejecting them, so + # saturation shows up purely as latency: logins, loadout applies and terminal purchases + # all wait behind each other. Five slots is far too few once character login alone issues + # a dozen sequential queries. + maxActiveConnections = 32 } # Enable non-standard game properties From 8d303449171b0f590eba4d03c4328d6f693cca78 Mon Sep 17 00:00:00 2001 From: GeekOfWires Date: Wed, 22 Jul 2026 05:25:00 -0400 Subject: [PATCH 12/18] Index the foreign keys used by login and character select PostgreSQL does not index a REFERENCES column automatically, and V015 (outfits) was the only migration that had declared any index at all. Every per-avatar lookup was therefore a sequential scan. killactivity is the most damaging. It is an append-only kill log that grows for the life of the server, and loadCampaignKdaData reads it on every login filtering on killer_id or victim_id, so login cost grew in proportion to every kill the server had ever recorded. Two single-column indexes let the OR resolve as a bitmap union. The rest cover loadouts, vehicle loadouts, lockers, certifications and implants, all read while the client waits on the character select or loading screen, plus avatar.account_id for assembling an account's character list. Tables whose existing UNIQUE constraint or primary key already yields a btree with avatar_id leading -- friend, ignored, shortcut, avatarmodepermission, savedplayer, savedavatar -- are deliberately left alone; indexing them again would only cost write throughput. Co-Authored-By: Claude Opus 4.8 --- .../db/migration/V018__PerformanceIndexes.sql | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 server/src/main/resources/db/migration/V018__PerformanceIndexes.sql diff --git a/server/src/main/resources/db/migration/V018__PerformanceIndexes.sql b/server/src/main/resources/db/migration/V018__PerformanceIndexes.sql new file mode 100644 index 000000000..f6548b053 --- /dev/null +++ b/server/src/main/resources/db/migration/V018__PerformanceIndexes.sql @@ -0,0 +1,39 @@ +-- Indexes for foreign keys that are queried on the login and gameplay hot paths. +-- +-- PostgreSQL does not create an index for a REFERENCES constraint, so before this migration +-- every one of the lookups below was a sequential scan. Only V015 (outfits) had declared any +-- indexes at all. +-- +-- Deliberately omitted, because an existing constraint already provides a usable btree with +-- avatar_id as its leading column: +-- friend, ignored UNIQUE(avatar_id, char_id) +-- shortcut UNIQUE(avatar_id, slot) +-- avatarmodepermission UNIQUE(avatar_id) +-- savedplayer, savedavatar avatar_id is the primary key +-- Adding further indexes there would only cost write throughput. + +-- Read for every character on the character select screen, and rewritten whenever a loadout +-- is saved or deleted. Primary key is id alone, so avatar_id was unindexed. +CREATE INDEX IF NOT EXISTS "loadout_avatar_id_idx" ON "loadout" ("avatar_id"); +CREATE INDEX IF NOT EXISTS "vehicleloadout_avatar_id_idx" ON "vehicleloadout" ("avatar_id"); + +-- Read on login and rewritten on every locker change. +CREATE INDEX IF NOT EXISTS "locker_avatar_id_idx" ON "locker" ("avatar_id"); + +-- Primary key is (id, avatar_id), so a lookup by avatar_id alone cannot use it. +CREATE INDEX IF NOT EXISTS "certification_avatar_id_idx" ON "certification" ("avatar_id"); + +-- Primary key is (name, avatar_id); loading every implant for an avatar does not supply +-- name and so cannot use the leading column. +CREATE INDEX IF NOT EXISTS "implant_avatar_id_idx" ON "implant" ("avatar_id"); + +-- Scanned to build the character list for an account. +CREATE INDEX IF NOT EXISTS "avatar_account_id_idx" ON "avatar" ("account_id"); + +-- killactivity is an append-only kill log that grows for the lifetime of the server, and the +-- campaign KDA query reads it on every single login with +-- WHERE (killer_id = ? OR victim_id = ?) AND killer_id <> victim_id +-- Without these, login cost grew in proportion to every kill ever recorded. The OR is +-- satisfied by two separate indexes via a bitmap union. +CREATE INDEX IF NOT EXISTS "killactivity_killer_id_idx" ON "killactivity" ("killer_id"); +CREATE INDEX IF NOT EXISTS "killactivity_victim_id_idx" ON "killactivity" ("victim_id"); From 6c6c603cb259d796a2408d8a6a6af6e10804dbff Mon Sep 17 00:00:00 2001 From: GeekOfWires Date: Wed, 22 Jul 2026 05:48:29 -0400 Subject: [PATCH 13/18] Select dispatchers at spawn instead of by actor path akka.conf and dispatchers.conf describe a careful thread allocation -- a pinned thread for the UDP read loop, an isolated pool for session work, a pool per zone so one busy continent cannot starve the others -- and none of it was ever applied. The deployment entries name paths no actor occupies, and config-based deployment only works for classic actors, while the network, session and zone actors are all spawned through the typed API. Everything therefore shared the default pool. Select the dispatcher explicitly where each actor is created: SocketActor -> network-listener (pinned; one thread per port) MiddlewareActor -> world-session SessionActor -> world-session LoginActor -> login-session ZoneActor -> -zone-dispatcher The zone lookup is guarded with hasPath and falls back to the default pool with a warning, because DispatcherSelector.fromConfig throws on a missing key and an unconfigured zone should not stop the server booting. That guard immediately earned itself: comparing ZoneInfo against the config showed the TR sanctuary dropship zone is "tzdrtr" while the dispatcher was spelled "tzsdrtr", so that zone had no pool and every other zone matched exactly. Corrected in both files. Verified by spawning an actor with all thirty-five selectors -- the three fixed pools plus one per ZoneInfo entry -- against the real merged configuration, which resolves without error. Co-Authored-By: Claude Opus 4.8 --- .../scala/net/psforever/server/Server.scala | 10 ++++++++-- src/main/resources/akka.conf | 8 ++++---- src/main/resources/dispatchers.conf | 2 +- .../psforever/actors/net/SocketActor.scala | 5 ++++- .../net/psforever/actors/net/SocketPane.scala | 15 +++++++++++---- .../services/InterstellarClusterService.scala | 19 +++++++++++++++++-- 6 files changed, 45 insertions(+), 14 deletions(-) diff --git a/server/src/main/scala/net/psforever/server/Server.scala b/server/src/main/scala/net/psforever/server/Server.scala index 686e39953..6000f60cf 100644 --- a/server/src/main/scala/net/psforever/server/Server.scala +++ b/server/src/main/scala/net/psforever/server/Server.scala @@ -115,7 +115,10 @@ object Server { val loginPlan = (ref: ActorRef[MiddlewareActor.Command], info: InetSocketAddress, connectionId: String) => { import net.psforever.services.account.IPAddress Behaviors.setup[PlanetSidePacket](context => { - val actor = context.actorOf(classic.Props(new LoginActor(ref, connectionId, Login.getNewId())), "login") + val actor = context.actorOf( + classic.Props(new LoginActor(ref, connectionId, Login.getNewId())).withDispatcher("login-session"), + "login" + ) actor ! ReceiveIPAddress(new IPAddress(info)) Behaviors.receiveMessage(message => { actor ! message @@ -127,7 +130,10 @@ object Server { Behaviors.setup[PlanetSidePacket](context => { val uuid = randomUUID().toString val actor = - context.actorOf(classic.Props(new SessionActor(ref, connectionId, Session.getNewId())), s"session-$uuid") + context.actorOf( + classic.Props(new SessionActor(ref, connectionId, Session.getNewId())).withDispatcher("world-session"), + s"session-$uuid" + ) Behaviors.receiveMessage(message => { actor ! message Behaviors.same diff --git a/src/main/resources/akka.conf b/src/main/resources/akka.conf index 32d1eafe1..a65208b0b 100644 --- a/src/main/resources/akka.conf +++ b/src/main/resources/akka.conf @@ -221,11 +221,11 @@ akka.actor.deployment { "/service/cluster/tzdrvs-actor/*" { dispatcher = tzdrvs-zone-dispatcher } - "/service/cluster/tzsdrtr-actor" { - dispatcher = tzsdrtr-zone-dispatcher + "/service/cluster/tzdrtr-actor" { + dispatcher = tzdrtr-zone-dispatcher } - "/service/cluster/tzsdrtr-actor/*" { - dispatcher = tzsdrtr-zone-dispatcher + "/service/cluster/tzdrtr-actor/*" { + dispatcher = tzdrtr-zone-dispatcher } "/service/cluster/tzshnc-actor" { dispatcher = tzshnc-zone-dispatcher diff --git a/src/main/resources/dispatchers.conf b/src/main/resources/dispatchers.conf index c471f16d5..c1654b59d 100644 --- a/src/main/resources/dispatchers.conf +++ b/src/main/resources/dispatchers.conf @@ -452,7 +452,7 @@ tzdrvs-zone-dispatcher { throughput = 50 throughput-deadline-time = 50ms } -tzsdrtr-zone-dispatcher { +tzdrtr-zone-dispatcher { type = Dispatcher executor = "fork-join-executor" diff --git a/src/main/scala/net/psforever/actors/net/SocketActor.scala b/src/main/scala/net/psforever/actors/net/SocketActor.scala index 09ca2e26d..619838d07 100644 --- a/src/main/scala/net/psforever/actors/net/SocketActor.scala +++ b/src/main/scala/net/psforever/actors/net/SocketActor.scala @@ -142,7 +142,10 @@ class SocketActor( val ref = context.spawn( MiddlewareActor(udpCommandAdapter, remote, nextPlan, connectionId), s"middleware-$connectionId", - ActorTags(s"uuid=$connectionId") + /* crypto, bundling and reordering for one connection; world-session is the + pool akka.conf meant to isolate this work onto, and it has to be selected + here because path-based deployment does not apply to typed actors */ + ActorTags(s"uuid=$connectionId").withDispatcherFromConfig("world-session") ) context.watch(ref) packetActors(remote) = ref diff --git a/src/main/scala/net/psforever/actors/net/SocketPane.scala b/src/main/scala/net/psforever/actors/net/SocketPane.scala index c82b05fbf..3100977fa 100644 --- a/src/main/scala/net/psforever/actors/net/SocketPane.scala +++ b/src/main/scala/net/psforever/actors/net/SocketPane.scala @@ -5,7 +5,7 @@ import akka.actor.typed.receptionist.{Receptionist, ServiceKey} import java.net.{InetAddress, InetSocketAddress} import akka.{actor => classic} -import akka.actor.typed.{ActorRef, Behavior, PostStop} +import akka.actor.typed.{ActorRef, Behavior, DispatcherSelector, PostStop} import akka.actor.typed.scaladsl.{ActorContext, Behaviors} import net.psforever.packet.PlanetSidePacket @@ -88,6 +88,13 @@ final case class SocketSetupInfo( object SocketPane { val SocketPaneKey: ServiceKey[Command] = ServiceKey[SocketPane.Command](id = "socketPane") + /** The UDP read loop gets the dedicated thread akka.conf always intended for it. + * network-listener is a PinnedDispatcher, so each socket actor -- one per bound port -- + * receives its own thread rather than competing with game logic on the default pool. + * Selecting it here rather than by actor path is deliberate: these are typed actors, and + * config-based deployment does not apply to them. */ + private val socketDispatcher: DispatcherSelector = DispatcherSelector.fromConfig("network-listener") + /** * Overrode constructor for `SocketPane` entities. * Registers the entity with the actor receptionist. @@ -138,7 +145,7 @@ class SocketPane( /** all sockets produced by the socket group information and any later socket creation commands */ private var socketActors: Array[ActorRef[SocketActor.Command]] = initialPortSetup.flatMap { case SocketSetup(_, SocketSetupInfo(address, ports, plan), _) => - ports.map { portNum => context.spawn(SocketActor(new InetSocketAddress(address, portNum), plan), name=s"world-socket-$portNum") } + ports.map { portNum => context.spawn(SocketActor(new InetSocketAddress(address, portNum), plan), name=s"world-socket-$portNum", SocketPane.socketDispatcher) } }.toArray /** load balancing for redirecting newly discovered packet input to different sockets (ports); * should be referenced externally to switch sockets; @@ -169,7 +176,7 @@ class SocketPane( val index = socketConfigs.indexWhere { setup => setup.groupId.equals(groupId) } val SocketSetup(_, SocketSetupInfo(address, ports, plan), _) = socketConfigs(index) socketActors = socketActors :+ - context.spawn(SocketActor(new InetSocketAddress(address, port), plan), name=s"world-socket-$port") + context.spawn(SocketActor(new InetSocketAddress(address, port), plan), name=s"world-socket-$port", SocketPane.socketDispatcher) socketConfigs = (socketConfigs.take(index) :+ SocketSetup(groupId, SocketSetupInfo(address, ports :+ port, plan))) ++ socketConfigs.drop(index + 1) socketRotations = (socketRotations.take(index) :+ SocketPanePortRotation(socketRotations(index), port)) ++ @@ -183,7 +190,7 @@ class SocketPane( case SocketPane.CreateNewSocketGroup(groupId, info @ SocketSetupInfo(address, ports, plan)) => socketActors = socketActors ++ - ports.map { portNum => context.spawn(SocketActor(new InetSocketAddress(address, portNum), plan), name=s"world-socket-$portNum") } + ports.map { portNum => context.spawn(SocketActor(new InetSocketAddress(address, portNum), plan), name=s"world-socket-$portNum", SocketPane.socketDispatcher) } socketConfigs = socketConfigs :+ SocketSetup(groupId, info) socketRotations = socketRotations :+ diff --git a/src/main/scala/net/psforever/services/InterstellarClusterService.scala b/src/main/scala/net/psforever/services/InterstellarClusterService.scala index 30a02008f..3f86f0e78 100644 --- a/src/main/scala/net/psforever/services/InterstellarClusterService.scala +++ b/src/main/scala/net/psforever/services/InterstellarClusterService.scala @@ -3,7 +3,7 @@ package net.psforever.services import akka.actor.typed.receptionist.{Receptionist, ServiceKey} import akka.actor.typed.scaladsl.{AbstractBehavior, ActorContext, Behaviors} -import akka.actor.typed.{ActorRef, Behavior, SupervisorStrategy} +import akka.actor.typed.{ActorRef, Behavior, DispatcherSelector, SupervisorStrategy} import net.psforever.actors.zone.ZoneActor import net.psforever.objects.avatar.Avatar import net.psforever.objects.{SpawnPoint, Vehicle} @@ -125,7 +125,22 @@ class InterstellarClusterService(context: ActorContext[InterstellarClusterServic mutable.Map( _zones.map { zone => - val zoneActor = context.spawn(ZoneActor(zone), s"zone-${zone.id}") + /* dispatchers.conf defines a pool per zone so that one busy continent cannot starve + the others, but nothing ever selected them: the deployment entries in akka.conf + name "-actor" paths that are never created, and config deployment does not + apply to typed actors in any case. + Not every zone id is guaranteed to have a matching entry, and + DispatcherSelector.fromConfig throws on a missing key, so fall back to the default + pool rather than prevent the server from starting. */ + val dispatcherPath = s"${zone.id}-zone-dispatcher" + val zoneDispatcher = + if (context.system.settings.config.hasPath(dispatcherPath)) { + DispatcherSelector.fromConfig(dispatcherPath) + } else { + context.log.warn(s"no dispatcher configured at $dispatcherPath; zone ${zone.id} shares the default pool") + DispatcherSelector.default() + } + val zoneActor = context.spawn(ZoneActor(zone), s"zone-${zone.id}", zoneDispatcher) (zone.id, (zoneActor, zone)) }.toSeq: _* ) From 601c86720e6863730a498f0e8b67b07c92637da5 Mon Sep 17 00:00:00 2001 From: GeekOfWires Date: Wed, 22 Jul 2026 05:50:55 -0400 Subject: [PATCH 14/18] Index GUID pools by position instead of walking lists Three linear walks over Lists on the GUID allocation path, all replaced with indexed access. The algorithms and their results are unchanged. GenericPool.first and rand build a sorted list of every number already handed out and then index it by position in a loop. List.apply walks the chain, so each step cost O(index) and the scan was quadratic overall. This matters because the generic pool is where every failed request is redirected: once any fixed pool is exhausted, allocation falls back here and receives the whole hub -- tens of thousands of numbers -- so the pool actor's mailbox blocks for a long time at exactly the moment the server is already under pressure. Sorting into an Array keeps the same scan at O(n log n). ExclusivePool did the same on both of its operations: Get resolved numbers(index) on a List, and Return performed an O(n) indexOf. The configured pools are large -- deployables 16000, ammo and kits 13500 -- and unregistration arrives in bulk when a player dies or a vehicle is deconstructed, serialised through a single pool actor. An indexed copy and a reverse lookup make both constant time; neither structure is mutated after construction and SimplePool already rejects duplicates. Deliberately not changed: RandomSelector.Return still reports failure when no number is outstanding, which does leak that GUID. Making it succeed unconditionally would let a double return insert the same number into the pool twice and eventually issue one GUID to two entities, which is far worse than the leak. Correcting it properly means tracking allocation state explicitly rather than inferring it from a cursor. Co-Authored-By: Claude Opus 4.8 --- .../objects/guid/pool/ExclusivePool.scala | 14 +++++++++++--- .../psforever/objects/guid/pool/GenericPool.scala | 10 ++++++++-- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/main/scala/net/psforever/objects/guid/pool/ExclusivePool.scala b/src/main/scala/net/psforever/objects/guid/pool/ExclusivePool.scala index 6b6904247..0b376fb42 100644 --- a/src/main/scala/net/psforever/objects/guid/pool/ExclusivePool.scala +++ b/src/main/scala/net/psforever/objects/guid/pool/ExclusivePool.scala @@ -9,6 +9,15 @@ class ExclusivePool(override val numbers: List[Int]) extends SimplePool(numbers) private val pool: Array[Int] = Array.ofDim[Int](numbers.length) numbers.indices.foreach(i => { pool(i) = i }) + /* `numbers` is a List, so both hot operations walked it: Get did numbers(index), which is + O(index), and Return did an O(n) indexOf. The configured pools are large -- deployables + 16000, ammo and kits 13500 each -- and unregistration happens in bulk when a player dies + or a vehicle is deconstructed, all serialised through one pool actor. + An indexed copy and a reverse lookup make both constant time. Neither is mutated after + construction, and SimplePool has already rejected duplicates. */ + private val numbersByIndex: Array[Int] = numbers.toArray + private val indexByNumber: Map[Int, Int] = numbers.zipWithIndex.toMap + override def Count: Int = pool.count(value => value == -1) override def Selector_=(slctr: NumberSelector): Unit = { @@ -21,12 +30,11 @@ class ExclusivePool(override val numbers: List[Int]) extends SimplePool(numbers) if (index == -1) { Failure(new Exception("there are no numbers available in the pool")) } else { - Success(numbers(index)) + Success(numbersByIndex(index)) } } override def Return(number: Int): Boolean = { - val index = Numbers.indexOf(number) - index != -1 && Selector.Return(index, pool) + indexByNumber.get(number).exists(index => Selector.Return(index, pool)) } } diff --git a/src/main/scala/net/psforever/objects/guid/pool/GenericPool.scala b/src/main/scala/net/psforever/objects/guid/pool/GenericPool.scala index 9a77aa7c1..8048cfe16 100644 --- a/src/main/scala/net/psforever/objects/guid/pool/GenericPool.scala +++ b/src/main/scala/net/psforever/objects/guid/pool/GenericPool.scala @@ -82,7 +82,12 @@ object GenericPool { */ def first(list: List[Long], domainSize: Int): Int = { if (list.size < domainSize) { - val sortedList: List[Long] = 0L +: list.sorted :+ domainSize + /* an Array, because this is indexed by position in a loop: List.apply walks the chain, + so the identical scan over a List cost O(index) per step. This pool is the fallback + for any exhausted pool and receives every number in the hub -- on the order of tens + of thousands -- which made the scan quadratic and left the pool actor's mailbox + blocked for a long time at precisely the moment the server was under pressure. */ + val sortedList: Array[Long] = (0L +: list.sorted :+ domainSize.toLong).toArray var index: Int = 0 val listLen = sortedList.length - 1 while(index < listLen && index < domainSize) { @@ -112,7 +117,8 @@ object GenericPool { def rand(list: List[Long], domainSize: Int): Int = { if (list.size < domainSize) { //get a list of all assigned numbers with an appended min and max - val sortedList: List[Long] = -1L +: list.sorted :+ domainSize.toLong + //an Array for the same reason as in `first`: the loop below indexes it by position + val sortedList: Array[Long] = (-1L +: list.sorted :+ domainSize.toLong).toArray //compare the delta between every two entries and find the start of that greatest delta comparison var maxDelta: Long = -1 var maxDeltaIndex = -1 From 05361c0f9725059e71b07a12bba3bd4a37261e96 Mon Sep 17 00:00:00 2001 From: GeekOfWires Date: Wed, 22 Jul 2026 05:53:13 -0400 Subject: [PATCH 15/18] Drain more than one bundle per run of the packet bundler processOutQueueBundle dispatches a single bundle per call and the timer called it once per tick, so each client's outbound throughput was fixed at one bundle per bundling delay -- roughly twenty 440 byte bundles a second, under 9 KB/s -- however much was queued behind it. Bursts are exactly when that hurts. A zone load, a base capture, or a squad arriving in render range queues hundreds of object creates, and they left at a rate that made the world visibly assemble in slow motion around the player. The bundling delay is meant to control how packets are coalesced, not to cap total bandwidth. Drain up to packet-bundling-drain-limit bundles per run, defaulting to 8. The budget doubles as the loop bound, so the loop always terminates after a fixed number of iterations even if a queue fails to shrink, and the value is exposed in configuration so it can be tuned down for constrained connections rather than being fixed in code. Verified that the new key resolves through pureconfig at runtime; a mismatch there would prevent the server from starting. Co-Authored-By: Claude Opus 4.8 --- src/main/resources/application.conf | 10 ++++++++++ .../psforever/actors/net/MiddlewareActor.scala | 17 ++++++++++++++++- src/main/scala/net/psforever/util/Config.scala | 1 + 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/main/resources/application.conf b/src/main/resources/application.conf index 168fe0155..257ee8fcc 100644 --- a/src/main/resources/application.conf +++ b/src/main/resources/application.conf @@ -560,6 +560,16 @@ network { # Affects the base value on the timer packet-bundling-delay-multiplier = 1.25 + # How many bundles may be dispatched in a single execution of the bundling process. + # Each bundle is capped at the 440 byte MTU, so this multiplied by the bundle size and + # divided by the delay above is the effective per-client outbound ceiling. + # Previously the process sent exactly one bundle per run, which capped every client at + # roughly 20 bundles a second no matter how much was queued, so any burst -- a zone load, + # a base capture, a squad coming into render range -- drained at under 9 KB/s and the + # world appeared to the player to arrive in slow motion. + # Raise for faster burst delivery, lower to be gentler on constrained connections. + packet-bundling-drain-limit = 8 + # Pause inbound packet transmission towards the network if the sequence number is out of order # Packets are put aside until the sequence is restored, or this timeout passes in-reorder-timeout = 50 milliseconds diff --git a/src/main/scala/net/psforever/actors/net/MiddlewareActor.scala b/src/main/scala/net/psforever/actors/net/MiddlewareActor.scala index 5f56491d3..9aa6fa30c 100644 --- a/src/main/scala/net/psforever/actors/net/MiddlewareActor.scala +++ b/src/main/scala/net/psforever/actors/net/MiddlewareActor.scala @@ -304,6 +304,10 @@ class MiddlewareActor( "milliseconds" ) + /** How many bundles a single run of the bundling process may dispatch; at least one */ + private val packetBundlingDrainLimit: Int = + math.max(1, Config.app.network.middleware.packetBundlingDrainLimit) + /** Timer that handles the bundling and throttling of outgoing packets and resolves disorganized inbound packets */ private var packetProcessor: Cancellable = Default.Cancellable @@ -671,7 +675,18 @@ class MiddlewareActor( */ private def processQueue(): Unit = { inReorderQueueFunc() - processOutQueueBundle() + /* processOutQueueBundle dispatches at most one bundle per call, so a single call per timer + tick capped each client's outbound throughput at one MTU-sized bundle per delay period + regardless of backlog. Drain up to the configured number of bundles instead, so a burst + clears in a few ticks rather than tens of seconds, while the delay continues to govern + how aggressively packets are coalesced. + The budget also bounds the loop: it runs a fixed number of times at most, so it cannot + spin even if a queue fails to shrink. */ + var budget: Int = packetBundlingDrainLimit + while (budget > 0 && (outQueueBundled.nonEmpty || outQueue.nonEmpty)) { + processOutQueueBundle() + budget -= 1 + } } /** diff --git a/src/main/scala/net/psforever/util/Config.scala b/src/main/scala/net/psforever/util/Config.scala index f32c2adaf..c9e341cbb 100644 --- a/src/main/scala/net/psforever/util/Config.scala +++ b/src/main/scala/net/psforever/util/Config.scala @@ -138,6 +138,7 @@ case class NetworkConfig( case class MiddlewareConfig( packetBundlingDelay: FiniteDuration, packetBundlingDelayMultiplier: Float, + packetBundlingDrainLimit: Int, inReorderTimeout: FiniteDuration, inSubslotMissingDelay: FiniteDuration, inSubslotMissingAttempts: Int From ec47d9a2ddc546f3ef508ce3e4fdf3f6cab116f9 Mon Sep 17 00:00:00 2001 From: GeekOfWires Date: Wed, 22 Jul 2026 06:02:45 -0400 Subject: [PATCH 16/18] Compute the packet MAC without boxing every byte Md5Mac runs over every inbound and every outbound packet, so it was the largest allocation source in the network layer, and the young-generation collections it drove stall every connected session at once. Three causes, all removed: State was held in ArrayBuffer[Byte], which is unspecialised and backed by Array[AnyRef], so every byte read and write boxed. It is now fixed primitive arrays allocated once per instance. mkInt allocated an iterator and dropped through it to read four bytes at a known offset, and hash() called it around 128 times per 64 byte block. It now reads the array directly. The four words derived from k2 are the same every round, so they are computed once at key setup rather than sixty-four times per block -- while preserving the ordering that leaves them zero during key expansion, which is load bearing. reset() rebuilt the entire state through a deep copy and is called twice per packet. It is now a fill plus an arraycopy from a template. The public API is unchanged and reset() still returns this, so the reset().updateFinal(...) form at the call sites is untouched. This code was verified live against the real server, so output has to be bit identical. CryptoTest, which asserts a known MAC vector, still passes with 26 expectations and no failures and was not modified. It was also checked differentially against the previous implementation over 6000 randomised cases -- varying keys, update counts, lengths straddling the 64 byte block boundary, output lengths and interleaved resets -- with no mismatches. Co-Authored-By: Claude Opus 4.8 --- .../scala/net/psforever/util/Md5Mac.scala | 470 ++++++++++-------- 1 file changed, 258 insertions(+), 212 deletions(-) diff --git a/src/main/scala/net/psforever/util/Md5Mac.scala b/src/main/scala/net/psforever/util/Md5Mac.scala index ddc03acae..7ec1607b8 100644 --- a/src/main/scala/net/psforever/util/Md5Mac.scala +++ b/src/main/scala/net/psforever/util/Md5Mac.scala @@ -1,38 +1,6 @@ package net.psforever.util import scodec.bits.ByteVector -import scala.collection.mutable.ArrayBuffer - -sealed case class PermanentMd5MacState( - buffer: Seq[Byte], - digest: Seq[Byte], - m: Seq[Byte], - k1: Seq[Byte], - k2: Seq[Byte], - k3: Seq[Byte] - ) - -sealed case class Md5MacState( - buffer: ArrayBuffer[Byte], - digest: ArrayBuffer[Byte], - m: ArrayBuffer[Byte], - k1: ArrayBuffer[Byte], - k2: ArrayBuffer[Byte], - k3: ArrayBuffer[Byte] - ) - -object PermanentMd5MacState { - def mutableCopy(in: PermanentMd5MacState): Md5MacState = { - Md5MacState( - ArrayBuffer[Byte]().addAll(in.buffer), - ArrayBuffer[Byte]().addAll(in.digest), - ArrayBuffer[Byte]().addAll(in.m), - ArrayBuffer[Byte]().addAll(in.k1), - ArrayBuffer[Byte]().addAll(in.k2), - ArrayBuffer[Byte]().addAll(in.k3), - ) - } -} object Md5Mac { val BLOCKSIZE = 64 @@ -40,25 +8,26 @@ object Md5Mac { val MACLENGTH = 16 val KEYLENGTH = 16 - private val t: Seq[Seq[Byte]] = Seq( - Seq(0x97, 0xef, 0x45, 0xac, 0x29, 0x0f, 0x43, 0xcd, 0x45, 0x7e, 0x1b, 0x55, 0x1c, 0x80, 0x11, 0x34), - Seq(0xb1, 0x77, 0xce, 0x96, 0x2e, 0x72, 0x8e, 0x7c, 0x5f, 0x5a, 0xab, 0x0a, 0x36, 0x43, 0xbe, 0x18), - Seq(0x9d, 0x21, 0xb4, 0x21, 0xbc, 0x87, 0xb9, 0x4d, 0xa2, 0x9d, 0x27, 0xbd, 0xc7, 0x5b, 0xd7, 0xc3) + private val t: Array[Array[Byte]] = Array( + Array(0x97, 0xef, 0x45, 0xac, 0x29, 0x0f, 0x43, 0xcd, 0x45, 0x7e, 0x1b, 0x55, 0x1c, 0x80, 0x11, 0x34), + Array(0xb1, 0x77, 0xce, 0x96, 0x2e, 0x72, 0x8e, 0x7c, 0x5f, 0x5a, 0xab, 0x0a, 0x36, 0x43, 0xbe, 0x18), + Array(0x9d, 0x21, 0xb4, 0x21, 0xbc, 0x87, 0xb9, 0x4d, 0xa2, 0x9d, 0x27, 0xbd, 0xc7, 0x5b, 0xd7, 0xc3) ).map(_.map(_.toByte)) - /** - * na - * @param lb na - * @param pos na - * @return na - */ - private def mkInt(lb: Iterable[Byte], pos: Int): Int = { - // get iterator - val it = lb.iterator.drop(pos) - (it.next().toInt & 0xFF) << 24 | - (it.next().toInt & 0xFF) << 16 | - (it.next().toInt & 0xFF) << 8 | - (it.next().toInt & 0xFF) + /** Read four bytes from `a` starting at `pos` as a big-endian, 32-bit integer. */ + private def mkInt(a: Array[Byte], pos: Int): Int = { + ((a(pos) & 0xff) << 24) | + ((a(pos + 1) & 0xff) << 16) | + ((a(pos + 2) & 0xff) << 8) | + (a(pos + 3) & 0xff) + } + + /** Write `v` into `a` at `pos` as a big-endian, 32-bit integer. */ + private def putInt(a: Array[Byte], pos: Int, v: Int): Unit = { + a(pos) = (v >>> 24).toByte + a(pos + 1) = (v >>> 16).toByte + a(pos + 2) = (v >>> 8).toByte + a(pos + 3) = v.toByte } /** Checks if two Message Authentication Codes are the same in constant time, @@ -90,6 +59,11 @@ object Md5Mac { * Both libraries have since removed this code. * This file is a Scala port of the OpenCL implementation. * Source: https://github.com/sghiassy/Code-Reading-Book/blob/master/OpenCL/src/md5mac.cpp + * + * This implementation is deliberately allocation-free on the hot path: + * every piece of working state is a primitive array owned by the instance, + * `reset` restores that state with `System.arraycopy` from a stored template, + * and the only object allocated per `doFinal` is the returned MAC itself. */ class Md5Mac(val key: ByteVector) { import Md5Mac._ @@ -97,176 +71,228 @@ class Md5Mac(val key: ByteVector) { private var count: Long = 0 private var position: Int = 0 - private var state: Md5MacState = _ //value initialized in doKey - private val originalState: PermanentMd5MacState = doKey() - - private def doKey(): PermanentMd5MacState = { - val buffer: ArrayBuffer[Byte] = ArrayBuffer.fill(BLOCKSIZE)(0) - val digest: ArrayBuffer[Byte] = ArrayBuffer.fill(DIGESTSIZE)(0) - val m: ArrayBuffer[Byte] = ArrayBuffer.fill(32)(0) - val k1: ArrayBuffer[Byte] = ArrayBuffer.fill(16)(0) - val k2: ArrayBuffer[Byte] = ArrayBuffer.fill(16)(0) - val k3: ArrayBuffer[Byte] = ArrayBuffer.fill(BLOCKSIZE)(0) - val ek: ArrayBuffer[Byte] = ArrayBuffer.fill(48)(0) - val data: ArrayBuffer[Byte] = ArrayBuffer.fill(128)(0) - state = Md5MacState(buffer, digest, m, k1, k2, k3) //initialize - - (0 until 16).foreach(j => { - data(j) = key(j % key.length) - data(j + 112) = key(j % key.length) - }) - - (0 until 3).foreach(j => { - digest.patchInPlace(0, ByteVector.fromInt(0x67452301).toArray, 4) - digest.patchInPlace(4, ByteVector.fromInt(0xefcdab89).toArray, 4) - digest.patchInPlace(8, ByteVector.fromInt(0x98badcfe).toArray, 4) - digest.patchInPlace(12, ByteVector.fromInt(0x10325476).toArray, 4) - - (16 until 112).foreach(k => data(k) = t((j + (k - 16) / 16) % 3)(k % 16)) - - hash(data.toSeq) - hash(data.drop(64).toSeq) - - ek.patchInPlace(4 * 4 * j, digest.slice(0, 4), 4) - ek.patchInPlace((4 * 4 * j) + 4, digest.slice(4, 8), 4) - ek.patchInPlace((4 * 4 * j) + 8, digest.slice(8, 12), 4) - ek.patchInPlace((4 * 4 * j) + 12, digest.slice(12, 16), 4) - }) - - k1.patchInPlace(0, ek.take(16), 16) - digest.patchInPlace(0, ek.take(16), 16) - k2.patchInPlace(0, ek.slice(16, 32), 16) - - (0 until 16).foreach(j => k3(j) = ek(((8 + j / 4) * 4) + (3 - j % 4))) - (16 until 64).foreach(j => k3(j) = (k3(j % 16) ^ t((j - 16) / 16)(j % 16)).toByte) - PermanentMd5MacState(buffer.toSeq, digest.toSeq, m.toSeq, k1.toSeq, k2.toSeq, k3.toSeq) + + /** the 64-byte accumulation block */ + private val buffer: Array[Byte] = new Array[Byte](BLOCKSIZE) + /** the running 16-byte digest, held as four big-endian words */ + private val digest: Array[Byte] = new Array[Byte](DIGESTSIZE) + /** the message schedule of the current block, already converted to little-endian words */ + private val m: Array[Int] = new Array[Int](16) + /** key-derived constants; `k2` is pre-split into the four words the round functions need */ + private val k1: Array[Byte] = new Array[Byte](16) + private val k3: Array[Byte] = new Array[Byte](BLOCKSIZE) + private var k2a: Int = 0 + private var k2b: Int = 0 + private var k2c: Int = 0 + private var k2d: Int = 0 + /** the value `digest` is restored to by `reset` */ + private val digestTemplate: Array[Byte] = new Array[Byte](DIGESTSIZE) + /** scratch space so whole blocks can be hashed straight out of a `ByteVector` without allocating */ + private val block: Array[Byte] = new Array[Byte](BLOCKSIZE) + /** scratch space for the byte-swapped digest produced by `doFinal` */ + private val macScratch: Array[Byte] = new Array[Byte](MACLENGTH) + + doKey() + + private def doKey(): Unit = { + val ek: Array[Byte] = new Array[Byte](48) + val data: Array[Byte] = new Array[Byte](128) + + var j = 0 + while (j < 16) { + data(j) = key(j.toLong % key.length) + data(j + 112) = key(j.toLong % key.length) + j += 1 + } + + j = 0 + while (j < 3) { + putInt(digest, 0, 0x67452301) + putInt(digest, 4, 0xefcdab89) + putInt(digest, 8, 0x98badcfe) + putInt(digest, 12, 0x10325476) + + var k = 16 + while (k < 112) { + data(k) = t((j + (k - 16) / 16) % 3)(k % 16) + k += 1 + } + + hash(data, 0) + hash(data, 64) + + System.arraycopy(digest, 0, ek, 4 * 4 * j, 16) + j += 1 + } + + System.arraycopy(ek, 0, k1, 0, 16) + System.arraycopy(ek, 0, digest, 0, 16) + // k2, split into the four big-endian words consumed by the round functions + k2a = mkInt(ek, 16) + k2b = mkInt(ek, 20) + k2c = mkInt(ek, 24) + k2d = mkInt(ek, 28) + + j = 0 + while (j < 16) { + k3(j) = ek(((8 + j / 4) * 4) + (3 - j % 4)) + j += 1 + } + j = 16 + while (j < 64) { + k3(j) = (k3(j % 16) ^ t((j - 16) / 16)(j % 16)).toByte + j += 1 + } + // `buffer` is still all zeroes here, which is exactly what `reset` needs to restore + System.arraycopy(digest, 0, digestTemplate, 0, DIGESTSIZE) } - private def hash(input: Seq[Byte]): Unit = { - val (digest, m) = (state.digest, state.m) - (0 until 16).foreach(j => { - m.patchInPlace(j * 4, Array[Byte](input(4 * j + 3), input(4 * j + 2), input(4 * j + 1), input(4 * j + 0)), 4) - }) - - var a = mkInt(digest.drop(0), 0) - var c = mkInt(digest.drop(2 * 4), 0) - var b = mkInt(digest.drop(1 * 4), 0) - var d = mkInt(digest.drop(3 * 4), 0) - - a = ff(a, b, c, d, mkInt(m, 0), 7, 0xd76aa478) - d = ff(d, a, b, c, mkInt(m, 1 * 4), 12, 0xe8c7b756) - c = ff(c, d, a, b, mkInt(m, 2 * 4), 17, 0x242070db) - b = ff(b, c, d, a, mkInt(m, 3 * 4), 22, 0xc1bdceee) - a = ff(a, b, c, d, mkInt(m, 4 * 4), 7, 0xf57c0faf) - d = ff(d, a, b, c, mkInt(m, 5 * 4), 12, 0x4787c62a) - c = ff(c, d, a, b, mkInt(m, 6 * 4), 17, 0xa8304613) - b = ff(b, c, d, a, mkInt(m, 7 * 4), 22, 0xfd469501) - a = ff(a, b, c, d, mkInt(m, 8 * 4), 7, 0x698098d8) - d = ff(d, a, b, c, mkInt(m, 9 * 4), 12, 0x8b44f7af) - c = ff(c, d, a, b, mkInt(m, 10 * 4), 17, 0xffff5bb1) - b = ff(b, c, d, a, mkInt(m, 11 * 4), 22, 0x895cd7be) - a = ff(a, b, c, d, mkInt(m, 12 * 4), 7, 0x6b901122) - d = ff(d, a, b, c, mkInt(m, 13 * 4), 12, 0xfd987193) - c = ff(c, d, a, b, mkInt(m, 14 * 4), 17, 0xa679438e) - b = ff(b, c, d, a, mkInt(m, 15 * 4), 22, 0x49b40821) - - a = gg(a, b, c, d, mkInt(m, 1 * 4), 5, 0xf61e2562) - d = gg(d, a, b, c, mkInt(m, 6 * 4), 9, 0xc040b340) - c = gg(c, d, a, b, mkInt(m, 11 * 4), 14, 0x265e5a51) - b = gg(b, c, d, a, mkInt(m, 0), 20, 0xe9b6c7aa) - a = gg(a, b, c, d, mkInt(m, 5 * 4), 5, 0xd62f105d) - d = gg(d, a, b, c, mkInt(m, 10 * 4), 9, 0x02441453) - c = gg(c, d, a, b, mkInt(m, 15 * 4), 14, 0xd8a1e681) - b = gg(b, c, d, a, mkInt(m, 4 * 4), 20, 0xe7d3fbc8) - a = gg(a, b, c, d, mkInt(m, 9 * 4), 5, 0x21e1cde6) - d = gg(d, a, b, c, mkInt(m, 14 * 4), 9, 0xc33707d6) - c = gg(c, d, a, b, mkInt(m, 3 * 4), 14, 0xf4d50d87) - b = gg(b, c, d, a, mkInt(m, 8 * 4), 20, 0x455a14ed) - a = gg(a, b, c, d, mkInt(m, 13 * 4), 5, 0xa9e3e905) - d = gg(d, a, b, c, mkInt(m, 2 * 4), 9, 0xfcefa3f8) - c = gg(c, d, a, b, mkInt(m, 7 * 4), 14, 0x676f02d9) - b = gg(b, c, d, a, mkInt(m, 12 * 4), 20, 0x8d2a4c8a) - - a = hh(a, b, c, d, mkInt(m, 5 * 4), 4, 0xfffa3942) - d = hh(d, a, b, c, mkInt(m, 8 * 4), 11, 0x8771f681) - c = hh(c, d, a, b, mkInt(m, 11 * 4), 16, 0x6d9d6122) - b = hh(b, c, d, a, mkInt(m, 14 * 4), 23, 0xfde5380c) - a = hh(a, b, c, d, mkInt(m, 1 * 4), 4, 0xa4beea44) - d = hh(d, a, b, c, mkInt(m, 4 * 4), 11, 0x4bdecfa9) - c = hh(c, d, a, b, mkInt(m, 7 * 4), 16, 0xf6bb4b60) - b = hh(b, c, d, a, mkInt(m, 10 * 4), 23, 0xbebfbc70) - a = hh(a, b, c, d, mkInt(m, 13 * 4), 4, 0x289b7ec6) - d = hh(d, a, b, c, mkInt(m, 0), 11, 0xeaa127fa) - c = hh(c, d, a, b, mkInt(m, 3 * 4), 16, 0xd4ef3085) - b = hh(b, c, d, a, mkInt(m, 6 * 4), 23, 0x04881d05) - a = hh(a, b, c, d, mkInt(m, 9 * 4), 4, 0xd9d4d039) - d = hh(d, a, b, c, mkInt(m, 12 * 4), 11, 0xe6db99e5) - c = hh(c, d, a, b, mkInt(m, 15 * 4), 16, 0x1fa27cf8) - b = hh(b, c, d, a, mkInt(m, 2 * 4), 23, 0xc4ac5665) - - a = ii(a, b, c, d, mkInt(m, 0), 6, 0xf4292244) - d = ii(d, a, b, c, mkInt(m, 7 * 4), 10, 0x432aff97) - c = ii(c, d, a, b, mkInt(m, 14 * 4), 15, 0xab9423a7) - b = ii(b, c, d, a, mkInt(m, 5 * 4), 21, 0xfc93a039) - a = ii(a, b, c, d, mkInt(m, 12 * 4), 6, 0x655b59c3) - d = ii(d, a, b, c, mkInt(m, 3 * 4), 10, 0x8f0ccc92) - c = ii(c, d, a, b, mkInt(m, 10 * 4), 15, 0xffeff47d) - b = ii(b, c, d, a, mkInt(m, 1 * 4), 21, 0x85845dd1) - a = ii(a, b, c, d, mkInt(m, 8 * 4), 6, 0x6fa87e4f) - d = ii(d, a, b, c, mkInt(m, 15 * 4), 10, 0xfe2ce6e0) - c = ii(c, d, a, b, mkInt(m, 6 * 4), 15, 0xa3014314) - b = ii(b, c, d, a, mkInt(m, 13 * 4), 21, 0x4e0811a1) - a = ii(a, b, c, d, mkInt(m, 4 * 4), 6, 0xf7537e82) - d = ii(d, a, b, c, mkInt(m, 11 * 4), 10, 0xbd3af235) - c = ii(c, d, a, b, mkInt(m, 2 * 4), 15, 0x2ad7d2bb) - b = ii(b, c, d, a, mkInt(m, 9 * 4), 21, 0xeb86d391) - - digest.patchInPlace(0, ByteVector.fromInt(mkInt(digest, 0) + a).toArray, 4) - digest.patchInPlace(4, ByteVector.fromInt(mkInt(digest, 4) + b).toArray, 4) - digest.patchInPlace(8, ByteVector.fromInt(mkInt(digest, 8) + c).toArray, 4) - digest.patchInPlace(12, ByteVector.fromInt(mkInt(digest, 12) + d).toArray, 4) + private def hash(input: Array[Byte], offset: Int): Unit = { + val mm = m + val digest = this.digest + var j = 0 + while (j < 16) { + val p = offset + j * 4 + // the original stored the bytes reversed and then read them back big-endian: a little-endian read + mm(j) = ((input(p + 3) & 0xff) << 24) | + ((input(p + 2) & 0xff) << 16) | + ((input(p + 1) & 0xff) << 8) | + (input(p) & 0xff) + j += 1 + } + + val a0 = mkInt(digest, 0) + val b0 = mkInt(digest, 4) + val c0 = mkInt(digest, 8) + val d0 = mkInt(digest, 12) + + var a = a0 + var b = b0 + var c = c0 + var d = d0 + + a = ff(a, b, c, d, mm(0), 7, 0xd76aa478) + d = ff(d, a, b, c, mm(1), 12, 0xe8c7b756) + c = ff(c, d, a, b, mm(2), 17, 0x242070db) + b = ff(b, c, d, a, mm(3), 22, 0xc1bdceee) + a = ff(a, b, c, d, mm(4), 7, 0xf57c0faf) + d = ff(d, a, b, c, mm(5), 12, 0x4787c62a) + c = ff(c, d, a, b, mm(6), 17, 0xa8304613) + b = ff(b, c, d, a, mm(7), 22, 0xfd469501) + a = ff(a, b, c, d, mm(8), 7, 0x698098d8) + d = ff(d, a, b, c, mm(9), 12, 0x8b44f7af) + c = ff(c, d, a, b, mm(10), 17, 0xffff5bb1) + b = ff(b, c, d, a, mm(11), 22, 0x895cd7be) + a = ff(a, b, c, d, mm(12), 7, 0x6b901122) + d = ff(d, a, b, c, mm(13), 12, 0xfd987193) + c = ff(c, d, a, b, mm(14), 17, 0xa679438e) + b = ff(b, c, d, a, mm(15), 22, 0x49b40821) + + a = gg(a, b, c, d, mm(1), 5, 0xf61e2562) + d = gg(d, a, b, c, mm(6), 9, 0xc040b340) + c = gg(c, d, a, b, mm(11), 14, 0x265e5a51) + b = gg(b, c, d, a, mm(0), 20, 0xe9b6c7aa) + a = gg(a, b, c, d, mm(5), 5, 0xd62f105d) + d = gg(d, a, b, c, mm(10), 9, 0x02441453) + c = gg(c, d, a, b, mm(15), 14, 0xd8a1e681) + b = gg(b, c, d, a, mm(4), 20, 0xe7d3fbc8) + a = gg(a, b, c, d, mm(9), 5, 0x21e1cde6) + d = gg(d, a, b, c, mm(14), 9, 0xc33707d6) + c = gg(c, d, a, b, mm(3), 14, 0xf4d50d87) + b = gg(b, c, d, a, mm(8), 20, 0x455a14ed) + a = gg(a, b, c, d, mm(13), 5, 0xa9e3e905) + d = gg(d, a, b, c, mm(2), 9, 0xfcefa3f8) + c = gg(c, d, a, b, mm(7), 14, 0x676f02d9) + b = gg(b, c, d, a, mm(12), 20, 0x8d2a4c8a) + + a = hh(a, b, c, d, mm(5), 4, 0xfffa3942) + d = hh(d, a, b, c, mm(8), 11, 0x8771f681) + c = hh(c, d, a, b, mm(11), 16, 0x6d9d6122) + b = hh(b, c, d, a, mm(14), 23, 0xfde5380c) + a = hh(a, b, c, d, mm(1), 4, 0xa4beea44) + d = hh(d, a, b, c, mm(4), 11, 0x4bdecfa9) + c = hh(c, d, a, b, mm(7), 16, 0xf6bb4b60) + b = hh(b, c, d, a, mm(10), 23, 0xbebfbc70) + a = hh(a, b, c, d, mm(13), 4, 0x289b7ec6) + d = hh(d, a, b, c, mm(0), 11, 0xeaa127fa) + c = hh(c, d, a, b, mm(3), 16, 0xd4ef3085) + b = hh(b, c, d, a, mm(6), 23, 0x04881d05) + a = hh(a, b, c, d, mm(9), 4, 0xd9d4d039) + d = hh(d, a, b, c, mm(12), 11, 0xe6db99e5) + c = hh(c, d, a, b, mm(15), 16, 0x1fa27cf8) + b = hh(b, c, d, a, mm(2), 23, 0xc4ac5665) + + a = ii(a, b, c, d, mm(0), 6, 0xf4292244) + d = ii(d, a, b, c, mm(7), 10, 0x432aff97) + c = ii(c, d, a, b, mm(14), 15, 0xab9423a7) + b = ii(b, c, d, a, mm(5), 21, 0xfc93a039) + a = ii(a, b, c, d, mm(12), 6, 0x655b59c3) + d = ii(d, a, b, c, mm(3), 10, 0x8f0ccc92) + c = ii(c, d, a, b, mm(10), 15, 0xffeff47d) + b = ii(b, c, d, a, mm(1), 21, 0x85845dd1) + a = ii(a, b, c, d, mm(8), 6, 0x6fa87e4f) + d = ii(d, a, b, c, mm(15), 10, 0xfe2ce6e0) + c = ii(c, d, a, b, mm(6), 15, 0xa3014314) + b = ii(b, c, d, a, mm(13), 21, 0x4e0811a1) + a = ii(a, b, c, d, mm(4), 6, 0xf7537e82) + d = ii(d, a, b, c, mm(11), 10, 0xbd3af235) + c = ii(c, d, a, b, mm(2), 15, 0x2ad7d2bb) + b = ii(b, c, d, a, mm(9), 21, 0xeb86d391) + + putInt(digest, 0, a0 + a) + putInt(digest, 4, b0 + b) + putInt(digest, 8, c0 + c) + putInt(digest, 12, d0 + d) } - private def ff(a: Int, b: Int, c: Int, d: Int, msg: Int, shift: Int, magic: Int): Int = { - val r = a + ((d ^ (b & (c ^ d))) + msg + magic + mkInt(state.k2, 0)) + @inline private def ff(a: Int, b: Int, c: Int, d: Int, msg: Int, shift: Int, magic: Int): Int = { + val r = a + ((d ^ (b & (c ^ d))) + msg + magic + k2a) Integer.rotateLeft(r, shift) + b } - private def gg(a: Int, b: Int, c: Int, d: Int, msg: Int, shift: Int, magic: Int): Int = { - val r = a + ((c ^ ((b ^ c) & d)) + msg + magic + mkInt(state.k2, 4)) + @inline private def gg(a: Int, b: Int, c: Int, d: Int, msg: Int, shift: Int, magic: Int): Int = { + val r = a + ((c ^ ((b ^ c) & d)) + msg + magic + k2b) Integer.rotateLeft(r, shift) + b } - private def hh(a: Int, b: Int, c: Int, d: Int, msg: Int, shift: Int, magic: Int): Int = { - val r = a + ((b ^ c ^ d) + msg + magic + mkInt(state.k2, 8)) + @inline private def hh(a: Int, b: Int, c: Int, d: Int, msg: Int, shift: Int, magic: Int): Int = { + val r = a + ((b ^ c ^ d) + msg + magic + k2c) Integer.rotateLeft(r, shift) + b } - private def ii(a: Int, b: Int, c: Int, d: Int, msg: Int, shift: Int, magic: Int): Int = { - val r = a + ((c ^ (b | ~d)) + msg + magic + mkInt(state.k2, 12)) + @inline private def ii(a: Int, b: Int, c: Int, d: Int, msg: Int, shift: Int, magic: Int): Int = { + val r = a + ((c ^ (b | ~d)) + msg + magic + k2d) Integer.rotateLeft(r, shift) + b } + /** Copy `size` bytes out of `src` (starting at `srcOffset`) into `dest` at `destOffset`. */ + private def copyOut(src: ByteVector, srcOffset: Long, dest: Array[Byte], destOffset: Int, size: Int): Unit = { + var i = 0 + while (i < size) { + dest(destOffset + i) = src(srcOffset + i) + i += 1 + } + } + def update(bytes: ByteVector): Unit = { - val buffer = state.buffer - count += bytes.length - var length = bytes.length - buffer.patchInPlace( - position, - bytes.take(math.min(BLOCKSIZE, bytes.length)).toIterable, - math.min(BLOCKSIZE, bytes.length).toInt - ) - - if (position + bytes.length >= BLOCKSIZE) { - hash(buffer.toSeq) - var input = bytes.drop(BLOCKSIZE - position) - length = bytes.length - (BLOCKSIZE - position) + val total = bytes.length + count += total + var length = total + // fill the accumulation block from `position` onwards, never past the end of the block + val head = math.min(math.min(BLOCKSIZE.toLong, total), (BLOCKSIZE - position).toLong).toInt + copyOut(bytes, 0L, buffer, position, head) + + if (position + total >= BLOCKSIZE) { + hash(buffer, 0) + var offset = (BLOCKSIZE - position).toLong + length = total - offset while (length >= BLOCKSIZE) { - hash(input.toSeq) - input = input.drop(BLOCKSIZE) + copyOut(bytes, offset, block, 0, BLOCKSIZE) + hash(block, 0) + offset += BLOCKSIZE length -= BLOCKSIZE } - buffer.patchInPlace(0, input.toIterable, input.length.toInt) + copyOut(bytes, offset, buffer, 0, length.toInt) position = 0 } position += length.toInt @@ -276,31 +302,50 @@ class Md5Mac(val key: ByteVector) { * @return the hash */ def doFinal(length: Int = MACLENGTH): ByteVector = { - val (buffer, digest, k1, k3) = (state.buffer, state.digest, state.k1, state.k3) - val output: ArrayBuffer[Byte] = ArrayBuffer.fill(MACLENGTH)(0) buffer(position) = 0x80.toByte - (position + 1 until BLOCKSIZE).foreach(i => buffer(i) = 0) + var i = position + 1 + while (i < BLOCKSIZE) { + buffer(i) = 0 + i += 1 + } if (position >= BLOCKSIZE - 8) { - hash(buffer.toSeq) - buffer.mapInPlace(_ => 0) + hash(buffer, 0) + java.util.Arrays.fill(buffer, 0.toByte) } - (BLOCKSIZE - 8 until BLOCKSIZE).foreach(i => buffer(i) = ByteVector.fromLong(8 * count)(7 - (i % 8))) + val bitCount = 8 * count + i = 0 + while (i < 8) { + // little-endian bit count in the last eight bytes of the block + buffer(BLOCKSIZE - 8 + i) = (bitCount >>> (8 * i)).toByte + i += 1 + } - hash(buffer.toSeq) - hash(k3.toSeq) + hash(buffer, 0) + hash(k3, 0) - (0 until MACLENGTH).foreach(i => output(i) = digest((i / 4) * 4 + (3 - (i % 4)))) + val out = macScratch + i = 0 + while (i < MACLENGTH) { + out(i) = digest((i / 4) * 4 + (3 - (i % 4))) + i += 1 + } count = 0 position = 0 - digest.patchInPlace(0, k1, digest.length) + System.arraycopy(k1, 0, digest, 0, DIGESTSIZE) + val result = new Array[Byte](length) if (length == MACLENGTH) { - ByteVector.view(output.toArray) + System.arraycopy(out, 0, result, 0, MACLENGTH) } else { - ByteVector.view((0 until length).map(i => output(i % Md5Mac.DIGESTSIZE)).toArray) + i = 0 + while (i < length) { + result(i) = out(i % Md5Mac.DIGESTSIZE) + i += 1 + } } + ByteVector.view(result) } /** Shorthand for `update` and `doFinal` */ @@ -319,7 +364,8 @@ class Md5Mac(val key: ByteVector) { def reset(): Md5Mac = { count = 0 position = 0 - state = PermanentMd5MacState.mutableCopy(originalState) + java.util.Arrays.fill(buffer, 0.toByte) + System.arraycopy(digestTemplate, 0, digest, 0, DIGESTSIZE) this } } From c15f40fd8e68a0c9dc8bc64222b34ea9cf2d14af Mon Sep 17 00:00:00 2001 From: GeekOfWires Date: Wed, 22 Jul 2026 06:05:42 -0400 Subject: [PATCH 17/18] Share the CommonFieldData codecs instead of rebuilding them Both codec and codec2 take a single Boolean and constructed a fresh tree of about a dozen primitive codecs, the named combinators and an xmap/exmap closure on every call. There are only two possible results each. These sit under ObjectClass.selectDataCodec, so they are reached once per object in every ObjectCreateMessage and ObjectCreateDetailedMessage. Spawning, zoning and entering render range of other players emit hundreds of those back to back on the session thread, and each one paid full codec construction before a single bit was written. Memoise the two shapes. scodec codecs are immutable and safe to share. They are lazy so that initialisation order inside the object -- the implicit val calls the dispatcher during construction -- cannot observe a null. The remaining per-call codec construction in this area, notably selectDataCodec's own arms and the offset-parameterised appearance and player codecs, is left for a follow-up; those take an Int rather than a Boolean and want a small table rather than two values. Co-Authored-By: Claude Opus 4.8 --- .../game/objectcreate/CommonFieldData.scala | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/main/scala/net/psforever/packet/game/objectcreate/CommonFieldData.scala b/src/main/scala/net/psforever/packet/game/objectcreate/CommonFieldData.scala index bf879c7cb..c19b36e8b 100644 --- a/src/main/scala/net/psforever/packet/game/objectcreate/CommonFieldData.scala +++ b/src/main/scala/net/psforever/packet/game/objectcreate/CommonFieldData.scala @@ -110,7 +110,7 @@ object CommonFieldData extends Marshallable[CommonFieldData] { CommonFieldData(faction, bops, destroyed, unk > 1, None, jammered = unk > 0, None, jammeredField, player_guid) } - def codec(extra: Boolean): Codec[CommonFieldData] = + private def buildCodec(extra: Boolean): Codec[CommonFieldData] = ( ("faction" | PlanetSideEmpire.codec) :: ("bops" | bool) :: @@ -131,9 +131,19 @@ object CommonFieldData extends Marshallable[CommonFieldData] { } ) + /* The parameter is a Boolean, so there are only ever two of these codecs, but a fresh tree of + roughly a dozen primitive codecs plus combinators and an xmap closure was constructed on + every call. This is reached once per object in every ObjectCreateMessage, and spawning or + zoning emits hundreds back to back. scodec codecs are immutable and safe to share. + lazy, so that initialisation order within this object cannot observe a null. */ + private lazy val codecPlain: Codec[CommonFieldData] = buildCodec(extra = false) + private lazy val codecExtra: Codec[CommonFieldData] = buildCodec(extra = true) + + def codec(extra: Boolean): Codec[CommonFieldData] = if (extra) codecExtra else codecPlain + implicit val codec: Codec[CommonFieldData] = codec(extra = false) - def codec2(extra: Boolean): Codec[CommonFieldData] = + private def buildCodec2(extra: Boolean): Codec[CommonFieldData] = ( ("faction" | PlanetSideEmpire.codec) :: ("bops" | bool) :: @@ -158,5 +168,11 @@ object CommonFieldData extends Marshallable[CommonFieldData] { } ) + /* see the note on codec above; same two-shape memoisation */ + private lazy val codec2Plain: Codec[CommonFieldData] = buildCodec2(extra = false) + private lazy val codec2Extra: Codec[CommonFieldData] = buildCodec2(extra = true) + + def codec2(extra: Boolean): Codec[CommonFieldData] = if (extra) codec2Extra else codec2Plain + val codec2: Codec[CommonFieldData] = codec2(extra = false) } From 291ef988e6146189429b35a90fd4e7e6b1aa8ff7 Mon Sep 17 00:00:00 2001 From: GeekOfWires Date: Wed, 22 Jul 2026 16:48:13 -0400 Subject: [PATCH 18/18] Improve comment etiquette for our previous work Reword the comments added by this branch so they describe the behaviour of the code as it now stands, rather than contrasting it with the code it replaced. A reader arriving at these files has no view of the previous implementation, so that framing carries no meaning for them; the before-and-after reasoning belongs in the commit messages, where it already is. No behavioural change: comments, configuration comments and one SQL comment block only. Co-Authored-By: Claude Opus 4.8 --- build.sbt | 10 ++++----- docker-compose.yml | 6 +++--- .../db/migration/V018__PerformanceIndexes.sql | 11 +++++----- src/main/resources/akka.conf | 21 +++++++++---------- src/main/resources/application.conf | 11 +++++----- .../actors/net/MiddlewareActor.scala | 12 +++++------ .../psforever/actors/net/SocketActor.scala | 7 ++++--- .../net/psforever/actors/net/SocketPane.scala | 8 +++---- .../session/support/ChatOperations.scala | 2 +- .../psforever/objects/guid/TaskWorkflow.scala | 11 +++++----- .../objects/guid/pool/ExclusivePool.scala | 13 ++++++------ .../objects/guid/pool/GenericPool.scala | 9 ++++---- .../containable/ContainableBehavior.scala | 9 ++++---- .../objects/zones/blockmap/Sector.scala | 11 +++++----- .../psforever/packet/GamePacketOpcode.scala | 9 ++++---- .../packet/control/MultiPacketEx.scala | 5 +++-- .../game/objectcreate/CommonFieldData.scala | 9 ++++---- .../services/InterstellarClusterService.scala | 14 ++++++------- .../local/support/HackCaptureActor.scala | 6 +++--- 19 files changed, 92 insertions(+), 92 deletions(-) diff --git a/build.sbt b/build.sbt index 5518b52e9..e00585535 100644 --- a/build.sbt +++ b/build.sbt @@ -110,11 +110,11 @@ lazy val server = (project in file("server")) inConfig(QuietTest)(Defaults.testTasks), packMain := Map("psforever-server" -> "net.psforever.server.Server"), packArchivePrefix := "psforever-server", - // The packaged launcher previously passed no memory or collector settings at all, so the - // server ran on whatever the JVM defaulted to -- in a container, a fraction of host RAM. - // The -Xmx in .jvmopts applies to sbt, not to this script. G1 with an explicit pause - // target keeps collections short; the server's hitching is sensitive to stop-the-world - // pauses because they stall every connected session simultaneously. + // Memory and collector settings for the packaged launcher. Without them the server takes + // the JVM defaults, which in a container is a fraction of host RAM. Note the -Xmx in + // .jvmopts governs sbt, not this script. G1 with an explicit pause target keeps + // collections short, which matters because a stop-the-world pause stalls every connected + // session simultaneously. packJvmOpts := Map("psforever-server" -> Seq( "-Dstacktrace.app.packages=net.psforever", "-Xms2g", diff --git a/docker-compose.yml b/docker-compose.yml index 322ace407..a5ba687cf 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,8 +1,8 @@ services: loginserver: - # Matches the JDK the Dockerfile builds with. This previously ran JDK 8, whose default - # collector is ParallelGC -- full collections there are stop-the-world and can run into - # seconds, stalling every connected client at once. + # Matches the JDK the Dockerfile builds with, so local runs and released builds share a + # collector. Keep this off JDK 8, whose default ParallelGC does stop-the-world full + # collections that can run into seconds and stall every connected client at once. image: sbtscala/scala-sbt:eclipse-temurin-focal-11.0.17_8_1.8.2_2.13.10 volumes: - .:/PSF-Loginserver:z diff --git a/server/src/main/resources/db/migration/V018__PerformanceIndexes.sql b/server/src/main/resources/db/migration/V018__PerformanceIndexes.sql index f6548b053..94d23005d 100644 --- a/server/src/main/resources/db/migration/V018__PerformanceIndexes.sql +++ b/server/src/main/resources/db/migration/V018__PerformanceIndexes.sql @@ -1,8 +1,7 @@ -- Indexes for foreign keys that are queried on the login and gameplay hot paths. -- --- PostgreSQL does not create an index for a REFERENCES constraint, so before this migration --- every one of the lookups below was a sequential scan. Only V015 (outfits) had declared any --- indexes at all. +-- PostgreSQL does not create an index for a REFERENCES constraint, so each foreign key that +-- is filtered on needs one declared explicitly or the lookup is a sequential scan. -- -- Deliberately omitted, because an existing constraint already provides a usable btree with -- avatar_id as its leading column: @@ -13,7 +12,7 @@ -- Adding further indexes there would only cost write throughput. -- Read for every character on the character select screen, and rewritten whenever a loadout --- is saved or deleted. Primary key is id alone, so avatar_id was unindexed. +-- is saved or deleted. The primary key is id alone, so it does not serve avatar_id lookups. CREATE INDEX IF NOT EXISTS "loadout_avatar_id_idx" ON "loadout" ("avatar_id"); CREATE INDEX IF NOT EXISTS "vehicleloadout_avatar_id_idx" ON "vehicleloadout" ("avatar_id"); @@ -33,7 +32,7 @@ CREATE INDEX IF NOT EXISTS "avatar_account_id_idx" ON "avatar" ("account_id"); -- killactivity is an append-only kill log that grows for the lifetime of the server, and the -- campaign KDA query reads it on every single login with -- WHERE (killer_id = ? OR victim_id = ?) AND killer_id <> victim_id --- Without these, login cost grew in proportion to every kill ever recorded. The OR is --- satisfied by two separate indexes via a bitmap union. +-- Two single-column indexes let the OR resolve as a bitmap union, keeping that query +-- proportional to one player's kills rather than to every kill the server has recorded. CREATE INDEX IF NOT EXISTS "killactivity_killer_id_idx" ON "killactivity" ("killer_id"); CREATE INDEX IF NOT EXISTS "killactivity_victim_id_idx" ON "killactivity" ("victim_id"); diff --git a/src/main/resources/akka.conf b/src/main/resources/akka.conf index a65208b0b..58bafa3a1 100644 --- a/src/main/resources/akka.conf +++ b/src/main/resources/akka.conf @@ -247,19 +247,18 @@ akka.actor.deployment { } } -# Safety net for the default dispatcher. +# Configuration for the default dispatcher. # -# The path-based assignments above do not correspond to any actor this server actually -# creates: there is no /world-udp-endpoint (the socket actors are named world-socket- -# under socketPane), no world-session-router, and the zone actors are named zone- rather -# than -actor. Akka's config-based deployment also only applies to classic actors, while -# the network and session actors are spawned through the typed API, so assigning their -# dispatchers by path cannot work regardless of the names. +# Anything that does not select a dispatcher explicitly runs here, so it is sized for real +# server load rather than left at Akka's stock throughput of 5, which produces frequent +# thread hand-off on the packet path. # -# The practical result is that socket I/O, session logic and zone logic all share -# akka.actor.default-dispatcher with its stock throughput of 5, which produces frequent -# thread hand-off on the packet path. Until each actor selects its dispatcher explicitly at -# spawn, give the pool everything actually runs on a configuration suited to that load. +# Note that the path-based assignments above do not reach the network, session or zone +# actors. Those are spawned through the typed API, which Akka's config-based deployment does +# not apply to, and they select their dispatchers at their spawn sites instead -- see +# SocketPane, SocketActor, Server and InterstellarClusterService. The paths named above +# (/world-udp-endpoint, world-session-router, -actor) do not correspond to actors this +# server creates. akka.actor.default-dispatcher { throughput = 20 diff --git a/src/main/resources/application.conf b/src/main/resources/application.conf index 257ee8fcc..4551164a9 100644 --- a/src/main/resources/application.conf +++ b/src/main/resources/application.conf @@ -68,8 +68,8 @@ database { # limit -- game clients never touch it. Every session, zone actor and kill report shares # this one pool, and the driver queues excess queries rather than rejecting them, so # saturation shows up purely as latency: logins, loadout applies and terminal purchases - # all wait behind each other. Five slots is far too few once character login alone issues - # a dozen sequential queries. + # waiting behind each other. Size it against concurrent database work, bearing in mind + # character login alone issues about a dozen sequential queries. maxActiveConnections = 32 } @@ -563,10 +563,9 @@ network { # How many bundles may be dispatched in a single execution of the bundling process. # Each bundle is capped at the 440 byte MTU, so this multiplied by the bundle size and # divided by the delay above is the effective per-client outbound ceiling. - # Previously the process sent exactly one bundle per run, which capped every client at - # roughly 20 bundles a second no matter how much was queued, so any burst -- a zone load, - # a base capture, a squad coming into render range -- drained at under 9 KB/s and the - # world appeared to the player to arrive in slow motion. + # This governs how quickly a backlog drains -- a zone load, a base capture, or a squad + # coming into render range all queue far more than one bundle -- while the delay above + # governs how aggressively packets are coalesced into each bundle. # Raise for faster burst delivery, lower to be gentler on constrained connections. packet-bundling-drain-limit = 8 diff --git a/src/main/scala/net/psforever/actors/net/MiddlewareActor.scala b/src/main/scala/net/psforever/actors/net/MiddlewareActor.scala index 9aa6fa30c..d94279dc7 100644 --- a/src/main/scala/net/psforever/actors/net/MiddlewareActor.scala +++ b/src/main/scala/net/psforever/actors/net/MiddlewareActor.scala @@ -675,13 +675,11 @@ class MiddlewareActor( */ private def processQueue(): Unit = { inReorderQueueFunc() - /* processOutQueueBundle dispatches at most one bundle per call, so a single call per timer - tick capped each client's outbound throughput at one MTU-sized bundle per delay period - regardless of backlog. Drain up to the configured number of bundles instead, so a burst - clears in a few ticks rather than tens of seconds, while the delay continues to govern - how aggressively packets are coalesced. - The budget also bounds the loop: it runs a fixed number of times at most, so it cannot - spin even if a queue fails to shrink. */ + /* Drain up to `packetBundlingDrainLimit` bundles per run. processOutQueueBundle dispatches + one bundle per call, so this budget governs how much of a backlog a single run clears, + while the bundling delay governs how aggressively packets are coalesced into each + bundle. The budget also bounds the loop, which therefore always terminates after a + fixed number of iterations even if a queue fails to shrink. */ var budget: Int = packetBundlingDrainLimit while (budget > 0 && (outQueueBundled.nonEmpty || outQueue.nonEmpty)) { processOutQueueBundle() diff --git a/src/main/scala/net/psforever/actors/net/SocketActor.scala b/src/main/scala/net/psforever/actors/net/SocketActor.scala index 619838d07..92ffa89e5 100644 --- a/src/main/scala/net/psforever/actors/net/SocketActor.scala +++ b/src/main/scala/net/psforever/actors/net/SocketActor.scala @@ -142,9 +142,10 @@ class SocketActor( val ref = context.spawn( MiddlewareActor(udpCommandAdapter, remote, nextPlan, connectionId), s"middleware-$connectionId", - /* crypto, bundling and reordering for one connection; world-session is the - pool akka.conf meant to isolate this work onto, and it has to be selected - here because path-based deployment does not apply to typed actors */ + /* crypto, bundling and reordering for one connection, isolated onto the + world-session pool. The dispatcher is selected here at the spawn site + because this is a typed actor, which config-based deployment in + akka.conf does not apply to. */ ActorTags(s"uuid=$connectionId").withDispatcherFromConfig("world-session") ) context.watch(ref) diff --git a/src/main/scala/net/psforever/actors/net/SocketPane.scala b/src/main/scala/net/psforever/actors/net/SocketPane.scala index 3100977fa..669a64877 100644 --- a/src/main/scala/net/psforever/actors/net/SocketPane.scala +++ b/src/main/scala/net/psforever/actors/net/SocketPane.scala @@ -88,11 +88,11 @@ final case class SocketSetupInfo( object SocketPane { val SocketPaneKey: ServiceKey[Command] = ServiceKey[SocketPane.Command](id = "socketPane") - /** The UDP read loop gets the dedicated thread akka.conf always intended for it. + /** Dispatcher for the UDP read loop. * network-listener is a PinnedDispatcher, so each socket actor -- one per bound port -- - * receives its own thread rather than competing with game logic on the default pool. - * Selecting it here rather than by actor path is deliberate: these are typed actors, and - * config-based deployment does not apply to them. */ + * owns a dedicated thread and is insulated from game logic running on other pools. + * The dispatcher is selected here at the spawn site because these are typed actors, which + * config-based deployment in akka.conf does not apply to. */ private val socketDispatcher: DispatcherSelector = DispatcherSelector.fromConfig("network-listener") /** diff --git a/src/main/scala/net/psforever/actors/session/support/ChatOperations.scala b/src/main/scala/net/psforever/actors/session/support/ChatOperations.scala index 2e671a451..31491e348 100644 --- a/src/main/scala/net/psforever/actors/session/support/ChatOperations.scala +++ b/src/main/scala/net/psforever/actors/session/support/ChatOperations.scala @@ -1487,7 +1487,7 @@ class ChatOperations( override protected[session] def stop(): Unit = { silenceTimer.cancel() chatService ! ChatService.LeaveAllChannels(chatServiceAdapter) - /* this pool is per-session; without this its threads outlive the session that made them */ + /* the scheduler pool is owned by this session, so it is released with the session */ scheduler.shutdown() } } diff --git a/src/main/scala/net/psforever/objects/guid/TaskWorkflow.scala b/src/main/scala/net/psforever/objects/guid/TaskWorkflow.scala index 507e10885..df0959f11 100644 --- a/src/main/scala/net/psforever/objects/guid/TaskWorkflow.scala +++ b/src/main/scala/net/psforever/objects/guid/TaskWorkflow.scala @@ -145,11 +145,12 @@ object TaskWorkflow { //no subtasks; just execute the main task promise.completeWith(task.action()) case list => - /* The monitor must be a dedicated object. Synchronising on a Boolean synchronises on - the boxed java.lang.Boolean, and valueOf returns the cached TRUE/FALSE singletons, - so every task bundle in the server contended the same two JVM-wide monitors. - It was also not a valid mutex: the monitor identity changed when the flag flipped, - so two threads could hold "the lock" at once and complete the promise twice. */ + /* A dedicated object, private to this bundle, guards `unassignedCompletion` so that + exactly one subtask completion resolves the promise. + The monitor must not be the flag itself: synchronising on a Boolean locks the boxed + java.lang.Boolean, and valueOf returns the cached TRUE/FALSE singletons, which are + shared JVM-wide and whose identity changes when the flag flips -- neither mutual + exclusion between threads nor isolation between bundles. */ val completionLock = new Object var unassignedCompletion: Boolean = true //wait for subtasks to complete diff --git a/src/main/scala/net/psforever/objects/guid/pool/ExclusivePool.scala b/src/main/scala/net/psforever/objects/guid/pool/ExclusivePool.scala index 0b376fb42..6e8e9e481 100644 --- a/src/main/scala/net/psforever/objects/guid/pool/ExclusivePool.scala +++ b/src/main/scala/net/psforever/objects/guid/pool/ExclusivePool.scala @@ -9,12 +9,13 @@ class ExclusivePool(override val numbers: List[Int]) extends SimplePool(numbers) private val pool: Array[Int] = Array.ofDim[Int](numbers.length) numbers.indices.foreach(i => { pool(i) = i }) - /* `numbers` is a List, so both hot operations walked it: Get did numbers(index), which is - O(index), and Return did an O(n) indexOf. The configured pools are large -- deployables - 16000, ammo and kits 13500 each -- and unregistration happens in bulk when a player dies - or a vehicle is deconstructed, all serialised through one pool actor. - An indexed copy and a reverse lookup make both constant time. Neither is mutated after - construction, and SimplePool has already rejected duplicates. */ + /* Constant-time views over `numbers`, which is a List: an indexed copy for resolving a + selector index to its number, and a reverse lookup for resolving a number back to its + index. Both operations are hot -- the configured pools are large (deployables 16000, + ammo and kits 13500 each) and unregistration arrives in bulk when a player dies or a + vehicle is deconstructed, all serialised through one pool actor. + Neither structure is mutated after construction, and SimplePool has already rejected + duplicates, so the reverse lookup is unambiguous. */ private val numbersByIndex: Array[Int] = numbers.toArray private val indexByNumber: Map[Int, Int] = numbers.zipWithIndex.toMap diff --git a/src/main/scala/net/psforever/objects/guid/pool/GenericPool.scala b/src/main/scala/net/psforever/objects/guid/pool/GenericPool.scala index 8048cfe16..ce961419b 100644 --- a/src/main/scala/net/psforever/objects/guid/pool/GenericPool.scala +++ b/src/main/scala/net/psforever/objects/guid/pool/GenericPool.scala @@ -82,11 +82,10 @@ object GenericPool { */ def first(list: List[Long], domainSize: Int): Int = { if (list.size < domainSize) { - /* an Array, because this is indexed by position in a loop: List.apply walks the chain, - so the identical scan over a List cost O(index) per step. This pool is the fallback - for any exhausted pool and receives every number in the hub -- on the order of tens - of thousands -- which made the scan quadratic and left the pool actor's mailbox - blocked for a long time at precisely the moment the server was under pressure. */ + /* An Array, because the loop below indexes it by position and Array.apply is constant + time. That matters here more than elsewhere: this pool is the fallback for any + exhausted pool and receives every number in the hub, on the order of tens of + thousands, and the scan runs on the pool actor's thread while its mailbox waits. */ val sortedList: Array[Long] = (0L +: list.sorted :+ domainSize.toLong).toArray var index: Int = 0 val listLen = sortedList.length - 1 diff --git a/src/main/scala/net/psforever/objects/serverobject/containable/ContainableBehavior.scala b/src/main/scala/net/psforever/objects/serverobject/containable/ContainableBehavior.scala index 6a206c5b7..163d04286 100644 --- a/src/main/scala/net/psforever/objects/serverobject/containable/ContainableBehavior.scala +++ b/src/main/scala/net/psforever/objects/serverobject/containable/ContainableBehavior.scala @@ -196,11 +196,10 @@ trait ContainableBehavior { case _ => ; //TODO what? } - /* exactly one Resume must answer the Wait() above. The previous form recovered an - AskTimeoutException by sending a Resume and, because recover yields a successful - future, the following onComplete sent a second one -- releasing an unrelated - concurrent move's guard and letting interleaved insertions through. - onComplete alone already covers both success and failure. */ + /* Exactly one Resume answers the Wait() above, and onComplete alone delivers it + for both success and failure -- including an AskTimeoutException. A second + Resume would release an unrelated concurrent move's guard and let insertions + interleave with a move that is still in flight. */ moveItemOver.onComplete { _ => destination.Actor ! ContainableBehavior.Resume() } } case _ => ; diff --git a/src/main/scala/net/psforever/objects/zones/blockmap/Sector.scala b/src/main/scala/net/psforever/objects/zones/blockmap/Sector.scala index 78de9d6ee..2520f3c0a 100644 --- a/src/main/scala/net/psforever/objects/zones/blockmap/Sector.scala +++ b/src/main/scala/net/psforever/objects/zones/blockmap/Sector.scala @@ -277,11 +277,12 @@ object Sector { * @param environmentList fields that represent the game world environment */ /** - * The entity lists are declared by-name and retained as `lazy val`s. - * A sector group frequently spans a great many sectors, and callers usually consult only one or two - * of the ten categories, so materializing all of them eagerly does a large amount of discarded work - * on hot paths such as the per-movement-packet local sector refresh. - * Each list is instead condensed on first access and memoized thereafter. + * The entity lists are declared by-name and retained as `lazy val`s, so each category is + * condensed on first access and memoized thereafter. + * A sector group frequently spans a great many sectors while callers usually consult only one or + * two of the ten categories, and the group is rebuilt on hot paths such as the per-movement-packet + * local sector refresh, so condensing a category only when it is read keeps that cost proportional + * to what is actually used. */ class SectorGroup( val rangeX: Float, diff --git a/src/main/scala/net/psforever/packet/GamePacketOpcode.scala b/src/main/scala/net/psforever/packet/GamePacketOpcode.scala index 249e21c71..719e62fa6 100644 --- a/src/main/scala/net/psforever/packet/GamePacketOpcode.scala +++ b/src/main/scala/net/psforever/packet/GamePacketOpcode.scala @@ -294,10 +294,11 @@ object GamePacketOpcode extends Enumeration { ClientCheatedMessage // last known message type (243, 0xf3) = Value - /* Err takes its message by value, so any interpolation here is built eagerly on every - packet that lands on an unimplemented opcode -- and a great many opcodes are still - stubs. Rendering the payload as hex allocated a string twice the packet's length each - time; the opcode alone identifies the gap. */ + /* The message names the opcode only, which is what identifies the missing marshaller. + Keep it cheap to build: Err takes its message by value, so anything interpolated here is + constructed eagerly for every packet that lands on an unimplemented opcode, and a great + many opcodes are still stubs. Rendering the payload would cost a string twice the + packet's length each time. */ private def noDecoder(opcode: GamePacketOpcode.Type) = (_: BitVector) => Attempt.failure(Err(s"Could not find a marshaller for game packet $opcode")) diff --git a/src/main/scala/net/psforever/packet/control/MultiPacketEx.scala b/src/main/scala/net/psforever/packet/control/MultiPacketEx.scala index 5d2bcf142..dab2ac1ce 100644 --- a/src/main/scala/net/psforever/packet/control/MultiPacketEx.scala +++ b/src/main/scala/net/psforever/packet/control/MultiPacketEx.scala @@ -21,8 +21,9 @@ object MultiPacketEx extends Marshallable[MultiPacketEx] { val MaxValue = (1L << 31) - 1 val MinValue = 0 - /* hoisted out of encode/decode; these run once per sub-packet of every inbound - and outbound bundle, and scodec codecs are immutable and safe to share */ + /* Shared across calls: sizeCodec runs once per sub-packet of every inbound and outbound + bundle, so these are built once here rather than per invocation. scodec codecs are + immutable and safe to share. */ private val mediumCodec = (constant(hex"ff") :: uint16L).dropUnits private val largeCodec = (constant(hex"ffffff") :: uint32L).dropUnits private val sizeTypes = Vector(8, 16, 32) diff --git a/src/main/scala/net/psforever/packet/game/objectcreate/CommonFieldData.scala b/src/main/scala/net/psforever/packet/game/objectcreate/CommonFieldData.scala index c19b36e8b..4769451b9 100644 --- a/src/main/scala/net/psforever/packet/game/objectcreate/CommonFieldData.scala +++ b/src/main/scala/net/psforever/packet/game/objectcreate/CommonFieldData.scala @@ -131,10 +131,11 @@ object CommonFieldData extends Marshallable[CommonFieldData] { } ) - /* The parameter is a Boolean, so there are only ever two of these codecs, but a fresh tree of - roughly a dozen primitive codecs plus combinators and an xmap closure was constructed on - every call. This is reached once per object in every ObjectCreateMessage, and spawning or - zoning emits hundreds back to back. scodec codecs are immutable and safe to share. + /* The parameter is a Boolean, so there are only ever two of these codecs; each is built once + and shared. Worth doing because this is reached once per object in every + ObjectCreateMessage and spawning or zoning emits hundreds back to back, while each tree + costs roughly a dozen primitive codecs plus combinators and an xmap closure to construct. + scodec codecs are immutable and safe to share. lazy, so that initialisation order within this object cannot observe a null. */ private lazy val codecPlain: Codec[CommonFieldData] = buildCodec(extra = false) private lazy val codecExtra: Codec[CommonFieldData] = buildCodec(extra = true) diff --git a/src/main/scala/net/psforever/services/InterstellarClusterService.scala b/src/main/scala/net/psforever/services/InterstellarClusterService.scala index 3f86f0e78..3ba55df86 100644 --- a/src/main/scala/net/psforever/services/InterstellarClusterService.scala +++ b/src/main/scala/net/psforever/services/InterstellarClusterService.scala @@ -125,13 +125,13 @@ class InterstellarClusterService(context: ActorContext[InterstellarClusterServic mutable.Map( _zones.map { zone => - /* dispatchers.conf defines a pool per zone so that one busy continent cannot starve - the others, but nothing ever selected them: the deployment entries in akka.conf - name "-actor" paths that are never created, and config deployment does not - apply to typed actors in any case. - Not every zone id is guaranteed to have a matching entry, and - DispatcherSelector.fromConfig throws on a missing key, so fall back to the default - pool rather than prevent the server from starting. */ + /* Each zone runs on its own pool from dispatchers.conf, so that one busy continent + cannot starve the others. The dispatcher is selected here at the spawn site + because this is a typed actor, which config-based deployment does not apply to. + Not every zone id is guaranteed to have a matching entry -- Zone.Nowhere, the + sentinel for an invalid location, has none -- and DispatcherSelector.fromConfig + throws on a missing key, so an absent pool falls back to the default rather than + preventing the server from starting. */ val dispatcherPath = s"${zone.id}-zone-dispatcher" val zoneDispatcher = if (context.system.settings.config.hasPath(dispatcherPath)) { diff --git a/src/main/scala/net/psforever/services/local/support/HackCaptureActor.scala b/src/main/scala/net/psforever/services/local/support/HackCaptureActor.scala index 568025829..9b4d98f32 100644 --- a/src/main/scala/net/psforever/services/local/support/HackCaptureActor.scala +++ b/src/main/scala/net/psforever/services/local/support/HackCaptureActor.scala @@ -342,9 +342,9 @@ class HackCaptureActor extends Actor { delayMillis: Long ): Unit = { val buildingIterator = buildings.iterator - /* the handle must be cancelled once the iterator is spent, or the task keeps waking - every delayMillis for the lifetime of the pool doing nothing; compare the equivalent - loop in ChatOperations.processBuildingsWithDelay, which already cancels */ + /* The handle is captured so the task can cancel itself once the iterator is spent; + otherwise it would keep waking every delayMillis for the lifetime of the pool with no + work left to do. ChatOperations.processBuildingsWithDelay follows the same pattern. */ var handle: ScheduledFuture[_] = null handle = scheduler.scheduleAtFixedRate( () => {