[internal]: Upgrade salsa (#16794)

## Summary

Another salsa upgrade. 

The main motivation is to stay on a recent salsa version because there
are still a lot of breaking changes happening.
The most significant changes in this update:

* Salsa no longer derives `Debug` by default. It now requires
`interned(debug)` (or similar)
* This version ships the foundation for garbage collecting interned
values. However, this comes at the cost that queries now track which
interned values they created (or read). The micro benchmarks in the
salsa repo showed a significant perf regression. Will see if this also
visible in our benchmarks.

## Test Plan

`cargo test`
This commit is contained in:
Micha Reiser 2025-03-17 11:05:54 +01:00 committed by GitHub
parent dbdb46dcd2
commit c100d519e9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
21 changed files with 70 additions and 56 deletions

6
Cargo.lock generated
View file

@ -3408,7 +3408,7 @@ checksum = "28d3b2b1366ec20994f1fd18c3c594f05c5dd4bc44d8bb0c1c632c8d6829481f"
[[package]]
name = "salsa"
version = "0.19.0"
source = "git+https://github.com/salsa-rs/salsa.git?rev=095d8b2b8115c3cf8bf31914dd9ea74648bb7cf9#095d8b2b8115c3cf8bf31914dd9ea74648bb7cf9"
source = "git+https://github.com/salsa-rs/salsa.git?rev=d758691ba17ee1a60c5356ea90888d529e1782ad#d758691ba17ee1a60c5356ea90888d529e1782ad"
dependencies = [
"boxcar",
"compact_str 0.8.1",
@ -3430,12 +3430,12 @@ dependencies = [
[[package]]
name = "salsa-macro-rules"
version = "0.19.0"
source = "git+https://github.com/salsa-rs/salsa.git?rev=095d8b2b8115c3cf8bf31914dd9ea74648bb7cf9#095d8b2b8115c3cf8bf31914dd9ea74648bb7cf9"
source = "git+https://github.com/salsa-rs/salsa.git?rev=d758691ba17ee1a60c5356ea90888d529e1782ad#d758691ba17ee1a60c5356ea90888d529e1782ad"
[[package]]
name = "salsa-macros"
version = "0.19.0"
source = "git+https://github.com/salsa-rs/salsa.git?rev=095d8b2b8115c3cf8bf31914dd9ea74648bb7cf9#095d8b2b8115c3cf8bf31914dd9ea74648bb7cf9"
source = "git+https://github.com/salsa-rs/salsa.git?rev=d758691ba17ee1a60c5356ea90888d529e1782ad#d758691ba17ee1a60c5356ea90888d529e1782ad"
dependencies = [
"heck",
"proc-macro2",

View file

@ -123,7 +123,7 @@ rayon = { version = "1.10.0" }
regex = { version = "1.10.2" }
rustc-hash = { version = "2.0.0" }
# When updating salsa, make sure to also update the revision in `fuzz/Cargo.toml`
salsa = { git = "https://github.com/salsa-rs/salsa.git", rev = "095d8b2b8115c3cf8bf31914dd9ea74648bb7cf9" }
salsa = { git = "https://github.com/salsa-rs/salsa.git", rev = "d758691ba17ee1a60c5356ea90888d529e1782ad" }
schemars = { version = "0.8.16" }
seahash = { version = "4.1.0" }
serde = { version = "1.0.197", features = ["derive"] }

View file

@ -59,9 +59,8 @@ impl ProjectDatabase {
self.with_db(|db| db.project().check(db))
}
#[tracing::instrument(level = "debug", skip(self))]
pub fn check_file(&self, file: File) -> Result<Vec<Box<dyn OldDiagnosticTrait>>, Cancelled> {
let _span = tracing::debug_span!("check_file", file=%file.path(self)).entered();
self.with_db(|db| self.project().check_file(db, file))
}

View file

@ -195,7 +195,8 @@ impl Project {
let project_span = project_span.clone();
scope.spawn(move |_| {
let check_file_span = tracing::debug_span!(parent: &project_span, "check_file", file=%file.path(&db));
let check_file_span =
tracing::debug_span!(parent: &project_span, "check_file", ?file);
let _entered = check_file_span.entered();
let file_diagnostics = check_file_impl(&db, file);
@ -325,7 +326,7 @@ impl Project {
self.files(db).contains(&file)
}
#[tracing::instrument(level = "debug", skip(db))]
#[tracing::instrument(level = "debug", skip(self, db))]
pub fn remove_file(self, db: &mut dyn Db, file: File) {
tracing::debug!(
"Removing file `{}` from project `{}`",

View file

@ -534,7 +534,7 @@ impl<'db> Iterator for PthFileIterator<'db> {
/// A thin wrapper around `ModuleName` to make it a Salsa ingredient.
///
/// This is needed because Salsa requires that all query arguments are salsa ingredients.
#[salsa::interned]
#[salsa::interned(debug)]
struct ModuleNameIngredient<'db> {
#[return_ref]
pub(super) name: ModuleName,

View file

@ -45,7 +45,7 @@ type SymbolMap = hashbrown::HashMap<ScopedSymbolId, (), FxBuildHasher>;
/// Prefer using [`symbol_table`] when working with symbols from a single scope.
#[salsa::tracked(return_ref, no_eq)]
pub(crate) fn semantic_index(db: &dyn Db, file: File) -> SemanticIndex<'_> {
let _span = tracing::trace_span!("semantic_index", file = %file.path(db)).entered();
let _span = tracing::trace_span!("semantic_index", ?file).entered();
let parsed = parsed_module(db.upcast(), file);
@ -60,8 +60,7 @@ pub(crate) fn semantic_index(db: &dyn Db, file: File) -> SemanticIndex<'_> {
#[salsa::tracked]
pub(crate) fn symbol_table<'db>(db: &'db dyn Db, scope: ScopeId<'db>) -> Arc<SymbolTable> {
let file = scope.file(db);
let _span =
tracing::trace_span!("symbol_table", scope=?scope.as_id(), file=%file.path(db)).entered();
let _span = tracing::trace_span!("symbol_table", scope=?scope.as_id(), ?file).entered();
let index = semantic_index(db, file);
index.symbol_table(scope.file_scope_id(db))
@ -91,8 +90,7 @@ pub(crate) fn imported_modules<'db>(db: &'db dyn Db, file: File) -> Arc<FxHashSe
#[salsa::tracked]
pub(crate) fn use_def_map<'db>(db: &'db dyn Db, scope: ScopeId<'db>) -> Arc<UseDefMap<'db>> {
let file = scope.file(db);
let _span =
tracing::trace_span!("use_def_map", scope=?scope.as_id(), file=%file.path(db)).entered();
let _span = tracing::trace_span!("use_def_map", scope=?scope.as_id(), ?file).entered();
let index = semantic_index(db, file);
index.use_def_map(scope.file_scope_id(db))
@ -120,7 +118,7 @@ pub(crate) fn attribute_assignments<'db>(
/// Returns the module global scope of `file`.
#[salsa::tracked]
pub(crate) fn global_scope(db: &dyn Db, file: File) -> ScopeId<'_> {
let _span = tracing::trace_span!("global_scope", file = %file.path(db)).entered();
let _span = tracing::trace_span!("global_scope", ?file).entered();
FileScopeId::global().to_scope_id(db, file)
}

View file

@ -22,7 +22,7 @@ use crate::Db;
/// * a return type of a cross-module query
/// * a field of a type that is a return type of a cross-module query
/// * an argument of a cross-module query
#[salsa::tracked]
#[salsa::tracked(debug)]
pub struct Definition<'db> {
/// The file in which the definition occurs.
pub(crate) file: File,

View file

@ -30,7 +30,7 @@ pub(crate) enum ExpressionKind {
/// * a return type of a cross-module query
/// * a field of a type that is a return type of a cross-module query
/// * an argument of a cross-module query
#[salsa::tracked]
#[salsa::tracked(debug)]
pub(crate) struct Expression<'db> {
/// The file in which the expression occurs.
pub(crate) file: File,

View file

@ -63,7 +63,7 @@ pub(crate) enum PatternPredicateKind<'db> {
Unsupported,
}
#[salsa::tracked]
#[salsa::tracked(debug)]
pub(crate) struct PatternPredicate<'db> {
pub(crate) file: File,

View file

@ -100,7 +100,7 @@ impl From<FileSymbolId> for ScopedSymbolId {
pub struct ScopedSymbolId;
/// A cross-module identifier of a scope that can be used as a salsa query parameter.
#[salsa::tracked]
#[salsa::tracked(debug)]
pub struct ScopeId<'db> {
pub file: File,

View file

@ -62,7 +62,7 @@ mod property_tests;
#[salsa::tracked(return_ref)]
pub fn check_types(db: &dyn Db, file: File) -> TypeCheckDiagnostics {
let _span = tracing::trace_span!("check_types", file=?file.path(db)).entered();
let _span = tracing::trace_span!("check_types", ?file).entered();
tracing::debug!("Checking file '{path}'", path = file.path(db));
@ -3605,7 +3605,7 @@ impl<'db> InvalidTypeExpression<'db> {
/// This must be a tracked struct, not an interned one, because typevar equivalence is by identity,
/// not by value. Two typevars that have the same name, bound/constraints, and default, are still
/// different typevars: if used in the same scope, they may be bound to different types.
#[salsa::tracked]
#[salsa::tracked(debug)]
pub struct TypeVarInstance<'db> {
/// The name of this TypeVar (e.g. `T`)
#[return_ref]
@ -4308,7 +4308,7 @@ impl From<bool> for Truthiness {
}
}
#[salsa::interned]
#[salsa::interned(debug)]
pub struct FunctionType<'db> {
/// name of the function at definition
#[return_ref]
@ -4538,7 +4538,7 @@ impl KnownFunction {
/// on an instance of a class. For example, the expression `Path("a.txt").touch` creates
/// a bound method object that represents the `Path.touch` method which is bound to the
/// instance `Path("a.txt")`.
#[salsa::tracked]
#[salsa::tracked(debug)]
pub struct BoundMethodType<'db> {
/// The function that is being bound. Corresponds to the `__func__` attribute on a
/// bound method object
@ -4550,7 +4550,7 @@ pub struct BoundMethodType<'db> {
/// This type represents a general callable type that are used to represent `typing.Callable`
/// and `lambda` expressions.
#[salsa::interned]
#[salsa::interned(debug)]
pub struct GeneralCallableType<'db> {
#[return_ref]
signature: Signature<'db>,
@ -4734,7 +4734,7 @@ enum ParameterExpectation {
TypeExpression,
}
#[salsa::interned]
#[salsa::interned(debug)]
pub struct ModuleLiteralType<'db> {
/// The file in which this module was imported.
///
@ -4783,7 +4783,7 @@ impl<'db> ModuleLiteralType<'db> {
}
}
#[salsa::interned]
#[salsa::interned(debug)]
pub struct TypeAliasType<'db> {
#[return_ref]
pub name: ast::name::Name,
@ -4811,7 +4811,7 @@ pub(super) struct MetaclassCandidate<'db> {
explicit_metaclass_of: Class<'db>,
}
#[salsa::interned]
#[salsa::interned(debug)]
pub struct UnionType<'db> {
/// The union type includes values in any of these types.
#[return_ref]
@ -5022,7 +5022,7 @@ impl<'db> UnionType<'db> {
}
}
#[salsa::interned]
#[salsa::interned(debug)]
pub struct IntersectionType<'db> {
/// The intersection type includes only values in all of these types.
#[return_ref]
@ -5253,7 +5253,7 @@ impl<'db> IntersectionType<'db> {
}
}
#[salsa::interned]
#[salsa::interned(debug)]
pub struct StringLiteralType<'db> {
#[return_ref]
value: Box<str>,
@ -5266,7 +5266,7 @@ impl<'db> StringLiteralType<'db> {
}
}
#[salsa::interned]
#[salsa::interned(debug)]
pub struct BytesLiteralType<'db> {
#[return_ref]
value: Box<[u8]>,
@ -5278,7 +5278,7 @@ impl<'db> BytesLiteralType<'db> {
}
}
#[salsa::interned]
#[salsa::interned(debug)]
pub struct SliceLiteralType<'db> {
start: Option<i32>,
stop: Option<i32>,
@ -5290,7 +5290,7 @@ impl SliceLiteralType<'_> {
(self.start(db), self.stop(db), self.step(db))
}
}
#[salsa::interned]
#[salsa::interned(debug)]
pub struct TupleType<'db> {
#[return_ref]
elements: Box<[Type<'db>]>,

View file

@ -32,7 +32,7 @@ use super::{
///
/// Does not in itself represent a type,
/// but is used as the inner data for several structs that *do* represent types.
#[salsa::interned]
#[salsa::interned(debug)]
pub struct Class<'db> {
/// Name of the class at definition
#[return_ref]

View file

@ -111,9 +111,7 @@ use super::{CallDunderError, ParameterExpectation, ParameterExpectations};
#[salsa::tracked(return_ref, cycle_fn=scope_cycle_recover, cycle_initial=scope_cycle_initial)]
pub(crate) fn infer_scope_types<'db>(db: &'db dyn Db, scope: ScopeId<'db>) -> TypeInference<'db> {
let file = scope.file(db);
let _span =
tracing::trace_span!("infer_scope_types", scope=?scope.as_id(), file=%file.path(db))
.entered();
let _span = tracing::trace_span!("infer_scope_types", scope=?scope.as_id(), ?file).entered();
// Using the index here is fine because the code below depends on the AST anyway.
// The isolation of the query is by the return inferred types.
@ -146,7 +144,7 @@ pub(crate) fn infer_definition_types<'db>(
let _span = tracing::trace_span!(
"infer_definition_types",
range = ?definition.kind(db).target_range(),
file = %file.path(db)
?file
)
.entered();
@ -185,7 +183,7 @@ pub(crate) fn infer_deferred_types<'db>(
"infer_deferred_types",
definition = ?definition.as_id(),
range = ?definition.kind(db).target_range(),
file = %file.path(db)
?file
)
.entered();
@ -221,7 +219,7 @@ pub(crate) fn infer_expression_types<'db>(
"infer_expression_types",
expression = ?expression.as_id(),
range = ?expression.node_ref(db).range(),
file = %file.path(db)
?file
)
.entered();
@ -302,8 +300,7 @@ fn single_expression_cycle_initial<'db>(
pub(super) fn infer_unpack_types<'db>(db: &'db dyn Db, unpack: Unpack<'db>) -> UnpackResult<'db> {
let file = unpack.file(db);
let _span =
tracing::trace_span!("infer_unpack_types", range=?unpack.range(db), file=%file.path(db))
.entered();
tracing::trace_span!("infer_unpack_types", range=?unpack.range(db), ?file).entered();
let mut unpacker = Unpacker::new(db, unpack.scope(db));
unpacker.unpack(unpack.target(db), unpack.value(db));

View file

@ -134,7 +134,8 @@ pub(crate) fn parse_string_annotation(
let file = context.file();
let db = context.db();
let _span = tracing::trace_span!("parse_string_annotation", string=?string_expr.range(), file=%file.path(db)).entered();
let _span = tracing::trace_span!("parse_string_annotation", string=?string_expr.range(), ?file)
.entered();
let source = source_text(db.upcast(), file);

View file

@ -26,7 +26,7 @@ use crate::Db;
/// * a return type of a cross-module query
/// * a field of a type that is a return type of a cross-module query
/// * an argument of a cross-module query
#[salsa::tracked]
#[salsa::tracked(debug)]
pub(crate) struct Unpack<'db> {
pub(crate) file: File,

View file

@ -1,14 +1,14 @@
use std::fmt::Formatter;
use std::fmt;
use std::sync::Arc;
use countme::Count;
use dashmap::mapref::entry::Entry;
use salsa::{Durability, Setter};
pub use file_root::{FileRoot, FileRootKind};
pub use path::FilePath;
use ruff_notebook::{Notebook, NotebookError};
use ruff_python_ast::PySourceType;
use salsa::plumbing::AsId;
use salsa::{Durability, Setter};
use crate::file_revision::FileRevision;
use crate::files::file_root::FileRoots;
@ -255,7 +255,7 @@ impl Files {
}
}
impl std::fmt::Debug for Files {
impl fmt::Debug for Files {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let mut map = f.debug_map();
@ -429,6 +429,24 @@ impl File {
}
}
impl fmt::Debug for File {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
salsa::with_attached_database(|db| {
if f.alternate() {
f.debug_struct("File")
.field("path", &self.path(db))
.field("status", &self.status(db))
.field("permissions", &self.permissions(db))
.field("revision", &self.revision(db))
.finish()
} else {
f.debug_tuple("File").field(&self.path(db)).finish()
}
})
.unwrap_or_else(|| f.debug_tuple("file").field(&self.as_id()).finish())
}
}
/// A virtual file that doesn't exist on the file system.
///
/// This is a wrapper around a [`File`] that provides additional methods to interact with a virtual
@ -481,8 +499,8 @@ pub enum FileError {
NotFound,
}
impl std::fmt::Display for FileError {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
impl fmt::Display for FileError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::fmt::Result {
match self {
FileError::IsADirectory => f.write_str("Is a directory"),
FileError::NotFound => f.write_str("Not found"),

View file

@ -16,7 +16,7 @@ use crate::Db;
/// The main usage of file roots is to determine a file's durability. But it can also be used
/// to make a salsa query dependent on whether a file in a root has changed without writing any
/// manual invalidation logic.
#[salsa::input]
#[salsa::input(debug)]
pub struct FileRoot {
/// The path of a root is guaranteed to never change.
#[return_ref]

View file

@ -22,7 +22,7 @@ use crate::Db;
/// for determining if a query result is unchanged.
#[salsa::tracked(return_ref, no_eq)]
pub fn parsed_module(db: &dyn Db, file: File) -> ParsedModule {
let _span = tracing::trace_span!("parsed_module", file = %file.path(db)).entered();
let _span = tracing::trace_span!("parsed_module", ?file).entered();
let source = source_text(db, file);
let path = file.path(db);

View file

@ -159,7 +159,7 @@ pub enum SourceTextError {
/// Computes the [`LineIndex`] for `file`.
#[salsa::tracked]
pub fn line_index(db: &dyn Db, file: File) -> LineIndex {
let _span = tracing::trace_span!("line_index", file = ?file.path(db)).entered();
let _span = tracing::trace_span!("line_index", ?file).entered();
let source = source_text(db, file);

View file

@ -223,7 +223,7 @@ fn query_was_not_run() {
use crate::tests::TestDb;
use salsa::prelude::*;
#[salsa::input]
#[salsa::input(debug)]
struct Input {
text: String,
}
@ -258,7 +258,7 @@ fn query_was_not_run_fails_if_query_was_run() {
use crate::tests::TestDb;
use salsa::prelude::*;
#[salsa::input]
#[salsa::input(debug)]
struct Input {
text: String,
}
@ -321,7 +321,7 @@ fn query_was_run_fails_if_query_was_not_run() {
use crate::tests::TestDb;
use salsa::prelude::*;
#[salsa::input]
#[salsa::input(debug)]
struct Input {
text: String,
}

View file

@ -29,7 +29,7 @@ ruff_python_formatter = { path = "../crates/ruff_python_formatter" }
ruff_text_size = { path = "../crates/ruff_text_size" }
libfuzzer-sys = { git = "https://github.com/rust-fuzz/libfuzzer", default-features = false }
salsa = { git = "https://github.com/salsa-rs/salsa.git", rev = "095d8b2b8115c3cf8bf31914dd9ea74648bb7cf9" }
salsa = { git = "https://github.com/salsa-rs/salsa.git", rev = "d758691ba17ee1a60c5356ea90888d529e1782ad" }
similar = { version = "2.5.0" }
tracing = { version = "0.1.40" }