Skip to content

Commit ef06ddd

Browse files
samwgoldmanmeta-codesync[bot]
authored andcommitted
Add laziness test suite for type checking dependencies (#3219)
Summary: Pull Request resolved: #3219 Adds a snapshot-based test suite documenting how much of a file's dependencies get type-checked when the file itself is checked. The laziness goal is that dependencies should stop at the earliest step (Load/Ast/Exports/Answers/Solutions) sufficient to resolve what callers actually need — e.g. a transitively-imported module used only as an annotation should not have its body inferred. Each test sets up a small module graph, checks one entry-point module, and records in a markdown snapshot the highest step each module reached plus the tree of cross-module demands observed. The snapshots capture current behavior — some document good laziness properties, others document opportunities where dependencies are computed further than callers need. Snapshots regenerate with `UPDATE_SNAPSHOTS=1`. Supporting infrastructure: - `DemandCollector` (`pyrefly_util::demand_tree`) records cross-module `LookupExport`/`LookupAnswer` events into a tree. Scoped to a single `Transaction` via `set_demand_collector` so parallel checks don't interfere. `enter()` returns an RAII span guard so the per-thread nesting stack stays balanced even when an Answer lookup panics. - `pyrefly check --report-demand-tree <path>` emits a JSON document (`{ module_steps, demand_tree }`). - The test harness renders the structured tree into a flat indented form for snapshot comparison, and aggregates pervasive `builtins`/`typing` demands into a single count so test output stays readable. Differential Revision: D102239243
1 parent d7ff8e4 commit ef06ddd

28 files changed

Lines changed: 2041 additions & 15 deletions
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
//! Demand tree collection for debugging and testing laziness.
9+
//!
10+
//! A [`DemandCollector`] records cross-module demand calls into a tree. A
11+
//! collector is usually owned by a `Transaction`, scoping collection to a
12+
//! single check run — so parallel checks don't interfere with one another.
13+
//! Parent/child nesting is tracked via a per-thread scratch stack.
14+
//!
15+
//! The tree is machine-readable via serde. `#[serde(default)]` on optional
16+
//! fields lets the schema grow without breaking existing consumers.
17+
18+
use std::cell::RefCell;
19+
use std::fmt;
20+
use std::sync::Arc;
21+
use std::sync::Mutex;
22+
23+
use serde::Deserialize;
24+
use serde::Serialize;
25+
26+
/// What kind of cross-module demand a node represents.
27+
#[derive(Debug, Clone, Serialize, Deserialize)]
28+
#[serde(tag = "type", rename_all = "snake_case")]
29+
pub enum DemandKind {
30+
/// A leaf event for an Exports-level demand (e.g. `module_exists`,
31+
/// `export_exists`). Carries the reason string identifying which
32+
/// `LookupExport` method triggered the demand.
33+
Exports { reason: String },
34+
/// A cross-module Answer lookup span. May have children if the
35+
/// computation recursively demanded data from other modules.
36+
/// `key` is the `Debug`-formatted lookup key.
37+
Answer { key: String },
38+
}
39+
40+
/// A node in the demand tree.
41+
#[derive(Debug, Clone, Serialize, Deserialize)]
42+
pub struct DemandNode {
43+
/// The module that made the demand.
44+
pub from: String,
45+
/// The module the demand was made against.
46+
pub target: String,
47+
/// What kind of demand this was.
48+
pub kind: DemandKind,
49+
/// Nested demands made while computing this one.
50+
#[serde(default, skip_serializing_if = "Vec::is_empty")]
51+
pub children: Vec<DemandNode>,
52+
}
53+
54+
thread_local! {
55+
/// Per-thread stack of in-flight parent spans, used to nest closed demand
56+
/// nodes under their caller. This is inherently per-thread scratch space:
57+
/// enter and exit always happen on the same thread, and the stack empties
58+
/// itself as long as every enter has a matching exit.
59+
static STACK: RefCell<Vec<DemandNode>> = const { RefCell::new(Vec::new()) };
60+
}
61+
62+
/// A demand-tree collection session. Cloning produces another handle to the
63+
/// same underlying roots (the collector is reference-counted internally).
64+
#[derive(Clone, Default)]
65+
pub struct DemandCollector {
66+
roots: Arc<Mutex<Vec<DemandNode>>>,
67+
}
68+
69+
impl DemandCollector {
70+
pub fn new() -> Self {
71+
Self::default()
72+
}
73+
74+
/// Open a demand span for a cross-module Answer lookup. Hold the returned
75+
/// guard for the duration of the demand — dropping the guard (including
76+
/// on unwind) closes the span and attaches it to its parent, keeping
77+
/// enter/exit balanced even across panics. `key` is the lookup key,
78+
/// formatted via `Debug`.
79+
#[inline]
80+
pub fn enter(
81+
&self,
82+
from: impl fmt::Display,
83+
target: impl fmt::Display,
84+
key: impl fmt::Debug,
85+
) -> DemandSpan<'_> {
86+
STACK.with(|stack| {
87+
stack.borrow_mut().push(DemandNode {
88+
from: from.to_string(),
89+
target: target.to_string(),
90+
kind: DemandKind::Answer {
91+
key: format!("{key:?}"),
92+
},
93+
children: Vec::new(),
94+
});
95+
});
96+
DemandSpan { collector: self }
97+
}
98+
99+
/// Record a leaf event for an Exports-level demand. `reason` identifies
100+
/// which `LookupExport` method triggered the demand.
101+
#[inline]
102+
pub fn exports_event(&self, from: impl fmt::Display, target: impl fmt::Display, reason: &str) {
103+
self.attach(DemandNode {
104+
from: from.to_string(),
105+
target: target.to_string(),
106+
kind: DemandKind::Exports {
107+
reason: reason.to_owned(),
108+
},
109+
children: Vec::new(),
110+
});
111+
}
112+
113+
/// Attach a completed node either to the current parent on the stack or,
114+
/// if no parent is in flight, to the collector's shared root list.
115+
fn attach(&self, node: DemandNode) {
116+
let leftover = STACK.with(|stack| {
117+
let mut stack = stack.borrow_mut();
118+
match stack.last_mut() {
119+
Some(parent) => {
120+
parent.children.push(node);
121+
None
122+
}
123+
None => Some(node),
124+
}
125+
});
126+
if let Some(node) = leftover {
127+
self.roots.lock().unwrap().push(node);
128+
}
129+
}
130+
131+
/// Take the collected tree roots, leaving the collector empty.
132+
pub fn take_roots(&self) -> Vec<DemandNode> {
133+
std::mem::take(&mut *self.roots.lock().unwrap())
134+
}
135+
}
136+
137+
/// RAII guard for an in-flight Answer demand span. Dropping the guard —
138+
/// whether via normal control flow or unwind — pops the span off the
139+
/// per-thread stack and attaches it to its parent (or to the collector's
140+
/// roots if no parent is in flight).
141+
#[must_use = "demand span is closed when the guard is dropped; hold it for the duration of the demand"]
142+
pub struct DemandSpan<'a> {
143+
collector: &'a DemandCollector,
144+
}
145+
146+
impl Drop for DemandSpan<'_> {
147+
fn drop(&mut self) {
148+
if let Some(node) = STACK.with(|stack| stack.borrow_mut().pop()) {
149+
self.collector.attach(node);
150+
}
151+
}
152+
}

