From f5c4000b1bdb04dccb1c45a2d3fa4755b5673269 Mon Sep 17 00:00:00 2001 From: Joshua Yanovski Date: Mon, 16 Sep 2019 03:38:53 +0200 Subject: [PATCH 1/7] Allow canceling chunk generation. Currently we only do this when no players are in range of the chunk. We also send the first client who posted the chunk a message indicating that it's canceled, the hope being that this will be a performance win in single player mode since you don't have to wait three seconds to realize that the server won't generate the chunk for you. We now check an atomic flag for every column sample in a chunk. We could probably do this less frequently, but since it's a relaxed load it has essentially no performance impact on Intel architectures. --- client/src/lib.rs | 4 +- common/src/msg/server.rs | 2 +- server/src/lib.rs | 104 +++++++++++++++++++++++---------------- 3 files changed, 66 insertions(+), 44 deletions(-) diff --git a/client/src/lib.rs b/client/src/lib.rs index 870cb96670..4074046647 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -488,7 +488,9 @@ impl Client { self.state.write_component(self.entity, inventory) } ServerMsg::TerrainChunkUpdate { key, chunk } => { - self.state.insert_chunk(key, *chunk); + if let Ok(chunk) = chunk { + self.state.insert_chunk(key, *chunk); + } self.pending_chunks.remove(&key); } ServerMsg::TerrainBlockUpdates(mut blocks) => blocks diff --git a/common/src/msg/server.rs b/common/src/msg/server.rs index 5b12429a2a..50ef9081c5 100644 --- a/common/src/msg/server.rs +++ b/common/src/msg/server.rs @@ -58,7 +58,7 @@ pub enum ServerMsg { InventoryUpdate(comp::Inventory), TerrainChunkUpdate { key: Vec2, - chunk: Box, + chunk: Result, ()>, }, TerrainBlockUpdates(HashMap, Block>), Error(ServerError), diff --git a/server/src/lib.rs b/server/src/lib.rs index e8886ba002..fc289373b0 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -27,7 +27,7 @@ use common::{ vol::{ReadVol, RectVolSize, Vox}, }; use crossbeam::channel; -use hashbrown::HashSet; +use hashbrown::{HashMap, hash_map::Entry}; use log::debug; use metrics::ServerMetrics; use rand::Rng; @@ -35,7 +35,7 @@ use specs::{join::Join, world::EntityBuilder as EcsEntityBuilder, Builder, Entit use std::{ i32, net::SocketAddr, - sync::Arc, + sync::{Arc, atomic::{AtomicBool, Ordering}}, time::{Duration, Instant}, }; use uvth::{ThreadPool, ThreadPoolBuilder}; @@ -68,9 +68,9 @@ pub struct Server { clients: Clients, thread_pool: ThreadPool, - chunk_tx: channel::Sender<(Vec2, (TerrainChunk, ChunkSupplement))>, - chunk_rx: channel::Receiver<(Vec2, (TerrainChunk, ChunkSupplement))>, - pending_chunks: HashSet>, + chunk_tx: channel::Sender<(Vec2, Result<(TerrainChunk, ChunkSupplement), EcsEntity>)>, + chunk_rx: channel::Receiver<(Vec2, Result<(TerrainChunk, ChunkSupplement), EcsEntity>)>, + pending_chunks: HashMap, Arc>, server_settings: ServerSettings, server_info: ServerInfo, @@ -113,7 +113,7 @@ impl Server { .build(), chunk_tx, chunk_rx, - pending_chunks: HashSet::new(), + pending_chunks: HashMap::new(), server_info: ServerInfo { name: settings.server_name.clone(), @@ -463,7 +463,20 @@ impl Server { let before_tick_5 = Instant::now(); // 5) Fetch any generated `TerrainChunk`s and insert them into the terrain. // Also, send the chunk data to anybody that is close by. - if let Ok((key, (chunk, supplement))) = self.chunk_rx.try_recv() { + 'insert_terrain_chunks: while let Ok((key, res)) = self.chunk_rx.try_recv() { + let (chunk, supplement) = match res { + Ok((chunk, supplement)) => (chunk, supplement), + Err(entity) => { + self.clients.notify( + entity, + ServerMsg::TerrainChunkUpdate { + key, + chunk: Err(()), + }, + ); + break 'insert_terrain_chunks; + } + }; // Send the chunk to all nearby players. for (entity, view_distance, pos) in ( &self.state.ecs().entities(), @@ -485,7 +498,7 @@ impl Server { entity, ServerMsg::TerrainChunkUpdate { key, - chunk: Box::new(chunk.clone()), + chunk: Ok(Box::new(chunk.clone())), }, ); } @@ -552,32 +565,38 @@ impl Server { // Remove chunks that are too far from players. let mut chunks_to_remove = Vec::new(); - self.state.terrain().iter().for_each(|(chunk_key, _)| { - let mut should_drop = true; + self.state.terrain() + .iter().map(|(k, _)| k) + .chain(self.pending_chunks.keys().cloned()) + .for_each(|chunk_key| { + let mut should_drop = true; - // For each player with a position, calculate the distance. - for (player, pos) in ( - &self.state.ecs().read_storage::(), - &self.state.ecs().read_storage::(), - ) - .join() - { - if player - .view_distance - .map(|vd| chunk_in_vd(pos.0, chunk_key, &self.state.terrain(), vd)) - .unwrap_or(false) + // For each player with a position, calculate the distance. + for (player, pos) in ( + &self.state.ecs().read_storage::(), + &self.state.ecs().read_storage::(), + ) + .join() { - should_drop = false; - break; + if player + .view_distance + .map(|vd| chunk_in_vd(pos.0, chunk_key, &self.state.terrain(), vd)) + .unwrap_or(false) + { + should_drop = false; + break; + } } - } - if should_drop { - chunks_to_remove.push(chunk_key); - } - }); + if should_drop { + chunks_to_remove.push(chunk_key); + } + }); for key in chunks_to_remove { self.state.remove_chunk(key); + if let Some(cancel) = self.pending_chunks.remove(&key) { + cancel.store(true, Ordering::Relaxed); + } } let before_tick_6 = Instant::now(); @@ -604,10 +623,10 @@ impl Server { entity, ServerMsg::TerrainChunkUpdate { key: *chunk_key, - chunk: Box::new(match self.state.terrain().get_key(*chunk_key) { + chunk: Ok(Box::new(match self.state.terrain().get_key(*chunk_key) { Some(chunk) => chunk.clone(), None => break 'chunk, - }), + })), }, ); } @@ -997,10 +1016,10 @@ impl Server { Some(chunk) => { client.postbox.send_message(ServerMsg::TerrainChunkUpdate { key, - chunk: Box::new(chunk.clone()), + chunk: Ok(Box::new(chunk.clone())), }) } - None => requested_chunks.push(key), + None => requested_chunks.push((entity, key)) } } ClientState::Pending => {} @@ -1083,8 +1102,8 @@ impl Server { } // Generate requested chunks. - for key in requested_chunks { - self.generate_chunk(key); + for (entity, key) in requested_chunks { + self.generate_chunk(entity, key); } for (pos, block) in modified_blocks { @@ -1298,14 +1317,15 @@ impl Server { .clear(); } - pub fn generate_chunk(&mut self, key: Vec2) { - if self.pending_chunks.insert(key) { - let chunk_tx = self.chunk_tx.clone(); - let world = self.world.clone(); - self.thread_pool.execute(move || { - let _ = chunk_tx.send((key, world.generate_chunk(key))); - }); - } + pub fn generate_chunk(&mut self, entity: EcsEntity, key: Vec2) { + let v = if let Entry::Vacant(v) = self.pending_chunks.entry(key) { v } else { return; }; + let cancel = Arc::new(AtomicBool::new(false)); + v.insert(Arc::clone(&cancel)); + let chunk_tx = self.chunk_tx.clone(); + let world = self.world.clone(); + self.thread_pool.execute(move || { + let _ = chunk_tx.send((key, world.generate_chunk(key, &cancel).map_err(|_| entity))); + }); } fn process_chat_cmd(&mut self, entity: EcsEntity, cmd: String) { From d5c441767f9641314b7b929f07addb69d5d3b379 Mon Sep 17 00:00:00 2001 From: Joshua Yanovski Date: Mon, 16 Sep 2019 03:51:08 +0200 Subject: [PATCH 2/7] Fixing previous commit. --- world/src/lib.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/world/src/lib.rs b/world/src/lib.rs index f0ce7896c7..a65e09749a 100644 --- a/world/src/lib.rs +++ b/world/src/lib.rs @@ -65,7 +65,8 @@ impl World { BlockGen::new(self, ColumnGen::new(&self.sim)) } - pub fn generate_chunk(&self, chunk_pos: Vec2) -> (TerrainChunk, ChunkSupplement) { + pub fn generate_chunk(&self, chunk_pos: Vec2, + flag: &AtomicBool) -> Result<(TerrainChunk, ChunkSupplement), ()> { let air = Block::empty(); let stone = Block::new(BlockKind::Dense, Rgb::new(200, 220, 255)); let water = Block::new(BlockKind::Water, Rgb::new(60, 90, 190)); @@ -81,7 +82,7 @@ impl World { { Some((base_z, sim_chunk)) => (base_z as i32, sim_chunk), None => { - return ( + return Ok(( TerrainChunk::new( CONFIG.sea_level as i32, water, @@ -89,7 +90,7 @@ impl World { TerrainChunkMeta::void(), ), ChunkSupplement::default(), - ) + )) } }; @@ -101,6 +102,7 @@ impl World { let mut chunk = TerrainChunk::new(base_z, stone, air, meta); for x in 0..TerrainChunkSize::RECT_SIZE.x as i32 { for y in 0..TerrainChunkSize::RECT_SIZE.y as i32 { + if flag.load(Ordering::Relaxed) { return Err(()) }; let wpos2d = Vec2::new(x, y) + Vec2::from(chunk_pos) * TerrainChunkSize::RECT_SIZE.map(|e| e as i32); @@ -151,7 +153,7 @@ impl World { }, }; - (chunk, supplement) + Ok((chunk, supplement)) } } From 00b3f4e4f49fe52929a4af651715130fe2b5053b Mon Sep 17 00:00:00 2001 From: Joshua Yanovski Date: Mon, 16 Sep 2019 03:54:32 +0200 Subject: [PATCH 3/7] Fixing previous commit. --- world/src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/world/src/lib.rs b/world/src/lib.rs index a65e09749a..6da38f578e 100644 --- a/world/src/lib.rs +++ b/world/src/lib.rs @@ -28,7 +28,10 @@ use common::{ vol::{ReadVol, RectVolSize, Vox, WriteVol}, }; use rand::Rng; -use std::time::Duration; +use std::{ + sync::atomic::{AtomicBool, Ordering}, + time::Duration, +}; use vek::*; #[derive(Debug)] From 661b9aee724d4658809dbbdb288ff6d66d646560 Mon Sep 17 00:00:00 2001 From: Joshua Yanovski Date: Mon, 16 Sep 2019 04:01:05 +0200 Subject: [PATCH 4/7] Fixing rustfmt. --- server/src/lib.rs | 29 ++++++++++++++++++++++------- world/src/lib.rs | 10 +++++++--- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/server/src/lib.rs b/server/src/lib.rs index fc289373b0..2b905e6f18 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -35,7 +35,10 @@ use specs::{join::Join, world::EntityBuilder as EcsEntityBuilder, Builder, Entit use std::{ i32, net::SocketAddr, - sync::{Arc, atomic::{AtomicBool, Ordering}}, + sync::{ + atomic::{AtomicBool, Ordering}, + Arc, + }, time::{Duration, Instant}, }; use uvth::{ThreadPool, ThreadPoolBuilder}; @@ -68,8 +71,14 @@ pub struct Server { clients: Clients, thread_pool: ThreadPool, - chunk_tx: channel::Sender<(Vec2, Result<(TerrainChunk, ChunkSupplement), EcsEntity>)>, - chunk_rx: channel::Receiver<(Vec2, Result<(TerrainChunk, ChunkSupplement), EcsEntity>)>, + chunk_tx: channel::Sender<( + Vec2, + Result<(TerrainChunk, ChunkSupplement), EcsEntity> + )>, + chunk_rx: channel::Receiver<( + Vec2, + Result<(TerrainChunk, ChunkSupplement), EcsEntity> + )>, pending_chunks: HashMap, Arc>, server_settings: ServerSettings, @@ -565,8 +574,10 @@ impl Server { // Remove chunks that are too far from players. let mut chunks_to_remove = Vec::new(); - self.state.terrain() - .iter().map(|(k, _)| k) + self.state + .terrain() + .iter() + .map(|(k, _)| k) .chain(self.pending_chunks.keys().cloned()) .for_each(|chunk_key| { let mut should_drop = true; @@ -1019,7 +1030,7 @@ impl Server { chunk: Ok(Box::new(chunk.clone())), }) } - None => requested_chunks.push((entity, key)) + None => requested_chunks.push((entity, key)), } } ClientState::Pending => {} @@ -1318,7 +1329,11 @@ impl Server { } pub fn generate_chunk(&mut self, entity: EcsEntity, key: Vec2) { - let v = if let Entry::Vacant(v) = self.pending_chunks.entry(key) { v } else { return; }; + let v = if let Entry::Vacant(v) = self.pending_chunks.entry(key) { + v + } else { + return; + }; let cancel = Arc::new(AtomicBool::new(false)); v.insert(Arc::clone(&cancel)); let chunk_tx = self.chunk_tx.clone(); diff --git a/world/src/lib.rs b/world/src/lib.rs index 6da38f578e..6ecac0b33c 100644 --- a/world/src/lib.rs +++ b/world/src/lib.rs @@ -68,8 +68,10 @@ impl World { BlockGen::new(self, ColumnGen::new(&self.sim)) } - pub fn generate_chunk(&self, chunk_pos: Vec2, - flag: &AtomicBool) -> Result<(TerrainChunk, ChunkSupplement), ()> { + pub fn generate_chunk( + &self, + chunk_pos: Vec2, + flag: &AtomicBool) -> Result<(TerrainChunk, ChunkSupplement), ()> { let air = Block::empty(); let stone = Block::new(BlockKind::Dense, Rgb::new(200, 220, 255)); let water = Block::new(BlockKind::Water, Rgb::new(60, 90, 190)); @@ -105,7 +107,9 @@ impl World { let mut chunk = TerrainChunk::new(base_z, stone, air, meta); for x in 0..TerrainChunkSize::RECT_SIZE.x as i32 { for y in 0..TerrainChunkSize::RECT_SIZE.y as i32 { - if flag.load(Ordering::Relaxed) { return Err(()) }; + if flag.load(Ordering::Relaxed) { + return Err(()) + }; let wpos2d = Vec2::new(x, y) + Vec2::from(chunk_pos) * TerrainChunkSize::RECT_SIZE.map(|e| e as i32); From c4eae2e1b1662559de7a450ede3e2849fc716d42 Mon Sep 17 00:00:00 2001 From: Joshua Yanovski Date: Mon, 16 Sep 2019 04:05:36 +0200 Subject: [PATCH 5/7] Fixing more rustfmt errors. --- server/src/lib.rs | 6 +++--- world/src/lib.rs | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/server/src/lib.rs b/server/src/lib.rs index 2b905e6f18..2680f98018 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -27,7 +27,7 @@ use common::{ vol::{ReadVol, RectVolSize, Vox}, }; use crossbeam::channel; -use hashbrown::{HashMap, hash_map::Entry}; +use hashbrown::{hash_map::Entry, HashMap}; use log::debug; use metrics::ServerMetrics; use rand::Rng; @@ -73,11 +73,11 @@ pub struct Server { thread_pool: ThreadPool, chunk_tx: channel::Sender<( Vec2, - Result<(TerrainChunk, ChunkSupplement), EcsEntity> + Result<(TerrainChunk, ChunkSupplement), EcsEntity>, )>, chunk_rx: channel::Receiver<( Vec2, - Result<(TerrainChunk, ChunkSupplement), EcsEntity> + Result<(TerrainChunk, ChunkSupplement), EcsEntity>, )>, pending_chunks: HashMap, Arc>, diff --git a/world/src/lib.rs b/world/src/lib.rs index 6ecac0b33c..65d929d9e1 100644 --- a/world/src/lib.rs +++ b/world/src/lib.rs @@ -71,7 +71,8 @@ impl World { pub fn generate_chunk( &self, chunk_pos: Vec2, - flag: &AtomicBool) -> Result<(TerrainChunk, ChunkSupplement), ()> { + flag: &AtomicBool, + ) -> Result<(TerrainChunk, ChunkSupplement), ()> { let air = Block::empty(); let stone = Block::new(BlockKind::Dense, Rgb::new(200, 220, 255)); let water = Block::new(BlockKind::Water, Rgb::new(60, 90, 190)); @@ -108,7 +109,7 @@ impl World { for x in 0..TerrainChunkSize::RECT_SIZE.x as i32 { for y in 0..TerrainChunkSize::RECT_SIZE.y as i32 { if flag.load(Ordering::Relaxed) { - return Err(()) + return Err(()); }; let wpos2d = Vec2::new(x, y) + Vec2::from(chunk_pos) * TerrainChunkSize::RECT_SIZE.map(|e| e as i32); From 4fb851263c75aff228f7038e0476cd1f4c7bf736 Mon Sep 17 00:00:00 2001 From: Joshua Yanovski Date: Mon, 16 Sep 2019 04:18:40 +0200 Subject: [PATCH 6/7] Continue instead of breaking. --- server/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/lib.rs b/server/src/lib.rs index 2680f98018..927edf7572 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -483,7 +483,7 @@ impl Server { chunk: Err(()), }, ); - break 'insert_terrain_chunks; + continue 'insert_terrain_chunks; } }; // Send the chunk to all nearby players. From 743e48110fe7326db57e34506782acbfbc4ad746 Mon Sep 17 00:00:00 2001 From: Joshua Yanovski Date: Mon, 16 Sep 2019 15:11:47 +0200 Subject: [PATCH 7/7] Addressing code review. --- server/src/lib.rs | 5 ++++- world/src/lib.rs | 9 +++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/server/src/lib.rs b/server/src/lib.rs index 927edf7572..8d140a980d 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -1339,7 +1339,10 @@ impl Server { let chunk_tx = self.chunk_tx.clone(); let world = self.world.clone(); self.thread_pool.execute(move || { - let _ = chunk_tx.send((key, world.generate_chunk(key, &cancel).map_err(|_| entity))); + let payload = world + .generate_chunk(key, || cancel.load(Ordering::Relaxed)) + .map_err(|_| entity); + let _ = chunk_tx.send((key, payload)); }); } diff --git a/world/src/lib.rs b/world/src/lib.rs index 65d929d9e1..a1a6ba4aa5 100644 --- a/world/src/lib.rs +++ b/world/src/lib.rs @@ -28,10 +28,7 @@ use common::{ vol::{ReadVol, RectVolSize, Vox, WriteVol}, }; use rand::Rng; -use std::{ - sync::atomic::{AtomicBool, Ordering}, - time::Duration, -}; +use std::time::Duration; use vek::*; #[derive(Debug)] @@ -71,7 +68,7 @@ impl World { pub fn generate_chunk( &self, chunk_pos: Vec2, - flag: &AtomicBool, + mut should_continue: impl FnMut() -> bool, ) -> Result<(TerrainChunk, ChunkSupplement), ()> { let air = Block::empty(); let stone = Block::new(BlockKind::Dense, Rgb::new(200, 220, 255)); @@ -108,7 +105,7 @@ impl World { let mut chunk = TerrainChunk::new(base_z, stone, air, meta); for x in 0..TerrainChunkSize::RECT_SIZE.x as i32 { for y in 0..TerrainChunkSize::RECT_SIZE.y as i32 { - if flag.load(Ordering::Relaxed) { + if should_continue() { return Err(()); }; let wpos2d = Vec2::new(x, y)