From 7004669fb0fbfeb1e5374d79bc18be5c9dad6cf8 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Sat, 20 Aug 2022 03:01:14 +0200 Subject: [PATCH] Replace parking_lot with std::sync types As of https://github.com/rust-lang/rust/pull/95035, the locking API provided by std::sync is improved on Linux, outperforming parking_lot in various cases. Using the built-in locking APIs means we can remove parking_lot as a dependency, along with its own (indirect) dependencies; removing a total of 10 dependencies. --- Cargo.lock | 87 +--------------------------------- vm/Cargo.toml | 1 - vm/src/permanent_space.rs | 12 ++--- vm/src/process.rs | 60 +++++++++++------------ vm/src/scheduler/park_group.rs | 6 +-- vm/src/state.rs | 6 +-- 6 files changed, 44 insertions(+), 128 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 15f8189ab..f74588632 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -60,9 +60,9 @@ dependencies = [ [[package]] name = "bumpalo" -version = "3.10.0" +version = "3.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "37ccbd214614c6783386c1af30caf03192f17891059cecc394b4fb119e363de3" +checksum = "c1ad822118d20d2c234f427000d5acc36eabe1e29a348c89b63dd60b13f28e5d" [[package]] name = "bytecode" @@ -312,16 +312,6 @@ dependencies = [ "winapi", ] -[[package]] -name = "lock_api" -version = "0.4.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "327fa5b6a6940e4699ec49a9beae1ea4845c6bab9314e4f84ac68742139d8c53" -dependencies = [ - "autocfg", - "scopeguard", -] - [[package]] name = "log" version = "0.4.17" @@ -385,29 +375,6 @@ version = "1.13.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "074864da206b4973b84eb91683020dbefd6a8c3f0f38e054d93954e891935e4e" -[[package]] -name = "parking_lot" -version = "0.12.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3742b2c103b9f06bc9fff0a37ff4912935851bee6d36f3c02bcc755bcfec228f" -dependencies = [ - "lock_api", - "parking_lot_core", -] - -[[package]] -name = "parking_lot_core" -version = "0.9.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "09a279cbf25cb0757810394fbc1e359949b59e348145c643a939a525692e6929" -dependencies = [ - "cfg-if", - "libc", - "redox_syscall", - "smallvec", - "windows-sys", -] - [[package]] name = "pin-utils" version = "0.1.0" @@ -612,12 +579,6 @@ dependencies = [ "similar", ] -[[package]] -name = "smallvec" -version = "1.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2fd0db749597d91ff862fd1d55ea87f7855a744a8425a64695b6fca237d1dad1" - [[package]] name = "socket2" version = "0.4.4" @@ -811,7 +772,6 @@ dependencies = [ "libloading", "nix", "num_cpus", - "parking_lot", "polling", "rand", "socket2", @@ -910,46 +870,3 @@ name = "winapi-x86_64-pc-windows-gnu" version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" - -[[package]] -name = "windows-sys" -version = "0.36.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ea04155a16a59f9eab786fe12a4a450e75cdb175f9e0d80da1e17db09f55b8d2" -dependencies = [ - "windows_aarch64_msvc", - "windows_i686_gnu", - "windows_i686_msvc", - "windows_x86_64_gnu", - "windows_x86_64_msvc", -] - -[[package]] -name = "windows_aarch64_msvc" -version = "0.36.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9bb8c3fd39ade2d67e9874ac4f3db21f0d710bee00fe7cab16949ec184eeaa47" - -[[package]] -name = "windows_i686_gnu" -version = "0.36.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "180e6ccf01daf4c426b846dfc66db1fc518f074baa793aa7d9b9aaeffad6a3b6" - -[[package]] -name = "windows_i686_msvc" -version = "0.36.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e2e7917148b2812d1eeafaeb22a97e4813dfa60a3f8f78ebe204bcc88f12f024" - -[[package]] -name = "windows_x86_64_gnu" -version = "0.36.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4dcd171b8776c41b97521e5da127a2d86ad280114807d0b2ab1e462bc764d9e1" - -[[package]] -name = "windows_x86_64_msvc" -version = "0.36.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c811ca4a8c853ef420abd8592ba53ddbbac90410fab6903b3e79972a631f7680" diff --git a/vm/Cargo.toml b/vm/Cargo.toml index 824f865e9..ab720a637 100644 --- a/vm/Cargo.toml +++ b/vm/Cargo.toml @@ -14,7 +14,6 @@ libffi-system = ["libffi/system"] [dependencies] num_cpus = "^1.13" -parking_lot = "^0.12" # Newer versions of the `time` crate only expose local time behind a # compile-time --cfg flag. This isn't viable for us, as it makes the compilation diff --git a/vm/src/permanent_space.rs b/vm/src/permanent_space.rs index 7a68198e8..0d22d771e 100644 --- a/vm/src/permanent_space.rs +++ b/vm/src/permanent_space.rs @@ -7,9 +7,9 @@ use crate::mem::{ }; use crate::process::Future; use ahash::AHashMap; -use parking_lot::Mutex; use std::mem::size_of; use std::ops::Drop; +use std::sync::Mutex; pub(crate) const INT_CLASS: usize = 0; pub(crate) const FLOAT_CLASS: usize = 1; @@ -169,7 +169,7 @@ impl PermanentSpace { /// If an Inko String has already been allocated for the given Rust String, /// the existing Inko String is returned; otherwise a new one is created. pub(crate) fn allocate_string(&self, string: String) -> Pointer { - let mut strings = self.interned_strings.lock(); + let mut strings = self.interned_strings.lock().unwrap(); if let Some(ptr) = strings.get(&string) { return *ptr; @@ -190,7 +190,7 @@ impl PermanentSpace { } else { let ptr = ptr.as_permanent(); - self.permanent_objects.lock().push(ptr); + self.permanent_objects.lock().unwrap().push(ptr); ptr } } @@ -198,7 +198,7 @@ impl PermanentSpace { pub(crate) fn allocate_float(&self, value: f64) -> Pointer { let ptr = Float::alloc(self.float_class(), value).as_permanent(); - self.permanent_objects.lock().push(ptr); + self.permanent_objects.lock().unwrap().push(ptr); ptr } @@ -290,11 +290,11 @@ impl PermanentSpace { impl Drop for PermanentSpace { fn drop(&mut self) { unsafe { - for pointer in self.interned_strings.lock().values() { + for pointer in self.interned_strings.lock().unwrap().values() { InkoString::drop_and_deallocate(*pointer); } - for ptr in self.permanent_objects.lock().iter() { + for ptr in self.permanent_objects.lock().unwrap().iter() { ptr.free(); } diff --git a/vm/src/process.rs b/vm/src/process.rs index 594e6cf6d..112ef77ed 100644 --- a/vm/src/process.rs +++ b/vm/src/process.rs @@ -4,7 +4,6 @@ use crate::indexes::{FieldIndex, MethodIndex}; use crate::location_table::Location; use crate::mem::{allocate, ClassPointer, Header, MethodPointer, Pointer}; use crate::scheduler::timeouts::Timeout; -use parking_lot::{Mutex, MutexGuard}; use std::alloc::{alloc, dealloc, handle_alloc_error, Layout}; use std::collections::VecDeque; use std::mem::{align_of, size_of, swap}; @@ -12,6 +11,7 @@ use std::ops::Drop; use std::ops::{Deref, DerefMut}; use std::ptr::{copy_nonoverlapping, drop_in_place, NonNull}; use std::slice; +use std::sync::{Mutex, MutexGuard}; /// The shared state of a future. pub(crate) struct FutureState { @@ -61,7 +61,7 @@ impl RcFutureState { /// The returned tuple contains a value that indicates if a process needs to /// be rescheduled, and a pointer to the process that sent the message. pub(crate) fn write(&self, result: Pointer, thrown: bool) -> WriteResult { - let mut future = self.lock(); + let mut future = self.lock().unwrap(); if future.disconnected { return WriteResult::Discard; @@ -71,7 +71,7 @@ impl RcFutureState { future.result = result; if let Some(consumer) = future.consumer.take() { - match consumer.state.lock().try_reschedule_for_future() { + match consumer.state.lock().unwrap().try_reschedule_for_future() { RescheduleRights::Failed => WriteResult::Continue, RescheduleRights::Acquired => WriteResult::Reschedule(consumer), RescheduleRights::AcquiredWithTimeout => { @@ -96,8 +96,8 @@ impl RcFutureState { // The locking order is important here: we _must_ lock the future // _before_ locking the consumer. If we lock the consumer first, we // could deadlock with processes writing to this future. - let mut future = self.lock(); - let mut state = consumer.state.lock(); + let mut future = self.lock().unwrap(); + let mut state = consumer.state.lock().unwrap(); let result = future.result; if result != Pointer::undefined_singleton() { @@ -636,11 +636,11 @@ impl Process { // here instead of `Process::main` as this makes writing some unit tests // a bit easier. self.header.reset_references(); - self.state.lock().status.set_main(); + self.state.lock().unwrap().status.set_main(); } pub(crate) fn is_main(&self) -> bool { - self.state.lock().status.is_main() + self.state.lock().unwrap().status.is_main() } /// Suspends this process for a period of time. @@ -648,7 +648,7 @@ impl Process { /// A process is sleeping when it simply isn't to be scheduled for a while, /// without waiting for a message, future, or something else. pub(crate) fn suspend(&mut self, timeout: ArcWithoutWeak) { - let mut state = self.state.lock(); + let mut state = self.state.lock().unwrap(); state.timeout = Some(timeout); @@ -665,7 +665,7 @@ impl Process { ) -> RescheduleRights { let write = if wait { Write::Direct(sender) } else { Write::Discard }; let message = Message::new(method, write, arguments); - let mut state = self.state.lock(); + let mut state = self.state.lock().unwrap(); state.mailbox.send(message); state.try_reschedule_for_message() @@ -679,7 +679,7 @@ impl Process { arguments: Vec, ) -> RescheduleRights { let message = Message::new(method, Write::Future(future), arguments); - let mut state = self.state.lock(); + let mut state = self.state.lock().unwrap(); state.mailbox.send(message); state.try_reschedule_for_message() @@ -687,7 +687,7 @@ impl Process { /// Schedules a task to run, if none is scheduled already. pub(crate) fn task_to_run(&mut self) -> Option { - let mut proc_state = self.state.lock(); + let mut proc_state = self.state.lock().unwrap(); if let Some(task) = self.task.as_ref() { return Some(TaskPointer::new(task)); @@ -723,7 +723,7 @@ impl Process { /// Finishes the exection of a task, and decides what to do next with this /// process. pub(crate) fn finish_task(&mut self) -> Finished { - let mut state = self.state.lock(); + let mut state = self.state.lock().unwrap(); self.task.take(); @@ -753,7 +753,7 @@ impl Process { } pub(crate) fn timeout_expired(&self) -> bool { - let mut state = self.state.lock(); + let mut state = self.state.lock().unwrap(); if state.status.timeout_expired() { state.status.set_timeout_expired(false); @@ -764,7 +764,7 @@ impl Process { } pub(crate) fn state(&self) -> MutexGuard { - self.state.lock() + self.state.lock().unwrap() } pub(crate) fn stacktrace(&self) -> Vec { @@ -921,11 +921,11 @@ impl Future { } pub(crate) fn lock(&self) -> MutexGuard { - self.state.lock() + self.state.lock().unwrap() } pub(crate) fn disconnect(&self) -> Pointer { - let mut future = self.state.lock(); + let mut future = self.state.lock().unwrap(); let result = future.result; future.disconnected = true; @@ -1406,7 +1406,7 @@ mod tests { let result = state.write(Pointer::int(42), false); assert_eq!(result, WriteResult::Continue); - assert_eq!(state.lock().thrown, false); + assert_eq!(state.lock().unwrap().thrown, false); } #[test] @@ -1415,14 +1415,14 @@ mod tests { let result = state.write(Pointer::int(42), true); assert_eq!(result, WriteResult::Continue); - assert_eq!(state.lock().thrown, true); + assert_eq!(state.lock().unwrap().thrown, true); } #[test] fn test_future_write_disconnected() { let state = FutureState::new(); - state.lock().disconnected = true; + state.lock().unwrap().disconnected = true; let result = state.write(Pointer::int(42), false); @@ -1435,7 +1435,7 @@ mod tests { let process = OwnedProcess::new(Process::alloc(*proc_class)); let state = FutureState::new(); - state.lock().consumer = Some(*process); + state.lock().unwrap().consumer = Some(*process); let result = state.write(Pointer::int(42), false); @@ -1449,7 +1449,7 @@ mod tests { let state = FutureState::new(); process.state().waiting_for_future(None); - state.lock().consumer = Some(*process); + state.lock().unwrap().consumer = Some(*process); let result = state.write(Pointer::int(42), false); @@ -1464,7 +1464,7 @@ mod tests { let timeout = Timeout::with_rc(Duration::from_secs(0)); process.state().waiting_for_future(Some(timeout)); - state.lock().consumer = Some(*process); + state.lock().unwrap().consumer = Some(*process); let result = state.write(Pointer::int(42), false); @@ -1480,7 +1480,7 @@ mod tests { let state = FutureState::new(); assert_eq!(state.get(*process, None), FutureResult::None); - assert_eq!(state.lock().consumer, Some(*process)); + assert_eq!(state.lock().unwrap().consumer, Some(*process)); assert!(process.state().status.is_waiting_for_future()); } @@ -1492,7 +1492,7 @@ mod tests { let timeout = Timeout::with_rc(Duration::from_secs(0)); assert_eq!(state.get(*process, Some(timeout)), FutureResult::None); - assert_eq!(state.lock().consumer, Some(*process)); + assert_eq!(state.lock().unwrap().consumer, Some(*process)); assert!(process.state().status.is_waiting_for_future()); assert!(process.state().timeout.is_some()); } @@ -1505,10 +1505,10 @@ mod tests { let value = Pointer::int(42); process.state().waiting_for_future(None); - state.lock().result = value; + state.lock().unwrap().result = value; assert_eq!(state.get(*process, None), FutureResult::Returned(value)); - assert!(state.lock().consumer.is_none()); + assert!(state.lock().unwrap().consumer.is_none()); assert!(!process.state().status.is_waiting_for_future()); } @@ -1520,11 +1520,11 @@ mod tests { let value = Pointer::int(42); process.state().waiting_for_future(None); - state.lock().result = value; - state.lock().thrown = true; + state.lock().unwrap().result = value; + state.lock().unwrap().thrown = true; assert_eq!(state.get(*process, None), FutureResult::Thrown(value)); - assert!(state.lock().consumer.is_none()); + assert!(state.lock().unwrap().consumer.is_none()); assert!(!process.state().status.is_waiting_for_future()); } @@ -1535,7 +1535,7 @@ mod tests { let fut = Future::alloc(*fut_class, state.clone()); let result = unsafe { fut.get::() }.disconnect(); - assert!(state.lock().disconnected); + assert!(state.lock().unwrap().disconnected); assert!(result == Pointer::undefined_singleton()); unsafe { diff --git a/vm/src/scheduler/park_group.rs b/vm/src/scheduler/park_group.rs index aa421c7f5..3c74dc14a 100644 --- a/vm/src/scheduler/park_group.rs +++ b/vm/src/scheduler/park_group.rs @@ -1,5 +1,5 @@ //! Parking and waking up of multiple threads. -use parking_lot::{Condvar, Mutex}; +use std::sync::{Condvar, Mutex}; macro_rules! lock_and_notify { ($parker: expr, $method: ident) => { @@ -47,10 +47,10 @@ impl ParkGroup { where F: Fn() -> bool, { - let mut lock = self.mutex.lock(); + let mut lock = self.mutex.lock().unwrap(); while condition() { - self.cvar.wait(&mut lock); + lock = self.cvar.wait(lock).unwrap(); } } } diff --git a/vm/src/state.rs b/vm/src/state.rs index 2392008cf..20bebcdfb 100644 --- a/vm/src/state.rs +++ b/vm/src/state.rs @@ -11,8 +11,8 @@ use crate::network_poller::NetworkPoller; use crate::permanent_space::PermanentSpace; use crate::scheduler::process::Scheduler; use crate::scheduler::timeout_worker::TimeoutWorker; -use parking_lot::Mutex; use std::panic::RefUnwindSafe; +use std::sync::Mutex; use std::time; /// A reference counted State. @@ -84,10 +84,10 @@ impl State { } pub(crate) fn set_exit_status(&self, new_status: i32) { - *self.exit_status.lock() = new_status; + *self.exit_status.lock().unwrap() = new_status; } pub(crate) fn current_exit_status(&self) -> i32 { - *self.exit_status.lock() + *self.exit_status.lock().unwrap() } }