Skip to content

Commit 9989d5a

Browse files
authored
chore: Enforce usage of parking_lot (#3325)
Mostly because of better ergonomics, but also has better performance in some cases and some useful additional features. Signed-off-by: Adam Gutglick <[email protected]>
1 parent 95081ab commit 9989d5a

File tree

21 files changed

+61
-84
lines changed

21 files changed

+61
-84
lines changed

.cargo/config.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ rustflags = ["-C", "link-arg=-undefined", "-C", "link-arg=dynamic_lookup"]
77
[target.x86_64-apple-darwin]
88
rustflags = ["-C", "link-arg=-undefined", "-C", "link-arg=dynamic_lookup"]
99
[target.wasm32-unknown-unknown]
10-
rustflags = ['--cfg', 'getrandom_backend="wasm_js"']
10+
rustflags = ['--cfg', 'getrandom_backend="wasm_js"', '-C', 'target-feature=+atomics']
1111

1212
[alias]
1313
xtask = "run -p xtask --"

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,7 @@ jobs:
419419
targets: "wasm32-wasip1"
420420
- name: Setup Wasmer
421421
uses: wasmerio/[email protected]
422-
- run: cargo build --target wasm32-wasip1
422+
- run: cargo -Zbuild-std=panic_abort,std build --target wasm32-wasip1
423423
working-directory: ./wasm-test
424424
- run: wasmer run ./target/wasm32-wasip1/debug/wasm-test.wasm
425425
working-directory: ./wasm-test

Cargo.lock

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,6 @@ object_store = "0.12.1"
122122
opentelemetry = "0.29.0"
123123
opentelemetry-otlp = "0.29.0"
124124
opentelemetry_sdk = "0.29.0"
125-
parking_lot = "0.12.3"
126125
parquet = "55"
127126
paste = "1.0.15"
128127
pin-project = "1.1.5"
@@ -134,6 +133,7 @@ pyo3 = { version = "0.24.1", features = ["extension-module", "abi3-py310"] }
134133
pyo3-log = "0.12.1"
135134
rancor = "0.1.0"
136135
rand = "0.9.0"
136+
parking_lot = { version = "0.12.3", features = ["nightly"] }
137137
rand_distr = "0.5"
138138
ratatui = "0.29"
139139
rayon = "1.10.0"

clippy.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,5 @@ allow-dbg-in-tests = true
22
allow-expect-in-tests = true
33
allow-unwrap-in-tests = true
44
single-char-binding-names-threshold = 2
5-
disallowed-types = ["std::collections::HashMap", "std::collections::HashSet"]
5+
# We prefer using parking_lot for improved ergonomics and performance.
6+
disallowed-types = ["std::collections::HashMap", "std::collections::HashSet", "std::sync::Mutex", "std::sync::RwLock"]

pyvortex/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ itertools = { workspace = true }
2929
log = { workspace = true }
3030
mimalloc = { workspace = true }
3131
object_store = { workspace = true, features = ["aws", "gcp", "azure", "http"] }
32+
parking_lot = { workspace = true }
3233
pyo3 = { workspace = true, features = ["abi3-py311"] }
3334
pyo3-log = { workspace = true }
3435
tokio = { workspace = true, features = ["fs", "rt-multi-thread"] }

pyvortex/src/iter/mod.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,15 @@ mod python;
22
pub(crate) mod stream;
33

44
use std::iter;
5-
use std::sync::Mutex;
65

76
use arrow::array::RecordBatchReader;
87
use arrow::pyarrow::IntoPyArrow;
8+
use parking_lot::Mutex;
99
use pyo3::prelude::*;
1010
use pyo3::types::PyIterator;
1111
use pyo3::{Bound, PyResult, Python};
1212
pub(crate) use stream::*;
1313
use vortex::dtype::DType;
14-
use vortex::error::VortexExpect;
1514
use vortex::iter::{ArrayIterator, ArrayIteratorAdapter, ArrayIteratorExt};
1615
use vortex::{Canonical, IntoArray};
1716

@@ -51,7 +50,7 @@ impl PyArrayIterator {
5150
}
5251

5352
pub fn take(&self) -> Option<Box<dyn ArrayIterator + Send>> {
54-
self.iter.lock().vortex_expect("poisoned lock").take()
53+
self.iter.lock().take()
5554
}
5655
}
5756

