Skip to content

Commit dba2c6e

Browse files
bedaHovorkaclaudeCopilot
authored
SP0.13a (#749): gate findNextReservationTarget on reservation-eligibility (~9% fast-sim win) (#754)
* fix(#749): gate findNextReservationTarget on reservation-eligibility ShuntingLoop.toBlockInputObservation resolved BlockInputObservation.toSeparatorName by calling PathReservationService.findNextReservationTarget for every block input on every tick, including inputs that provably cannot take a forward reservation. The call is a BFS (findNextSemaphoresVia) plus a full topological-path enumeration per candidate (isPathAvailable -> findAllTopologicalPaths). Measured on vyhybna: 906 calls per `example shuntingLoop 300`, of which 892 (98.5%) are discarded unread by RuleBasedDispatcher.checkInput, which already returns null for FREE, not-approaching and already-extended inputs. Cost: 30.3 ms of 342 ms wall (~9%). Gate the search on eligibility. The three facts it needs (isApproachingThisInput, pathSetUpTowardThisInput, pathAlreadyExtendedBeyond) are already computed in the same function, so the gate is free. This narrows the sensor-port contract: toSeparatorName is now populated only where a forward reservation is possible, and is null elsewhere without the search having run. RuleBasedDispatcher behaviour is unchanged by construction. Documented on BlockInputObservation so a future LLM dispatcher does not read null as "track ahead is occupied". Native linuxX64 release, example shuntingLoop 300, median of 7: before 341 ms after 318 ms Golden output byte-for-byte identical (90/90 event lines, 5 trains, 211.1 s sim time). ShuntingLoopReservationTargetLazinessTest locks the contract and fails 2/2 without the gate (a FREE input resolves k1->doA1=A). Gates: core:jvmTest 2194, core:integrationTest 725, core:heavyTest 1001, dispatcher-agent:test 51, desktop-ui:test 650, desktop-ui:integrationTest 247 — all green. purity/detekt/ktlint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Extract reservation-target helper --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent 51784ac commit dba2c6e

3 files changed

Lines changed: 187 additions & 9 deletions

File tree

‎core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/sim/DispatchObservation.kt‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,22 @@ data class QueuedTrainObservation(
8282
* [ShuntingLoop]). `null` when no FREE next separator exists, in which case the
8383
* dispatcher emits [DispatchDecision.NoAction] for this input (the train waits
8484
* and is reconsidered next tick).
85+
*
86+
* **Populated only where a forward reservation is possible** (Issue #749). The shell
87+
* resolves it exclusively for inputs satisfying
88+
* `!pathAlreadyExtendedBeyond && (isApproachingThisInput || pathSetUpTowardThisInput)`;
89+
* for every other input — FREE, not approaching this input, or already extended beyond
90+
* it — this is `null` **without the search having been run**. Resolving it means a BFS
91+
* plus a per-candidate topological-path enumeration
92+
* ([PathReservationService.findNextReservationTarget][cz.vutbr.fit.interlockSim.context.navigation.PathReservationService.findNextReservationTarget]);
93+
* running it for the ~98% of inputs whose value is then discarded cost ~9% of fast-sim
94+
* wall time.
95+
*
96+
* A `null` here therefore means *"no forward-reservation target applies"*, not
97+
* *"the search found nothing"* — the two are indistinguishable to a dispatcher, and
98+
* both call for the same response (no reservation for this input on this tick).
99+
* Dispatcher implementations — including future LLM-backed ones — must not read
100+
* `toSeparatorName == null` as evidence that the track ahead is occupied.
85101
* @property state Occupancy state of the block.
86102
* @property ownerTrainId Name of the train associated with this block: the
87103
* occupant's name when OCCUPIED, [cz.vutbr.fit.interlockSim.objects.tracks.DynamicTrackBlock.trainName]

‎core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/sim/ShuntingLoop.kt‎

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -392,23 +392,50 @@ class ShuntingLoop(
392392
TrackFacility.State.RESERVED -> block.trainName
393393
TrackFacility.State.OCCUPIED -> requireSimulationNotNull(block.getTrackOccupant()).name
394394
}
395-
val toSeparatorName = pathReservationService.findNextReservationTarget(to)?.let(::nameOf)
395+
val isApproachingThisInput =
396+
state == TrackFacility.State.OCCUPIED &&
397+
requireSimulationNotNull(block.getTrackOccupant()).nextSemaphore() == to
398+
val pathSetUpTowardThisInput =
399+
state == TrackFacility.State.RESERVED &&
400+
block.isSetUpPath(env.toDynamic(block.getSecondEnd(to)))
401+
val pathAlreadyExtendedBeyond = ownerTrainId != null && registry.isPathExtendedBeyond(ownerTrainId, to)
396402
return BlockInputObservation(
397403
blockId = requireNotNull(block.name) { "ShuntingLoop-owned blocks are always named" },
398404
towardSemaphoreName = to.name,
399-
toSeparatorName = toSeparatorName,
405+
toSeparatorName =
406+
findForwardReservationTargetName(
407+
to,
408+
isApproachingThisInput,
409+
pathSetUpTowardThisInput,
410+
pathAlreadyExtendedBeyond
411+
),
400412
state = state,
401413
ownerTrainId = ownerTrainId,
402-
isApproachingThisInput =
403-
state == TrackFacility.State.OCCUPIED &&
404-
requireSimulationNotNull(block.getTrackOccupant()).nextSemaphore() == to,
405-
pathSetUpTowardThisInput =
406-
state == TrackFacility.State.RESERVED &&
407-
block.isSetUpPath(env.toDynamic(block.getSecondEnd(to))),
408-
pathAlreadyExtendedBeyond = ownerTrainId != null && registry.isPathExtendedBeyond(ownerTrainId, to)
414+
isApproachingThisInput = isApproachingThisInput,
415+
pathSetUpTowardThisInput = pathSetUpTowardThisInput,
416+
pathAlreadyExtendedBeyond = pathAlreadyExtendedBeyond
409417
)
410418
}
411419

420+
// Only inputs that can actually yield a forward reservation need a target searched for.
421+
// findNextReservationTarget is a graph walk (BFS + per-candidate path enumeration); running
422+
// it for FREE and already-extended inputs cost ~9% of fast-sim wall time (#738).
423+
private fun findForwardReservationTargetName(
424+
to: DynamicRailSemaphore,
425+
isApproachingThisInput: Boolean,
426+
pathSetUpTowardThisInput: Boolean,
427+
pathAlreadyExtendedBeyond: Boolean
428+
): String? {
429+
val canReserveForward =
430+
!pathAlreadyExtendedBeyond &&
431+
(isApproachingThisInput || pathSetUpTowardThisInput)
432+
return if (canReserveForward) {
433+
pathReservationService.findNextReservationTarget(to)?.let(::nameOf)
434+
} else {
435+
null
436+
}
437+
}
438+
412439
/**
413440
* Name of a [DynamicPathSeparator] returned by
414441
* [PathReservationService.findNextReservationTarget] — a [DynamicInOut] or a

‎core/src/jvmTest/kotlin/cz/vutbr/fit/interlockSim/sim/ShuntingLoopReservationTargetLazinessTest.kt‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
package cz.vutbr.fit.interlockSim.sim
2+
3+
import assertk.assertThat
4+
import assertk.assertions.isEqualTo
5+
import assertk.assertions.isGreaterThan
6+
import assertk.assertions.isNull
7+
import cz.vutbr.fit.interlockSim.context.DefaultSimulationContext
8+
import cz.vutbr.fit.interlockSim.context.EditingContext
9+
import cz.vutbr.fit.interlockSim.context.JvmEditingContextFactory
10+
import cz.vutbr.fit.interlockSim.context.SimulationContextFactory
11+
import cz.vutbr.fit.interlockSim.objects.core.TrackFacility
12+
import cz.vutbr.fit.interlockSim.testutil.KoinTestBase
13+
import cz.vutbr.fit.interlockSim.testutil.TestFixtures
14+
import org.junit.jupiter.api.DisplayName
15+
import org.junit.jupiter.api.Tag
16+
import org.junit.jupiter.api.Test
17+
import org.junit.jupiter.api.Timeout
18+
import org.koin.test.inject
19+
import java.util.concurrent.TimeUnit
20+
21+
/**
22+
* Locks in the sensor-port contract that keeps the forward-reservation graph search
23+
* off the per-tick hot path.
24+
*
25+
* [ShuntingLoop.toBlockInputObservation] resolves [BlockInputObservation.toSeparatorName]
26+
* via [PathReservationService.findNextReservationTarget][cz.vutbr.fit.interlockSim.context.navigation.PathReservationService.findNextReservationTarget],
27+
* which is a BFS plus a per-candidate topological-path enumeration. Running it for every
28+
* block input on every tick — including inputs that provably cannot take a forward
29+
* reservation — accounted for ~9% of `fast-sim example shuntingLoop 300` wall time.
30+
*
31+
* The contract: `toSeparatorName` is populated **only** for inputs that could actually
32+
* yield a [DispatchDecision.ReservePath] — a train occupying the block and approaching this
33+
* input, or a path already set up toward it — and only when the path is not already extended
34+
* beyond it. For every other input it is `null`.
35+
*
36+
* [RuleBasedDispatcher.checkInput] returns `null` for exactly those non-eligible cases, so
37+
* narrowing the contract is behaviour-preserving; the golden `shuntingLoop` output is
38+
* byte-for-byte unchanged.
39+
*
40+
* Reverting the gate (computing `toSeparatorName` unconditionally) makes this test fail:
41+
* FREE inputs with clear track ahead resolve to a non-null target.
42+
*/
43+
@DisplayName("ShuntingLoop forward-reservation-target laziness contract")
44+
@Tag("integration-test")
45+
class ShuntingLoopReservationTargetLazinessTest : KoinTestBase() {
46+
private val editingContextFactory: JvmEditingContextFactory by inject()
47+
private val simulationContextFactory: SimulationContextFactory by inject()
48+
49+
private fun loadVyhybnaContext(): DefaultSimulationContext =
50+
TestFixtures.loadShuntingXml().use { xmlStream ->
51+
val editingContext = editingContextFactory.createContext(xmlStream) as EditingContext
52+
simulationContextFactory.createContext(editingContext) as DefaultSimulationContext
53+
}
54+
55+
@Test
56+
@Timeout(value = 60, unit = TimeUnit.SECONDS)
57+
fun `toSeparatorName is null for every input that cannot take a forward reservation`() {
58+
val context = loadVyhybnaContext()
59+
context.getInOuts()
60+
61+
val loop = ShuntingLoop(context, endTime = 120L)
62+
wireSynchronousDispatcher(context, loop)
63+
64+
// Observations are published before the control-step listener fires, so reading them
65+
// from a listener installed *after* the wiring sees the current tick's data.
66+
val wired = loop.controlStepListener
67+
var inputsSeen = 0
68+
var freeInputsSeen = 0
69+
var eligibleInputsSeen = 0
70+
val violations = mutableListOf<String>()
71+
72+
loop.controlStepListener =
73+
ControlStepListener {
74+
val inputs = loop.getInnerBlockInputs() + loop.getOuterBlockInputs()
75+
for (input in inputs) {
76+
inputsSeen++
77+
if (input.state == TrackFacility.State.FREE) freeInputsSeen++
78+
79+
val canReserveForward =
80+
!input.pathAlreadyExtendedBeyond &&
81+
(input.isApproachingThisInput || input.pathSetUpTowardThisInput)
82+
if (canReserveForward) {
83+
eligibleInputsSeen++
84+
} else if (input.toSeparatorName != null) {
85+
violations +=
86+
"block=${input.blockId} toward=${input.towardSemaphoreName} state=${input.state} " +
87+
"approaching=${input.isApproachingThisInput} " +
88+
"setUpToward=${input.pathSetUpTowardThisInput} " +
89+
"extendedBeyond=${input.pathAlreadyExtendedBeyond} " +
90+
"toSeparatorName=${input.toSeparatorName}"
91+
}
92+
}
93+
wired?.onControlStep()
94+
}
95+
96+
context.setMainProcess(loop)
97+
context.run()
98+
99+
// The run must have exercised the interesting states, otherwise the assertion is vacuous.
100+
assertThat(inputsSeen, "block inputs observed").isGreaterThan(0)
101+
assertThat(freeInputsSeen, "FREE inputs observed").isGreaterThan(0)
102+
assertThat(eligibleInputsSeen, "reservation-eligible inputs observed").isGreaterThan(0)
103+
104+
assertThat(violations.size, "non-eligible inputs carrying a toSeparatorName: $violations").isEqualTo(0)
105+
}
106+
107+
@Test
108+
@Timeout(value = 60, unit = TimeUnit.SECONDS)
109+
fun `a FREE block input never carries a forward-reservation target`() {
110+
val context = loadVyhybnaContext()
111+
context.getInOuts()
112+
113+
val loop = ShuntingLoop(context, endTime = 120L)
114+
wireSynchronousDispatcher(context, loop)
115+
val wired = loop.controlStepListener
116+
117+
val freeWithTarget = mutableListOf<String>()
118+
loop.controlStepListener =
119+
ControlStepListener {
120+
(loop.getInnerBlockInputs() + loop.getOuterBlockInputs())
121+
.filter { it.state == TrackFacility.State.FREE }
122+
.forEach { input ->
123+
if (input.toSeparatorName != null) {
124+
freeWithTarget += "${input.blockId}->${input.towardSemaphoreName}=${input.toSeparatorName}"
125+
}
126+
}
127+
wired?.onControlStep()
128+
}
129+
130+
context.setMainProcess(loop)
131+
context.run()
132+
133+
assertThat(freeWithTarget.firstOrNull(), "FREE input with a forward-reservation target").isNull()
134+
}
135+
}

0 commit comments

Comments
 (0)