Skip to content

Commit cb0fe69

Browse files
authored
perf: cache Windows Store normalized symlinks (#418)
Summary: - Cache Windows Store environments with pre-normalized symlinks at discovery/cache-fill time. - Update `try_from` to compare incoming normalized candidates against cached normalized symlinks. - Preserve cached entries through refresh-state sync and add tests for cached/incoming extended-path prefix matching. Validation: - cargo test -p pet-windows-store - cargo fmt --all - cargo clippy --all -- -D warnings Fixes #404
1 parent 3ad9399 commit cb0fe69

1 file changed

Lines changed: 150 additions & 62 deletions

File tree

  • crates/pet-windows-store/src

crates/pet-windows-store/src/lib.rs

Lines changed: 150 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,49 @@ use pet_core::LocatorKind;
1515
use pet_core::{
1616
os_environment::Environment, Locator, RefreshStatePersistence, RefreshStateSyncScope,
1717
};
18-
use std::path::Path;
18+
#[cfg(any(windows, test))]
19+
use pet_fs::path::norm_case;
20+
use std::path::{Path, PathBuf};
1921
use std::sync::{Arc, RwLock};
2022

23+
#[derive(Clone, Debug)]
24+
#[cfg_attr(not(any(windows, test)), allow(dead_code))]
25+
struct CachedStoreEnvironment {
26+
environment: PythonEnvironment,
27+
normalized_symlinks: Vec<PathBuf>,
28+
}
29+
30+
impl CachedStoreEnvironment {
31+
#[cfg(any(windows, test))]
32+
fn from_environment(environment: PythonEnvironment) -> Self {
33+
let normalized_symlinks = environment
34+
.symlinks
35+
.as_deref()
36+
.unwrap_or_default()
37+
.iter()
38+
.map(|path| normalize_for_comparison(path))
39+
.collect();
40+
41+
Self {
42+
environment,
43+
normalized_symlinks,
44+
}
45+
}
46+
}
47+
48+
#[cfg(any(windows, test))]
49+
fn normalize_for_comparison(path: &Path) -> PathBuf {
50+
let normalized = norm_case(path);
51+
let path_str = normalized.to_string_lossy();
52+
if let Some(unc_path) = path_str.strip_prefix(r"\\?\UNC\") {
53+
PathBuf::from(format!(r"\\{unc_path}"))
54+
} else if let Some(path_without_prefix) = path_str.strip_prefix(r"\\?\") {
55+
PathBuf::from(path_without_prefix)
56+
} else {
57+
normalized
58+
}
59+
}
60+
2161
pub fn is_windows_app_folder_in_program_files(path: &Path) -> bool {
2262
let path = path.to_str().unwrap_or_default().to_ascii_lowercase();
2363
path.get(1..)
@@ -27,7 +67,7 @@ pub fn is_windows_app_folder_in_program_files(path: &Path) -> bool {
2767
pub struct WindowsStore {
2868
pub env_vars: EnvVariables,
2969
#[allow(dead_code)]
30-
environments: Arc<RwLock<Option<Vec<PythonEnvironment>>>>,
70+
environments: Arc<RwLock<Option<Arc<Vec<CachedStoreEnvironment>>>>>,
3171
}
3272

3373
impl WindowsStore {
@@ -38,7 +78,7 @@ impl WindowsStore {
3878
}
3979
}
4080
#[cfg(windows)]
41-
fn find_with_cache(&self) -> Option<Vec<PythonEnvironment>> {
81+
fn find_with_cache(&self) -> Option<Arc<Vec<CachedStoreEnvironment>>> {
4282
// First check if we have cached results
4383
{
4484
let environments = self.environments.read().unwrap();
@@ -47,7 +87,13 @@ impl WindowsStore {
4787
}
4888
}
4989

50-
let envs = list_store_pythons(&self.env_vars).unwrap_or_default();
90+
let envs = Arc::new(
91+
list_store_pythons(&self.env_vars)
92+
.unwrap_or_default()
93+
.into_iter()
94+
.map(CachedStoreEnvironment::from_environment)
95+
.collect::<Vec<_>>(),
96+
);
5197
self.environments.write().unwrap().replace(envs.clone());
5298
Some(envs)
5399
}
@@ -96,26 +142,13 @@ impl Locator for WindowsStore {
96142

97143
#[cfg(windows)]
98144
fn try_from(&self, env: &PythonEnv) -> Option<PythonEnvironment> {
99-
use std::path::PathBuf;
100-
101145
use pet_core::python_environment::PythonEnvironmentBuilder;
102-
use pet_fs::path::norm_case;
103146
use pet_virtualenv::is_virtualenv;
104147

105-
// Helper to normalize paths for comparison by stripping \\?\ prefix
106-
fn normalize_for_comparison(path: &PathBuf) -> PathBuf {
107-
let normalized = norm_case(path);
108-
let path_str = normalized.to_string_lossy();
109-
if path_str.starts_with(r"\\?\") {
110-
PathBuf::from(path_str.trim_start_matches(r"\\?\"))
111-
} else {
112-
normalized
113-
}
114-
}
115-
116-
// Assume we create a virtual env from a python install,
117-
// Then the exe in the virtual env bin will be a symlink to the homebrew python install.
118-
// Hence the first part of the condition will be true, but the second part will be false.
148+
// A virtual environment created from a Windows Store Python may still have an
149+
// executable path or symlink chain that resolves back to the base Store install.
150+
// Even in that case, the environment itself is a virtualenv and should not be
151+
// classified as a Windows Store environment here.
119152
if is_virtualenv(env) {
120153
return None;
121154
}
@@ -126,15 +159,12 @@ impl Locator for WindowsStore {
126159
.map(|p| normalize_for_comparison(&p))
127160
.collect();
128161
if let Some(environments) = self.find_with_cache() {
129-
for found_env in environments {
130-
if let Some(symlinks) = &found_env.symlinks {
131-
// Normalize symlinks for comparison
132-
let normalized_symlinks: Vec<PathBuf> =
133-
symlinks.iter().map(normalize_for_comparison).collect();
162+
for found_env in environments.iter() {
163+
if !found_env.normalized_symlinks.is_empty() {
134164
// Check if we have found this exe.
135165
if list_of_possible_exes
136166
.iter()
137-
.any(|exe| normalized_symlinks.contains(exe))
167+
.any(|exe| found_env.normalized_symlinks.contains(exe))
138168
{
139169
// Its possible the env discovery was not aware of the symlink
140170
// E.g. if we are asked to resolve `../WindowsApp/python.exe`
@@ -143,8 +173,10 @@ impl Locator for WindowsStore {
143173
// However the env found by the locator will almost never contain python.exe nor python3.exe
144174
// See README.md
145175
// As a result, we need to add those symlinks here.
146-
let builder = PythonEnvironmentBuilder::from_environment(found_env.clone())
147-
.symlinks(env.symlinks.clone());
176+
let builder = PythonEnvironmentBuilder::from_environment(
177+
found_env.environment.clone(),
178+
)
179+
.symlinks(env.symlinks.clone());
148180
return Some(builder.build());
149181
}
150182
}
@@ -164,7 +196,7 @@ impl Locator for WindowsStore {
164196
if let Some(environments) = self.find_with_cache() {
165197
environments
166198
.iter()
167-
.for_each(|e| reporter.report_environment(e))
199+
.for_each(|e| reporter.report_environment(&e.environment))
168200
}
169201
}
170202

@@ -179,6 +211,13 @@ mod tests {
179211
use super::*;
180212
use pet_core::os_environment::EnvironmentApi;
181213

214+
fn cached_environment(name: &str) -> CachedStoreEnvironment {
215+
CachedStoreEnvironment::from_environment(PythonEnvironment {
216+
name: Some(name.to_string()),
217+
..Default::default()
218+
})
219+
}
220+
182221
#[test]
183222
fn windows_store_reports_kind_supported_categories_and_refresh_state() {
184223
let environment = EnvironmentApi::new();
@@ -206,6 +245,76 @@ mod tests {
206245
assert!(!is_windows_app_folder_in_program_files(Path::new("")));
207246
}
208247

248+
#[test]
249+
fn cached_store_environment_normalizes_symlinks_once() {
250+
let cached = CachedStoreEnvironment::from_environment(PythonEnvironment {
251+
symlinks: Some(vec![PathBuf::from(r"\\?\C:\Users\User\python.exe")]),
252+
..Default::default()
253+
});
254+
255+
assert_eq!(
256+
cached.normalized_symlinks,
257+
vec![PathBuf::from(r"C:\Users\User\python.exe")]
258+
);
259+
}
260+
261+
#[test]
262+
fn cached_store_environment_normalizes_extended_unc_symlinks() {
263+
let cached = CachedStoreEnvironment::from_environment(PythonEnvironment {
264+
symlinks: Some(vec![PathBuf::from(r"\\?\UNC\server\share\python.exe")]),
265+
..Default::default()
266+
});
267+
268+
assert_eq!(
269+
cached.normalized_symlinks,
270+
vec![PathBuf::from(r"\\server\share\python.exe")]
271+
);
272+
}
273+
274+
#[cfg(windows)]
275+
#[test]
276+
fn try_from_matches_cached_normalized_symlink() {
277+
let environment = EnvironmentApi::new();
278+
let locator = WindowsStore::from(&environment);
279+
let symlink =
280+
PathBuf::from(r"C:\Users\User\AppData\Local\Microsoft\WindowsApps\python3.11.exe");
281+
let store_environment = PythonEnvironment {
282+
kind: Some(PythonEnvironmentKind::WindowsStore),
283+
symlinks: Some(vec![PathBuf::from(format!(r"\\?\{}", symlink.display()))]),
284+
..Default::default()
285+
};
286+
locator.environments.write().unwrap().replace(Arc::new(vec![
287+
CachedStoreEnvironment::from_environment(store_environment),
288+
]));
289+
290+
let mut env = PythonEnv::new(symlink.clone(), None, None);
291+
env.symlinks = Some(vec![symlink]);
292+
293+
assert!(locator.try_from(&env).is_some());
294+
}
295+
296+
#[cfg(windows)]
297+
#[test]
298+
fn try_from_normalizes_incoming_extended_prefix_symlink() {
299+
let environment = EnvironmentApi::new();
300+
let locator = WindowsStore::from(&environment);
301+
let symlink =
302+
PathBuf::from(r"C:\Users\User\AppData\Local\Microsoft\WindowsApps\python3.11.exe");
303+
let store_environment = PythonEnvironment {
304+
kind: Some(PythonEnvironmentKind::WindowsStore),
305+
symlinks: Some(vec![symlink.clone()]),
306+
..Default::default()
307+
};
308+
locator.environments.write().unwrap().replace(Arc::new(vec![
309+
CachedStoreEnvironment::from_environment(store_environment),
310+
]));
311+
312+
let mut env = PythonEnv::new(symlink.clone(), None, None);
313+
env.symlinks = Some(vec![PathBuf::from(format!(r"\\?\{}", symlink.display()))]);
314+
315+
assert!(locator.try_from(&env).is_some());
316+
}
317+
209318
#[test]
210319
fn test_full_refresh_sync_replaces_store_cache() {
211320
let environment = EnvironmentApi::new();
@@ -216,23 +325,17 @@ mod tests {
216325
.environments
217326
.write()
218327
.unwrap()
219-
.replace(vec![PythonEnvironment {
220-
name: Some("stale".to_string()),
221-
..Default::default()
222-
}]);
328+
.replace(Arc::new(vec![cached_environment("stale")]));
223329
refreshed
224330
.environments
225331
.write()
226332
.unwrap()
227-
.replace(vec![PythonEnvironment {
228-
name: Some("fresh".to_string()),
229-
..Default::default()
230-
}]);
333+
.replace(Arc::new(vec![cached_environment("fresh")]));
231334

232335
shared.sync_refresh_state_from(&refreshed, &RefreshStateSyncScope::Full);
233336

234337
let result = shared.environments.read().unwrap().clone().unwrap();
235-
assert_eq!(result[0].name.as_deref(), Some("fresh"));
338+
assert_eq!(result[0].environment.name.as_deref(), Some("fresh"));
236339
}
237340

238341
#[test]
@@ -245,23 +348,17 @@ mod tests {
245348
.environments
246349
.write()
247350
.unwrap()
248-
.replace(vec![PythonEnvironment {
249-
name: Some("stale".to_string()),
250-
..Default::default()
251-
}]);
351+
.replace(Arc::new(vec![cached_environment("stale")]));
252352
refreshed
253353
.environments
254354
.write()
255355
.unwrap()
256-
.replace(vec![PythonEnvironment {
257-
name: Some("fresh".to_string()),
258-
..Default::default()
259-
}]);
356+
.replace(Arc::new(vec![cached_environment("fresh")]));
260357

261358
shared.sync_refresh_state_from(&refreshed, &RefreshStateSyncScope::Workspace);
262359

263360
let result = shared.environments.read().unwrap().clone().unwrap();
264-
assert_eq!(result[0].name.as_deref(), Some("stale"));
361+
assert_eq!(result[0].environment.name.as_deref(), Some("stale"));
265362
}
266363

267364
#[test]
@@ -274,39 +371,30 @@ mod tests {
274371
.environments
275372
.write()
276373
.unwrap()
277-
.replace(vec![PythonEnvironment {
278-
name: Some("stale".to_string()),
279-
..Default::default()
280-
}]);
374+
.replace(Arc::new(vec![cached_environment("stale")]));
281375
refreshed
282376
.environments
283377
.write()
284378
.unwrap()
285-
.replace(vec![PythonEnvironment {
286-
name: Some("fresh".to_string()),
287-
..Default::default()
288-
}]);
379+
.replace(Arc::new(vec![cached_environment("fresh")]));
289380

290381
shared.sync_refresh_state_from(
291382
&refreshed,
292383
&RefreshStateSyncScope::GlobalFiltered(PythonEnvironmentKind::WindowsStore),
293384
);
294385
let result = shared.environments.read().unwrap().clone().unwrap();
295-
assert_eq!(result[0].name.as_deref(), Some("fresh"));
386+
assert_eq!(result[0].environment.name.as_deref(), Some("fresh"));
296387

297388
shared
298389
.environments
299390
.write()
300391
.unwrap()
301-
.replace(vec![PythonEnvironment {
302-
name: Some("stale".to_string()),
303-
..Default::default()
304-
}]);
392+
.replace(Arc::new(vec![cached_environment("stale")]));
305393
shared.sync_refresh_state_from(
306394
&refreshed,
307395
&RefreshStateSyncScope::GlobalFiltered(PythonEnvironmentKind::Conda),
308396
);
309397
let result = shared.environments.read().unwrap().clone().unwrap();
310-
assert_eq!(result[0].name.as_deref(), Some("stale"));
398+
assert_eq!(result[0].environment.name.as_deref(), Some("stale"));
311399
}
312400
}

0 commit comments

Comments
 (0)