Skip to content

Commit

Permalink
add performance note
Browse files Browse the repository at this point in the history
  • Loading branch information
kostmo committed Mar 12, 2024
1 parent 2511863 commit 219727e
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 1 deletion.
7 changes: 6 additions & 1 deletion src/swarm-engine/Swarm/Game/State/Robot.hs
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,12 @@ wakeWatchingRobots myID currentTick loc = do
newInsertions = M.filter (not . null) $ M.fromList wakeTimeGroups

-- Contract: This must be emptied immediately
-- in 'iterateRobots'
-- in 'iterateRobots'.
--
-- Tracking the wakeups due on the current tick separately from other
-- waiting robots is a performance optimization (see #1736);
-- it avoids an O(log N) 'Map' lookup on 'internalWaitingRobots' in favor
-- of an O(1) 'null' check in the common case.
currentTickWakeableBots .= currTickWakeable

-- NOTE: There are two "sources of truth" for the waiting state of robots:
Expand Down
31 changes: 31 additions & 0 deletions src/swarm-engine/Swarm/Game/Step.hs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,37 @@ runRobotIDs maybeStopBeforeRID robotNames = do

-- | This function must be utilized in every loop that iterates over
-- robots and invokes 'stepRobot' (i.e. steps a robot's CESK machine).
--
-- = Performance notes
--
-- == Guarding the 'Map' lookup in 'wakeUpRobotsDoneSleeping'
--
-- We use the extra 'currentTickWakeableBots' collection
-- to supplement the 'internalWaitingRobots' Map
-- so that we can avoid a 'Map' lookup in the common case.
--
-- See comment in the body of 'wakeWatchingRobots'.
--
-- == Avoiding a linear 'union' operation?
--
-- Note that although this:
-- @
-- IS.union somePopulatedSet usuallyEmptySet
-- @
--
-- produces the same output as this:
-- @
-- if null usuallyEmptySet
-- then somePopulatedSet
-- else IS.union somePopulatedSet usuallyEmptySet
-- @
--
-- It may be the case that the shorter code would end up doing an O(N) operation every time,
-- rather than only when the "usuallyEmptySet" is empty.
-- Therefore, aside from the primary motivation of 'currentTickWakeableBots' described above,
-- it is also useful for us to "guard" the 'union' operation with an O(1) 'null' check.
--
-- TODO: Actually verify this claim with benchmarks?
getExtraRunnableRobots :: Has (State GameState) sig m => TickNumber -> m (IS.IntSet -> IS.IntSet)
getExtraRunnableRobots time = do
-- NOTE: We could use 'IS.split thisRobotId activeRIDsThisTick'
Expand Down

0 comments on commit 219727e

Please sign in to comment.