Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions crates/ruff_benchmark/benches/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1408,6 +1408,7 @@ struct ProjectBenchmark<'a> {
project: InstalledProject<'a>,
fs: MemoryFileSystem,
max_diagnostics: usize,
freeze_inputs: bool,
}

impl<'a> ProjectBenchmark<'a> {
Expand All @@ -1421,9 +1422,15 @@ impl<'a> ProjectBenchmark<'a> {
project: setup_project,
fs,
max_diagnostics,
freeze_inputs: false,
}
}

fn freeze_inputs(mut self) -> Self {
self.freeze_inputs = true;
self
}

fn setup_iteration(&self) -> ProjectDatabase {
let system = TestSystem::new(InMemorySystem::from_memory_fs(self.fs.clone()));

Expand All @@ -1450,12 +1457,25 @@ impl<'a> ProjectBenchmark<'a> {
.collect(),
);

if self.freeze_inputs {
db.freeze();
}

db
}
}

#[track_caller]
fn bench_project(benchmark: &ProjectBenchmark, criterion: &mut Criterion) {
bench_project_named(benchmark, criterion, benchmark.project.config.name);
}

#[track_caller]
fn bench_project_named(
benchmark: &ProjectBenchmark,
criterion: &mut Criterion,
benchmark_name: &str,
) {
fn check_project(db: &mut ProjectDatabase, project_name: &str, max_diagnostics: usize) {
let result = db.check();
let diagnostics = result.len();
Expand All @@ -1477,10 +1497,10 @@ fn bench_project(benchmark: &ProjectBenchmark, criterion: &mut Criterion) {

let mut group = criterion.benchmark_group("project");
group.sampling_mode(criterion::SamplingMode::Flat);
group.bench_function(benchmark.project.config.name, |b| {
group.bench_function(benchmark_name, |b| {
b.iter_batched_ref(
|| benchmark.setup_iteration(),
|db| check_project(db, benchmark.project.config.name, benchmark.max_diagnostics),
|db| check_project(db, benchmark_name, benchmark.max_diagnostics),
BatchSize::SmallInput,
);
});
Expand Down Expand Up @@ -1552,6 +1572,10 @@ fn datetype(criterion: &mut Criterion) {
);

bench_project(&benchmark, criterion);

// Keep one cheap real-world benchmark frozen to catch regressions from newly added inputs.
let frozen_benchmark = benchmark.freeze_inputs();
bench_project_named(&frozen_benchmark, criterion, "DateType (frozen inputs)");
}

criterion_group!(check_file, benchmark_cold, benchmark_incremental);
Expand Down
58 changes: 55 additions & 3 deletions crates/ruff_db/src/files.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::collections::BTreeSet;
use std::fmt;
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, Ordering};

use dashmap::mapref::entry::Entry;
pub use directory::{
Expand Down Expand Up @@ -64,6 +65,9 @@ pub struct Files {

#[derive(Default)]
struct FilesInner {
/// Whether inputs on newly created files should be frozen.
frozen: AtomicBool,

/// Lookup table that maps [`SystemPathBuf`]s to salsa interned [`File`] instances.
///
/// The map also stores entries for files that don't exist on the file system. This is necessary
Expand All @@ -81,6 +85,22 @@ struct FilesInner {
}

impl Files {
/// Freezes all inputs on files created from now on.
///
/// Existing files retain their current durability. Callers should therefore call this before
/// discovering any files if they need the freeze to apply to the entire project.
pub fn freeze(&self) {
self.inner.frozen.store(true, Ordering::Relaxed);
}

fn input_durability(&self, default: Durability) -> Durability {
if self.inner.frozen.load(Ordering::Relaxed) {
Durability::NEVER_CHANGE
} else {
default
}
}

/// Looks up a file by its `path`.
///
/// For a non-existing file, creates a new salsa [`File`] ingredient and stores it for future lookups.
Expand All @@ -104,9 +124,10 @@ impl Files {

tracing::trace!("Adding file '{absolute}'");

let durability = self
.root(db, &absolute)
.map_or(Durability::default(), |root| root.durability(db));
let durability = self.input_durability(
self.root(db, &absolute)
.map_or(Durability::default(), |root| root.durability(db)),
);

let builder = File::builder(FilePath::from(absolute))
.durability(durability)
Expand Down Expand Up @@ -181,6 +202,7 @@ impl Files {
tracing::trace!("Adding virtual file {}", path);
let virtual_file = VirtualFile(
File::builder(FilePath::from(path))
.durability(self.input_durability(Durability::LOW))
.path_durability(Durability::NEVER_CHANGE)
.status(FileStatus::Exists)
.revision(FileRevision::zero())
Expand Down Expand Up @@ -677,8 +699,12 @@ impl TryFrom<Span> for FileRange {

#[cfg(test)]
mod tests {
use salsa::Setter;

use crate::Db as _;
use crate::file_revision::FileRevision;
use crate::files::{FileError, system_path_to_file, vendored_path_to_file};
use crate::source::source_text;
use crate::system::DbWithWritableSystem as _;
use crate::tests::TestDb;
use crate::vendored::VendoredFileSystemBuilder;
Expand Down Expand Up @@ -723,6 +749,32 @@ mod tests {
);
}

#[test]
#[should_panic]
fn freeze_applies_to_new_file_overrides() {
let mut db = TestDb::new();
db.write_file("test.py", "x = 1").unwrap();
db.files().freeze();

let file = system_path_to_file(&db, "test.py").unwrap();
let source = source_text(&db, file);
file.set_source_text_override(&mut db).to(Some(source));
}

#[test]
fn freeze_does_not_change_existing_files() {
let mut db = TestDb::new();
db.write_file("test.py", "x = 1").unwrap();

let file = system_path_to_file(&db, "test.py").unwrap();
db.files().freeze();

let source = source_text(&db, file);
file.set_source_text_override(&mut db)
.to(Some(source.clone()));
assert_eq!(file.source_text_override(&db).as_ref(), Some(&source));
}

#[test]
fn stubbed_vendored_file() -> crate::system::Result<()> {
let mut db = TestDb::new();
Expand Down
30 changes: 21 additions & 9 deletions crates/ty/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ fn run_check(args: CheckCommand) -> anyhow::Result<ExitStatus> {
let system = OsSystem::new(&cwd);
let watch = args.watch;
let exit_zero = args.exit_zero;
let memory_report = std::env::var(EnvVars::TY_MEMORY_REPORT).ok();
let config_file = args
.config_file
.as_ref()
Expand All @@ -165,9 +166,6 @@ fn run_check(args: CheckCommand) -> anyhow::Result<ExitStatus> {
project_metadata.apply_overrides(&project_options_overrides);

let mut db = ProjectDatabase::fallible(project_metadata, system)?;
if !watch {
ruff_db::disable_lru(&mut db);
}
let project = db.project();

project.set_verbose(&mut db, verbosity >= VerbosityLevel::Verbose);
Expand All @@ -177,6 +175,20 @@ fn run_check(args: CheckCommand) -> anyhow::Result<ExitStatus> {
project.set_included_paths(&mut db, check_paths);
}

// Disabling LRU only assumes that the database is short-lived; unlike freezing below, it does
// not require immutable inputs.
if !watch {
ruff_db::disable_lru(&mut db);
}

// A one-shot check never mutates these heavily read inputs, so freezing them avoids recording
// unnecessary Salsa dependencies. Watch mode updates inputs incrementally, fix modes apply
// source-text overrides, and memory reports measure the database without this optimization, so
// they must keep the inputs mutable.
if !watch && matches!(mode, MainLoopMode::Check) && memory_report.is_none() {
db.freeze();
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we couple this with ruff_db::disable_lru(&mut db);? Like, should we make the LRU disablement conditional on the same things, or not necessary?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could move the two checks closer together, but I wouldn't unify them. We don't need LRU for --fix, because it's still a short lived check. We can also use it when running the memory report.


let (main_loop, main_loop_cancellation_token) =
MainLoop::new(mode, project_options_overrides, printer);

Expand All @@ -197,16 +209,16 @@ fn run_check(args: CheckCommand) -> anyhow::Result<ExitStatus> {
};

let mut stdout = printer.stream_for_requested_summary().lock();
match std::env::var(EnvVars::TY_MEMORY_REPORT).as_deref() {
Ok("short") => write!(stdout, "{}", db.salsa_memory_dump().display_short())?,
Ok("full") => write!(stdout, "{}", db.salsa_memory_dump().display_full())?,
Ok("json") => writeln!(stdout, "{}", db.salsa_memory_dump().to_json())?,
Ok(other) => {
match memory_report.as_deref() {
Some("short") => write!(stdout, "{}", db.salsa_memory_dump().display_short())?,
Some("full") => write!(stdout, "{}", db.salsa_memory_dump().display_full())?,
Some("json") => writeln!(stdout, "{}", db.salsa_memory_dump().to_json())?,
Some(other) => {
tracing::warn!(
"Unknown value for `TY_MEMORY_REPORT`: `{other}`. Valid values are `short`, `full`, and `json`."
);
}
Err(_) => {}
None => {}
}

std::mem::forget(db);
Expand Down
32 changes: 32 additions & 0 deletions crates/ty_project/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,21 @@ impl ProjectDatabase {
db
}

/// Permanently freezes the most heavily read inputs that are immutable during a one-shot check.
///
/// This is intentionally not exhaustive. It includes every [`Program`] input, the most heavily
/// read immutable [`Project`] inputs, and every field on files created after this call. Existing
/// files retain their durability. This must not be used by incremental consumers or checks that
/// apply fixes.
pub fn freeze(&mut self) {
let program = Program::try_get(self).expect("the program should be initialized");
let project = self.project();

program.freeze(self);
project.freeze(self);
self.files.freeze();
}

fn new<S, Strategy: MisconfigurationStrategy>(
project_metadata: ProjectMetadata,
system: S,
Expand Down Expand Up @@ -796,6 +811,23 @@ mod tests {

use crate::{ProjectDatabase, ProjectMetadata};

#[test]
fn frozen_inputs_support_a_one_shot_check() -> anyhow::Result<()> {
let system = TestSystem::default();
let project = SystemPathBuf::from("/project");
system
.memory_file_system()
.write_file_all(project.join("main.py"), "x: int = 'not an int'")?;

let metadata = ProjectMetadata::discover(&project, &system)?;
let mut db = ProjectDatabase::fallible(metadata, system)?;
db.freeze();

assert_eq!(db.check().len(), 1);

Ok(())
}

#[test]
fn search_root_registration() -> anyhow::Result<()> {
let system = TestSystem::default();
Expand Down
17 changes: 16 additions & 1 deletion crates/ty_project/src/files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::ops::Deref;
use std::sync::Arc;

use rustc_hash::FxHashSet;
use salsa::Setter;
use salsa::{Durability, Setter};

use ruff_db::diagnostic::Diagnostic;
use ruff_db::files::File;
Expand Down Expand Up @@ -55,6 +55,21 @@ impl IndexedFiles {
matches!(*self.state.lock().unwrap(), State::Lazy)
}

/// Permanently freezes the project's file-set input without cloning the indexed files.
pub(super) fn freeze(db: &mut dyn Db, project: Project) {
let state = {
let files = project.file_set(db);
std::mem::replace(&mut *files.state.lock().unwrap(), State::Lazy)
};

project
.set_file_set(db)
.with_durability(Durability::NEVER_CHANGE)
.to(Self {
state: std::sync::Mutex::new(state),
});
}

/// Returns a mutable view on the index that allows cheap in-place mutations.
///
/// The changes are automatically written back to the database once the view is dropped.
Expand Down
38 changes: 38 additions & 0 deletions crates/ty_project/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,44 @@ impl Project {
.new(db)
}

/// Permanently freezes the most heavily read immutable project inputs.
///
/// This is intentionally not exhaustive.
pub(crate) fn freeze(self, db: &mut dyn Db) {
let durability = Durability::NEVER_CHANGE;
let metadata = Box::new(self.metadata(db).clone());
let settings = Box::new(self.settings(db).clone());
let included_paths = self.included_paths_list(db).to_vec();
let open_files = self.open_fileset(db).clone();
let check_mode = self.check_mode(db);
let verbose = self.verbose_flag(db);
let force_exclude = self.force_exclude_flag(db);

self.set_metadata(db)
.with_durability(durability)
.to(metadata);
self.set_settings(db)
.with_durability(durability)
.to(settings);
self.set_included_paths_list(db)
.with_durability(durability)
.to(included_paths);
self.set_open_fileset(db)
.with_durability(durability)
.to(open_files);
self.set_check_mode(db)
.with_durability(durability)
.to(check_mode);
self.set_verbose_flag(db)
.with_durability(durability)
.to(verbose);
self.set_force_exclude_flag(db)
.with_durability(durability)
.to(force_exclude);

IndexedFiles::freeze(db, self);
}

pub fn root(self, db: &dyn Db) -> &SystemPath {
self.metadata(db).root()
}
Expand Down
2 changes: 1 addition & 1 deletion crates/ty_project/src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub mod python_version;
pub mod settings;
pub mod value;

#[derive(Debug, PartialEq, Eq, get_size2::GetSize)]
#[derive(Debug, Clone, PartialEq, Eq, get_size2::GetSize)]
#[cfg_attr(test, derive(serde::Serialize))]
pub struct ProjectMetadata {
pub(super) name: Name,
Expand Down
Loading
Loading