crates/pyrefly_util/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ pub mod absolutize;
3131
pub mod arc_id;
3232
pub mod args;
3333
pub mod assert_size;
34+
pub mod demand_tree;
3435
pub mod display;
3536
pub mod events;
3637
pub mod forgetter;

pyrefly/Cargo.toml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# @generated by autocargo from //pyrefly/pyrefly:[pyrefly,pyrefly_library,pyrefly_lsp_interaction_tests]
1+
# @generated by autocargo from //pyrefly/pyrefly:[pyrefly,pyrefly_laziness_tests,pyrefly_library,pyrefly_lsp_interaction_tests]
22

33
[package]
44
name = "pyrefly"
@@ -7,6 +7,7 @@ authors = ["Meta"]
77
edition = "2024"
88
repository = "https://github.com/facebook/pyrefly"
99
license = "MIT"
10+
build = "test_laziness/build.rs"
1011

1112
[lib]
1213
path = "lib/lib.rs"
@@ -15,6 +16,10 @@ path = "lib/lib.rs"
1516
name = "pyrefly"
1617
path = "bin/main.rs"
1718

19+
[[test]]
20+
name = "pyrefly_laziness_tests"
21+
path = "test_laziness/mod.rs"
22+
1823
[[test]]
1924
name = "pyrefly_lsp_interaction_tests"
2025
path = "lib/test/lsp/lsp_interaction/mod.rs"

