From 3e2cf5d7c4662d0e2adafedda10d2a4f09cf0c63 Mon Sep 17 00:00:00 2001 From: cake-monotone Date: Tue, 18 Mar 2025 17:09:57 +0900 Subject: [PATCH] [red-knot] Improve property test performance by cloning db instead of holding `MutexGuard` (#16823) ## Summary This PR brings an optimization. - `get_cached_db` no longer returns a `MutexGuard`; instead, it returns a cloned database. ### `get_cached_db` Previously, the `MutexGuard` was held inside the property test function (defined in the macro), which prevented multiple property tests from running in parallel. More specifically, the program could only test one random test case at a time, which likely caused a significant bottleneck. On my local machine, running: ``` QUICKCHECK_TESTS=100000 cargo test --release -p red_knot_python_semantic -- --ignored stable ``` showed about **a 75% speedup** (from \~60s to \~15s). --- .../red_knot_python_semantic/src/types/property_tests.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types/property_tests.rs b/crates/red_knot_python_semantic/src/types/property_tests.rs index 2614ec490b..ecc64b24c0 100644 --- a/crates/red_knot_python_semantic/src/types/property_tests.rs +++ b/crates/red_knot_python_semantic/src/types/property_tests.rs @@ -24,7 +24,7 @@ //! --ignored types::property_tests::stable; do :; done //! ``` -use std::sync::{Arc, Mutex, MutexGuard, OnceLock}; +use std::sync::{Arc, Mutex, OnceLock}; use crate::db::tests::{setup_db, TestDb}; use crate::symbol::{builtins_symbol, known_module_symbol}; @@ -327,9 +327,9 @@ impl Arbitrary for Ty { static CACHED_DB: OnceLock>> = OnceLock::new(); -fn get_cached_db() -> MutexGuard<'static, TestDb> { +fn get_cached_db() -> TestDb { let db = CACHED_DB.get_or_init(|| Arc::new(Mutex::new(setup_db()))); - db.lock().unwrap() + db.lock().unwrap().clone() } /// A macro to define a property test for types. @@ -348,8 +348,7 @@ macro_rules! type_property_test { #[quickcheck_macros::quickcheck] #[ignore] fn $test_name($($types: super::Ty),+) -> bool { - let db_cached = super::get_cached_db(); - let $db = &*db_cached; + let $db = &super::get_cached_db(); $(let $types = $types.into_type($db);)+ $property