Skip to content

Commit 4a6d2b9

Browse files
committed
refactor(locking): Created locking strategy to control locking mode
This commit introduces a `LockingStrategy` struct and `LockMode` enum to allow more control over the locking strategy used during compilation. This work is done in preparation to enable fine grain locking.
1 parent 6c1b610 commit 4a6d2b9

File tree

7 files changed

+110
-18
lines changed

7 files changed

+110
-18
lines changed

src/cargo/core/compiler/build_runner/mod.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use std::sync::{Arc, Mutex};
66

77
use crate::core::PackageId;
88
use crate::core::compiler::compilation::{self, UnitOutput};
9+
use crate::core::compiler::locking::LockingStrategy;
910
use crate::core::compiler::{self, Unit, artifact};
1011
use crate::util::cache_lock::CacheLockMode;
1112
use crate::util::errors::CargoResult;
@@ -89,6 +90,10 @@ pub struct BuildRunner<'a, 'gctx> {
8990
/// because the target has a type error. This is in an Arc<Mutex<..>>
9091
/// because it is continuously updated as the job progresses.
9192
pub failed_scrape_units: Arc<Mutex<HashSet<UnitHash>>>,
93+
94+
/// The locking mode to use for this build.
95+
/// By default we use coarse grain locking, but disable locking on some filesystems like NFS
96+
pub locking_strategy: LockingStrategy,
9297
}
9398

9499
impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
@@ -111,6 +116,8 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
111116
}
112117
};
113118

119+
let locking_strategy = LockingStrategy::determine_locking_strategy(&bcx.ws)?;
120+
114121
Ok(Self {
115122
bcx,
116123
compilation: Compilation::new(bcx)?,
@@ -128,6 +135,7 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
128135
lto: HashMap::new(),
129136
metadata_for_doc_units: HashMap::new(),
130137
failed_scrape_units: Arc::new(Mutex::new(HashSet::new())),
138+
locking_strategy,
131139
})
132140
}
133141