pyrefly/lib/alt/answers.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,27 @@ use crate::alt::traits::Solve;
3737
use crate::binding::binding::AnyIdx;
3838
use crate::binding::binding::Exported;
3939
use crate::binding::binding::Key;
40+
use crate::binding::binding::KeyAbstractClassCheck;
41+
use crate::binding::binding::KeyAnnotation;
42+
use crate::binding::binding::KeyClass;
43+
use crate::binding::binding::KeyClassBaseType;
44+
use crate::binding::binding::KeyClassField;
45+
use crate::binding::binding::KeyClassMetadata;
46+
use crate::binding::binding::KeyClassMro;
47+
use crate::binding::binding::KeyClassSynthesizedFields;
48+
use crate::binding::binding::KeyConsistentOverrideCheck;
49+
use crate::binding::binding::KeyDecoratedFunction;
50+
use crate::binding::binding::KeyDecorator;
51+
use crate::binding::binding::KeyExpect;
52+
use crate::binding::binding::KeyExport;
53+
use crate::binding::binding::KeyLegacyTypeParam;
54+
use crate::binding::binding::KeyTParams;
55+
use crate::binding::binding::KeyTypeAlias;
56+
use crate::binding::binding::KeyUndecoratedFunction;
57+
use crate::binding::binding::KeyVariance;
58+
use crate::binding::binding::KeyVarianceCheck;
59+
use crate::binding::binding::KeyYield;
60+
use crate::binding::binding::KeyYieldFrom;
4061
use crate::binding::binding::Keyed;
4162
use crate::binding::bindings::BindingEntry;
4263
use crate::binding::bindings::BindingTable;

pyrefly/lib/binding/bindings.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ use vec1::Vec1;
5252
use vec1::vec1;
5353

5454
use crate::binding::binding::AnnotationTarget;
55+
use crate::binding::binding::AnyIdx;
5556
use crate::binding::binding::Binding;
5657
use crate::binding::binding::BindingAnnotation;
5758
use crate::binding::binding::BindingExport;

pyrefly/lib/commands/check.rs

Lines changed: 70 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ use crate::state::require::RequireLevels;
7676
use crate::state::state::State;
7777
use crate::state::state::Transaction;
7878
use crate::state::subscriber::ProgressBarSubscriber;
79+
use crate::state::subscriber::TestSubscriber;
7980

