From 842ddb4127b76d3dcd1fe37dd06025beb1d83851 Mon Sep 17 00:00:00 2001 From: Ryan Johnson Date: Wed, 30 Nov 2022 15:55:46 -0800 Subject: [PATCH] Optimize sending tracked entity data (#163) --- .gitignore | 3 +- Cargo.toml | 4 +-- {performance_tests => benchmarks}/README.md | 4 +-- .../bench_players}/Cargo.toml | 2 +- .../bench_players}/src/main.rs | 10 +++--- build/entity.rs | 26 ++++++--------- src/chunk.rs | 4 +-- src/client.rs | 33 ++++++++++++------- src/entity.rs | 14 ++++---- 9 files changed, 55 insertions(+), 45 deletions(-) rename {performance_tests => benchmarks}/README.md (92%) rename {performance_tests/players => benchmarks/bench_players}/Cargo.toml (84%) rename {performance_tests/players => benchmarks/bench_players}/src/main.rs (95%) diff --git a/.gitignore b/.gitignore index 2564d72..5ae1327 100644 --- a/.gitignore +++ b/.gitignore @@ -8,7 +8,8 @@ Cargo.lock /extractor/out /extractor/classes /extractor/run -/performance_tests/rust-mc-bot +/benchmarks/rust-mc-bot +/velocity flamegraph.svg perf.data perf.data.old diff --git a/Cargo.toml b/Cargo.toml index dd6ef27..8023cb2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -70,9 +70,9 @@ members = [ "valence_nbt", "valence_protocol", "packet_inspector", - "performance_tests/players" + "benchmarks/bench_players" ] -exclude = ["performance_tests/rust-mc-bot"] +exclude = ["benchmarks/rust-mc-bot"] [profile.dev.package."*"] opt-level = 3 diff --git a/performance_tests/README.md b/benchmarks/README.md similarity index 92% rename from performance_tests/README.md rename to benchmarks/README.md index 27c3225..771eeac 100644 --- a/performance_tests/README.md +++ b/benchmarks/README.md @@ -3,7 +3,7 @@ Run the server ```shell -cargo r -r -p players +cargo r -r -p bench_players ``` In a separate terminal, start [rust-mc-bot](https://github.com/Eoghanmc22/rust-mc-bot). @@ -25,7 +25,7 @@ run the server like this: ```shell # You can also try setting the `CARGO_PROFILE_RELEASE_DEBUG` environment variable to `true`. -cargo flamegraph -p players +cargo flamegraph -p bench_players ``` Run rust-mc-bot as above, and then stop the server after a few seconds. Flamegraph will generate a flamegraph.svg in the diff --git a/performance_tests/players/Cargo.toml b/benchmarks/bench_players/Cargo.toml similarity index 84% rename from performance_tests/players/Cargo.toml rename to benchmarks/bench_players/Cargo.toml index 7b62956..c81c194 100644 --- a/performance_tests/players/Cargo.toml +++ b/benchmarks/bench_players/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "players" +name = "bench_players" version = "0.1.0" edition = "2021" diff --git a/performance_tests/players/src/main.rs b/benchmarks/bench_players/src/main.rs similarity index 95% rename from performance_tests/players/src/main.rs rename to benchmarks/bench_players/src/main.rs index af2d9fc..2cb676e 100644 --- a/performance_tests/players/src/main.rs +++ b/benchmarks/bench_players/src/main.rs @@ -17,7 +17,6 @@ pub fn main() -> ShutdownResult { } const WITH_PLAYER_ENTITIES: bool = true; -const MAX_PLAYERS: usize = 10_000; struct Game; @@ -38,7 +37,7 @@ impl Config for Game { type InventoryState = (); fn max_connections(&self) -> usize { - MAX_PLAYERS + 64 + 10_000 } fn connection_mode(&self) -> ConnectionMode { @@ -70,7 +69,7 @@ impl Config for Game { online_players: -1, max_players: -1, player_sample: Default::default(), - description: "Test server!".into(), + description: "Player Benchmark Server".into(), favicon_png: None, } } @@ -137,7 +136,10 @@ impl Config for Game { .entities .insert_with_uuid(EntityKind::Player, client.uuid(), ()) { - Some((id, _)) => client.state = id, + Some((id, entity)) => { + entity.set_world(world_id); + client.state = id + } None => { client.disconnect("Conflicting UUID"); return false; diff --git a/build/entity.rs b/build/entity.rs index d3dbc8b..408a59c 100644 --- a/build/entity.rs +++ b/build/entity.rs @@ -460,33 +460,27 @@ pub fn build() -> anyhow::Result { } } - pub(super) fn initial_tracked_data(&self) -> Option> { - let mut data = Vec::new(); + pub(super) fn write_initial_tracked_data(&self, buf: &mut Vec) { + debug_assert!(buf.is_empty()); match self { - #(Self::#concrete_entity_names(e) => e.initial_tracked_data(&mut data),)* + #(Self::#concrete_entity_names(e) => e.initial_tracked_data(buf),)* } - if data.is_empty() { - None - } else { - data.push(0xff); - Some(data) + if !buf.is_empty() { + buf.push(0xff); } } - pub(super) fn updated_tracked_data(&self) -> Option> { - let mut data = Vec::new(); + pub(super) fn write_updated_tracked_data(&self, buf: &mut Vec) { + debug_assert!(buf.is_empty()); match self { - #(Self::#concrete_entity_names(e) => e.updated_tracked_data(&mut data),)* + #(Self::#concrete_entity_names(e) => e.updated_tracked_data(buf),)* } - if data.is_empty() { - None - } else { - data.push(0xff); - Some(data) + if !buf.is_empty() { + buf.push(0xff); } } diff --git a/src/chunk.rs b/src/chunk.rs index 1a75ebf..7644175 100644 --- a/src/chunk.rs +++ b/src/chunk.rs @@ -561,14 +561,14 @@ impl LoadedChunk { } /// Queues the chunk data packet for this chunk with the given position. - pub(crate) fn chunk_data_packet( + pub(crate) fn send_chunk_data_packet( &self, send: &mut PlayPacketSender, scratch: &mut Vec, pos: ChunkPos, biome_registry_len: usize, ) -> anyhow::Result<()> { - scratch.clear(); + debug_assert!(scratch.is_empty()); for sect in self.sections.iter() { sect.non_air_count.encode(&mut *scratch)?; diff --git a/src/client.rs b/src/client.rs index 16f4aec..c11e3c4 100644 --- a/src/client.rs +++ b/src/client.rs @@ -201,6 +201,8 @@ pub struct Client { /// Ensures that we don't allow more connections to the server until the /// client is dropped. _permit: OwnedSemaphorePermit, + /// General purpose reusable buffer. + scratch: Vec, username: Username, uuid: Uuid, ip: IpAddr, @@ -297,6 +299,7 @@ impl Client { send: Some(send), recv, _permit: permit, + scratch: Vec::new(), username: ncd.username, uuid: ncd.uuid, ip: ncd.ip, @@ -979,6 +982,8 @@ impl Client { player_lists: &PlayerLists, inventories: &Inventories, ) -> anyhow::Result<()> { + debug_assert!(self.scratch.is_empty()); + let world = match worlds.get(self.world) { Some(world) => world, None => bail!("client is in an invalid world and must be disconnected"), @@ -1135,13 +1140,17 @@ impl Client { // Load new chunks within the view distance { - let mut scratch = Vec::new(); let biome_registry_len = shared.biomes().len(); - for pos in chunks_in_view_distance(center, self.view_distance) { if let Some(chunk) = world.chunks.get(pos) { if self.loaded_chunks.insert(pos) { - chunk.chunk_data_packet(send, &mut scratch, pos, biome_registry_len)?; + chunk.send_chunk_data_packet( + send, + &mut self.scratch, + pos, + biome_registry_len, + )?; + self.scratch.clear(); } } } @@ -1167,7 +1176,8 @@ impl Client { if self.world == entity.world() && self.position.distance(entity.position()) <= self.view_distance as f64 * 16.0 { - let _ = entity.send_updated_tracked_data(send, id); + let _ = entity.send_updated_tracked_data(send, &mut self.scratch, id); + self.scratch.clear(); let position_delta = entity.position() - entity.old_position(); let needs_teleport = position_delta.map(f64::abs).reduce_partial_max() >= 8.0; @@ -1244,16 +1254,16 @@ impl Client { } // Update the client's own player metadata. - let mut data = Vec::new(); - self.player_data.updated_tracked_data(&mut data); - - if !data.is_empty() { - data.push(0xff); + self.player_data.updated_tracked_data(&mut self.scratch); + if !self.scratch.is_empty() { + self.scratch.push(0xff); send.append_packet(&SetEntityMetadata { entity_id: VarInt(0), - metadata: RawBytes(&data), + metadata: RawBytes(&self.scratch), })?; + + self.scratch.clear(); } // Spawn new entities within the view distance. @@ -1276,9 +1286,10 @@ impl Client { return Some(e); } - if let Err(e) = entity.send_initial_tracked_data(send, id) { + if let Err(e) = entity.send_initial_tracked_data(send, &mut self.scratch, id) { return Some(e); } + self.scratch.clear(); if let Err(e) = send_entity_events(send, id.to_raw(), entity.events()) { return Some(e); diff --git a/src/entity.rs b/src/entity.rs index 7ef4276..551e76e 100644 --- a/src/entity.rs +++ b/src/entity.rs @@ -737,13 +737,14 @@ impl Entity { pub(crate) fn send_initial_tracked_data( &self, send: &mut PlayPacketSender, + scratch: &mut Vec, this_id: EntityId, ) -> anyhow::Result<()> { - // TODO: cache metadata buffer? - if let Some(metadata) = self.variants.initial_tracked_data() { + self.variants.write_initial_tracked_data(scratch); + if !scratch.is_empty() { send.append_packet(&SetEntityMetadata { entity_id: VarInt(this_id.to_raw()), - metadata: RawBytes(&metadata), + metadata: RawBytes(scratch), })?; } @@ -755,13 +756,14 @@ impl Entity { pub(crate) fn send_updated_tracked_data( &self, send: &mut PlayPacketSender, + scratch: &mut Vec, this_id: EntityId, ) -> anyhow::Result<()> { - // TODO: cache metadata buffer? - if let Some(metadata) = self.variants.updated_tracked_data() { + self.variants.write_updated_tracked_data(scratch); + if !scratch.is_empty() { send.append_packet(&SetEntityMetadata { entity_id: VarInt(this_id.to_raw()), - metadata: RawBytes(&metadata), + metadata: RawBytes(scratch), })?; }