@@ -360,11 +368,11 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
360368
#[tracing::instrument(skip_all)]
361369
pub fn prepare_units(&mut self) -> CargoResult<()> {
362370
let dest = self.bcx.profiles.get_dir_name();
363-
let host_layout = Layout::new(self.bcx.ws, None, &dest)?;
371+
let host_layout = Layout::new(self.bcx.ws, None, &dest, &self.locking_strategy)?;
364372
let mut targets = HashMap::new();
365373
for kind in self.bcx.all_kinds.iter() {
366374
if let CompileKind::Target(target) = *kind {
367-
let layout = Layout::new(self.bcx.ws, Some(target), &dest)?;
375+
let layout = Layout::new(self.bcx.ws, Some(target), &dest, &self.locking_strategy)?;
368376
targets.insert(target, layout);
369377
}
370378
}

src/cargo/core/compiler/layout.rs

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@
103103
104104
use crate::core::Workspace;
105105
use crate::core::compiler::CompileTarget;
106+
use crate::core::compiler::locking::{LockingMode, LockingStrategy};
106107
use crate::util::{CargoResult, FileLock};
107108
use cargo_util::paths;
108109
use std::path::{Path, PathBuf};
@@ -126,6 +127,7 @@ impl Layout {
126127
ws: &Workspace<'_>,
127128
target: Option<CompileTarget>,
128129
dest: &str,
130+
locking_strategy: &LockingStrategy,
129131
) -> CargoResult<Layout> {
130132
let is_new_layout = ws.gctx().cli_unstable().build_dir_new_layout;
131133
let mut root = ws.target_dir();
@@ -152,18 +154,29 @@ impl Layout {
152154
// For now we don't do any more finer-grained locking on the artifact
153155
// directory, so just lock the entire thing for the duration of this
154156
// compile.
155-
let artifact_dir_lock =
156-
dest.open_rw_exclusive_create(".cargo-lock", ws.gctx(), "build directory")?;
157-
158-
let build_dir_lock = if root != build_root {
159-
Some(build_dest.open_rw_exclusive_create(
157+
let artifact_dir_lock = match locking_strategy.artifact_dir() {
158+
LockingMode::Disabled => None,
159+
LockingMode::Coarse => Some(dest.open_rw_exclusive_create(
160160
".cargo-lock",
161161
ws.gctx(),
162-
"build directory",
163-
)?)
162+
"artifact directory",
163+
)?),
164+
};
165+
166+
assert_eq!(locking_strategy.is_unified_output_dir(), root == build_root);
167+
let build_dir_lock = if !locking_strategy.is_unified_output_dir() {
168+
match locking_strategy.build_dir() {
169+
LockingMode::Disabled => None,
170+
LockingMode::Coarse => Some(build_dest.open_rw_exclusive_create(
171+
".cargo-lock",
172+
ws.gctx(),
173+
"build directory",
174+
)?),
175+
}
164176
} else {
165177
None
166178
};
179+
167180
let root = root.into_path_unlocked();
168181
let build_root = build_root.into_path_unlocked();
169182
let dest = dest.into_path_unlocked();
@@ -222,7 +235,7 @@ pub struct ArtifactDirLayout {
222235
timings: PathBuf,
223236
/// The lockfile for a build (`.cargo-lock`). Will be unlocked when this
224237
/// struct is `drop`ped.
225-
_lock: FileLock,
238+
_lock: Option<FileLock>,
226239
}
227240

228241
impl ArtifactDirLayout {

src/cargo/core/compiler/locking.rs

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
use tracing::debug;
2+
3+
use crate::{CargoResult, core::Workspace, util::flock::is_on_nfs_mount};
4+
5+
/// The strategy to use for locking during a build.
6+
///
7+
/// A strategy is made up of multiple locking modes for different directories.
8+
#[derive(Debug)]
9+
pub struct LockingStrategy {
10+
/// The locking mode for the artifact-dir
11+
artifact_dir: LockingMode,
12+
/// The locking mode for the build-dir
13+
///
14+
/// Will be `None` when artifact_dir and build_dir are the same directory.
15+
build_dir: Option<LockingMode>,
16+
}
17+
18+
impl LockingStrategy {
19+
/// Determines the locking strategy the current environment can support.
20+
pub fn determine_locking_strategy(ws: &Workspace<'_>) -> CargoResult<Self> {
21+
let artifact_dir_locking_mode = match is_on_nfs_mount(ws.target_dir().as_path_unlocked()) {
22+
true => {
23+
debug!("NFS detected. Disabling file system locking for artifact-dir");
24+
LockingMode::Disabled
25+
}
26+
false => LockingMode::Coarse,
27+
};
28+
let build_dir_locking_mode = if ws.target_dir() == ws.build_dir() {
29+
None
30+
} else {
31+
Some(match is_on_nfs_mount(ws.build_dir().as_path_unlocked()) {
32+
true => {
33+
debug!("NFS detected. Disabling file system locking for build-dir");
34+
LockingMode::Disabled
35+
}
36+
false => LockingMode::Coarse,
37+
})
38+
};
39+
Ok(Self {
40+
artifact_dir: artifact_dir_locking_mode,
41+
build_dir: build_dir_locking_mode,
42+
})
43+
}
44+
45+
pub fn artifact_dir(&self) -> &LockingMode {
46+
&self.artifact_dir
47+
}
48+
49+
pub fn build_dir(&self) -> &LockingMode {
50+
self.build_dir.as_ref().unwrap_or(&self.artifact_dir)
51+
}
52+
53+
/// If the artifact_dir and build_dir are the same directory.
54+
pub fn is_unified_output_dir(&self) -> bool {
55+
self.build_dir.is_none()
56+
}
57+
}
58+
59+
/// The locking mode that will be used for output directories.
60+
#[derive(Debug)]
61+
pub enum LockingMode {
62+
/// Completely disables locking (used for filesystems that do not support locking)
63+
Disabled,
64+
/// Coarse grain locking (Profile level)
65+
Coarse,
66+
}

src/cargo/core/compiler/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ pub mod future_incompat;
4545
pub(crate) mod job_queue;
4646
pub(crate) mod layout;
4747
mod links;
48+
pub mod locking;
4849
mod lto;
4950
mod output_depinfo;
5051
mod output_sbom;

src/cargo/ops/cargo_clean.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use crate::core::compiler::locking::LockingStrategy;
12
use crate::core::compiler::{CompileKind, CompileMode, Layout, RustcTargetData};
23
use crate::core::profiles::Profiles;
34
use crate::core::{PackageIdSpec, PackageIdSpecQuery, TargetKind, Workspace};
@@ -116,15 +117,18 @@ fn clean_specs(
116117
let target_data = RustcTargetData::new(ws, &requested_kinds)?;
117118
let (pkg_set, resolve) = ops::resolve_ws(ws, dry_run)?;
118119
let prof_dir_name = profiles.get_dir_name();
119-
let host_layout = Layout::new(ws, None, &prof_dir_name)?;
120+
let locking_strategy = LockingStrategy::determine_locking_strategy(&ws)?;
121+
let host_layout = Layout::new(ws, None, &prof_dir_name, &locking_strategy)?;
120122
// Convert requested kinds to a Vec of layouts.
121123
let target_layouts: Vec<(CompileKind, Layout)> = requested_kinds
122124
.into_iter()
123125
.filter_map(|kind| match kind {
124-
CompileKind::Target(target) => match Layout::new(ws, Some(target), &prof_dir_name) {
125-
Ok(layout) => Some(Ok((kind, layout))),
126-
Err(e) => Some(Err(e)),
127-
},
126+
CompileKind::Target(target) => {
127+
match Layout::new(ws, Some(target), &prof_dir_name, &locking_strategy) {
128+
Ok(layout) => Some(Ok((kind, layout))),
129+
Err(e) => Some(Err(e)),
130+
}
131+
}
128132
CompileKind::Host => None,
129133
})
130134
.collect::<CargoResult<_>>()?;

src/cargo/util/flock.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,7 @@ fn acquire(
427427
}
428428

429429
#[cfg(all(target_os = "linux", not(target_env = "musl")))]
430-
fn is_on_nfs_mount(path: &Path) -> bool {
430+
pub fn is_on_nfs_mount(path: &Path) -> bool {
431431
use std::ffi::CString;
432432
use std::mem;
433433
use std::os::unix::prelude::*;
@@ -445,7 +445,7 @@ fn is_on_nfs_mount(path: &Path) -> bool {
445445
}
446446

447447
#[cfg(any(not(target_os = "linux"), target_env = "musl"))]
448-
fn is_on_nfs_mount(_path: &Path) -> bool {
448+
pub fn is_on_nfs_mount(_path: &Path) -> bool {
449449
false
450450
}
451451

src/cargo/util/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ mod dependency_queue;
4242
pub mod diagnostic_server;
4343
pub mod edit_distance;
4444
pub mod errors;
45-
mod flock;
45+
pub mod flock;
4646
pub mod frontmatter;
4747
pub mod graph;
4848
mod hasher;

0 commit comments

Comments
 (0)