8081
/// Result data from a non-watch check run, used for telemetry logging.
8182
pub struct CheckResult {
@@ -262,6 +263,10 @@ struct OutputArgs {
262263
/// Format for pysa report output (json or capnp)
263264
#[arg(long, value_enum, default_value_t = report::pysa::PysaFormat::Capnp)]
264265
report_pysa_format: report::pysa::PysaFormat,
266+
/// Report the cross-module demand tree (aggregated summary of LookupAnswer
267+
/// and LookupExport calls). Useful for analyzing laziness properties.
268+
#[arg(long, value_name = "OUTPUT_FILE")]
269+
report_demand_tree: Option<PathBuf>,
265270
/// Generate a CinderX-format type report (experimental, internal-only).
266271
#[arg(long, value_name = "OUTPUT_DIR", hide = true)]
267272
report_cinderx: Option<PathBuf>,
@@ -949,15 +954,22 @@ impl CheckArgs {
949954
}
950955

951956
let type_check_start = Instant::now();
952-
let show_progress_bar =
953-
self.output.summary != Summary::None && !self.output.no_progress_bar;
954-
if show_progress_bar {
955-
transaction.set_subscriber(Some(Box::new(ProgressBarSubscriber::new())));
956-
}
957+
let demand_tree_subscriber = if self.output.report_demand_tree.is_some() {
958+
transaction
959+
.set_demand_collector(Some(pyrefly_util::demand_tree::DemandCollector::new()));
960+
let sub = TestSubscriber::new();
961+
transaction.set_subscriber(Some(Box::new(sub.dupe())));
962+
Some(sub)
963+
} else {
964+
let show_progress_bar =
965+
self.output.summary != Summary::None && !self.output.no_progress_bar;
966+
if show_progress_bar {
967+
transaction.set_subscriber(Some(Box::new(ProgressBarSubscriber::new())));
968+
}
969+
None
970+
};
957971
transaction.run(handles, require, None);
958-
if show_progress_bar {
959-
transaction.set_subscriber(None);
960-
}
972+
transaction.set_subscriber(None);
961973

962974
let loads = if self.behavior.check_all {
963975
transaction.get_all_errors()
@@ -1201,6 +1213,12 @@ impl CheckArgs {
12011213
report::dependency_graph::dependency_graph(transaction, handles),
12021214
)?;
12031215
}
1216+
if let Some(path) = &self.output.report_demand_tree {
1217+
let roots = transaction.take_demand_roots();
1218+
let module_steps = demand_tree_subscriber.unwrap().finish_detailed();
1219+
let output = demand_tree_report_json(&roots, &module_steps);
1220+
fs_anyhow::write(path, output)?;
1221+
}
12041222
if self.behavior.expectations {
12051223
loads.check_against_expectations()?;
12061224
Ok((CommandExitStatus::Success, output_errors))
@@ -1212,6 +1230,50 @@ impl CheckArgs {
12121230
}
12131231
}
12141232

1233+
/// Serialize the demand tree and per-module step info as a pretty-printed
1234+
/// JSON document.
1235+
fn demand_tree_report_json(
1236+
roots: &[pyrefly_util::demand_tree::DemandNode],
1237+
module_steps: &SmallMap<Handle, crate::state::subscriber::TestModuleInfo>,
1238+
) -> String {
1239+
use serde::Serialize;
1240+
1241+
#[derive(Serialize)]
1242+
struct ModuleStep {
1243+
module: String,
1244+
last_step: &'static str,
1245+
}
1246+
1247+
#[derive(Serialize)]
1248+
struct Report<'a> {
1249+
module_steps: Vec<ModuleStep>,
1250+
demand_tree: &'a [pyrefly_util::demand_tree::DemandNode],
1251+
}
1252+
1253+
let mut steps: Vec<ModuleStep> = module_steps
1254+
.iter()
1255+
.map(|(handle, info)| ModuleStep {
1256+
module: handle.module().as_str().to_owned(),
1257+
last_step: match info.last_step {
1258+
Some(crate::state::steps::Step::Load) => "Load",
1259+
Some(crate::state::steps::Step::Ast) => "Ast",
1260+
Some(crate::state::steps::Step::Exports) => "Exports",
1261+
Some(crate::state::steps::Step::Answers) => "Answers",
1262+
Some(crate::state::steps::Step::Solutions) => "Solutions",
1263+
None => "Nothing",
1264+
},
1265+
})
1266+
.collect();
1267+
// Stable ordering makes diffing reports tractable.
1268+
steps.sort_by(|a, b| a.module.cmp(&b.module));
1269+
1270+
let report = Report {
1271+
module_steps: steps,
1272+
demand_tree: roots,
1273+
};
1274+
serde_json::to_string_pretty(&report).expect("demand tree report should always serialize")
1275+
}
1276+
12151277
#[cfg(test)]
12161278
mod tests {
12171279
use std::path::PathBuf;

0 commit comments

Comments
 (0)