[red-knot] Improved file watching (#12382)

This commit is contained in:
Micha Reiser 2024-07-23 08:18:59 +02:00 committed by GitHub
parent a9f8bd59b2
commit 40d9324f5a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
16 changed files with 1476 additions and 381 deletions

View file

@ -5,8 +5,8 @@ use dashmap::mapref::entry::Entry;
use crate::file_revision::FileRevision;
use crate::files::private::FileStatus;
use crate::system::SystemPath;
use crate::vendored::VendoredPath;
use crate::system::{SystemPath, SystemPathBuf};
use crate::vendored::{VendoredPath, VendoredPathBuf};
use crate::{Db, FxDashMap};
pub use path::FilePath;
use ruff_notebook::{Notebook, NotebookError};
@ -24,10 +24,7 @@ pub fn system_path_to_file(db: &dyn Db, path: impl AsRef<SystemPath>) -> Option<
// exist anymore so that Salsa can track that the caller of this function depends on the existence of
// that file. This function filters out files that don't exist, but Salsa will know that it must
// re-run the calling query whenever the `file`'s status changes (because of the `.status` call here).
match file.status(db) {
FileStatus::Exists => Some(file),
FileStatus::Deleted => None,
}
file.exists(db).then_some(file)
}
/// Interns a vendored file path. Returns `Some` if the vendored file for `path` exists and `None` otherwise.
@ -44,11 +41,14 @@ pub struct Files {
#[derive(Default)]
struct FilesInner {
/// Lookup table that maps [`FilePath`]s to salsa interned [`File`] instances.
/// 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
/// so that queries that depend on the existence of a file are re-executed when the file is created.
files_by_path: FxDashMap<FilePath, File>,
system_by_path: FxDashMap<SystemPathBuf, File>,
/// Lookup table that maps vendored files to the salsa [`File`] ingredients.
vendored_by_path: FxDashMap<VendoredPathBuf, File>,
}
impl Files {
@ -61,11 +61,10 @@ impl Files {
#[tracing::instrument(level = "trace", skip(self, db), ret)]
fn system(&self, db: &dyn Db, path: &SystemPath) -> File {
let absolute = SystemPath::absolute(path, db.system().current_directory());
let absolute = FilePath::System(absolute);
*self
.inner
.files_by_path
.system_by_path
.entry(absolute.clone())
.or_insert_with(|| {
let metadata = db.system().path_metadata(path);
@ -73,7 +72,7 @@ impl Files {
match metadata {
Ok(metadata) if metadata.file_type().is_file() => File::new(
db,
absolute,
FilePath::System(absolute),
metadata.permissions(),
metadata.revision(),
FileStatus::Exists,
@ -81,7 +80,7 @@ impl Files {
),
_ => File::new(
db,
absolute,
FilePath::System(absolute),
None,
FileRevision::zero(),
FileStatus::Deleted,
@ -92,11 +91,11 @@ impl Files {
}
/// Tries to look up the file for the given system path, returns `None` if no such file exists yet
fn try_system(&self, db: &dyn Db, path: &SystemPath) -> Option<File> {
pub fn try_system(&self, db: &dyn Db, path: &SystemPath) -> Option<File> {
let absolute = SystemPath::absolute(path, db.system().current_directory());
self.inner
.files_by_path
.get(&FilePath::System(absolute))
.system_by_path
.get(&absolute)
.map(|entry| *entry.value())
}
@ -104,11 +103,7 @@ impl Files {
/// exists and `None` otherwise.
#[tracing::instrument(level = "trace", skip(self, db), ret)]
fn vendored(&self, db: &dyn Db, path: &VendoredPath) -> Option<File> {
let file = match self
.inner
.files_by_path
.entry(FilePath::Vendored(path.to_path_buf()))
{
let file = match self.inner.vendored_by_path.entry(path.to_path_buf()) {
Entry::Occupied(entry) => *entry.get(),
Entry::Vacant(entry) => {
let metadata = db.vendored().metadata(path).ok()?;
@ -131,6 +126,44 @@ impl Files {
Some(file)
}
/// Refreshes the state of all known files under `path` recursively.
///
/// The most common use case is to update the [`Files`] state after removing or moving a directory.
///
/// # Performance
/// Refreshing the state of every file under `path` is expensive. It requires iterating over all known files
/// and making system calls to get the latest status of each file in `path`.
/// That's why [`File::sync_path`] and [`File::sync_path`] is preferred if it is known that the path is a file.
#[tracing::instrument(level = "debug", skip(db))]
pub fn sync_recursively(db: &mut dyn Db, path: &SystemPath) {
let path = SystemPath::absolute(path, db.system().current_directory());
let inner = Arc::clone(&db.files().inner);
for entry in inner.system_by_path.iter_mut() {
if entry.key().starts_with(&path) {
let file = entry.value();
file.sync(db);
}
}
}
/// Refreshes the state of all known files.
///
/// This is a last-resort method that should only be used when more granular updates aren't possible
/// (for example, because the file watcher failed to observe some changes). Use responsibly!
///
/// # Performance
/// Refreshing the state of every file is expensive. It requires iterating over all known files and
/// issuing a system call to get the latest status of each file.
#[tracing::instrument(level = "debug", skip(db))]
pub fn sync_all(db: &mut dyn Db) {
let inner = Arc::clone(&db.files().inner);
for entry in inner.system_by_path.iter_mut() {
let file = entry.value();
file.sync(db);
}
}
/// Creates a salsa like snapshot. The instances share
/// the same path-to-file mapping.
pub fn snapshot(&self) -> Self {
@ -144,7 +177,7 @@ impl std::fmt::Debug for Files {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let mut map = f.debug_map();
for entry in self.inner.files_by_path.iter() {
for entry in self.inner.system_by_path.iter() {
map.entry(entry.key(), entry.value());
}
map.finish()
@ -219,18 +252,20 @@ impl File {
}
/// Refreshes the file metadata by querying the file system if needed.
/// TODO: The API should instead take all observed changes from the file system directly
/// and then apply the VfsFile status accordingly. But for now, this is sufficient.
pub fn touch_path(db: &mut dyn Db, path: &SystemPath) {
Self::touch_impl(db, path, None);
#[tracing::instrument(level = "debug", skip(db))]
pub fn sync_path(db: &mut dyn Db, path: &SystemPath) {
let absolute = SystemPath::absolute(path, db.system().current_directory());
Self::sync_impl(db, &absolute, None);
}
pub fn touch(self, db: &mut dyn Db) {
/// Syncs the [`File`]'s state with the state of the file on the system.
#[tracing::instrument(level = "debug", skip(db))]
pub fn sync(self, db: &mut dyn Db) {
let path = self.path(db).clone();
match path {
FilePath::System(system) => {
Self::touch_impl(db, &system, Some(self));
Self::sync_impl(db, &system, Some(self));
}
FilePath::Vendored(_) => {
// Readonly, can never be out of date.
@ -238,23 +273,31 @@ impl File {
}
}
/// Private method providing the implementation for [`Self::touch_path`] and [`Self::touch`].
fn touch_impl(db: &mut dyn Db, path: &SystemPath, file: Option<File>) {
let metadata = db.system().path_metadata(path);
let (status, revision) = match metadata {
Ok(metadata) if metadata.file_type().is_file() => {
(FileStatus::Exists, metadata.revision())
}
_ => (FileStatus::Deleted, FileRevision::zero()),
};
/// Private method providing the implementation for [`Self::sync_path`] and [`Self::sync_path`].
fn sync_impl(db: &mut dyn Db, path: &SystemPath, file: Option<File>) {
let Some(file) = file.or_else(|| db.files().try_system(db, path)) else {
return;
};
let metadata = db.system().path_metadata(path);
let (status, revision, permission) = match metadata {
Ok(metadata) if metadata.file_type().is_file() => (
FileStatus::Exists,
metadata.revision(),
metadata.permissions(),
),
_ => (FileStatus::Deleted, FileRevision::zero(), None),
};
file.set_status(db).to(status);
file.set_revision(db).to(revision);
file.set_permissions(db).to(permission);
}
/// Returns `true` if the file exists.
pub fn exists(self, db: &dyn Db) -> bool {
self.status(db) == FileStatus::Exists
}
}

View file

@ -34,6 +34,7 @@ pub trait Db: DbWithJar<Jar> {
/// Trait for upcasting a reference to a base trait object.
pub trait Upcast<T: ?Sized> {
fn upcast(&self) -> &T;
fn upcast_mut(&mut self) -> &mut T;
}
#[cfg(test)]

View file

@ -145,7 +145,7 @@ pub trait DbWithTestSystem: Db + Sized {
.write_file(path, content);
if result.is_ok() {
File::touch_path(self, path);
File::sync_path(self, path);
}
result