react immediately to wakeups

[kostmo]

Fixes #1598

Demo

Illustrates immediate (same-tick) reaction to a change of a watched cell:

scripts/play.sh -i data/scenarios/Testing/1598-detect-entity-change.yaml --autoplay --speed 1

Background

Robots can be scheduled to wakeup from a watch by the wakeWatchingRobots function.
Robots may be actually awakened by the wakeUpRobotsDoneSleeping function.

Previously, wakeWatchingRobots would only ever schedule wakeups at currentTick + 1. Then, once the next tick is reached, in wakeUpRobotsDoneSleeping only robots that had been scheduled to wake on precisely the current tick could actually be awakened.

But this made it impossible for one robot to cause another robot, which had been sleeping, to actually wake up within the same tick that scheduling of the wakeup is performed.

The importance of this comes into play for system robots that are watching a cell and intended to react instantaneously to player actions (usually by running code in an instant block).

During each "tick", every active robot gets exactly one "turn" to execute code. Given the following ID assignments:

Robot ID	Robot name
0	base
1	systemBot

the systemBot will always execute after the base robot in a given tick.

If the systemBot is watching a given cell, a modification of that cell by the base will schedule a wakeup of the systemBot. If the scheduled wakeup tick is currentTick + 1, then the base will have another execution turn before the systemBot gets an opportunity to react to the base's action at the current tick, causing the systemBot to overlook actions that may be relevant to a goal condition.

Solution

In contast, if we schedule the systemBot wakeup for currentTick instead of currentTick + 1, then it should have an opportunity to wake, run, and react to the base's action before the base's next turn.

But in the status quo, the selection of which robots to run in a given tick is made only once, before iteration over those robots begins. We need instead to update the robots-to-run pool dynamically, after each iteration on an individual robot. The most appropriate data structure for the iteration pool is probably a Monotone priority queue, but we approximate this with an IntSet of RIDs and the minView function.

Being able to alter the list of active robots in the middle of a given tick's robot iteration has required a change to the runRobotIDs function. Instead of using a forM_ to iterate over a static list of RIDs, we have a custom iteration function iterateRobots to handle the dynamic set.

Performance

Benchmarks were performed locally with stack and GHC 9.6.

I had tested the replacement of the forM_ loop with iterateRobots in isolation, and it had zero effect on benchmarks. But some other trivial changes tested in isolation, such as the addition of docstrings, yielded a +6% increase in benchmarks. So there seems to be some instability in the ability of the compiler to perform certain optimizations.

With this PR, increases in benchmark times ranging from 7% to 11% were observed. The obvious culprit is that wakeUpRobotsDoneSleeping is now called N times per tick, rather than once per tick, where N is the number of robots that are initially (or become) active during that tick.

Mitigating with an "awakened" set

In the common case (i.e. when there are no watching robots), the wakeUpRobotsDoneSleeping that is now executed N times per tick (instead of once per tick) incurs a Map lookup, which is O(log M) in the number of distinct future wakeup times across all robots.

However, those extra N invocations only exist to serve the purpose of watching robots that may need to wake up at the current tick. It may be more efficient to have a dedicated Set in the GameState for such robots that gets populated parallel to this insertion:

swarm/src/swarm-engine/Swarm/Game/State/Robot.hs

Line 387 in ad5c589

[ (currentTick, currTickWakeable)

Then if this set remains null, we can avoid paying for an additional O(N * log M) operations entailed by the Map lookups into internalWaitingRobots.

Result

Indeed, storing awakened bots in a new currentTickWakeableBots set restored the benchmarks nearly to the baseline. Instead of the regressions in the 10-20% range observed before, now only a few benchmarks had at most 3-4% increases.

CI regressions

In CI, (snake, powerset, and word-search) exceeded their timeout thresholds in GHC 9.2 and GHC 9.4. No regressions were observed with GHC 9.6. To accommodate the lowest common denominator, I bumped the thresholds for those three scenarios to get a passing build.

I don't believe it is worth further effort or investigation to optimize for GHC 9.4, since such efforts will be moot for GHC 9.6 and onward.

Test invocation:

cabal test swarm-integration --test-show-details streaming --test-options '--color always --pattern "word-search"'

Scenario	GHC 9.4.8	GHC 9.4.8	GHC 9.6.4	GHC 9.6.4
	Before	After	Before	After
snake	1.84s	5.38s	1.62s	1.67s
powerset	1.66s	5.09s	1.56s	1.66s
word-search	0.56s	1.91s	0.44s	0.48s

Potential improvements

TickNumber as Map key

waitingRobots is of type Map TickNumber [RID]. Instead of Map, we could have a newtype that encapsulates a IntMap to make lookups by TickNumber more efficient.

[byorgey]

Currently looking through this. How does it interact with the single-step debugging mode? Do we need to update the singleStep function so that it has corresponding behavior?

[kostmo]

How does it interact with the single-step debugging mode? Do we need to update the singleStep function so that it has corresponding behavior?

Yes, I suppose we would. Will look into this.

[byorgey]

This all looks good to me, with the possible caveat of needing to update singleStep.