Skip to content

Commit a25009d

Browse files
FrozenPandazFrozenPandaz
committed
fix(core): intern hash instructions in a pool and plan with id lists (#36249)
Hash plans are `HashMap<task, Vec<HashInstruction>>` — every task owns deep copies of every instruction in its dependency closure. The distinct-instruction population is only a few thousand values (a couple per project × input), but plans store them O(tasks × closure) times: ~1.1M copies × ~200 bytes on a 1,110-task benchmark with deep closures. This is the structure that #35071's sibling-inputs correctness fix legitimately inflated (the real mechanism behind NXC-4605's +91 MiB), and it exists in every parallel DTE agent process (#36152). Plans store `u32` ids into an `InstructionPool` interner and travel with it as `HashPlans` through the planner→hasher `External`: - The interner's entry()-serialized id allocation guarantees value-equal ⇒ id-equal, so integer sort+dedup ≡ value-level dedup. - Dependency subtrees are memoized per (project, propagated input) as id lists and spliced into plans as integer memcpy — zero materialization on the O(tasks × closure) path. Deps-outputs subtrees and cyclic graphs use the existing per-task traversal, interned at the boundary. - `task_hasher` / `hash_plan_inspector` resolve ids in place; the string-returning `getPlans` API materializes and Ord-sorts, keeping observable behavior identical. **Measured** (densified bench: 1,110 tasks, avg closure ~555, 3 runs): planning 373 ms → 144 ms (~2.6×), peak process RSS 947 MB → 658 MB (−31%), `hash_plans` unchanged. Task hashes byte-for-byte identical — 72 TS hasher+planner tests and all Rust tests pass. Negative control: the same memo returning materialized instructions measured 2.8× slower; the representation change is the entire win. > [!NOTE] > **Stacked on #36248** (uses its `VisitedTracker`); retarget to `master` once that merges. Draft pending: full e2e sweep, a committed `bench:plan` variant (the stock benchmarks workspace has no dependency inputs and cannot exercise this path), scoped clone of the pool `Ref` in the Runtime hash arm, and a proper opaque d.ts type name. Fixes NXC-4607 <!-- polygraph-session-start --> --- [View session information ↗](https://app.trypolygraph.com/orgs/6a061dcb561c062131116eca/sessions/NXC-4603-Workspace-Fileset-Cache-Memory-Fix-69f28e06) <!-- polygraph-session-end --> --------- Co-authored-by: nx-cloud[bot] <71083854+nx-cloud[bot]@users.noreply.github.com> Backport notes: - 22.7.x builds `external_deps_mapped` per get_plans call rather than memoizing it on the planner, so the struct keeps no such field here. The subtree memo is still safe: its values derive only from the project graph and nx_json, which are immutable either way. - `memoized_dep_subtree` and `compute_dep_subtree` take 22.7.x borrowed `hashbrown::HashMap<&String, Vec<&String>>` instead of the owned map. - `OnceCache` comes from #36244, which is otherwise skipped: 22.7.x has no workspace fileset cache for that PR to fix. Only the type is taken.
1 parent c885d84 commit a25009d

6 files changed

Lines changed: 389 additions & 75 deletions

File tree

‎packages/nx/src/native/tasks/hash_plan_inspector.rs‎

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use crate::native::tasks::hashers::{
33
hash_workspace_files_with_inputs, resolve_task_output_files,
44
};
55
use crate::native::tasks::task_hasher::{HashInputs, HashInputsBuilder};
6-
use crate::native::tasks::types::HashInstruction;
6+
use crate::native::tasks::types::{HashInstruction, HashPlans};
77
use crate::native::types::FileData;
88
use hashbrown::HashSet;
99
use napi::bindgen_prelude::External;
@@ -40,18 +40,19 @@ impl HashPlanInspector {
4040
#[napi(ts_return_type = "Record<string, string[]>")]
4141
pub fn inspect(
4242
&self,
43-
hash_plans: &External<HashMap<String, Vec<HashInstruction>>>,
43+
#[napi(ts_arg_type = "ExternalObject<Record<string, Array<HashInstruction>>>")]
44+
hash_plans: &External<HashPlans>,
4445
) -> anyhow::Result<HashMap<String, Vec<String>>> {
4546
let project_file_set_cache = ProjectFileSetCache::new();
47+
let pool = &hash_plans.pool;
4648
let results: Vec<(&String, Vec<String>)> = hash_plans
49+
.plans
4750
.iter()
48-
.flat_map(|(task_id, instructions)| {
49-
instructions
50-
.iter()
51-
.map(move |instruction| (task_id, instruction))
52-
})
51+
.flat_map(|(task_id, ids)| ids.iter().map(move |id| (task_id, *id)))
5352
.par_bridge()
54-
.map(|(task_id, instruction)| {
53+
.map(|(task_id, id)| {
54+
let instruction_ref = pool.get(id);
55+
let instruction = instruction_ref.value();
5556
let strings = match instruction {
5657
// File-set instructions: resolve to actual file paths
5758
HashInstruction::WorkspaceFileSet(_)
@@ -88,20 +89,20 @@ impl HashPlanInspector {
8889
#[napi(ts_return_type = "Record<string, HashInputs>")]
8990
pub fn inspect_inputs(
9091
&self,
91-
hash_plans: &External<HashMap<String, Vec<HashInstruction>>>,
92+
#[napi(ts_arg_type = "ExternalObject<Record<string, Array<HashInstruction>>>")]
93+
hash_plans: &External<HashPlans>,
9294
) -> anyhow::Result<HashMap<String, HashInputs>> {
9395
let project_file_set_cache = ProjectFileSetCache::new();
96+
let pool = &hash_plans.pool;
9497
let results: Vec<(&String, HashInputsBuilder)> = hash_plans
98+
.plans
9599
.iter()
96-
.flat_map(|(task_id, instructions)| {
97-
instructions
98-
.iter()
99-
.map(move |instruction| (task_id, instruction))
100-
})
100+
.flat_map(|(task_id, ids)| ids.iter().map(move |id| (task_id, *id)))
101101
.par_bridge()
102-
.map(|(task_id, instruction)| {
103-
let builder =
104-
self.resolve_instruction_inputs(instruction, &project_file_set_cache)?;
102+
.map(|(task_id, id)| {
103+
let instruction_ref = pool.get(id);
104+
let builder = self
105+
.resolve_instruction_inputs(instruction_ref.value(), &project_file_set_cache)?;
105106
Ok::<_, anyhow::Error>((task_id, builder))
106107
})
107108
.collect::<anyhow::Result<_>>()?;

0 commit comments

Comments
 (0)