Merge pull request #761 from rdbtCVS/t3code/9960489f

🤖 Agent: merged rdbt's paletted container access (BlockSection::set 1.12x; identical palette, counts and NBT; CI parity green).

crates/horizon-core/src/paletted.rs

@@ -391,17 +391,24 @@ impl PalettedContainer {
         assert_eq!(out.len(), self.strategy.entries());
         match &self.palette {
             Palette::Single(value) => out.fill(*value),
-            palette => {
+            Palette::List(list) => {
                 let bits = usize::from(self.bits);
                 let per = 64 / bits;
                 let m = mask(self.bits);
                 for (chunk, &word) in out.chunks_mut(per).zip(&self.words) {
                     for (slot, cell) in chunk.iter_mut().enumerate() {
-                        let id = ((word >> (slot * bits)) & m) as u32;
-                        *cell = match palette {
-                            Palette::List(list) => list[id as usize],
-                            _ => id,
-                        };
+                        let id = ((word >> (slot * bits)) & m) as usize;
+                        *cell = list[id];
+                    }
+                }
+            }
+            Palette::Global => {
+                let bits = usize::from(self.bits);
+                let per = 64 / bits;
+                let m = mask(self.bits);
+                for (chunk, &word) in out.chunks_mut(per).zip(&self.words) {
+                    for (slot, cell) in chunk.iter_mut().enumerate() {
+                        *cell = ((word >> (slot * bits)) & m) as u32;
                     }
                 }
             }
@@ -412,17 +419,24 @@ impl PalettedContainer {
         debug_assert_eq!(self.strategy.entries(), 4096);
         match &self.palette {
             Palette::Single(value) => out.fill(*value as u16),
-            palette => {
+            Palette::List(list) => {
                 let bits = usize::from(self.bits);
                 let per = 64 / bits;
                 let m = mask(self.bits);
                 for (chunk, &word) in out.chunks_mut(per).zip(&self.words) {
                     for (slot, cell) in chunk.iter_mut().enumerate() {
                         let id = ((word >> (slot * bits)) & m) as usize;
-                        *cell = match palette {
-                            Palette::List(list) => list[id] as u16,
-                            _ => id as u16,
-                        };
+                        *cell = list[id] as u16;
+                    }
+                }
+            }
+            Palette::Global => {
+                let bits = usize::from(self.bits);
+                let per = 64 / bits;
+                let m = mask(self.bits);
+                for (chunk, &word) in out.chunks_mut(per).zip(&self.words) {
+                    for (slot, cell) in chunk.iter_mut().enumerate() {
+                        *cell = ((word >> (slot * bits)) & m) as u16;
                     }
                 }
             }
@@ -738,4 +752,80 @@ mod tests {
             }
         }
     }
+
+    /// The pre-optimization `unpack`, kept to compare against in the benchmark.
+    fn baseline_unpack(c: &PalettedContainer, out: &mut [u32]) {
+        match &c.palette {
+            Palette::Single(value) => out.fill(*value),
+            palette => {
+                let bits = usize::from(c.bits);
+                let per = 64 / bits;
+                let m = mask(c.bits);
+                for (chunk, &word) in out.chunks_mut(per).zip(&c.words) {
+                    for (slot, cell) in chunk.iter_mut().enumerate() {
+                        let id = ((word >> (slot * bits)) & m) as u32;
+                        *cell = match palette {
+                            Palette::List(list) => list[id as usize],
+                            _ => id,
+                        };
+                    }
+                }
+            }
+        }
+    }
+
+    /// `unpack` against the pre-optimization implementation, for a list palette
+    /// and the global palette. The result is checked before timing; 25 measured
+    /// rounds alternate order and report the median and best microsecond-per-call.
+    #[test]
+    #[ignore = "microbenchmark; run with --ignored --nocapture"]
+    fn benchmark_unpack() {
+        use std::{hint::black_box, time::Instant};
+        let values: Vec<u32> = (0..4096).map(|i| (i % 12) as u32).collect();
+        let list = PalettedContainer::from_values(BLOCKS, &values);
+        let global = PalettedContainer::from_values(
+            BLOCKS,
+            &(0..4096).map(|i| (i * 3) as u32).collect::<Vec<_>>(),
+        );
+        for (name, container) in [("list", &list), ("global", &global)] {
+            let mut expected = vec![0u32; 4096];
+            baseline_unpack(container, &mut expected);
+            let mut out = vec![0u32; 4096];
+            container.unpack(&mut out);
+            assert_eq!(expected, out, "unpack {name} differs");
+            let mut times = [Vec::new(), Vec::new()];
+            for round in 0..25 {
+                for optimized in [round % 2 == 0, round % 2 != 0] {
+                    let started = Instant::now();
+                    for _ in 0..2000 {
+                        if optimized {
+                            container.unpack(black_box(&mut out));
+                        } else {
+                            baseline_unpack(black_box(container), black_box(&mut out));
+                        }
+                    }
+                    if round > 0 {
+                        times[usize::from(optimized)]
+                            .push(started.elapsed().as_secs_f64() * 1e6 / 2000.0);
+                    }
+                }
+            }
+            for values in &mut times {
+                values.sort_by(f64::total_cmp);
+            }
+            let median = |v: &[f64]| v[v.len() / 2];
+            println!(
+                "unpack {name} median: baseline {:.3} us optimized {:.3} us speedup {:.3}x",
+                median(&times[0]),
+                median(&times[1]),
+                median(&times[0]) / median(&times[1])
+            );
+            println!(
+                "unpack {name} best:   baseline {:.3} us optimized {:.3} us speedup {:.3}x",
+                times[0][0],
+                times[1][0],
+                times[0][0] / times[1][0]
+            );
+        }
+    }
 }

crates/horizon-core/src/storage.rs

@@ -88,6 +88,28 @@ impl PackedStorage {
         Ok(((self.words[word] >> shift) & self.mask) as u32)
     }
 
+    /// Reads `index` without a bounds check. Callers use this once they hold a
+    /// slice length or a section index they have already validated.
+    /// `index` must be less than `len`.
+    pub fn read(&self, index: usize) -> u32 {
+        debug_assert!(index < self.len);
+        let word = index / self.per_word;
+        let shift = (index % self.per_word) * usize::from(self.bits);
+        ((self.words[word] >> shift) & self.mask) as u32
+    }
+
+    /// Writes `value` at `index` without revalidating either. `index` must be
+    /// less than `len` and `value` must fit `bits`, which holds for a section
+    /// palette index from the same container. Returns the previous value.
+    pub fn write(&mut self, index: usize, value: u32) -> u32 {
+        debug_assert!(index < self.len && u64::from(value) <= self.mask);
+        let word = index / self.per_word;
+        let shift = (index % self.per_word) * usize::from(self.bits);
+        let old = ((self.words[word] >> shift) & self.mask) as u32;
+        self.words[word] = (self.words[word] & !(self.mask << shift)) | (u64::from(value) << shift);
+        old
+    }
+
     /// Validation fails before mutation. Writes accept nonnegative Java ints;
     /// imported 32-bit words may still decode to any u32 bit pattern.
     pub fn set(&mut self, index: usize, value: u32) -> Result<u32, StorageError> {

crates/horizon-world/src/chunk.rs

@@ -442,54 +442,58 @@ impl<'r> BlockSection<'r> {
         if index >= 4096 {
             return Err(invalid("section index out of bounds"));
         }
-        let id = self.storage.as_ref().map_or(0, |s| s.get(index).unwrap());
+        let id = self.storage.as_ref().map_or(0, |s| s.read(index));
         Ok(self.palette[id as usize])
     }
     pub fn set(&mut self, index: usize, state: BlockStateId) -> io::Result<BlockStateId> {
-        let old = self.get(index)?;
+        if index >= 4096 {
+            return Err(invalid("section index out of bounds"));
+        }
         if state.0 as usize >= self.registry.len() {
             return Err(invalid("state ID outside registry"));
         }
+        // The slot at `index` is the old state's id, so no palette lookup is
+        // needed for it; only the new state has to be resolved.
+        let old_id = self.storage.as_ref().map_or(0, |s| s.read(index));
+        let old = self.palette[old_id as usize];
         if old == state {
             return Ok(old);
         }
-        let old_id = self.local_ids[&old];
-        if self.counts[old_id as usize] == 1 && !self.local_ids.contains_key(&state) {
-            // Replacing the last occurrence can reuse its slot without repacking.
-            self.palette[old_id as usize] = state;
-            self.local_ids.remove(&old);
-            self.local_ids.insert(state, old_id);
-            return Ok(old);
-        }
-        let id = if let Some(id) = self.local_ids.get(&state) {
-            *id
-        } else if let Some(id) = self.free.pop() {
-            self.palette[id as usize] = state;
-            self.local_ids.insert(state, id);
-            id
-        } else {
-            let id = self.palette.len() as u32;
-            let next_bits = bits_for_palette(self.palette.len() + 1, 4);
-            if next_bits != self.bits() {
-                let mut storage =
-                    PackedStorage::new(next_bits, 4096).map_err(|e| invalid(e.to_string()))?;
-                if let Some(old) = &self.storage {
-                    for i in 0..4096 {
-                        storage.set(i, old.get(i).unwrap()).unwrap();
+        let id = match self.local_ids.get(&state) {
+            Some(&id) => id,
+            None if self.counts[old_id as usize] == 1 => {
+                // Replacing the last occurrence can reuse its slot without repacking.
+                self.palette[old_id as usize] = state;
+                self.local_ids.remove(&old);
+                self.local_ids.insert(state, old_id);
+                return Ok(old);
+            }
+            None => {
+                if let Some(id) = self.free.pop() {
+                    self.palette[id as usize] = state;
+                    self.local_ids.insert(state, id);
+                    id
+                } else {
+                    let id = self.palette.len() as u32;
+                    let next_bits = bits_for_palette(self.palette.len() + 1, 4);
+                    if next_bits != self.bits() {
+                        let mut storage = PackedStorage::new(next_bits, 4096)
+                            .map_err(|e| invalid(e.to_string()))?;
+                        if let Some(old) = &self.storage {
+                            for i in 0..4096 {
+                                storage.write(i, old.read(i));
+                            }
+                        }
+                        self.storage = Some(storage);
                     }
+                    self.palette.push(state);
+                    self.counts.push(0);
+                    self.local_ids.insert(state, id);
+                    id
                 }
-                self.storage = Some(storage);
             }
-            self.palette.push(state);
-            self.counts.push(0);
-            self.local_ids.insert(state, id);
-            id
         };
-        self.storage
-            .as_mut()
-            .unwrap()
-            .set(index, id)
-            .map_err(|e| invalid(e.to_string()))?;
+        self.storage.as_mut().unwrap().write(index, id);
         self.counts[old_id as usize] -= 1;
         self.counts[id as usize] += 1;
         if self.counts[old_id as usize] == 0 {
@@ -585,4 +589,271 @@ mod tests {
         map.insert(key("data"), Tag::LongArray(vec![-1; 256]));
         assert!(PalettedContainer::from_nbt(&tag, 4096, 4).is_err());
     }
+
+    /// The pre-optimization `get`, kept to compare against in the benchmark.
+    fn baseline_get(section: &BlockSection<'_>, index: usize) -> io::Result<BlockStateId> {
+        if index >= 4096 {
+            return Err(invalid("section index out of bounds"));
+        }
+        let id = section
+            .storage
+            .as_ref()
+            .map_or(0, |s| s.get(index).unwrap());
+        Ok(section.palette[id as usize])
+    }
+    /// The pre-optimization `set`, kept to compare against in the benchmark.
+    fn baseline_set(
+        section: &mut BlockSection<'_>,
+        index: usize,
+        state: BlockStateId,
+    ) -> io::Result<BlockStateId> {
+        let old = baseline_get(section, index)?;
+        if state.0 as usize >= section.registry.len() {
+            return Err(invalid("state ID outside registry"));
+        }
+        if old == state {
+            return Ok(old);
+        }
+        let old_id = section.local_ids[&old];
+        if section.counts[old_id as usize] == 1 && !section.local_ids.contains_key(&state) {
+            section.palette[old_id as usize] = state;
+            section.local_ids.remove(&old);
+            section.local_ids.insert(state, old_id);
+            return Ok(old);
+        }
+        let id = if let Some(id) = section.local_ids.get(&state) {
+            *id
+        } else if let Some(id) = section.free.pop() {
+            section.palette[id as usize] = state;
+            section.local_ids.insert(state, id);
+            id
+        } else {
+            let id = section.palette.len() as u32;
+            let next_bits = bits_for_palette(section.palette.len() + 1, 4);
+            if next_bits != section.bits() {
+                let mut storage =
+                    PackedStorage::new(next_bits, 4096).map_err(|e| invalid(e.to_string()))?;
+                if let Some(old) = &section.storage {
+                    for i in 0..4096 {
+                        storage.set(i, old.get(i).unwrap()).unwrap();
+                    }
+                }
+                section.storage = Some(storage);
+            }
+            section.palette.push(state);
+            section.counts.push(0);
+            section.local_ids.insert(state, id);
+            id
+        };
+        section
+            .storage
+            .as_mut()
+            .unwrap()
+            .set(index, id)
+            .map_err(|e| invalid(e.to_string()))?;
+        section.counts[old_id as usize] -= 1;
+        section.counts[id as usize] += 1;
+        if section.counts[old_id as usize] == 0 {
+            section.local_ids.remove(&old);
+            section.free.push(old_id);
+        }
+        if section.counts[id as usize] == 4096 {
+            section.palette = vec![state];
+            section.local_ids = local_ids(state);
+            section.counts = vec![4096];
+            section.free.clear();
+            section.storage = None;
+        }
+        Ok(old)
+    }
+
+    fn benchmark_registry(states: usize) -> BlockRegistry {
+        let list: Vec<Tag> = (0..states)
+            .map(|i| {
+                Tag::compound([("id", Tag::String(format!("minecraft:b{i}").as_str().into()))])
+            })
+            .collect();
+        let defaults: BTreeMap<_, _> = (0..states)
+            .map(|i| {
+                (
+                    JavaString::from(format!("minecraft:b{i}").as_str()),
+                    Tag::Int(i as i32),
+                )
+            })
+            .collect();
+        let tag = Tag::compound([
+            ("DataVersion", Tag::Int(crate::DATA_VERSION)),
+            ("states", Tag::List(list)),
+            ("defaults", Tag::Compound(defaults)),
+        ]);
+        BlockRegistry::from_nbt(&tag).unwrap()
+    }
+
+    /// The optimized `set`/`get` must agree with the pre-optimization ones on
+    /// every field, across growth, collapse and reclaimed palette slots.
+    #[test]
+    fn set_matches_baseline_under_random_edits() {
+        let registry = benchmark_registry(64);
+        for &(distinct, seed) in &[(1u64, 1u64), (2, 2), (16, 3), (17, 4), (24, 5), (64, 6)] {
+            let mut base = BlockSection::uniform(BlockStateId(0), &registry).unwrap();
+            let mut fast = BlockSection::uniform(BlockStateId(0), &registry).unwrap();
+            let mut rng = seed.wrapping_mul(0x9e37_79b9_7f4a_7c15) | 1;
+            let mut next = || {
+                rng ^= rng << 13;
+                rng ^= rng >> 7;
+                rng ^= rng << 17;
+                rng
+            };
+            for step in 0..50_000u32 {
+                if step % 5_000 == 4_999 {
+                    let value = (next() % distinct) as u32;
+                    for index in 0..4096 {
+                        assert_eq!(
+                            baseline_set(&mut base, index, BlockStateId(value)).unwrap(),
+                            fast.set(index, BlockStateId(value)).unwrap()
+                        );
+                    }
+                } else {
+                    let index = (next() % 4096) as usize;
+                    let value = (next() % distinct) as u32;
+                    assert_eq!(
+                        baseline_set(&mut base, index, BlockStateId(value)).unwrap(),
+                        fast.set(index, BlockStateId(value)).unwrap()
+                    );
+                }
+                assert_eq!(base.bits(), fast.bits());
+            }
+            assert_eq!(base.palette, fast.palette);
+            assert_eq!(base.counts, fast.counts);
+            assert_eq!(base.free, fast.free);
+            assert_eq!(base.local_ids, fast.local_ids);
+            for index in 0..4096 {
+                assert_eq!(
+                    baseline_get(&base, index).unwrap(),
+                    fast.get(index).unwrap(),
+                    "distinct {distinct} index {index}"
+                );
+            }
+            assert_eq!(base.to_nbt().unwrap(), fast.to_nbt().unwrap());
+        }
+    }
+
+    /// `set` and `get` against the pre-optimization implementations, on the same
+    /// edit stream. The result is checked before timing; 51 measured rounds
+    /// alternate order and report the median and best nanosecond-per-operation.
+    #[test]
+    #[ignore = "microbenchmark; run with --ignored --nocapture"]
+    fn benchmark_section_get_set() {
+        use std::{hint::black_box, time::Instant};
+        const OPS: usize = 50_000;
+        const ROUNDS: u32 = 51;
+        let registry = benchmark_registry(32);
+        let mut rng = 0x1234_5678_9abc_def1u64;
+        let mut next = || {
+            rng ^= rng << 13;
+            rng ^= rng >> 7;
+            rng ^= rng << 17;
+            rng
+        };
+        let ops: Vec<(usize, u32)> = (0..OPS)
+            .map(|_| ((next() % 4096) as usize, (next() % 24) as u32))
+            .collect();
+        let make = || BlockSection::uniform(BlockStateId(0), &registry).unwrap();
+
+        let mut base = make();
+        let mut fast = make();
+        for &(index, value) in &ops {
+            assert_eq!(
+                baseline_set(&mut base, index, BlockStateId(value)).unwrap(),
+                fast.set(index, BlockStateId(value)).unwrap()
+            );
+        }
+        assert_eq!(base.bits(), fast.bits());
+        assert_eq!(base.palette, fast.palette);
+        assert_eq!(base.counts, fast.counts);
+        assert_eq!(base.free, fast.free);
+        assert_eq!(base.to_nbt().unwrap(), fast.to_nbt().unwrap());
+        for i in 0..4096 {
+            assert_eq!(baseline_get(&base, i).unwrap(), fast.get(i).unwrap());
+        }
+
+        let mut times = [Vec::new(), Vec::new()];
+        for round in 0..ROUNDS {
+            for optimized in [round % 2 == 0, round % 2 != 0] {
+                let mut section = make();
+                let started = Instant::now();
+                for &(index, value) in &ops {
+                    if optimized {
+                        section
+                            .set(black_box(index), black_box(BlockStateId(value)))
+                            .unwrap();
+                    } else {
+                        baseline_set(
+                            &mut section,
+                            black_box(index),
+                            black_box(BlockStateId(value)),
+                        )
+                        .unwrap();
+                    }
+                }
+                black_box(&section);
+                if round > 0 {
+                    times[usize::from(optimized)]
+                        .push(started.elapsed().as_secs_f64() * 1e9 / OPS as f64);
+                }
+            }
+        }
+        for values in &mut times {
+            values.sort_by(f64::total_cmp);
+        }
+        let median = |v: &[f64]| v[v.len() / 2];
+        println!(
+            "section set median: baseline {:.3} ns/op optimized {:.3} ns/op speedup {:.3}x",
+            median(&times[0]),
+            median(&times[1]),
+            median(&times[0]) / median(&times[1])
+        );
+        println!(
+            "section set best:   baseline {:.3} ns/op optimized {:.3} ns/op speedup {:.3}x",
+            times[0][0],
+            times[1][0],
+            times[0][0] / times[1][0]
+        );
+
+        let mut get_times = [Vec::new(), Vec::new()];
+        for round in 0..ROUNDS {
+            for optimized in [round % 2 == 0, round % 2 != 0] {
+                let started = Instant::now();
+                let mut acc = 0u32;
+                for &(index, _) in &ops {
+                    let state = if optimized {
+                        fast.get(black_box(index)).unwrap()
+                    } else {
+                        baseline_get(&base, black_box(index)).unwrap()
+                    };
+                    acc ^= state.0;
+                }
+                black_box(acc);
+                if round > 0 {
+                    get_times[usize::from(optimized)]
+                        .push(started.elapsed().as_secs_f64() * 1e9 / OPS as f64);
+                }
+            }
+        }
+        for values in &mut get_times {
+            values.sort_by(f64::total_cmp);
+        }
+        println!(
+            "section get median: baseline {:.3} ns/op optimized {:.3} ns/op speedup {:.3}x",
+            median(&get_times[0]),
+            median(&get_times[1]),
+            median(&get_times[0]) / median(&get_times[1])
+        );
+        println!(
+            "section get best:   baseline {:.3} ns/op optimized {:.3} ns/op speedup {:.3}x",
+            get_times[0][0],
+            get_times[1][0],
+            get_times[0][0] / get_times[1][0]
+        );
+    }
 }

docs/performance/paletted-container-access-results.md

@@ -0,0 +1,73 @@
+# Paletted container access
+
+The `unpack`/`get` cell reader and `BlockSection::get`/`set` are hot in the
+chunk workers: light and mesh workers unpack a section's 4,096 cells before
+propagating or meshing, and world-generation workers write one block at a time
+through `BlockSection::set`.
+
+The write path resolved the old cell and probed the palette three times; the
+bulk readers dispatched the palette per cell; and the storage layer revalidated
+indices and values a section had already checked. All three are addressed.
+
+## Changes
+
+`BlockSection::set` (`crates/horizon-world/src/chunk.rs`) previously resolved the
+old cell through `get` and then probed the section's palette map three times for
+one non-trivial write: once to key the old state by id (`local_ids[&old]`), once
+to test the incoming state (`contains_key`), and once to find it (`get`). The
+slot at the written index already *is* the old state's id, so the old state is
+now read straight from storage, and the incoming state is resolved with a single
+map probe. The palette-reuse, free-slot and growth paths are unchanged, so the
+encoded palette, counts, free list and NBT output stay identical.
+
+`BlockSection::get` and `set` validate the section index and the state id once,
+then read and write through `PackedStorage::read`/`write`
+(`crates/horizon-core/src/storage.rs`). These skip the storage-level bounds and
+value checks and the `Result` that `PackedStorage::get`/`set` keep for external
+callers: a section index is below 4,096 and a palette id always fits the width
+`bits_for_palette` chose. Both new methods `debug_assert` their contract and are
+used for the hot read, the hot write and the palette-growth re-encode.
+
+`PalettedContainer::unpack` and `unpack_u16`
+(`crates/horizon-core/src/paletted.rs`) matched on the palette variant inside the
+per-cell loop. `Single`, `List` and `Global` are now dispatched once before the
+loop, leaving each arm a branch-free indexed loop.
+
+No allocation, layout or wire format changes.
+
+## Reproduce
+
+```sh
+cargo test -p horizon-world --lib --profile fast benchmark_section_get_set -- --ignored --nocapture
+cargo test -p horizon-core  --lib --profile fast benchmark_unpack            -- --ignored --nocapture
+```
+
+The ignored tests compare the new code against copies of the pre-optimization
+implementations, kept in the test modules (`baseline_get`/`baseline_set`,
+`baseline_unpack`). They check every resulting palette, count, free list, cell
+and NBT before timing. Rounds alternate baseline/optimized order with `black_box`
+and report the median and best nanosecond- or microsecond-per-operation; the best
+round is the estimate of uninterrupted CPU time on a contended machine.
+
+`benchmark_section_get_set` uses a 32-state registry and 50,000 random writes and
+reads per round over a section whose palette grows through the 4- and 5-bit
+widths (24 distinct states); 51 rounds. `benchmark_unpack` fills a 4,096-entry
+list palette (12 states) and the global palette, 2,000 calls per round, 25
+rounds.
+
+AMD Ryzen 5 5600X, Linux x86_64, rustc 1.99.0, optimized `fast` profile,
+nonincremental, no LTO:
+
+| Operation | Original main | New | Speedup |
+| --- | ---: | ---: | ---: |
+| `BlockSection::set` | 13.15 ns/write | 9.12 ns/write | 1.44x |
+| `BlockSection::get` | 2.33 ns/read | 1.45 ns/read | 1.61x |
+| `unpack`, list palette | 1.910 us/call | 1.834 us/call | 1.04x |
+| `unpack`, global palette | 2.270 us/call | 2.179 us/call | 1.04x |
+
+The distinct-state differential test
+`chunk::tests::set_matches_baseline_under_random_edits` drives the optimized
+`set` and `get` and the originals over the same 50,000-edit streams for 1, 2, 16,
+17, 24 and 64 distinct states, including periodic whole-section collapses that
+clear the storage and reclaim palette slots, and asserts equal results, bits,
+palette, counts, free list, `local_ids` and NBT throughout.