From 0f5bfc49718d8aec93bd337fa4e91704995a0c52 Mon Sep 17 00:00:00 2001 From: random-geek <35757396+random-geek@users.noreply.github.com> Date: Mon, 8 Feb 2021 23:51:04 -0800 Subject: [PATCH] MapBlock caching part 2 --- .gitignore | 6 ++- .vscode/settings.json | 5 --- Manual.md | 11 +++--- src/commands/clone.rs | 84 +++++++++++++++++++++++----------------- src/commands/mod.rs | 3 ++ src/commands/overlay.rs | 82 ++++++++++++++++++++++++++------------- src/instance.rs | 4 +- src/map_block/mod.rs | 2 +- src/utils.rs | 44 ++++++--------------- workspace.code-workspace | 7 ---- 10 files changed, 134 insertions(+), 114 deletions(-) delete mode 100644 .vscode/settings.json delete mode 100644 workspace.code-workspace diff --git a/.gitignore b/.gitignore index f0e3bca..6ce87ef 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,6 @@ /target -**/*.rs.bk \ No newline at end of file +**/*.rs.bk + +# VSCode stuff +/.vscode +*.code_workspace diff --git a/.vscode/settings.json b/.vscode/settings.json deleted file mode 100644 index c0cf6a0..0000000 --- a/.vscode/settings.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "cSpell.words": [ - "minetest" - ] -} \ No newline at end of file diff --git a/Manual.md b/Manual.md index e1265af..bb7fd93 100644 --- a/Manual.md +++ b/Manual.md @@ -53,16 +53,17 @@ WorldEdit `//fixlight` command. Usage: `clone --p1 x y z --p2 x y z --offset x y z` -Clone (copy) the given area to a new location. +Clone (copy) a given area to a new location. Arguments: -- `--p1, --p2`: Area to copy from. -- `--offset`: Offset to shift the area by. For example, to copy an area 50 -nodes upward (positive Y direction), use `--offset 0 50 0`. +- `--p1, --p2`: Area to clone. +- `--offset`: Vector to shift the area by. For example, to copy an area 50 +nodes downward (negative Y direction), use `--offset 0 -50 0`. Directions may +be determined using Minetest's F5 debug menu. This command copies nodes, param1, param2, and metadata. Nothing will be copied -into mapblocks that are not yet generated. +from or into mapblocks that are not yet generated. ### deleteblocks diff --git a/src/commands/clone.rs b/src/commands/clone.rs index 201f58d..f6b69f4 100644 --- a/src/commands/clone.rs +++ b/src/commands/clone.rs @@ -1,23 +1,42 @@ -use super::Command; +use super::{Command, BLOCK_CACHE_SIZE}; -use crate::unwrap_or; +use crate::{unwrap_or, opt_unwrap_or}; use crate::spatial::{Vec3, area_rel_block_overlap, area_abs_block_overlap}; -use crate::map_block::{MapBlock, is_valid_generated, NodeMetadataList}; +use crate::map_database::MapDatabase; +use crate::map_block::{MapBlock, MapBlockError, is_valid_generated, + NodeMetadataList}; use crate::block_utils::{merge_blocks, merge_metadata, clean_name_id_map}; use crate::instance::{ArgType, InstBundle}; -use crate::utils::{CacheMap, CachedMapDatabase, query_keys}; +use crate::utils::{CacheMap, query_keys}; use crate::time_keeper::TimeKeeper; -// TODO: This and overlay--cache mapblocks in deserialized form. +type BlockResult = Option>; + +fn get_cached( + db: &mut MapDatabase, + cache: &mut CacheMap, + key: i64 +) -> BlockResult { + match cache.get(&key) { + Some(data) => data.clone(), + None => { + let block = db.get_block(key).ok() + .filter(|d| is_valid_generated(d)) + .map(|d| MapBlock::deserialize(&d)); + cache.insert(key, block.clone()); + block + } + } +} fn clone(inst: &mut InstBundle) { let src_area = inst.args.area.unwrap(); let offset = inst.args.offset.unwrap(); let dst_area = src_area + offset; - let mut keys = query_keys(&mut inst.db, &inst.status, + let mut dst_keys = query_keys(&mut inst.db, &inst.status, &[], Some(dst_area), false, true); // Sort blocks according to offset such that we don't read blocks that @@ -26,25 +45,27 @@ fn clone(inst: &mut InstBundle) { // Subtract one from inverted axes to keep values from overflowing. let sort_offset = sort_dir.map(|v| if v == -1 { -1 } else { 0 }); - keys.sort_unstable_by_key(|k| { + dst_keys.sort_unstable_by_key(|k| { (Vec3::from_block_key(*k) * sort_dir + sort_offset).to_block_key() }); - // let mut db = CachedMapDatabase::new(&mut inst.db, 256); - let mut block_cache = CacheMap::::with_capacity(256); + let mut block_cache = CacheMap::with_capacity(BLOCK_CACHE_SIZE); let mut tk = TimeKeeper::new(); inst.status.begin_editing(); - for dst_key in keys { + for dst_key in dst_keys { inst.status.inc_done(); - let dst_data = inst.db.get_block(dst_key).unwrap(); - if !is_valid_generated(&dst_data) { - continue; - } - let mut dst_block = MapBlock::deserialize(&dst_data).unwrap(); - let mut dst_meta = NodeMetadataList::deserialize( - dst_block.metadata.get_ref()).unwrap(); + let (mut dst_block, mut dst_meta) = unwrap_or!( + opt_unwrap_or!( + get_cached(&mut inst.db, &mut block_cache, dst_key), + continue + ).and_then(|b| + NodeMetadataList::deserialize(b.metadata.get_ref()) + .map(|m| (b, m)) + ), + { inst.status.inc_failed(); continue; } + ); let dst_pos = Vec3::from_block_key(dst_key); let dst_part_abs = area_abs_block_overlap(&dst_area, dst_pos) @@ -57,22 +78,15 @@ fn clone(inst: &mut InstBundle) { continue; } let src_key = src_pos.to_block_key(); - let src_block = if let Some(block) = block_cache.get(&src_key) { - let _t = tk.get_timer("get_block (cached)"); - block.clone() - } else { - let _t = tk.get_timer("get_block (database)"); - let src_data = unwrap_or!(inst.db.get_block(src_key), - continue); - if !is_valid_generated(&src_data) { - continue; - } - let src_block = MapBlock::deserialize(&src_data).unwrap(); - block_cache.insert(src_key, src_block.clone()); - src_block - }; - let src_meta = NodeMetadataList::deserialize( - &src_block.metadata.get_ref()).unwrap(); + let (src_block, src_meta) = opt_unwrap_or!( + get_cached(&mut inst.db, &mut block_cache, src_key) + .map(Result::ok).flatten() + .and_then(|b| + NodeMetadataList::deserialize(b.metadata.get_ref()) + .ok().map(|m| (b, m)) + ), + continue + ); let src_frag_abs = area_abs_block_overlap(&src_part_abs, src_pos) .unwrap(); @@ -112,8 +126,8 @@ pub fn get_command() -> Command { verify_args: None, args: vec![ (ArgType::Area(true), "Area to clone"), - (ArgType::Offset(true), "Vector to shift nodes by") + (ArgType::Offset(true), "Vector to shift the area by") ], - help: "Clone a given area to a new location." + help: "Clone (copy) a given area to a new location." } } diff --git a/src/commands/mod.rs b/src/commands/mod.rs index e8c9ef7..f4f4144 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -16,6 +16,9 @@ mod set_param2; mod vacuum; +pub const BLOCK_CACHE_SIZE: usize = 1024; + + pub struct Command { pub func: fn(&mut InstBundle), pub verify_args: Option anyhow::Result<()>>, diff --git a/src/commands/overlay.rs b/src/commands/overlay.rs index 838f787..dd33392 100644 --- a/src/commands/overlay.rs +++ b/src/commands/overlay.rs @@ -1,11 +1,14 @@ -use super::Command; +use super::{Command, BLOCK_CACHE_SIZE}; +use crate::opt_unwrap_or; use crate::spatial::{Vec3, Area, area_rel_block_overlap, area_abs_block_overlap, area_contains_block, area_touches_block}; use crate::instance::{ArgType, InstArgs, InstBundle}; -use crate::map_block::{MapBlock, NodeMetadataList, is_valid_generated}; +use crate::map_database::MapDatabase; +use crate::map_block::{MapBlock, MapBlockError, NodeMetadataList, + is_valid_generated}; use crate::block_utils::{merge_blocks, merge_metadata, clean_name_id_map}; -use crate::utils::query_keys; +use crate::utils::{query_keys, CacheMap}; fn verify_args(args: &InstArgs) -> anyhow::Result<()> { @@ -97,6 +100,26 @@ fn overlay_no_offset(inst: &mut InstBundle) { } +type BlockResult = Option>; + +fn get_cached( + db: &mut MapDatabase, + cache: &mut CacheMap, + key: i64 +) -> BlockResult { + match cache.get(&key) { + Some(data) => data.clone(), + None => { + let block = db.get_block(key).ok() + .filter(|d| is_valid_generated(d)) + .map(|d| MapBlock::deserialize(&d)); + cache.insert(key, block.clone()); + block + } + } +} + + /// Overlay with offset, with or without area. #[inline] fn overlay_with_offset(inst: &mut InstBundle) { @@ -106,21 +129,27 @@ fn overlay_with_offset(inst: &mut InstBundle) { let idb = inst.idb.as_mut().unwrap(); // Get keys from output database. - let keys = query_keys(&mut inst.db, &inst.status, + let dst_keys = query_keys(&mut inst.db, &inst.status, &[], dst_area, inst.args.invert, true); - inst.status.begin_editing(); - for key in keys { + let mut src_block_cache = CacheMap::with_capacity(BLOCK_CACHE_SIZE); + + inst.status.begin_editing(); + for dst_key in dst_keys { inst.status.inc_done(); - let dst_pos = Vec3::from_block_key(key); - let dst_data = inst.db.get_block(key).unwrap(); - if !is_valid_generated(&dst_data) { - continue; - } - let mut dst_block = MapBlock::deserialize(&dst_data).unwrap(); - let mut dst_meta = NodeMetadataList::deserialize( - dst_block.metadata.get_ref()).unwrap(); + let dst_pos = Vec3::from_block_key(dst_key); + let dst_data = opt_unwrap_or!( + inst.db.get_block(dst_key).ok().filter(|d| is_valid_generated(d)), + continue + ); + let (mut dst_block, mut dst_meta) = opt_unwrap_or!( + MapBlock::deserialize(&dst_data).ok().and_then(|b| + NodeMetadataList::deserialize(b.metadata.get_ref()) + .ok().map(|m| (b, m)) + ), + { inst.status.inc_failed(); continue; } + ); let dst_part_abs = dst_area.map_or( Area::new(dst_pos * 16, dst_pos * 16 + 15), @@ -133,17 +162,16 @@ fn overlay_with_offset(inst: &mut InstBundle) { if !src_pos.is_valid_block_pos() { continue; } - let src_data = match idb.get_block(src_pos.to_block_key()) { - Ok(d) => if is_valid_generated(&d) { - d - } else { - continue - }, - Err(_) => continue - }; - let src_block = MapBlock::deserialize(&src_data).unwrap(); - let src_meta = NodeMetadataList::deserialize( - src_block.metadata.get_ref()).unwrap(); + let src_key = src_pos.to_block_key(); + let (src_block, src_meta) = opt_unwrap_or!( + get_cached(idb, &mut src_block_cache, src_key) + .map(Result::ok).flatten() + .and_then(|b| + NodeMetadataList::deserialize(b.metadata.get_ref()) + .ok().map(|m| (b, m)) + ), + continue + ); let src_frag_abs = area_abs_block_overlap(&src_part_abs, src_pos) .unwrap(); @@ -159,7 +187,7 @@ fn overlay_with_offset(inst: &mut InstBundle) { clean_name_id_map(&mut dst_block); *dst_block.metadata.get_mut() = dst_meta.serialize(dst_block.version); - inst.db.set_block(key, &dst_block.serialize()).unwrap(); + inst.db.set_block(dst_key, &dst_block.serialize()).unwrap(); } inst.status.end_editing(); @@ -186,6 +214,6 @@ pub fn get_command() -> Command { (ArgType::Invert, "Overlay all nodes outside the given area"), (ArgType::Offset(false), "Vector to offset nodes by"), ], - help: "Copy part or all of one map into another." + help: "Copy part or all of one world/map into another." } } diff --git a/src/instance.rs b/src/instance.rs index ff7e39c..286721a 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -317,10 +317,12 @@ pub fn spawn_compute_thread(args: InstArgs) -> (std::thread::JoinHandle<()>, StatusClient) { let (status_tx, status_rx) = status_channel(); + // Clone within this thread to avoid issue #39364 (hopefully). + let status_tx_2 = status_tx.clone(); let h = std::thread::Builder::new() .name("compute".to_string()) .spawn(move || { - compute_thread(args, status_tx.clone()).unwrap_or_else( + compute_thread(args, status_tx_2).unwrap_or_else( |err| status_tx.log_error(&err.to_string()) ); }) diff --git a/src/map_block/mod.rs b/src/map_block/mod.rs index d33fe0c..ea6a2b3 100644 --- a/src/map_block/mod.rs +++ b/src/map_block/mod.rs @@ -22,7 +22,7 @@ use node_timer::{serialize_timers, deserialize_timers}; pub use name_id_map::NameIdMap; -#[derive(Debug)] +#[derive(Clone, Debug)] pub enum MapBlockError { InvalidVersion, DataError, diff --git a/src/utils.rs b/src/utils.rs index 18b966b..ac8b58b 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -5,8 +5,7 @@ use memmem::{Searcher, TwoWaySearcher}; use byteorder::{WriteBytesExt, BigEndian}; use crate::instance::{InstState, StatusServer}; -use crate::map_block::MapBlock; -use crate::map_database::{MapDatabase, DBError}; +use crate::map_database::MapDatabase; use crate::spatial::{Area, Vec3}; @@ -101,36 +100,6 @@ impl CacheMap { } -pub struct CachedMapDatabase<'a, 'b> { - db: &'a mut MapDatabase<'b>, - cache: CacheMap> -} - -impl<'a, 'b> CachedMapDatabase<'a, 'b> { - pub fn new(db: &'a mut MapDatabase<'b>, cap: usize) -> Self { - Self { db, cache: CacheMap::with_capacity(cap) } - } - - pub fn get_block(&mut self, key: i64) -> Option { - if let Some(block) = self.cache.get(&key) { - block.clone() - } else { - let data = self.db.get_block(key).ok(); - let block = match data { - Some(d) => MapBlock::deserialize(&d).ok(), - None => None - }; - self.cache.insert(key, block.clone()); - block - } - } - - pub fn set_block(&mut self, key: i64, data: &[u8]) -> Result<(), DBError> { - self.db.set_block(key, data) - } -} - - pub fn to_bytes(s: &String) -> Vec { s.as_bytes().to_vec() } @@ -155,6 +124,17 @@ macro_rules! unwrap_or { } +#[macro_export] +macro_rules! opt_unwrap_or { + ($res:expr, $alt:expr) => { + match $res { + Some(val) => val, + None => $alt + } + } +} + + pub fn fmt_duration(dur: Duration) -> String { let s = dur.as_secs(); if s < 3600 { diff --git a/workspace.code-workspace b/workspace.code-workspace deleted file mode 100644 index 362d7c2..0000000 --- a/workspace.code-workspace +++ /dev/null @@ -1,7 +0,0 @@ -{ - "folders": [ - { - "path": "." - } - ] -} \ No newline at end of file