Merge remote-tracking branch 'origin/main' into t1181-step-canvas
This commit is contained in:
@@ -197,20 +197,58 @@ pub enum GenWorkItem {
|
||||
/// `drain_generation_completions` documents).
|
||||
///
|
||||
/// **TerrainAnalysis availability (T-1137 binding decision, with numbers;
|
||||
/// corrected 2026-07-21 per PR #187 review — Tyre C1):** `BodyWorldState`
|
||||
/// does NOT retain `TerrainAnalysis` after cascade completion (T-1044
|
||||
/// scoped its transient-carry fix to *within-cascade* reuse only —
|
||||
/// `cascade.rs` drops it once `DistrictProfile`+`RoadGraph` finish; see the
|
||||
/// doc on `CascadeSnapshot::terrain_analysis`). Caching it alongside every
|
||||
/// `BodyWorldStateCache` entry would cost ~2 MB × 50-body capacity ≈
|
||||
/// 100 MB of PERMANENT resident cost, paid by every cached body whether or
|
||||
/// not a window is ever requested for it — the exact D-203 budget concern
|
||||
/// T-1044's own ticket text guarded against. So `run_work_item` re-derives
|
||||
/// via `run_layer1` (matching `aliveness_probe`'s existing `--render`
|
||||
/// workaround) rather than persisting a field on `BodyWorldState` — but
|
||||
/// NOT unconditionally on every work item: the actual model is a small
|
||||
/// **per-body LRU** (`TerrainAnalysisCache`, capacity 8, ~16 MB worst
|
||||
/// case), consulted before every re-derive. The FIRST `DeriveWindow` on a
|
||||
/// corrected 2026-07-21 per PR #187 review — Tyre C1; RE-CORRECTED
|
||||
/// 2026-07-25 per PR #200 review — Hoshe finding 1, T-1184's hydrology
|
||||
/// field addition):** `BodyWorldState` does NOT retain `TerrainAnalysis`
|
||||
/// after cascade completion (T-1044 scoped its transient-carry fix to
|
||||
/// *within-cascade* reuse only — `cascade.rs` drops it once
|
||||
/// `DistrictProfile`+`RoadGraph` finish; see the doc on
|
||||
/// `CascadeSnapshot::terrain_analysis`). Caching it alongside every
|
||||
/// `BodyWorldStateCache` entry would cost **~2.62 MB × 50-body capacity ≈
|
||||
/// 131 MB** of PERMANENT resident cost (real per-entry figure below —
|
||||
/// this multiplier is unchanged, only the per-entry base moved), paid by
|
||||
/// every cached body whether or not a window is ever requested for it —
|
||||
/// the exact D-203 budget concern T-1044's own ticket text guarded
|
||||
/// against. So `run_work_item` re-derives via `run_layer1` (matching
|
||||
/// `aliveness_probe`'s existing `--render` workaround) rather than
|
||||
/// persisting a field on `BodyWorldState` — but NOT unconditionally on
|
||||
/// every work item: the actual model is a small **per-body LRU**
|
||||
/// (`TerrainAnalysisCache`, capacity 8), consulted before every
|
||||
/// re-derive.
|
||||
///
|
||||
/// **Real per-entry size (T-1184, corrected from the stale "~2 MB"
|
||||
/// figure the PR #187 pass computed before hydrology existed):** at the
|
||||
/// real `GRID_W × GRID_H = 512 × 256` working grid (131,072 cells, the
|
||||
/// SAME grid every `TerrainAnalysisCache` entry is built at — NOT the
|
||||
/// 1024×512 stored-heightmap-PNG resolution, D-202, which is downsampled
|
||||
/// away before `run_layer1` ever runs) — pre-T-1184 dense fields
|
||||
/// (`ocean_mask`/`lake_mask`: `Vec<bool>`, 1 B/cell each; `water_dist`:
|
||||
/// `Vec<u16>`, 2 B/cell; `slope_deg`/`elev_pct`: `Vec<f32>`, 4 B/cell
|
||||
/// each) sum to **~1.57 MB**. T-1184's `HydrologySample` (`elevation` +
|
||||
/// `filled`, both `Vec<f32>`, 4 B/cell) adds **~1.05 MB** — **not quite a
|
||||
/// doubling** (×1.67, not ×2), but a real, non-trivial per-entry growth:
|
||||
/// **post-T-1184 total ~2.62 MB/entry.** At capacity 8 that is **~21.0 MB
|
||||
/// worst case** (was ~12.6 MB pre-T-1184; the old "~16 MB" comment was
|
||||
/// itself a round-up of the pre-T-1184 figure, not a post-hydrology one).
|
||||
/// Both this cache and `BodyWorldStateCache`'s counterfactual above use
|
||||
/// the corrected ~2.62 MB base.
|
||||
///
|
||||
/// **Clone-on-HIT, not just insert (Hoshe finding 1, binding for the next
|
||||
/// sizing decision):** `get_or_derive`'s cache-hit branch
|
||||
/// (`return (l1.clone(), ta.clone())`) deep-copies the FULL
|
||||
/// `TerrainAnalysis` — including both new `HydrologySample` `Vec<f32>`
|
||||
/// fields — on EVERY hit, not merely on the first miss/insert. A
|
||||
/// pan-heavy session hitting the same body's cache entry repeatedly pays
|
||||
/// the ~2.62 MB clone cost per `DeriveWindow` work item, not once per
|
||||
/// body. This was already true pre-T-1184 for the smaller ~1.57 MB
|
||||
/// struct; T-1184 makes the per-hit clone cost ~1.67× larger, not a new
|
||||
/// category of cost. **Not fixed this round** (an `Arc<TerrainAnalysis>`
|
||||
/// refactor — sharing one heap allocation across hits instead of
|
||||
/// deep-copying — is the obvious next step if hit-heavy clone cost ever
|
||||
/// shows up in a profile, but is out of scope for T-1184; flagged for a
|
||||
/// follow-up ticket rather than done speculatively here).
|
||||
///
|
||||
/// The FIRST `DeriveWindow` on a
|
||||
/// body pays the full ~45 ms `run_layer1` cost and populates that body's
|
||||
/// cache entry; EVERY SUBSEQUENT window on the SAME body (any `center`/`n`,
|
||||
/// not just an exact repeat — that narrower case is what
|
||||
@@ -795,18 +833,30 @@ impl TerrainAnalysisCache {
|
||||
}
|
||||
|
||||
/// Look up a cached `(Layer1Output, TerrainAnalysis)` pair for `body_id`,
|
||||
/// re-deriving via `run_layer1` on a miss and inserting the result
|
||||
/// (evicting the LRU entry first if at capacity). Bumps the access clock
|
||||
/// on both a hit and a fresh insert (both are "this body was just used").
|
||||
/// re-deriving via `run_layer1_with_moisture` on a miss and inserting the
|
||||
/// result (evicting the LRU entry first if at capacity). Bumps the access
|
||||
/// clock on both a hit and a fresh insert (both are "this body was just
|
||||
/// used").
|
||||
///
|
||||
/// Returns both halves of `run_layer1`'s output (T-1170 Ruling 4b) — the
|
||||
/// window derive path (`GenWorkItem::DeriveWindow`) needs `Layer1Output`'s
|
||||
/// `RiverNetwork` to know which river edges exist near the requested
|
||||
/// window, in addition to the `TerrainAnalysis` it always needed.
|
||||
///
|
||||
/// `body_params` (T-1184) is `Option` — `None` when the caller has no DB
|
||||
/// row for this body, matching the same "params absent → fall back"
|
||||
/// posture every other `body_params: Option<&BodyParams>` consumer in this
|
||||
/// module already has (`resolve_settlement_morphology_zone`). Falling
|
||||
/// through to `run_layer1`'s body-agnostic moisture default in that case
|
||||
/// is a body-classification-quality concern (which basins read Endorheic
|
||||
/// vs. Overflow), never a correctness one — lake EXTENT never depends on
|
||||
/// moisture (only the elevation-geometry-gated filled-surface comparison
|
||||
/// does; see `district_profile::derive_morphology_zone`'s lake tier).
|
||||
fn get_or_derive(
|
||||
&mut self,
|
||||
body_id: &str,
|
||||
heightmap: &crate::atlas::heightmap::BodyHeightmap,
|
||||
body_params: Option<&BodyParams>,
|
||||
) -> (Layer1Output, TerrainAnalysis) {
|
||||
self.clock += 1;
|
||||
let now = self.clock;
|
||||
@@ -815,7 +865,13 @@ impl TerrainAnalysisCache {
|
||||
return (l1.clone(), ta.clone());
|
||||
}
|
||||
|
||||
let (l1, ta) = crate::atlas::layer1::run_layer1(heightmap);
|
||||
let (l1, ta) = match body_params {
|
||||
Some(params) => crate::atlas::layer1::run_layer1_with_moisture(
|
||||
heightmap,
|
||||
crate::atlas::district_profile::derive_moisture_ceiling_q(params),
|
||||
),
|
||||
None => crate::atlas::layer1::run_layer1(heightmap),
|
||||
};
|
||||
|
||||
if self.entries.len() >= self.capacity && !self.entries.contains_key(body_id) {
|
||||
if let Some(victim) = self
|
||||
@@ -870,7 +926,7 @@ pub(crate) fn resolve_settlement_morphology_zone(
|
||||
let (_l1, ta) = terrain_cache
|
||||
.lock()
|
||||
.unwrap()
|
||||
.get_or_derive(body_id, heightmap);
|
||||
.get_or_derive(body_id, heightmap, Some(params));
|
||||
let climate = ClimateConstants::default();
|
||||
let profile = crate::atlas::district_profile::derive_at_metres(
|
||||
body_seed,
|
||||
@@ -1072,10 +1128,11 @@ fn run_work_item(
|
||||
// river edges exist near this window (T-1170 A2) without a
|
||||
// second drainage pass — the fix for the former
|
||||
// `let (_, ta) = run_layer1(...)` discard (Ruling 4b).
|
||||
let (l1, ta) = terrain_cache
|
||||
.lock()
|
||||
.unwrap()
|
||||
.get_or_derive(body_id, &working);
|
||||
let (l1, ta) = terrain_cache.lock().unwrap().get_or_derive(
|
||||
body_id,
|
||||
&working,
|
||||
Some(body_params),
|
||||
);
|
||||
let climate = ClimateConstants::default();
|
||||
let layer = build_district_window_layer(
|
||||
*body_seed,
|
||||
@@ -1815,11 +1872,11 @@ mod tests {
|
||||
let hm = window_test_hm();
|
||||
|
||||
assert!(!cache.contains("BodyA"));
|
||||
let (l1_first, ta_first) = cache.get_or_derive("BodyA", &hm);
|
||||
let (l1_first, ta_first) = cache.get_or_derive("BodyA", &hm, None);
|
||||
assert_eq!(cache.len(), 1);
|
||||
assert!(cache.contains("BodyA"));
|
||||
|
||||
let (l1_second, ta_second) = cache.get_or_derive("BodyA", &hm);
|
||||
let (l1_second, ta_second) = cache.get_or_derive("BodyA", &hm, None);
|
||||
assert_eq!(
|
||||
cache.len(),
|
||||
1,
|
||||
@@ -1849,16 +1906,16 @@ mod tests {
|
||||
let mut cache = TerrainAnalysisCache::new(2);
|
||||
let hm = window_test_hm();
|
||||
|
||||
cache.get_or_derive("BodyA", &hm);
|
||||
cache.get_or_derive("BodyB", &hm);
|
||||
cache.get_or_derive("BodyA", &hm, None);
|
||||
cache.get_or_derive("BodyB", &hm, None);
|
||||
assert_eq!(cache.len(), 2);
|
||||
|
||||
// Touch BodyA again — it is now the MOST recently used, so BodyB
|
||||
// (untouched since its own insert) is the true LRU victim.
|
||||
cache.get_or_derive("BodyA", &hm);
|
||||
cache.get_or_derive("BodyA", &hm, None);
|
||||
|
||||
// Insert a third body — capacity 2 forces an eviction.
|
||||
cache.get_or_derive("BodyC", &hm);
|
||||
cache.get_or_derive("BodyC", &hm, None);
|
||||
assert_eq!(cache.len(), 2);
|
||||
assert!(
|
||||
cache.contains("BodyA"),
|
||||
|
||||
Reference in New Issue
Block a user