From b4ad76372b4a9c644bd50013d70dd961423be80d 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 9a4e5e06f20088b44db14b4ad5313a5459e90714 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 b9f545f97c424d6f3b251cdfef42d4f1d2a42764 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 95b02912b695a1015c5becf04e1710ae53aa88a6 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 2abf7cd016cec35007be198087670196d00079d6 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 20f57cf7f3dec7d1a66c633a20a648ddf6f8d94c 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 9ba64ca57f42cc33bc8607c5529a073ae05f8bb1 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)