@@ -75,7 +74,6 @@ impl PyArrayIterator {
7574
Ok(self
7675
.iter
7776
.lock()
78-
.vortex_expect("poisoned lock")
7977
.as_mut()
8078
.and_then(|iter| iter.next())
8179
.transpose()?
@@ -87,7 +85,7 @@ impl PyArrayIterator {
8785
/// this will be a :class:`vortex.ChunkedArray`, otherwise it will be a single array.
8886
fn read_all(&self, py: Python) -> PyResult<PyArrayRef> {
8987
let array = py.allow_threads(|| {
90-
if let Some(iter) = self.iter.lock().vortex_expect("poisoned lock").take() {
88+
if let Some(iter) = self.iter.lock().take() {
9189
iter.read_all()
9290
} else {
9391
// Otherwise, we continue to return an empty array.

pyvortex/src/registry.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
use std::sync::{Arc, RwLock};
1+
use std::sync::Arc;
22

33
use itertools::Itertools;
4+
use parking_lot::RwLock;
45
use pyo3::prelude::*;
56
use pyo3::{Bound, PyResult, Python};
67
use vortex::ArrayRegistry;
7-
use vortex::error::VortexExpect;
88
use vortex::file::DEFAULT_REGISTRY;
99
use vortex::layout::{LayoutRegistry, LayoutRegistryExt};
1010

@@ -58,16 +58,13 @@ impl PyRegistry {
5858
/// It's not currently possible to register a layout encoding from Python.
5959
pub(crate) fn register(&self, cls: PythonEncoding) -> PyResult<()> {
6060
let encoding = cls.to_encoding();
61-
self.array_registry
62-
.write()
63-
.vortex_expect("poisoned lock")
64-
.register(encoding);
61+
self.array_registry.write().register(encoding);
6562
Ok(())
6663
}
6764

6865
/// Create an :class:`~vortex.ArrayContext` containing the given encodings.
6966
fn array_ctx(&self, encodings: Vec<Bound<PyAny>>) -> PyResult<PyArrayContext> {
70-
let registry = self.array_registry.read().vortex_expect("poisoned lock");
67+
let registry = self.array_registry.read();
7168
Ok(PyArrayContext::from(registry.new_context(
7269
encoding_ids(&encodings)?.iter().map(|s| s.as_str()),
7370
)?))

vortex-array/src/compute/mod.rs

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
99
use std::any::{Any, type_name};
1010
use std::fmt::{Debug, Formatter};
11-
use std::sync::RwLock;
1211

1312
use arcref::ArcRef;
1413
pub use between::*;
@@ -27,10 +26,11 @@ pub use mask::*;
2726
pub use min_max::*;
2827
pub use nan_count::*;
2928
pub use numeric::*;
29+
use parking_lot::RwLock;
3030
pub use sum::*;
3131
pub use take::*;
3232
use vortex_dtype::DType;
33-
use vortex_error::{VortexError, VortexExpect, VortexResult, vortex_bail, vortex_err};
33+
use vortex_error::{VortexError, VortexResult, vortex_bail, vortex_err};
3434
use vortex_mask::Mask;
3535
use vortex_scalar::Scalar;
3636

@@ -84,10 +84,7 @@ impl ComputeFn {
8484

8585
/// Register a kernel for the compute function.
8686
pub fn register_kernel(&self, kernel: ArcRef<dyn Kernel>) {
87-
self.kernels
88-
.write()
89-
.vortex_expect("poisoned lock")
90-
.push(kernel);
87+
self.kernels.write().push(kernel);
9188
}
9289

9390
/// Invokes the compute function with the given arguments.
@@ -112,9 +109,7 @@ impl ComputeFn {
112109
let expected_dtype = self.vtable.return_dtype(args)?;
113110
let expected_len = self.vtable.return_len(args)?;
114111

115-
let output = self
116-
.vtable
117-
.invoke(args, &self.kernels.read().vortex_expect("poisoned lock"))?;
112+
let output = self.vtable.invoke(args, &self.kernels.read())?;
118113

119114
if output.dtype() != &expected_dtype {
120115
vortex_bail!(
@@ -154,7 +149,7 @@ impl ComputeFn {
154149

155150
/// Returns the compute function's kernels.
156151
pub fn kernels(&self) -> Vec<ArcRef<dyn Kernel>> {
157-
self.kernels.read().vortex_expect("poisoned lock").to_vec()
152+
self.kernels.read().to_vec()
158153
}
159154
}
160155

vortex-array/src/context.rs

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
use std::fmt::Display;
2-
use std::sync::{Arc, RwLock, RwLockReadGuard};
2+
use std::sync::Arc;
33

44
use itertools::Itertools;
5+
use parking_lot::RwLock;
56
use vortex_error::{VortexExpect, VortexResult, vortex_err};
67

78
use crate::EncodingRef;
@@ -56,7 +57,7 @@ impl<T: Clone + Eq> VTableContext<T> {
5657

5758
pub fn with(self, encoding: T) -> Self {
5859
{
59-
let mut write = self.0.write().vortex_expect("poisoned lock");
60+
let mut write = self.0.write();
6061
if write.iter().all(|e| e != &encoding) {
6162
write.push(encoding);
6263
}
@@ -68,13 +69,13 @@ impl<T: Clone + Eq> VTableContext<T> {
6869
items.into_iter().fold(self, |ctx, e| ctx.with(e))
6970
}
7071

71-
pub fn encodings(&self) -> RwLockReadGuard<Vec<T>> {
72-
self.0.read().vortex_expect("poisoned lock")
72+
pub fn encodings(&self) -> Vec<T> {
73+
self.0.read().clone()
7374
}
7475

7576
/// Returns the index of the encoding in the context, or adds it if it doesn't exist.
7677
pub fn encoding_idx(&self, encoding: &T) -> u16 {
77-
let mut write = self.0.write().vortex_expect("poisoned lock");
78+
let mut write = self.0.write();
7879
if let Some(idx) = write.iter().position(|e| e == encoding) {
7980
return u16::try_from(idx).vortex_expect("Cannot have more than u16::MAX encodings");
8081
}
@@ -88,11 +89,7 @@ impl<T: Clone + Eq> VTableContext<T> {
8889

8990
/// Find an encoding by its position.
9091
pub fn lookup_encoding(&self, idx: u16) -> Option<T> {
91-
self.0
92-
.read()
93-
.vortex_expect("poisoned lock")
94-
.get(idx as usize)
95-
.cloned()
92+
self.0.read().get(idx as usize).cloned()
9693
}
9794
}
9895

0 commit comments

Comments
 (0)