diff --git a/core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/sim/DispatchObservation.kt b/core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/sim/DispatchObservation.kt index 6d2322306..9eea89a3a 100644 --- a/core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/sim/DispatchObservation.kt +++ b/core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/sim/DispatchObservation.kt @@ -82,6 +82,22 @@ data class QueuedTrainObservation( * [ShuntingLoop]). `null` when no FREE next separator exists, in which case the * dispatcher emits [DispatchDecision.NoAction] for this input (the train waits * and is reconsidered next tick). + * + * **Populated only where a forward reservation is possible** (Issue #749). The shell + * resolves it exclusively for inputs satisfying + * `!pathAlreadyExtendedBeyond && (isApproachingThisInput || pathSetUpTowardThisInput)`; + * for every other input — FREE, not approaching this input, or already extended beyond + * it — this is `null` **without the search having been run**. Resolving it means a BFS + * plus a per-candidate topological-path enumeration + * ([PathReservationService.findNextReservationTarget][cz.vutbr.fit.interlockSim.context.navigation.PathReservationService.findNextReservationTarget]); + * running it for the ~98% of inputs whose value is then discarded cost ~9% of fast-sim + * wall time. + * + * A `null` here therefore means *"no forward-reservation target applies"*, not + * *"the search found nothing"* — the two are indistinguishable to a dispatcher, and + * both call for the same response (no reservation for this input on this tick). + * Dispatcher implementations — including future LLM-backed ones — must not read + * `toSeparatorName == null` as evidence that the track ahead is occupied. * @property state Occupancy state of the block. * @property ownerTrainId Name of the train associated with this block: the * occupant's name when OCCUPIED, [cz.vutbr.fit.interlockSim.objects.tracks.DynamicTrackBlock.trainName] diff --git a/core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/sim/ShuntingLoop.kt b/core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/sim/ShuntingLoop.kt index 1c9ffd00c..13aa4bee2 100644 --- a/core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/sim/ShuntingLoop.kt +++ b/core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/sim/ShuntingLoop.kt @@ -392,23 +392,50 @@ class ShuntingLoop( TrackFacility.State.RESERVED -> block.trainName TrackFacility.State.OCCUPIED -> requireSimulationNotNull(block.getTrackOccupant()).name } - val toSeparatorName = pathReservationService.findNextReservationTarget(to)?.let(::nameOf) + val isApproachingThisInput = + state == TrackFacility.State.OCCUPIED && + requireSimulationNotNull(block.getTrackOccupant()).nextSemaphore() == to + val pathSetUpTowardThisInput = + state == TrackFacility.State.RESERVED && + block.isSetUpPath(env.toDynamic(block.getSecondEnd(to))) + val pathAlreadyExtendedBeyond = ownerTrainId != null && registry.isPathExtendedBeyond(ownerTrainId, to) return BlockInputObservation( blockId = requireNotNull(block.name) { "ShuntingLoop-owned blocks are always named" }, towardSemaphoreName = to.name, - toSeparatorName = toSeparatorName, + toSeparatorName = + findForwardReservationTargetName( + to, + isApproachingThisInput, + pathSetUpTowardThisInput, + pathAlreadyExtendedBeyond + ), state = state, ownerTrainId = ownerTrainId, - isApproachingThisInput = - state == TrackFacility.State.OCCUPIED && - requireSimulationNotNull(block.getTrackOccupant()).nextSemaphore() == to, - pathSetUpTowardThisInput = - state == TrackFacility.State.RESERVED && - block.isSetUpPath(env.toDynamic(block.getSecondEnd(to))), - pathAlreadyExtendedBeyond = ownerTrainId != null && registry.isPathExtendedBeyond(ownerTrainId, to) + isApproachingThisInput = isApproachingThisInput, + pathSetUpTowardThisInput = pathSetUpTowardThisInput, + pathAlreadyExtendedBeyond = pathAlreadyExtendedBeyond ) } + // Only inputs that can actually yield a forward reservation need a target searched for. + // findNextReservationTarget is a graph walk (BFS + per-candidate path enumeration); running + // it for FREE and already-extended inputs cost ~9% of fast-sim wall time (#738). + private fun findForwardReservationTargetName( + to: DynamicRailSemaphore, + isApproachingThisInput: Boolean, + pathSetUpTowardThisInput: Boolean, + pathAlreadyExtendedBeyond: Boolean + ): String? { + val canReserveForward = + !pathAlreadyExtendedBeyond && + (isApproachingThisInput || pathSetUpTowardThisInput) + return if (canReserveForward) { + pathReservationService.findNextReservationTarget(to)?.let(::nameOf) + } else { + null + } + } + /** * Name of a [DynamicPathSeparator] returned by * [PathReservationService.findNextReservationTarget] — a [DynamicInOut] or a diff --git a/core/src/jvmTest/kotlin/cz/vutbr/fit/interlockSim/sim/ShuntingLoopReservationTargetLazinessTest.kt b/core/src/jvmTest/kotlin/cz/vutbr/fit/interlockSim/sim/ShuntingLoopReservationTargetLazinessTest.kt new file mode 100644 index 000000000..791637aa2 --- /dev/null +++ b/core/src/jvmTest/kotlin/cz/vutbr/fit/interlockSim/sim/ShuntingLoopReservationTargetLazinessTest.kt @@ -0,0 +1,135 @@ +package cz.vutbr.fit.interlockSim.sim + +import assertk.assertThat +import assertk.assertions.isEqualTo +import assertk.assertions.isGreaterThan +import assertk.assertions.isNull +import cz.vutbr.fit.interlockSim.context.DefaultSimulationContext +import cz.vutbr.fit.interlockSim.context.EditingContext +import cz.vutbr.fit.interlockSim.context.JvmEditingContextFactory +import cz.vutbr.fit.interlockSim.context.SimulationContextFactory +import cz.vutbr.fit.interlockSim.objects.core.TrackFacility +import cz.vutbr.fit.interlockSim.testutil.KoinTestBase +import cz.vutbr.fit.interlockSim.testutil.TestFixtures +import org.junit.jupiter.api.DisplayName +import org.junit.jupiter.api.Tag +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.Timeout +import org.koin.test.inject +import java.util.concurrent.TimeUnit + +/** + * Locks in the sensor-port contract that keeps the forward-reservation graph search + * off the per-tick hot path. + * + * [ShuntingLoop.toBlockInputObservation] resolves [BlockInputObservation.toSeparatorName] + * via [PathReservationService.findNextReservationTarget][cz.vutbr.fit.interlockSim.context.navigation.PathReservationService.findNextReservationTarget], + * which is a BFS plus a per-candidate topological-path enumeration. Running it for every + * block input on every tick — including inputs that provably cannot take a forward + * reservation — accounted for ~9% of `fast-sim example shuntingLoop 300` wall time. + * + * The contract: `toSeparatorName` is populated **only** for inputs that could actually + * yield a [DispatchDecision.ReservePath] — a train occupying the block and approaching this + * input, or a path already set up toward it — and only when the path is not already extended + * beyond it. For every other input it is `null`. + * + * [RuleBasedDispatcher.checkInput] returns `null` for exactly those non-eligible cases, so + * narrowing the contract is behaviour-preserving; the golden `shuntingLoop` output is + * byte-for-byte unchanged. + * + * Reverting the gate (computing `toSeparatorName` unconditionally) makes this test fail: + * FREE inputs with clear track ahead resolve to a non-null target. + */ +@DisplayName("ShuntingLoop forward-reservation-target laziness contract") +@Tag("integration-test") +class ShuntingLoopReservationTargetLazinessTest : KoinTestBase() { + private val editingContextFactory: JvmEditingContextFactory by inject() + private val simulationContextFactory: SimulationContextFactory by inject() + + private fun loadVyhybnaContext(): DefaultSimulationContext = + TestFixtures.loadShuntingXml().use { xmlStream -> + val editingContext = editingContextFactory.createContext(xmlStream) as EditingContext + simulationContextFactory.createContext(editingContext) as DefaultSimulationContext + } + + @Test + @Timeout(value = 60, unit = TimeUnit.SECONDS) + fun `toSeparatorName is null for every input that cannot take a forward reservation`() { + val context = loadVyhybnaContext() + context.getInOuts() + + val loop = ShuntingLoop(context, endTime = 120L) + wireSynchronousDispatcher(context, loop) + + // Observations are published before the control-step listener fires, so reading them + // from a listener installed *after* the wiring sees the current tick's data. + val wired = loop.controlStepListener + var inputsSeen = 0 + var freeInputsSeen = 0 + var eligibleInputsSeen = 0 + val violations = mutableListOf() + + loop.controlStepListener = + ControlStepListener { + val inputs = loop.getInnerBlockInputs() + loop.getOuterBlockInputs() + for (input in inputs) { + inputsSeen++ + if (input.state == TrackFacility.State.FREE) freeInputsSeen++ + + val canReserveForward = + !input.pathAlreadyExtendedBeyond && + (input.isApproachingThisInput || input.pathSetUpTowardThisInput) + if (canReserveForward) { + eligibleInputsSeen++ + } else if (input.toSeparatorName != null) { + violations += + "block=${input.blockId} toward=${input.towardSemaphoreName} state=${input.state} " + + "approaching=${input.isApproachingThisInput} " + + "setUpToward=${input.pathSetUpTowardThisInput} " + + "extendedBeyond=${input.pathAlreadyExtendedBeyond} " + + "toSeparatorName=${input.toSeparatorName}" + } + } + wired?.onControlStep() + } + + context.setMainProcess(loop) + context.run() + + // The run must have exercised the interesting states, otherwise the assertion is vacuous. + assertThat(inputsSeen, "block inputs observed").isGreaterThan(0) + assertThat(freeInputsSeen, "FREE inputs observed").isGreaterThan(0) + assertThat(eligibleInputsSeen, "reservation-eligible inputs observed").isGreaterThan(0) + + assertThat(violations.size, "non-eligible inputs carrying a toSeparatorName: $violations").isEqualTo(0) + } + + @Test + @Timeout(value = 60, unit = TimeUnit.SECONDS) + fun `a FREE block input never carries a forward-reservation target`() { + val context = loadVyhybnaContext() + context.getInOuts() + + val loop = ShuntingLoop(context, endTime = 120L) + wireSynchronousDispatcher(context, loop) + val wired = loop.controlStepListener + + val freeWithTarget = mutableListOf() + loop.controlStepListener = + ControlStepListener { + (loop.getInnerBlockInputs() + loop.getOuterBlockInputs()) + .filter { it.state == TrackFacility.State.FREE } + .forEach { input -> + if (input.toSeparatorName != null) { + freeWithTarget += "${input.blockId}->${input.towardSemaphoreName}=${input.toSeparatorName}" + } + } + wired?.onControlStep() + } + + context.setMainProcess(loop) + context.run() + + assertThat(freeWithTarget.firstOrNull(), "FREE input with a forward-reservation target").isNull() + } +}