From 32e58c4b17dfaad4927f0021b9474fdd6b6bb725 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcel=20M=C3=A4rtens?= Date: Tue, 10 Aug 2021 15:55:58 +0200 Subject: [PATCH 1/3] remove futures_executor from renderer and rather pass tokio runtime to it properly. therefore the runtime is now created with Voxygen rather than with the Connect attempt --- Cargo.lock | 1 - voxygen/Cargo.toml | 1 - voxygen/src/lib.rs | 4 ++ voxygen/src/main.rs | 25 +++++++- voxygen/src/menu/main/client_init.rs | 21 +------ voxygen/src/menu/main/mod.rs | 12 ++-- voxygen/src/render/renderer.rs | 88 ++++++++++++++-------------- voxygen/src/singleplayer.rs | 43 ++++---------- voxygen/src/window.rs | 4 +- 9 files changed, 94 insertions(+), 105 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e142e4a17c..722a0f2cf3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6161,7 +6161,6 @@ dependencies = [ "egui_winit_platform", "enum-iterator", "euc", - "futures-executor", "gilrs", "glyph_brush", "guillotiere", diff --git a/voxygen/Cargo.toml b/voxygen/Cargo.toml index eab7fa4639..2b1bebc83f 100644 --- a/voxygen/Cargo.toml +++ b/voxygen/Cargo.toml @@ -98,7 +98,6 @@ crossbeam-channel = "0.5" directories-next = "2.0" dot_vox = "4.0" enum-iterator = "0.6" -futures-executor = "0.3" guillotiere = "0.6" hashbrown = {version = "0.11", features = ["rayon", "serde", "nightly"]} image = {version = "0.23.12", default-features = false, features = ["ico", "png"]} diff --git a/voxygen/src/lib.rs b/voxygen/src/lib.rs index 6404738e50..f0f63ac1f4 100644 --- a/voxygen/src/lib.rs +++ b/voxygen/src/lib.rs @@ -51,6 +51,9 @@ use common_base::span; use i18n::LocalizationHandle; use std::path::PathBuf; +use std::sync::Arc; +use tokio::runtime::Runtime; + /// A type used to store state that is shared between all play states. pub struct GlobalState { pub userdata_dir: PathBuf, @@ -58,6 +61,7 @@ pub struct GlobalState { pub settings: Settings, pub profile: Profile, pub window: Window, + pub tokio_runtime: Arc, #[cfg(feature = "egui-ui")] pub egui_state: EguiState, pub lazy_init: scene::terrain::SpriteRenderContextLazy, diff --git a/voxygen/src/main.rs b/voxygen/src/main.rs index dd602949d8..36e08769a8 100644 --- a/voxygen/src/main.rs +++ b/voxygen/src/main.rs @@ -165,6 +165,28 @@ fn main() { default_hook(panic_info); })); + //Setup tokio runtime + use common::consts::MIN_RECOMMENDED_TOKIO_THREADS; + use std::sync::{ + atomic::{AtomicUsize, Ordering}, + Arc, + }; + use tokio::runtime::Builder; + + let cores = num_cpus::get(); + let tokio_runtime = Arc::new( + Builder::new_multi_thread() + .enable_all() + .worker_threads((cores / 4).max(MIN_RECOMMENDED_TOKIO_THREADS)) + .thread_name_fn(|| { + static ATOMIC_ID: AtomicUsize = AtomicUsize::new(0); + let id = ATOMIC_ID.fetch_add(1, Ordering::SeqCst); + format!("tokio-voxygen-{}", id) + }) + .build() + .unwrap(), + ); + #[cfg(feature = "hot-reloading")] assets::start_hot_reloading(); @@ -210,7 +232,7 @@ fn main() { // Create window use veloren_voxygen::{error::Error, render::RenderError}; - let (mut window, event_loop) = match Window::new(&settings) { + let (mut window, event_loop) = match tokio_runtime.block_on(Window::new(&settings)) { Ok(ok) => ok, // Custom panic message when a graphics backend could not be found Err(Error::RenderError(RenderError::CouldNotFindAdapter)) => { @@ -247,6 +269,7 @@ fn main() { audio, profile, window, + tokio_runtime, #[cfg(feature = "egui-ui")] egui_state, lazy_init, diff --git a/voxygen/src/menu/main/client_init.rs b/voxygen/src/menu/main/client_init.rs index 784e0ecbb0..e2e173fb93 100644 --- a/voxygen/src/menu/main/client_init.rs +++ b/voxygen/src/menu/main/client_init.rs @@ -3,11 +3,10 @@ use client::{ error::{Error as ClientError, NetworkConnectError, NetworkError}, Client, ServerInfo, }; -use common::consts::MIN_RECOMMENDED_TOKIO_THREADS; use crossbeam_channel::{unbounded, Receiver, Sender, TryRecvError}; use std::{ sync::{ - atomic::{AtomicBool, AtomicUsize, Ordering}, + atomic::{AtomicBool, Ordering}, Arc, }, time::Duration, @@ -47,29 +46,13 @@ impl ClientInit { connection_args: ConnectionArgs, username: String, password: String, - runtime: Option>, + runtime: Arc, ) -> Self { let (tx, rx) = unbounded(); let (trust_tx, trust_rx) = unbounded(); let cancel = Arc::new(AtomicBool::new(false)); let cancel2 = Arc::clone(&cancel); - let runtime = runtime.unwrap_or_else(|| { - // TODO: evaluate std::thread::available_concurrency as a num_cpus replacement - let cores = num_cpus::get(); - Arc::new( - runtime::Builder::new_multi_thread() - .enable_all() - .worker_threads((cores / 4).max(MIN_RECOMMENDED_TOKIO_THREADS)) - .thread_name_fn(|| { - static ATOMIC_ID: AtomicUsize = AtomicUsize::new(0); - let id = ATOMIC_ID.fetch_add(1, Ordering::SeqCst); - format!("tokio-voxygen-{}", id) - }) - .build() - .unwrap(), - ) - }); let runtime2 = Arc::clone(&runtime); runtime.spawn(async move { diff --git a/voxygen/src/menu/main/mod.rs b/voxygen/src/menu/main/mod.rs index 910174901a..5914cd5f1f 100644 --- a/voxygen/src/menu/main/mod.rs +++ b/voxygen/src/menu/main/mod.rs @@ -93,7 +93,7 @@ impl PlayState for MainMenuState { { if let Some(singleplayer) = &global_state.singleplayer { match singleplayer.receiver.try_recv() { - Ok(Ok(runtime)) => { + Ok(Ok(())) => { // Attempt login after the server is finished initializing attempt_login( &mut global_state.info_message, @@ -101,7 +101,7 @@ impl PlayState for MainMenuState { "".to_owned(), ConnectionArgs::Mpsc(14004), &mut self.init, - Some(runtime), + &global_state.tokio_runtime, ); }, Ok(Err(e)) => { @@ -261,7 +261,7 @@ impl PlayState for MainMenuState { password, connection_args, &mut self.init, - None, + &global_state.tokio_runtime, ); }, MainMenuEvent::CancelLoginAttempt => { @@ -290,7 +290,7 @@ impl PlayState for MainMenuState { }, #[cfg(feature = "singleplayer")] MainMenuEvent::StartSingleplayer => { - let singleplayer = Singleplayer::new(); + let singleplayer = Singleplayer::new(&global_state.tokio_runtime); global_state.singleplayer = Some(singleplayer); }, @@ -450,7 +450,7 @@ fn attempt_login( password: String, connection_args: ConnectionArgs, init: &mut InitState, - runtime: Option>, + runtime: &Arc, ) { if let Err(err) = comp::Player::alias_validate(&username) { *info_message = Some(err.to_string()); @@ -463,7 +463,7 @@ fn attempt_login( connection_args, username, password, - runtime, + Arc::clone(runtime), )); } } diff --git a/voxygen/src/render/renderer.rs b/voxygen/src/render/renderer.rs index 603fb2a915..2432868009 100644 --- a/voxygen/src/render/renderer.rs +++ b/voxygen/src/render/renderer.rs @@ -183,7 +183,10 @@ pub struct Renderer { impl Renderer { /// Create a new `Renderer` from a variety of backend-specific components /// and the window targets. - pub fn new(window: &winit::window::Window, mode: RenderMode) -> Result { + pub async fn new( + window: &winit::window::Window, + mode: RenderMode, + ) -> Result { let (pipeline_modes, mut other_modes) = mode.split(); // Enable seamless cubemaps globally, where available--they are essentially a // strict improvement on regular cube maps. @@ -220,13 +223,13 @@ impl Renderer { #[allow(unsafe_code)] let surface = unsafe { instance.create_surface(window) }; - let adapter = futures_executor::block_on(instance.request_adapter( - &wgpu::RequestAdapterOptionsBase { + let adapter = instance + .request_adapter(&wgpu::RequestAdapterOptionsBase { power_preference: wgpu::PowerPreference::HighPerformance, compatible_surface: Some(&surface), - }, - )) - .ok_or(RenderError::CouldNotFindAdapter)?; + }) + .await + .ok_or(RenderError::CouldNotFindAdapter)?; let info = adapter.get_info(); info!( @@ -244,47 +247,44 @@ impl Renderer { ..Default::default() }; - let (device, queue) = futures_executor::block_on(adapter.request_device( - &wgpu::DeviceDescriptor { - // TODO - label: None, - features: wgpu::Features::DEPTH_CLAMPING - | wgpu::Features::ADDRESS_MODE_CLAMP_TO_BORDER - | wgpu::Features::PUSH_CONSTANTS - | (adapter.features() & wgpu_profiler::GpuProfiler::REQUIRED_WGPU_FEATURES), - limits, - }, - std::env::var_os("WGPU_TRACE_DIR").as_ref().map(|v| { - let path = std::path::Path::new(v); - // We don't want to continue if we can't actually collect the api trace - if !path.exists() { - panic!( - "WGPU_TRACE_DIR is set to the path \"{}\" which doesn't exist", - path.display() - ); - } - if !path.is_dir() { - panic!( - "WGPU_TRACE_DIR is set to the path \"{}\" which is not a directory", - path.display() - ); - } - if path - .read_dir() + let trace_path = std::env::var_os("WGPU_TRACE_DIR").map(|v| { + let path = std::path::PathBuf::from(v); + // We don't want to continue if we can't actually collect the api trace + assert!( + path.exists(), + "WGPU_TRACE_DIR is set to the path \"{}\" which doesn't exist", + path.display() + ); + assert!( + path.is_dir(), + "WGPU_TRACE_DIR is set to the path \"{}\" which is not a directory", + path.display() + ); + assert!( + path.read_dir() .expect("Could not read the directory that is specified by WGPU_TRACE_DIR") .next() - .is_some() - { - panic!( - "WGPU_TRACE_DIR is set to the path \"{}\" which already contains other \ - files", - path.display() - ); - } + .is_some(), + "WGPU_TRACE_DIR is set to the path \"{}\" which already contains other files", + path.display() + ); - path - }), - ))?; + path + }); + let (device, queue) = adapter + .request_device( + &wgpu::DeviceDescriptor { + // TODO + label: None, + features: wgpu::Features::DEPTH_CLAMPING + | wgpu::Features::ADDRESS_MODE_CLAMP_TO_BORDER + | wgpu::Features::PUSH_CONSTANTS + | (adapter.features() & wgpu_profiler::GpuProfiler::REQUIRED_WGPU_FEATURES), + limits, + }, + trace_path.as_deref(), + ) + .await?; // Set error handler for wgpu errors // This is better for use than their default because it includes the error in diff --git a/voxygen/src/singleplayer.rs b/voxygen/src/singleplayer.rs index cb9fd50f20..bd0b6f6357 100644 --- a/voxygen/src/singleplayer.rs +++ b/voxygen/src/singleplayer.rs @@ -1,4 +1,4 @@ -use common::{clock::Clock, consts::MIN_RECOMMENDED_TOKIO_THREADS}; +use common::clock::Clock; use crossbeam_channel::{bounded, unbounded, Receiver, Sender, TryRecvError}; use server::{ persistence::{DatabaseSettings, SqlLogMode}, @@ -6,14 +6,14 @@ use server::{ }; use std::{ sync::{ - atomic::{AtomicBool, AtomicUsize, Ordering}, + atomic::{AtomicBool, Ordering}, Arc, }, thread::{self, JoinHandle}, time::Duration, }; use tokio::runtime::Runtime; -use tracing::{debug, error, info, trace, warn}; +use tracing::{error, info, trace, warn}; const TPS: u64 = 30; @@ -22,7 +22,7 @@ const TPS: u64 = 30; pub struct Singleplayer { _server_thread: JoinHandle<()>, stop_server_s: Sender<()>, - pub receiver: Receiver, ServerError>>, + pub receiver: Receiver>, // Wether the server is stopped or not paused: Arc, // Settings that the server was started with @@ -30,8 +30,7 @@ pub struct Singleplayer { } impl Singleplayer { - #[allow(clippy::new_without_default)] - pub fn new() -> Self { + pub fn new(runtime: &Arc) -> Self { let (stop_server_s, stop_server_r) = unbounded(); // Determine folder to save server data in @@ -82,22 +81,6 @@ impl Singleplayer { let settings = server::Settings::singleplayer(&server_data_dir); let editable_settings = server::EditableSettings::singleplayer(&server_data_dir); - // TODO: evaluate std::thread::available_concurrency as a num_cpus replacement - let cores = num_cpus::get(); - debug!("Creating a new runtime for server"); - let runtime = Arc::new( - tokio::runtime::Builder::new_multi_thread() - .enable_all() - .worker_threads((cores / 4).max(MIN_RECOMMENDED_TOKIO_THREADS)) - .thread_name_fn(|| { - static ATOMIC_ID: AtomicUsize = AtomicUsize::new(0); - let id = ATOMIC_ID.fetch_add(1, Ordering::SeqCst); - format!("tokio-sp-{}", id) - }) - .build() - .unwrap(), - ); - let settings2 = settings.clone(); // Relative to data_dir @@ -117,24 +100,22 @@ impl Singleplayer { let (result_sender, result_receiver) = bounded(1); let builder = thread::Builder::new().name("singleplayer-server-thread".into()); + let runtime = Arc::clone(runtime); let thread = builder .spawn(move || { trace!("starting singleplayer server thread"); let mut server = None; if let Err(e) = result_sender.send( - match Server::new( + Server::new( settings2, editable_settings, database_settings, &server_data_dir, - Arc::clone(&runtime), - ) { - Ok(s) => { - server = Some(s); - Ok(runtime) - }, - Err(e) => Err(e), - }, + runtime, + ) + .map(|s| { + server = Some(s); + }), ) { warn!( ?e, diff --git a/voxygen/src/window.rs b/voxygen/src/window.rs index f29c197fb6..b74be45c9e 100644 --- a/voxygen/src/window.rs +++ b/voxygen/src/window.rs @@ -398,7 +398,7 @@ pub struct Window { } impl Window { - pub fn new(settings: &Settings) -> Result<(Window, EventLoop), Error> { + pub async fn new(settings: &Settings) -> Result<(Window, EventLoop), Error> { let event_loop = EventLoop::new(); let size = settings.graphics.window_size; @@ -418,7 +418,7 @@ impl Window { let window = win_builder.build(&event_loop).unwrap(); - let renderer = Renderer::new(&window, settings.graphics.render_mode.clone())?; + let renderer = Renderer::new(&window, settings.graphics.render_mode.clone()).await?; let keypress_map = HashMap::new(); From 889a8d11f51dd3707e8afd311355900957ccc6a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcel=20M=C3=A4rtens?= Date: Wed, 11 Aug 2021 01:23:48 +0200 Subject: [PATCH 2/3] zest prefers this over await --- voxygen/src/main.rs | 2 +- voxygen/src/render/renderer.rs | 36 ++++++++++++++++------------------ voxygen/src/window.rs | 7 +++++-- 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/voxygen/src/main.rs b/voxygen/src/main.rs index 36e08769a8..2d3f86a0aa 100644 --- a/voxygen/src/main.rs +++ b/voxygen/src/main.rs @@ -232,7 +232,7 @@ fn main() { // Create window use veloren_voxygen::{error::Error, render::RenderError}; - let (mut window, event_loop) = match tokio_runtime.block_on(Window::new(&settings)) { + let (mut window, event_loop) = match Window::new(&settings, &tokio_runtime) { Ok(ok) => ok, // Custom panic message when a graphics backend could not be found Err(Error::RenderError(RenderError::CouldNotFindAdapter)) => { diff --git a/voxygen/src/render/renderer.rs b/voxygen/src/render/renderer.rs index 2432868009..b8094dc0ea 100644 --- a/voxygen/src/render/renderer.rs +++ b/voxygen/src/render/renderer.rs @@ -183,9 +183,10 @@ pub struct Renderer { impl Renderer { /// Create a new `Renderer` from a variety of backend-specific components /// and the window targets. - pub async fn new( + pub fn new( window: &winit::window::Window, mode: RenderMode, + runtime: &tokio::runtime::Runtime, ) -> Result { let (pipeline_modes, mut other_modes) = mode.split(); // Enable seamless cubemaps globally, where available--they are essentially a @@ -223,12 +224,11 @@ impl Renderer { #[allow(unsafe_code)] let surface = unsafe { instance.create_surface(window) }; - let adapter = instance - .request_adapter(&wgpu::RequestAdapterOptionsBase { + let adapter = runtime + .block_on(instance.request_adapter(&wgpu::RequestAdapterOptionsBase { power_preference: wgpu::PowerPreference::HighPerformance, compatible_surface: Some(&surface), - }) - .await + })) .ok_or(RenderError::CouldNotFindAdapter)?; let info = adapter.get_info(); @@ -271,20 +271,18 @@ impl Renderer { path }); - let (device, queue) = adapter - .request_device( - &wgpu::DeviceDescriptor { - // TODO - label: None, - features: wgpu::Features::DEPTH_CLAMPING - | wgpu::Features::ADDRESS_MODE_CLAMP_TO_BORDER - | wgpu::Features::PUSH_CONSTANTS - | (adapter.features() & wgpu_profiler::GpuProfiler::REQUIRED_WGPU_FEATURES), - limits, - }, - trace_path.as_deref(), - ) - .await?; + let (device, queue) = runtime.block_on(adapter.request_device( + &wgpu::DeviceDescriptor { + // TODO + label: None, + features: wgpu::Features::DEPTH_CLAMPING + | wgpu::Features::ADDRESS_MODE_CLAMP_TO_BORDER + | wgpu::Features::PUSH_CONSTANTS + | (adapter.features() & wgpu_profiler::GpuProfiler::REQUIRED_WGPU_FEATURES), + limits, + }, + trace_path.as_deref(), + ))?; // Set error handler for wgpu errors // This is better for use than their default because it includes the error in diff --git a/voxygen/src/window.rs b/voxygen/src/window.rs index b74be45c9e..409407b06f 100644 --- a/voxygen/src/window.rs +++ b/voxygen/src/window.rs @@ -398,7 +398,10 @@ pub struct Window { } impl Window { - pub async fn new(settings: &Settings) -> Result<(Window, EventLoop), Error> { + pub fn new( + settings: &Settings, + runtime: &tokio::runtime::Runtime, + ) -> Result<(Window, EventLoop), Error> { let event_loop = EventLoop::new(); let size = settings.graphics.window_size; @@ -418,7 +421,7 @@ impl Window { let window = win_builder.build(&event_loop).unwrap(); - let renderer = Renderer::new(&window, settings.graphics.render_mode.clone()).await?; + let renderer = Renderer::new(&window, settings.graphics.render_mode.clone(), runtime)?; let keypress_map = HashMap::new(); From bc821a46cde8c27daf54ab1cf4f7fde480a720ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcel=20M=C3=A4rtens?= Date: Wed, 11 Aug 2021 09:42:29 +0200 Subject: [PATCH 3/3] code cleanup --- voxygen/src/main.rs | 3 ++- voxygen/src/render/renderer.rs | 6 ++++-- voxygen/src/singleplayer.rs | 39 +++++++++++++++------------------- 3 files changed, 23 insertions(+), 25 deletions(-) diff --git a/voxygen/src/main.rs b/voxygen/src/main.rs index 2d3f86a0aa..98e97f5db7 100644 --- a/voxygen/src/main.rs +++ b/voxygen/src/main.rs @@ -165,7 +165,7 @@ fn main() { default_hook(panic_info); })); - //Setup tokio runtime + // Setup tokio runtime use common::consts::MIN_RECOMMENDED_TOKIO_THREADS; use std::sync::{ atomic::{AtomicUsize, Ordering}, @@ -173,6 +173,7 @@ fn main() { }; use tokio::runtime::Builder; + // TODO: evaluate std::thread::available_concurrency as a num_cpus replacement let cores = num_cpus::get(); let tokio_runtime = Arc::new( Builder::new_multi_thread() diff --git a/voxygen/src/render/renderer.rs b/voxygen/src/render/renderer.rs index b8094dc0ea..a77d86d368 100644 --- a/voxygen/src/render/renderer.rs +++ b/voxygen/src/render/renderer.rs @@ -247,8 +247,9 @@ impl Renderer { ..Default::default() }; - let trace_path = std::env::var_os("WGPU_TRACE_DIR").map(|v| { - let path = std::path::PathBuf::from(v); + let trace_env = std::env::var_os("WGPU_TRACE_DIR"); + let trace_path = trace_env.as_ref().map(|v| { + let path = std::path::Path::new(v); // We don't want to continue if we can't actually collect the api trace assert!( path.exists(), @@ -271,6 +272,7 @@ impl Renderer { path }); + let (device, queue) = runtime.block_on(adapter.request_device( &wgpu::DeviceDescriptor { // TODO diff --git a/voxygen/src/singleplayer.rs b/voxygen/src/singleplayer.rs index bd0b6f6357..44e0a789c3 100644 --- a/voxygen/src/singleplayer.rs +++ b/voxygen/src/singleplayer.rs @@ -104,33 +104,28 @@ impl Singleplayer { let thread = builder .spawn(move || { trace!("starting singleplayer server thread"); - let mut server = None; - if let Err(e) = result_sender.send( - Server::new( - settings2, - editable_settings, - database_settings, - &server_data_dir, - runtime, - ) - .map(|s| { - server = Some(s); - }), + + let (server, init_result) = match Server::new( + settings2, + editable_settings, + database_settings, + &server_data_dir, + runtime, ) { - warn!( + Ok(server) => (Some(server), Ok(())), + Err(err) => (None, Err(err)), + }; + + match (result_sender.send(init_result), server) { + (Err(e), _) => warn!( ?e, "Failed to send singleplayer server initialization result. Most likely \ the channel was closed by cancelling server creation. Stopping Server" - ); - return; - }; + ), + (Ok(()), None) => (), + (Ok(()), Some(server)) => run_server(server, stop_server_r, paused1), + } - let server = match server { - Some(s) => s, - None => return, - }; - - run_server(server, stop_server_r, paused1); trace!("ending singleplayer server thread"); }) .unwrap();