Skip to content

Commit 92c5f62

Browse files
authored
[ty] Enable LRU collection for parsed module (#21749)
1 parent 21e5a57 commit 92c5f62

File tree

6 files changed

+65
-22
lines changed

6 files changed

+65
-22
lines changed

crates/ruff_db/src/parsed.rs

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,11 @@ use crate::source::source_text;
2121
/// reflected in the changed AST offsets.
2222
/// The other reason is that Ruff's AST doesn't implement `Eq` which Salsa requires
2323
/// for determining if a query result is unchanged.
24-
#[salsa::tracked(returns(ref), no_eq, heap_size=ruff_memory_usage::heap_size)]
24+
///
25+
/// The LRU capacity of 200 was picked without any empirical evidence that it's optimal,
26+
/// instead it's a wild guess that it should be unlikely that incremental changes involve
27+
/// more than 200 modules. Parsed ASTs within the same revision are never evicted by Salsa.
28+
#[salsa::tracked(returns(ref), no_eq, heap_size=ruff_memory_usage::heap_size, lru=200)]
2529
pub fn parsed_module(db: &dyn Db, file: File) -> ParsedModule {
2630
let _span = tracing::trace_span!("parsed_module", ?file).entered();
2731

@@ -92,14 +96,9 @@ impl ParsedModule {
9296
self.inner.store(None);
9397
}
9498

95-
/// Returns the pointer address of this [`ParsedModule`].
96-
///
97-
/// The pointer uniquely identifies the module within the current Salsa revision,
98-
/// regardless of whether particular [`ParsedModuleRef`] instances are garbage collected.
99-
pub fn addr(&self) -> usize {
100-
// Note that the outer `Arc` in `inner` is stable across garbage collection, while the inner
101-
// `Arc` within the `ArcSwap` may change.
102-
Arc::as_ptr(&self.inner).addr()
99+
/// Returns the file to which this module belongs.
100+
pub fn file(&self) -> File {
101+
self.file
103102
}
104103
}
105104

crates/ruff_memory_usage/src/lib.rs

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,46 @@
1-
use std::sync::{LazyLock, Mutex};
1+
use std::cell::RefCell;
22

33
use get_size2::{GetSize, StandardTracker};
44
use ordermap::{OrderMap, OrderSet};
55

6+
thread_local! {
7+
pub static TRACKER: RefCell<Option<StandardTracker>>= const { RefCell::new(None) };
8+
}
9+
10+
struct TrackerGuard(Option<StandardTracker>);
11+
12+
impl Drop for TrackerGuard {
13+
fn drop(&mut self) {
14+
TRACKER.set(self.0.take());
15+
}
16+
}
17+
18+
pub fn attach_tracker<R>(tracker: StandardTracker, f: impl FnOnce() -> R) -> R {
19+
let prev = TRACKER.replace(Some(tracker));
20+
let _guard = TrackerGuard(prev);
21+
f()
22+
}
23+
24+
fn with_tracker<F, R>(f: F) -> R
25+
where
26+
F: FnOnce(Option<&mut StandardTracker>) -> R,
27+
{
28+
TRACKER.with(|tracker| {
29+
let mut tracker = tracker.borrow_mut();
30+
f(tracker.as_mut())
31+
})
32+
}
33+
634
/// Returns the memory usage of the provided object, using a global tracker to avoid
735
/// double-counting shared objects.
836
pub fn heap_size<T: GetSize>(value: &T) -> usize {
9-
static TRACKER: LazyLock<Mutex<StandardTracker>> =
10-
LazyLock::new(|| Mutex::new(StandardTracker::new()));
11-
12-
value
13-
.get_heap_size_with_tracker(&mut *TRACKER.lock().unwrap())
14-
.0
37+
with_tracker(|tracker| {
38+
if let Some(tracker) = tracker {
39+
value.get_heap_size_with_tracker(tracker).0
40+
} else {
41+
value.get_heap_size()
42+
}
43+
})
1544
}
1645

1746
/// An implementation of [`GetSize::get_heap_size`] for [`OrderSet`].

crates/ty_ide/src/symbols.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,16 @@ pub(crate) fn symbols_for_file_global_only(db: &dyn Db, file: File) -> FlatSymbo
379379
global_only: true,
380380
};
381381
visitor.visit_body(&module.syntax().body);
382+
383+
if file
384+
.path(db)
385+
.as_system_path()
386+
.is_none_or(|path| !db.project().is_file_included(db, path))
387+
{
388+
// Eagerly clear ASTs of third party files.
389+
parsed.clear();
390+
}
391+
382392
FlatSymbols {
383393
symbols: visitor.symbols,
384394
}

crates/ty_project/src/db.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ pub use self::changes::ChangeResult;
77
use crate::CollectReporter;
88
use crate::metadata::settings::file_settings;
99
use crate::{ProgressReporter, Project, ProjectMetadata};
10+
use get_size2::StandardTracker;
1011
use ruff_db::Db as SourceDb;
1112
use ruff_db::diagnostic::Diagnostic;
1213
use ruff_db::files::{File, Files};
@@ -129,7 +130,10 @@ impl ProjectDatabase {
129130
/// Returns a [`SalsaMemoryDump`] that can be use to dump Salsa memory usage information
130131
/// to the CLI after a typechecker run.
131132
pub fn salsa_memory_dump(&self) -> SalsaMemoryDump {
132-
let memory_usage = <dyn salsa::Database>::memory_usage(self);
133+
let memory_usage = ruff_memory_usage::attach_tracker(StandardTracker::new(), || {
134+
<dyn salsa::Database>::memory_usage(self)
135+
});
136+
133137
let mut ingredients = memory_usage
134138
.structs
135139
.into_iter()

crates/ty_python_semantic/src/ast_node_ref.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
use std::fmt::Debug;
22
use std::marker::PhantomData;
33

4+
#[cfg(debug_assertions)]
5+
use ruff_db::files::File;
46
use ruff_db::parsed::ParsedModuleRef;
57
use ruff_python_ast::{AnyNodeRef, NodeIndex};
68
use ruff_python_ast::{AnyRootNodeRef, HasNodeIndex};
@@ -44,7 +46,7 @@ pub struct AstNodeRef<T> {
4446
// cannot implement `Eq`, as indices are only unique within a given instance of the
4547
// AST.
4648
#[cfg(debug_assertions)]
47-
module_addr: usize,
49+
file: File,
4850

4951
_node: PhantomData<T>,
5052
}
@@ -72,7 +74,7 @@ where
7274
Self {
7375
index,
7476
#[cfg(debug_assertions)]
75-
module_addr: module_ref.module().addr(),
77+
file: module_ref.module().file(),
7678
#[cfg(debug_assertions)]
7779
kind: AnyNodeRef::from(node).kind(),
7880
#[cfg(debug_assertions)]
@@ -88,8 +90,7 @@ where
8890
#[track_caller]
8991
pub fn node<'ast>(&self, module_ref: &'ast ParsedModuleRef) -> &'ast T {
9092
#[cfg(debug_assertions)]
91-
assert_eq!(module_ref.module().addr(), self.module_addr);
92-
93+
assert_eq!(module_ref.module().file(), self.file);
9394
// The user guarantees that the module is from the same file and Salsa
9495
// revision, so the file contents cannot have changed.
9596
module_ref

crates/ty_python_semantic/src/module_resolver/module.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ impl std::fmt::Debug for Module<'_> {
120120
}
121121

122122
#[allow(clippy::ref_option)]
123-
#[salsa::tracked(returns(ref))]
123+
#[salsa::tracked(returns(ref), heap_size=ruff_memory_usage::heap_size)]
124124
fn all_submodule_names_for_package<'db>(
125125
db: &'db dyn Db,
126126
module: Module<'db>,

0 commit comments

Comments
 (0)