Swap fetching jobs between units when both trips get shorter - #197
Swap fetching jobs between units when both trips get shorter#197Giszmo wants to merge 4 commits into
Conversation
711cd2f to
3613f93
Compare
|
My disposition is similar to #193: this is an interesting concept, and the reported delivery improvements are encouraging, but I want stronger evidence that it works before committing to merge. Please put reproducible before/after comparisons on the PR, ideally side-by-side examples showing workers changing assignments and the resulting deliveries/travel/congestion. Include representative ordinary and large games, multiple seeds, and neutral or negative results. Separate the benefit of swaps from the underlying #193 changes. If you already have more evidence privately, please share the fixtures, commands and results here. I am especially concerned about the reported additional 13–18% reduction in large-game simulation throughput. That is a substantial cost on top of #193. Can we make the assignment search more efficient while retaining most of the benefit? The current full-team scan after every hire, plus the rolling scans, seems like the main place to investigate. Some alternatives worth profiling rather than assuming:
I do not require that any particular approach above be used. I would like profiles identifying the actual cost, and a measured tradeoff between search effort, simulation throughput and gameplay benefit, against an updated baseline incorporating master's pathfinding optimizations. Dedicated regression coverage is also needed: valid working-list/assignment bookkeeping after swaps, carried-resource compatibility, hunger/qualification checks, repeated swaps or oscillation, and deterministic behavior including save/load. The existing unchanged test suite and one repeated-seed run do not establish those properties. One clarification for the presentation: the implementation requires the combined journey to improve; it does not require both individual trips to get shorter, as the title suggests. Please make that explicit. Interested in pursuing this, but not approving it for merge yet. |
|
A more concrete performance recommendation after tracing the implementation at 3613f93: First, make speculative scoring genuinely read-only. The current jobCost precheck only verifies that b->globalGradient[swimClass] is non-null. buildingAvailable then calls buildingGradient, which can rebuild a dirty field once DIRTY_REBUILD_TICKS has elapsed. resourceAvailable calls getGradient -> getResourceGradient, which allocates and propagates a missing resource field. roundTripDistance also calls resourceAvailable. Thus a candidate comparison can trigger a whole-map calculation despite the current comment. These calls also update some gradient-use timestamps, potentially retaining fields because of speculative comparisons. I suggest explicit cached-distance probe helpers that never allocate, refresh, or change cache-use timestamps. Preserve reachability/locked checks and the existing building-neighbor probe semantics; skip/defer a candidate when a required field is missing or invalid under the chosen freshness policy. Ordinary navigation remains responsible for constructing fields. This can change swap availability/timing, so measure it as a behavioral variant, not a byte-identical optimization. First instrument rebuilds, allocations and time attributable to scoring to establish how much this actually costs. Then put all swap searches under one deterministic work budget. Remove the synchronous exhaustive search from the hire path. Enqueue the newly hired unit once, and share a fixed candidate-inspection budget with the periodic search. Try 8/16/32 candidate inspections per team per tick as experimental settings; these are sweep points, not a claim that one is optimal. Use simulation counters, never a wall-clock timeout, because this is a lockstep engine. A simple initial implementation can retain a stable unit-slot cursor and bound every inspected slot, including ineligible slots. If sparse slots dominate, build a stable eligible-unit list once per team per tick (or maintain one with measured overhead), then inspect candidates from that list. Do not rebuild that list for every hire. Keep periodic work from starving behind the hire queue and coalesce duplicate queue entries. An exhaustive search can be spread across ticks, but positions/jobs may change while it runs. Store unit identities safely, not unvalidated long-lived pointers, and recompute both current jobs, all eligibility checks and the gain immediately before a swap. A best candidate collected across ticks is not guaranteed to remain the instantaneous exhaustive optimum. A bounded batch that chooses its best valid partner is simpler; either way this deliberately trades some search quality/latency for bounded work. Queue/cursor state that influences future swaps also needs deterministic save/load handling. For scale, today an eligible search visits 1,024 slots and evaluates up to three additional job costs per compatible teammate. The four periodic calls do not necessarily all search (empty/ineligible initiators return early), so 4,096 slot visits per team per tick is an upper bound, not the actual workload. Every eligible hire adds another full scan without a shared cap. This explains why reducing only periodic checks need not help the measured hire-dominated case. I would benchmark incremental variants: current swaps; read-only scoring; then read-only scoring plus the shared budget. Report total simulation CPU/throughput, worst tick times, comparisons, scoring-induced gradient builds, queue delay, and deliveries/travel against both no-swaps and exhaustive-swaps baselines. A reasonable proposed acceptance target is around <=1–2% total simulation overhead on representative large games while retaining most of the delivery benefit, with uncertainty reported. That is a target to demonstrate, not a speedup established by this analysis. My preference is to start with these two changes before introducing a spatial index: they directly address unbounded work and unintended full-map computations, and avoid assuming that nearby workers are necessarily the best partners on obstacle-heavy maps. |
687b0c6 to
78692f5
Compare
6bb00b0 to
b5e5986
Compare
|
Rebased onto the reduced #193 (round trip only, on current master); the three swap commits applied with one trivial SConscript conflict. 172 unit tests, all harnesses and the resource-fetch-target regression pass. Next here, per the review above: read-only cached-distance probes for scoring (no allocation, refresh or timestamp updates during a comparison), one deterministic per-team inspection budget shared by hires and the periodic sweep, the bookkeeping/oscillation/save-load tests, and an evidence package against the current master baseline. Not asking for review until those are in. |
931367a to
6d75cc9
Compare
3c530b5 to
f2134a1
Compare
targetX/Y for a fetch task (also the debug path line, hotkey T) are set once, by ascending the plain resource gradient, when the task starts. pathfindResource() re-reads whichever gradient actually governs the per-tick step -- the round-trip field when attachedBuilding has one and it is valid here, the plain resource gradient otherwise -- and either field can be rebuilt, or the preference between them can flip, while the unit is still walking. The stored target never tracked either change, so the debug line and the range checks that read targetX/Y could point at the nearest tile to the unit while the unit actually walked toward the cheaper round trip through a farther one. Add Map::isGradientPeak: whether a tile is a local maximum of a gradient, true for anywhere getGlobalGradientDestination's ascent could end, including a round-trip field whose seeded goal is a finite cost rather than the type's max the way GRADIENT_AT_GOAL is (so its own 'reached exact goal' check does not generalize). handleMovementGoingToResource now resolves the same gradient pathfindResource prefers and re-ascends from the unit's position whenever the stored target stops being a peak of it -- a cheap check every action, the ascent itself only when stale. New ResourceFetchTargetHarness (test/, wired as the resource-fetch-target-test scons alias) seeds a tile close to the unit but far from the building and one far from the unit but close to it, confirms the target follows the cheaper round trip rather than the nearer tile, and that it refreshes once that tile is harvested away. Fails without the fix. Full test/TestsRunner suite (169 tests) passes; scons release=1 and release=1 server=1 build clean. (cherry picked from commit 6bb00b0) Fable 5.1 helped authoring this commit.
A unit that becomes free takes the job of whichever building asks first, however far away, and units never trade jobs. Team::swapTask compares a fetcher's job with every team mate's: a job costs the round trip for an empty-handed unit or the delivery distance for one carrying that resource, and when the two trips together shrink by more than four tiles the units exchange building, resource and target and keep walking from where they are. It runs for a unit right after it is hired and, four units a tick, over every fetcher every 256 ticks. (cherry picked from commit 4ba5c91) Fable 5.1 helped authoring this commit.
Requiring the same swim class was only a proxy for what matters: that every cost in the comparison is read from a gradient some unit already keeps alive, so a comparison never builds one. Check that directly, so a swimmer and a walker swap whenever both fields are there. (cherry picked from commit 3613f93) Fable 5.1 helped authoring this commit.
A hungry unit books the nearest inn with a free meal, and the booking takes the meal: the next unit, standing closer to that inn, finds it full and walks to the next one, crossing the first on the way. Two units heading for the wrong inns each is the food side of the crossing that swapTask fixes for fetching. Team::swapInn runs the moment a unit books its place. It compares the unit's walk with every team mate still walking to an inn of this team, reading only the building gradients that choosing an inn already built (or the crow-flight distance for a flyer), and when trading inns shortens the two walks together by more than four tiles it moves both bookings: guest lists, attached and target building. Each inn keeps its head count, so the meals stay reserved. A trade that would put an inn beyond what a unit can still walk is not made. Inn bookings are rare next to hiring, so one scan per booking is cheap. InnSwapHarness covers the crossing and a booking that is already the shorter one, and walks both units into their inns afterwards. Fable 5.1 helped authoring this commit.
f2134a1 to
1470aa4
Compare
|
Petri benchmark delta of this PR over its base #244 (full tables and setup in #244 (comment)): 20 seeds, 15000 ticks, four AINone colonies on Leo's petri map.
Petri2 (team 2's inns moved further apart, 80 game minutes, 10 seeds; chart in the #244 comment): team 2 keeps 26.9 workers alive against 22.4 on #244 and 22.2 on master, the whole difference being the inn swap at booking time; team 0 76.3 against 73.8 / 65.8. Deliveries over the 80 minutes: team 0 2150 vs 2057, team 2 783 vs 673, team 3 536 vs 528. On the school island all 8 workers are trained by minute 12 (16 to 17 on #244 and master); the colony goes to school as a whole between minute 11 and 13 and deliveries pause for those two minutes. |
|
@genixpro please reconsider. I now provide solid data with AINone. The AINicowar games distorted all experiments. |

On top of #244 (round-trip gradients; formerly #193). Rebased onto the current round-trip branch, four linear commits, no merge commits. The fetch-target harness now carries master's seven-class stale-target cases and the round-trip case.
Inn swaps (new, commit 4). A hungry unit books the nearest inn with a free meal and the booking takes the meal, so the next unit, standing closer to that inn, finds it full and walks to the next one, crossing the first on the way.
Team::swapInnruns the moment a unit books its place: it compares the unit's walk with every team mate still walking to one of the team's inns, reading only the building gradients that choosing an inn already built (crow-flight distance for flyers), and when trading inns shortens the two walks together by more than four tiles it moves both bookings (guest lists, attached and target building). Each inn keeps its head count, so the meals stay reserved; a trade that would put an inn beyond what a unit can still walk is not made. Inn bookings are rare next to hiring, so one scan per booking is cheap.InnSwapHarness(in CI) covers the crossing and a booking that is already the shorter one, and walks both units into their inns afterwards.Task swaps. A unit that becomes free takes the job of whichever building asks first, however far away, and units never trade jobs, so with no unemployment Alice walks north for wheat while Bob starts south for algae from right next to her wheat field.
Team::swapTaskcompares a fetcher's job with every team mate's. A job is (building, resource) and costs the round trip for an empty-handed unit or the delivery distance for a unit carrying that resource (a carrier cannot take a job for another resource, jobs served through a market are left alone). When the two trips together shrink by more than four tiles the two units exchange building, resource and target and keep walking from where they are. Both units must qualify for the other's building (canUnitWorkHere) and have the hunger range for the new trip. Every cost is read from gradients that already exist (the building's gradient in the unit's swim class, its round-trip field if there is one, else the plain distances hiring uses); a comparison never builds a gradient, and a swimmer and a walker swap whenever both fields are there. It runs for a unit right after it is hired and, four units a tick (every fetcher every 256 ticks), over every fetcher.Measured (same setup as #193; swaps = this branch, lanes = #193's head; 10 game minutes, 5 seeds on the small maps, 2 on 512²):
Variants tried: same swim class only (fewer swaps, Mazury 74.4, balanced_for_2 93.7, 512² 0.89 of #193's tick rate); sweep thinned to one unit a tick (Mazury 74.4, balanced_for_2 90.0, no tick-rate gain, the per-hire scans dominate). The 512² cost is the scan over the team's units at every hire with 8 teams of 90 units.
Deterministic (same seed twice, identical timelines and counters); unit tests unchanged (169 pass);
server=1unaffected (the server build does not compile team or building step code).Known limits: O(units) per hire and per swept unit, which is the 13–18% on 512² with 8 teams; a swap only pairs two jobs, it never re-targets a single unit; the 4-tile threshold is a first value.