# XCE Resource Requirement AST & CSE Design > **Status**: Draft > **Last Updated**: 2026-04-27 > **Revision**: 6 (multi-round review: resmgr API, atomic memory order, sel_make_key op2, makeString TOCTOU) > **Target**: `src/lib/` (C shared library), `src/include/resreq.h` --- ## Table of Contents 1. [Background](#1-background) 2. [Full Resreq AST Goals](#2-full-resreq-ast-goals) 3. [Select Expression AST](#3-select-expression-ast) 4. [Resource Types](#4-resource-types) 5. [CSE via Hash-Consing](#5-cse-via-hash-consing) 6. [Commutativity Canonicalization](#6-commutativity-canonicalization) 7. [Parse-Time Type Validation](#7-parse-time-type-validation) 8. [Evaluation in Host Context](#8-evaluation-in-host-context) 9. [Job-Level Sharing](#9-job-level-sharing) *(§9.2–§9.3 are future work, not in current scope)* 10. [Scheduling Session Semantics](#10-scheduling-session-semantics) 11. [Implementation Plan](#11-implementation-plan) 12. [Appendix: Data Structures](#12-appendix-data-structures) --- ## 1. Background The existing resreq parser has a 3-level hierarchy: ``` xce_resreq_t (alter / compound / primary) └─ xce_primary_resreq_t ├─ xce_select_t ← EMPTY STUB (struct { }) ├─ xlist_t rusages ← fully parsed (rusage.peg) ├─ xce_span_t ← EMPTY STUB ├─ xce_same_t ← EMPTY STUB └─ xce_affinity_t ← EMPTY STUB ``` `section.peg` splits the raw resreq string into clause sub-strings. The `select` clause may appear in two forms: explicit (`select[expr]`) or implicit (bare expression before any other clause keyword). The parser extracts `selectString`, `rusageString`, `spanString`, etc. `rusage.peg` fully parses the rusage clause. The select clause is extracted but never parsed — `EvalSelectForHost` in the Go backend is a no-CGO stub that always returns `errNoCGO`. Phase 6 of this design wires evaluation directly into the C scheduler; the Go stub is not replaced — the Go API server uses the unoptimized parse path (`sel_optimize_level=0`, §5.0). The API server may also call `eval_select_simple` for client-side host-filtering queries (§5.0), but at far lower frequency than the scheduler. The primary host-matching evaluation loop runs in the C scheduler (`sel_optimize_level=1`). This document describes the design to: - Fully parse the select clause into a boolean expression AST. - Apply **Common Subexpression Elimination (CSE)** at the AST node level so identical sub-expressions across all jobs share a single allocation. In production HPC workloads, many jobs share identical or overlapping select expressions (e.g., `type==x86_64 && gpu`). CSE at the AST node level allows these jobs to share interned subtrees, yielding significant memory savings and reducing allocation pressure during scheduling — validated by experience with IBM LSF's select clause implementation. A direct example: when a user submits a large job array, all elements of the array share the same resource requirement string, so CSE allows a single interned AST to represent thousands of jobs. In a busy EDA cluster (10,000 nodes, 10M jobs, 1M/hour throughput), the number of unique EDA tool configurations is limited (< 100), so many jobs share the same select expression. Node-level CSE provides structural sharing even when full resreq strings differ (e.g., same `select[...]` but different `rusage[...]`), which string-level interning alone cannot achieve. - **Evaluate each unique AST node at most once per host per scheduling session** — the core performance guarantee of this design. Because CSE ensures all structurally identical sub-expressions resolve to a single shared `xce_select_node_t*`, a per-host memoization bitmap (`bool_bits` in §8.5) can record whether a node has already been evaluated for the current host. In a session with 10,000 jobs all sharing the node `type==linux`, that node is evaluated at most **once** per host per evaluation path that reaches it — AND/OR short-circuit may skip nodes in un-taken branches, but any node that IS reached is evaluated at most once. Without this guarantee, evaluation cost would scale with jobs × hosts rather than with unique_nodes × hosts. - Evaluate that AST efficiently in the context of a host's live resource values. - Run a **mark-and-sweep GC** between scheduling sessions to recycle interned nodes that are no longer reachable from any live job (§5.5). --- ## 2. Full Resreq AST Goals All clauses in `xce_primary_resreq_t` will eventually be filled. This document focuses on `select` as the first clause. - **`span`** controls task span across nodes (e.g., for MPI jobs). - **`affinity`** controls hardware topology allocation (e.g., for OpenMP jobs). - **`same`** constrains co-allocation of tasks on the same node. These are advanced features; their design and implementation are separate future work. The CSE and intern table technique described here is applicable to them but should not be considered final for those use cases. **Scope of this document:** The `select` clause grammar, AST, CSE, and evaluation are in scope. The `rusage`, `span`, `same`, and `affinity` clauses are out of scope; they are listed above for context only. **In-scope implementation requirements for scheduler integration (Phase 6):** - GC idle invariant predicates (`sched_session_active`, `sched_eval_in_progress`, `staging_merge_complete`) — required to enforce the §5.5 runtime guard that prevents use-after-free when GC runs. These are tested as stubs in Phase 1 and wired to real `xce_scheduler_t` fields in Phase 6 (see Phase 6 for the stub-to-production migration contract). - live_jobs materialization — iterating `sched->jobs` to produce the flat `const xce_sched_job_t**` array required by `gc_sweep_select_nodes`. This is a Phase 6 deliverable, not a standalone library feature. **Note on implicit select syntax:** The implicit select form (bare expression before any other clause keyword, §3.1) requires the select parser to recognize `rusage`, `span`, `same`, and `affinity` as clause-boundary tokens in order to know where the select expression ends. Recognizing these keyword boundaries is in scope; implementing the other clauses is not. The resreq string grammar (outer structure, already implemented): ``` resreq ::= alter_resreq | compound_resreq | primary_resreq alter_resreq ::= '{' compound '}' ( '||' '{' compound '}' )* compound_resreq ::= n '*' '{' primary '}' ( '+' n '*' '{' primary '}' )* primary_resreq ::= select_clause rusage_clause span_clause same_clause affinity_clause select_clause ::= 'select' '[' select_expr ']' | select_expr (see §3) rusage_clause ::= 'rusage' '[' rusage_list ']' | ε span_clause ::= 'span' '[' span_spec ']' | ε same_clause ::= 'same' '[' same_spec ']' | ε affinity_clause ::= 'affinity' '[' affinity_spec ']' | ε ``` **Clause keyword semantics when empty (ε):** | Clause | Meaning when absent | Equivalent | |--------|-------------------|------------| | `select_clause` | Match all hosts | `type == any` → `SEL_TRUE` | | `rusage_clause` | No per-job resource consumption; scheduler checks only user/queue/project limits | — | | `span_clause` | Skip span constraint handling | — | | `same_clause` | Skip same-node co-allocation constraint | — | | `affinity_clause` | Skip hardware topology affinity constraint | — | **Implicit select (bracket-free):** If the first token of a `primary_resreq` is not one of the clause keywords (`select`, `rusage`, `span`, `same`, `affinity`), the parser treats it as the start of a `select_expr`. This means the `select` keyword and brackets can be omitted when select is the first (and often only) clause: ``` type==linux && gpu ← implicit select (no keyword, no brackets) select[type==linux && gpu] ← explicit select (keyword + brackets) rusage[mem=100] ← no select clause, match all hosts type==linux rusage[mem=100] ← implicit select + rusage ``` The implicit form is the common case in LSF-compatible workflows. The explicit bracketed form (`select[...]`) is kept for compatibility and for cases where the expression starts with a clause keyword name. For example, if a resource is named `rusage`, writing `rusage > 5` unquoted would be parsed as the start of a `rusage[...]` clause — use `select[rusage > 5]` instead. --- ## 3. Select Expression AST ### 3.1 Grammar (new `lib/xce_select.peg`) ``` select_expr ::= select_or EOF select_or ::= select_and ( '||' select_and )* select_and ::= select_not ( '&&' select_not )* select_not ::= '!' select_primary | select_primary select_primary ::= '(' select_or ')' | comparison | resname comparison ::= resname cmp_op value cmp_op ::= '>=' | '<=' | '!=' | '==' | '>' | '<' resname ::= [a-zA-Z_][a-zA-Z_0-9]* (excluding 'any') value ::= string_literal | number | ANY ANY ::= 'any' number ::= [0-9]+ ('.' [0-9]*)? string_literal ::= [a-zA-Z_][a-zA-Z_0-9]* (unquoted word, excluding 'any') ``` **Resource name / clause keyword conflict (implementation requirement):** The `resname` production matches any identifier except `any`. This means a resource named `rusage`, `span`, `same`, `affinity`, or `select` is syntactically valid in the inner `xce_select.peg` grammar, but causes ambiguity in the outer `section.peg` implicit-select boundary detection (see §2 implicit select discussion). The document at §2 explains the workaround (use explicit `select[...]` form), but this relies on users knowing the constraint. **Runtime enforcement required (Phase 4b `resname` PEG action):** In the `resname` action of `xce_select.peg`, after the identifier is matched, add a validation check before calling `makeSelectNode`: ```c /* In the resname action — after identifier capture: */ static const char* clause_keywords[] = { "select", "rusage", "span", "same", "affinity", NULL }; for (int i = 0; clause_keywords[i]; i++) { if (strcmp(ident, clause_keywords[i]) == 0) { snprintf(auxil->errmsg, ERRMSG_MAX, "resource name '%s' conflicts with a clause keyword — " "use explicit select[%s ...] form if this is intentional", ident, ident); return -1; /* parse failure */ } } ``` This gives a clear error at parse time rather than silently misparsing. `any` is already excluded by the grammar rule; the clause keywords are not and must be checked explicitly in the action. **No `true`/`false` keywords.** The words `true` and `false` are **not** reserved and are **not** part of the select grammar. Boolean resources use the standalone idiom only: - `gpu` — the resource's value IS the boolean (idiomatic) - `!gpu` — negation of a boolean resource Writing `gpu == true` or `gpu == false` is a **type mismatch parse error**: `true`/`false` are not recognized as keywords, so they are parsed as `SEL_STR` string literals. Since `gpu` is a `RES_TYPE_BOOL` resource, comparing it to a string literal (`SEL_STR`) fails the type-compatibility check in the `==` action (§7). This produces a clear error message at job submission time rather than silently const-folding as in the previous design. **Why remove `true`/`false`:** The previous design parsed `true`/`false` as keywords and const-folded `bool_res == true` → `bool_res`. This was confusing for two reasons: (1) it encouraged a non-idiomatic form that only works for boolean resources — writing `type == true` for a string resource would const-fold incorrectly; (2) the const-fold made `gpu == true` and `gpu` behave identically, obscuring whether the user understood boolean resource semantics. Rejecting `gpu == true` with a type error forces the idiomatic `gpu` form and makes the type system visible to users. `SEL_TRUE` and `SEL_FALSE` still exist as **internal node types** produced by const-folding (e.g., `type == any` → `SEL_TRUE`), but they are never produced directly from user input — only by transform passes. **LSF-compatible disambiguation:** `resname` and `string_literal` share the same lexical pattern. The parser resolves the ambiguity using the resource manager: if a bare word matches a known resource name, it is parsed as `SEL_RESNAME`; otherwise it is treated as `SEL_STR` (a string literal). This is compatible with IBM LSF's select clause semantics. For example, `type==linux` — `type` resolves to `SEL_RESNAME` (known resource), `linux` resolves to `SEL_STR` (not a known resource name). String values in LSF select expressions are unquoted words. **`any` keyword:** The special keyword `any` matches any value. It is only valid as the right-hand side of `==` or `!=` (e.g., `type == any`, `model == any`). At parse time, `any` produces a `SEL_ANY` leaf node. The const-fold pass (§6.3) reduces `resname == any` to `SEL_TRUE`, so `eval_select` never encounters `SEL_ANY` at runtime. **`!= any` is valid and const-folds to `SEL_FALSE`:** Both `resname != any` and `!(resname == any)` are accepted at parse time and produce identical results. `resname == any` const-folds to `SEL_TRUE` (§6.3); `resname != any` const-folds directly to `SEL_FALSE` (§6.3); `!(resname == any)` becomes `!SEL_TRUE` → `SEL_FALSE` via the complement law. All three paths yield the same interned singleton `SEL_FALSE` — no host matches, since `any` matches all resource values. Accepting `!= any` explicitly removes a surprising asymmetry and avoids user confusion. **String literals in comparisons:** Unquoted words that are not known resource names and not the reserved keyword `any` are treated as string literals. For example, `type == x86_64` — `type` resolves to `SEL_RESNAME` (known resource), `x86_64` resolves to `SEL_STR` (not a known resource name). String values in comparisons do not need quotes — LSF select expressions use unquoted words. ### 3.2 Node Types ```c typedef enum { SEL_AND, /* && — commutative */ SEL_OR, /* || — commutative */ SEL_NOT, /* ! — unary */ SEL_GT, /* > — numeric only */ SEL_LT, /* < — numeric only */ SEL_GE, /* >= — numeric only */ SEL_LE, /* <= — numeric only */ SEL_EQ, /* == — commutative, all types */ SEL_NE, /* != — commutative, all types */ SEL_RESNAME, /* leaf: resource variable */ SEL_NUM, /* leaf: numeric literal */ SEL_STR, /* leaf: string literal */ SEL_ANY, /* leaf: any keyword — always eliminated by const-fold: * resname == any → SEL_TRUE * resname != any → SEL_FALSE (see §6.3) */ SEL_TRUE, /* leaf: constant true — produced by const-fold only, not a keyword */ SEL_FALSE, /* leaf: constant false — produced by const-fold only, not a keyword */ } sel_node_type_t; ``` ### 3.3 Node Structure ```c struct xce_select_node_ { sel_node_type_t type; uint32_t hashcode; /* cached at construction */ uint32_t node_id; /* dense index into eval context arrays (§8.5) */ unsigned gc_mark; /* set during mark-and-sweep GC (§5.5) */ union { struct { xce_select_node_t *left, *right; } binop; /* AND / OR / GT / LT / GE / LE / EQ / NE */ xce_select_node_t* operand; /* NOT */ struct { int res_idx; /* resolved at parse time via resmgr */ res_type_t res_type; /* stored for eval dispatch, not in CSE key */ } resname; double numval; /* SEL_NUM */ char* strval; /* SEL_STR — interned via makeString() */ } u; }; ``` `struct xce_resreq_select_` becomes: ```c struct xce_resreq_select_ { xce_select_node_t* root; /* reference to interned node tree */ }; ``` --- ## 4. Resource Types Resources have exactly one of three types, known from the resource definition: ```c typedef enum { RES_TYPE_NUMERIC = 0, /* float — supports all comparison operators */ RES_TYPE_STRING, /* char* — supports == != only */ RES_TYPE_BOOL, /* int — standalone boolean use */ } res_type_t; ``` Operator validity by resource type: | Operator | Numeric | String | Boolean | |----------|:-------:|:------:|:-------:| | `>` `<` `>=` `<=` | ✓ | parse error | parse error | | `==` `!=` | ✓ | ✓ | ✓ (vs numeric literal, string literal, or boolean;¹ `true`/`false` not keywords) | | standalone (no op) | ✓ (non-zero) | parse error | ✓ | ¹ Boolean resources in `==`/`!=` comparisons: the RHS can be a boolean resource (e.g., `gpu == gpu2`: BOOL vs BOOL) or a numeric literal (e.g., `gpu == 1`, `gpu != 0`: BOOL vs NUMERIC). String literals cause a type mismatch error (e.g., `gpu == true` is rejected: BOOL vs STRING, since `true`/`false` are not keywords and parse as strings). **Migration diagnostic:** When `true` or `false` appears as an identifier (parsed as SEL_RESNAME with no matching resource), the parser should emit a helpful error: `"unknown resource 'true' — did you mean the boolean resource name directly (e.g. 'gpu') or a numeric literal (1/0)?"`. This aids migration from IBM LSF where `true`/`false` are accepted boolean literals. **Note:** `res_type_t` maps directly to the existing `XCE_VAL_*` constants in `src/include/xce/types.h` (`XCE_VAL_STRING=0`, `XCE_VAL_NUMERIC=1`, `XCE_VAL_BOOLEAN=2`). The numeric values differ in order, so an explicit mapping is required: ```c /* res_type_t → existing valtype mapping. * NOTE: This function has no consumer in the core eval path and is * intentionally omitted from the Phase 2 build — see Phase 2 scope decision * ("res_type_to_valtype has no current consumer … omit it to avoid dead code"). * It is documented here for reference only; do not compile it into the * production build without a concrete consumer. */ static inline int res_type_to_valtype(res_type_t t) { static const int map[] = { [RES_TYPE_NUMERIC] = XCE_VAL_NUMERIC, /* 0 → 1 */ [RES_TYPE_STRING] = XCE_VAL_STRING, /* 1 → 0 */ [RES_TYPE_BOOL] = XCE_VAL_BOOLEAN, /* 2 → 2 */ }; return map[t]; } /* valtype → res_type_t inverse mapping. * Declared as an API helper in Phase 2; used in error-reporting paths * that need to describe mismatched operand types back to the caller. * No direct caller in the core eval path — eval works with res_type * cached at parse time in each node. */ static inline res_type_t valtype_to_res_type(int v) { switch (v) { case XCE_VAL_NUMERIC: return RES_TYPE_NUMERIC; case XCE_VAL_STRING: return RES_TYPE_STRING; case XCE_VAL_BOOLEAN: return RES_TYPE_BOOL; default: /* Unknown valtype — should never occur. Use log_fatal (not assert) * so the invariant is enforced in production builds too (§5.5). */ log_fatal("unknown valtype %d in valtype_to_res_type", v); return RES_TYPE_NUMERIC; /* unreachable — log_fatal terminates */ } } ``` The PEG action for `resname` resolution must look up the resource in the resmgr by name, retrieve its `valtype` (using `XCE_VAL_*`), and map it to `res_type_t` via the inverse mapping before storing it in the SEL_RESNAME node. **res_idx validity guard:** `key_find(resmgr, name)` returns a non-negative index on success, or a negative sentinel (e.g., `-ENOENT`) on failure. Before calling `makeSelectNode(SEL_RESNAME, ...)`, the action **must** check that the returned index is non-negative: ```c /* PEG action for a bare word that resolves to a known resource name. * Uses the real resmgr API: key_find returns 1 on hit (r != NULL), * -ENOENT on miss — matching the pattern in resource.c (acct->residx = r->idx). */ xce_resource_t* r = NULL; int rc = key_find(auxil->resmgr, (void*)name, (void**)&r); /* GUARD: key_find returns 1 on hit; anything <= 0 means unknown resource. * Passing a non-negative res_idx is mandatory before calling makeSelectNode: * a negative value cast to uintptr_t produces UINTPTR_MAX (or similar large * value), which would intern a SEL_RESNAME node with a permanently wrong * node_id, causing false CSE cache hits and incorrect eval results. * Unknown resource names are a parse error — see §7. */ if (rc <= 0 || r == NULL) { auxil->has_error = 1; $$ = NULL; } else { int res_idx = r->idx; res_type_t res_type = valtype_to_res_type(r->valtype); $$ = (char*)makeSelectNode(SEL_RESNAME, (uintptr_t)res_idx, (uintptr_t)res_type); } ``` This guard matches the stated invariant in §7: "Unknown resource names fail the parse (rejected at job submission time)." **String resources in host values:** `xvec_idval_t` stores only numeric resource values (float pairs). String host resources (e.g., `type`, `arch`) are resolved differently — the host's string properties are looked up by `res_idx` through a separate `(res_idx → const char*)` table, not through `xvec_idval_t`. Boolean resources are stored as nonzero/zero float values in `xvec_idval_t`. See §8 for the accessor functions. --- ## 5. CSE via Hash-Consing ### 5.0 Optimization Level (sel_optimize_level) CSE (hash-consing via the intern table) and related optimizations are controlled by a **process-lifetime tri-state flag** set once at process startup and never changed at runtime: ```c static _Atomic int sel_optimize_level = -1; /* -1 = uninitialized (default; write-once enforcement uses this as sentinel) * 0 = optimization disabled (API server mode: plain tree, no intern table) * >0 = CSE + const-fold enabled (scheduler mode: intern table + memoization) * * The sign encodes the initialization state — a separate lock variable is not * needed. Set exactly once via sel_optimize_set() before any parse_resreq or * makeSelectNode call. Never read or write directly; use the accessor below. */ ``` **Write-once enforcement via CAS on the sentinel:** The initial value of -1 is the uninitialized sentinel. `sel_optimize_set(level)` atomically transitions from -1 to `level` with a single compare-exchange: ```c void sel_optimize_set(int level) { if (level < 0) { log_fatal("sel_optimize_set: level must be >= 0 (got %d)", level); return; } int expected = -1; if (!atomic_compare_exchange_strong_explicit( &sel_optimize_level, &expected, level, memory_order_release, memory_order_acquire)) { /* CAS failed — already initialized */ int current = atomic_load_explicit(&sel_optimize_level, memory_order_acquire); if (current != level) log_fatal("sel_optimize_level cannot be changed after first use " "(current=%d, attempted=%d)", current, level); /* else: idempotent re-set to the same value — tolerated for test convenience */ } } static inline int sel_get_optimize_level(void) { return atomic_load_explicit(&sel_optimize_level, memory_order_acquire); } ``` The scheduler calls `sel_optimize_set(1)` at startup; the API server calls `sel_optimize_set(0)`. Neither assigns `sel_optimize_level` directly. Unit tests that need the disabled path call `sel_optimize_set(0)` or rely on the -1 sentinel (makeSelectNode treats both < 0 and == 0 as non-CSE paths; a guard at the top of makeSelectNode logs_fatal if level is still -1 at call time, catching unconfigured processes early). **Test setup for CSE-enabled path:** Unit tests that exercise CSE interning (`sel_optimize_level > 0`) must call `sel_optimize_set(1)` **at test setup**, before any `makeSelectNode` call. Each test that needs a fresh CSE state must call `sel_node_table_fini()` and then reset `sel_optimize_level` to -1 (via `atomic_store_explicit(&sel_optimize_level, -1, memory_order_seq_cst)`) in a teardown step, then call `sel_optimize_set(1)` again in the next test's setup. A shared `sel_optimize_test_reset()` helper (in a test-only header, guarded by `#ifdef UNIT_TEST`) should encapsulate this teardown — including the `atomic_store_explicit` — to prevent copy-paste in individual test files. All test-spawned threads that call `makeSelectNode` must be joined before `sel_optimize_test_reset()` is called, to avoid a concurrent CAS racing against the reset. | Process | Level | Rationale | |---------|-------|-----------| | **Scheduler** | `sel_optimize_set(1)` | Evaluates thousands of jobs per session; CSE eliminates redundant node allocation and enables evaluate-once-per-host memoization (§8.5). | | **API server** | `sel_optimize_set(0)` | Parses resreq for validation and optionally evaluates select expressions for client-side host-filtering queries (much less frequently than the scheduler — see below). No intern table is created; `eval_select_simple()` handles host evaluation without memoization overhead. | **API server host evaluation (unoptimized):** The API server may evaluate select expressions against host data for client-side host-filtering queries (e.g., a REST endpoint returning matching hosts for a given resreq, or a dry-run submission check). This happens far less frequently than the scheduler's per-session evaluation, so memoization is not needed. The API server uses `eval_select_simple(root, host)` — a plain recursive tree-walk that takes no `select_eval_ctx_t` and performs no caching: ```c /* Simple (non-memoized) tree-walk evaluator for sel_optimize_level == 0. * Returns 1 (match), 0 (no match), -1 (error). Thread-safe — no shared state. */ int eval_select_simple(const xce_select_node_t* root, const host_eval_ctx_t* host); ``` `eval_select_simple` is declared in `include/resreq.h` and shares the `host_eval_ctx_t` definition with the scheduler. It does not use `node_id` (all nodes have `node_id=0` in unoptimized mode — no collision risk because no shared bitmap exists). When `sel_optimize_level == 0`, `makeSelectNode` allocates a fresh node on every call (no intern table lookup or insertion), `gc_sweep_select_nodes` and `recompact_node_ids` are no-ops, and `sel_node_table_init` skips kvtab allocation. The `node_id` field is intentionally left as 0 (calloc zero-initialization) — it has no meaning without an intern table. `select_eval_ctx_t` is never constructed in this mode. **`eval_select` guard (optimized path only):** The memoized `eval_select` requires an initialized intern table and valid `node_id` values. It must reject calls in unoptimized mode: ```c /* At the top of eval_select / select_eval_ctx_init */ if (sel_get_optimize_level() <= 0) { log_fatal("eval_select called without optimization — use eval_select_simple"); return -1; } ``` Phase 4e must add this guard. A corresponding unit test verifies that `eval_select` is never reached when `sel_get_optimize_level() <= 0`. **Memory management in unoptimized mode:** Since nodes are not tracked in the intern table, the caller (parse layer) is responsible for freeing the AST when it is no longer needed. `kill_primary_resreq` must call a recursive `free_select_tree(root)` helper (not needed in enabled mode where GC handles reclamation) to walk and free each node, then free the `xce_select_t` container struct itself. For SEL_STR nodes, call `dereference(node->u.strval)` before `free(node)`, as all string values are xstring-interned via `makeString()`. **Conditional cleanup contract:** When `sel_get_optimize_level() == 0`, `kill_primary_resreq` must call `free_select_tree(primary->select->root)` then `FREEUP(primary->select)`. When `sel_get_optimize_level() > 0`, `kill_primary_resreq` must only null the root pointer (`primary->select->root = NULL`) — GC between sessions handles node reclamation. A debug assertion in `free_select_tree` should verify `sel_get_optimize_level() <= 0` to prevent accidental use in the wrong mode. ```c /* Free a select AST tree (unoptimized path only). * When sel_optimize_level > 0, GC reclaims nodes — do NOT call this. * Walks the tree post-order: free children before parent. */ static void free_select_tree(xce_select_node_t* node) { if (sel_get_optimize_level() > 0) { log_fatal("free_select_tree called in CSE-enabled mode — use GC for node reclamation"); return; } if (node == NULL) return; switch (node->type) { case SEL_AND: case SEL_OR: case SEL_GT: case SEL_LT: case SEL_GE: case SEL_LE: case SEL_EQ: case SEL_NE: free_select_tree(node->u.binop.left); free_select_tree(node->u.binop.right); break; case SEL_NOT: free_select_tree(node->u.operand); break; case SEL_STR: dereference(node->u.strval); /* release xstring refcount */ break; default: break; /* SEL_RESNAME, SEL_NUM, SEL_ANY, SEL_TRUE, SEL_FALSE: no extra allocs */ } free(node); } /* STACK DEPTH NOTE: free_select_tree has the same recursion structure as * eval_select_inner. Depth is bounded by SELECT_MAX_DEPTH (64) because * parse_select enforces that limit. Stack usage per frame is comparable to * eval_select_inner (~80-120 B), so the same 8 KB analysis applies — well * within the 8 MB default Linux thread stack. Any increase to SELECT_MAX_DEPTH * must revisit this analysis for BOTH eval_select_inner AND free_select_tree. */ ``` **Unoptimized ownership model (single rule):** When `sel_optimize_level == 0`, every allocated `xce_select_node_t` has exactly one owner at all times. Initially the PEG action that calls `makeSelectNode` owns the fresh allocation. Ownership transfers via two paths: (1) the node becomes a child of a composite node (parent owns it), or (2) the flatten pass (§6.2) replaces it — the replacing node becomes the new child and the original becomes an orphan that the flatten pass must free immediately (see below). At no point is a node both interned (tracked by the table) and individually owned — the two modes are mutually exclusive. `free_select_tree` walks from root and frees every reachable node; the flatten pass must free every node it replaces *before* returning, ensuring `free_select_tree` never encounters a freed node and never leaks an unreachable one. **Precondition:** `free_select_tree` must only be called after `sel_optimize_set(0)` has been called — calling it with `sel_optimize_level == -1` (uninitialized) is undefined behavior. A startup guard in `makeSelectNode` (`log_fatal` if level == -1) ensures this precondition is met before any tree is constructed. **Flatten-pass orphan handling in unoptimized mode:** The flatten pass (§6.2) replaces sub-trees via `makeSelectNode`, obtaining a new node and discarding the old one. When `sel_optimize_level > 0` this is safe — the intern table tracks all nodes and GC reclaims them. When `sel_optimize_level == 0`, the flatten pass must explicitly free every replaced (orphaned) node, because no intern table tracks them and `free_select_tree` only walks the final root. Implementation contract: `flatten_pass` must maintain a list of nodes it replaces (e.g. a stack-local `zarray_t`), and after re-interning the parent it must call `free(old_node)` (with `dereference(old_node->u.strval)` first for `SEL_STR`) on each orphaned node before returning. **Important:** only the replaced node itself is freed — its children are not, because the children have been re-parented (they are now children of the new node or of the existing interned node that `makeSelectNode` returned). Freeing a child that has been re-parented would cause a double-free when `free_select_tree` later walks the tree from root. **Concurrency in unoptimized mode:** The API server calls `parse_resreq` from multiple goroutines concurrently. When `sel_optimize_level == 0`, `makeSelectNode` **bypasses the shared intern table entirely** — it allocates a private (non-shared) node tree directly via `XCEMALLOC` without touching `xce_select_table`. There is therefore no shared intern table state, and concurrent parse calls are safe as long as each uses an independent `select_context_t` (stack-allocated per call — already the case). The private tree is owned by the caller and freed via `free_select_tree` when the request completes. Concurrent `eval_select_simple` calls are also safe — the function takes no shared state beyond the read-only AST tree and the caller-supplied `host_eval_ctx_t`. ### 5.1 Principle Every `xce_select_node_t` is interned in a global table keyed by its *logical content*. Two nodes with the same type and same operands always return the **same pointer**. This is **hash-consing**, used in compilers and functional language runtimes. **Pointer equality guarantee scope:** When `sel_optimize_level > 0`, pointer equality is guaranteed — `A && B` and `B && A` produce the same `xce_select_node_t*`, and equality checks can use `==` on pointers. When `sel_optimize_level == 0` (API server path), no interning occurs and each `makeSelectNode` call returns a fresh allocation. Callers in the disabled path must not rely on pointer equality for comparison — use structural equality or compare by value instead. The packcc PEG parser is bottom-up: leaves are reduced before their parents. By the time a parent node is constructed, all its children are already interned canonical pointers. Therefore: - **Equality of a composite node = pointer equality of its children.** - No recursive comparison is ever needed. - Hash and equality are both O(1) regardless of expression depth. **Immutability invariant:** Once a node is inserted into the intern table, its content (`type`, `u.*`, `hashcode`) is **never mutated**. `node_id` is reassigned by `recompact_node_ids` in the pre-session idle phase (§8.5.4) — this is the **only** permitted mutation of an interned node field, and it occurs only when no scheduling session is active and no `eval_select` call is in progress. The `type`, `u.*`, and `hashcode` fields are load-bearing: `eval_select` reads them lock-free during a session (§8.4), and the GC mark pass reads `type` and child pointers without holding any lock. Any code that modifies `type`, `u.*`, or `hashcode` of an interned node after insertion violates this invariant and is incorrect. ### 5.2 CSE Key ```c typedef struct { sel_node_type_t type; uintptr_t op1; /* left child ptr, res_idx, or numval bits */ uintptr_t op2; /* right child ptr, or 0 for unary / leaf */ } sel_node_key_t; ``` Key encoding per node type: | Node type | `op1` | `op2` | |-----------|-------|-------| | `SEL_AND` `SEL_OR` | `(uintptr_t)left` | `(uintptr_t)right` | | `SEL_GT` `SEL_LT` `SEL_GE` `SEL_LE` | `(uintptr_t)left` | `(uintptr_t)right` | | `SEL_EQ` `SEL_NE` | `(uintptr_t)min(left,right)` | `(uintptr_t)max(left,right)` | | `SEL_NOT` | `(uintptr_t)operand` | `0` | | `SEL_RESNAME` | `(uintptr_t)res_idx` | `0` (key only; caller passes `(uintptr_t)res_type` as op2, which `sel_make_key` zeroes for key purposes since `res_idx` uniquely determines `res_type`; the fill step stores the original op2 as `node->u.resname.res_type`) | | `SEL_NUM` | bit-cast of `double` | `0` | | `SEL_STR` | `(uintptr_t)xstring_ptr` | `0` | | `SEL_ANY` | `0` | `0` | | `SEL_TRUE` | `0` | `0` | | `SEL_FALSE` | `0` | `0` | **Singleton disambiguation:** `SEL_ANY`, `SEL_TRUE`, and `SEL_FALSE` all encode `(op1=0, op2=0)`, so their CSE keys differ only in the `type` field. Since `type` is always compared first in `sel_key_cmp`, they produce three distinct intern table entries with no collision. `SEL_NUM` uses the exact bit-pattern of the `double` value reinterpreted as `uintptr_t` (via `memcpy` or union cast). This is safe on all supported 64-bit platforms where `sizeof(uintptr_t) >= sizeof(double)`, but must be guarded by a compile-time assertion in `xce_select.c`: `static_assert(sizeof(uintptr_t) == sizeof(double), "uintptr_t must exactly hold a double bit-pattern — equality required, not merely >=, so sizeof(op1) copies all 8 bytes without truncation");` Two numeric literals are equivalent for CSE if their bit-patterns are identical; `0.0` and `-0.0` are distinct CSE keys (they differ in the sign bit). NaN payloads are also compared by bit-pattern. **Boundary note:** while IEEE 754 defines `0.0 == -0.0` as true, CSE treats them as distinct interned nodes. At evaluation time, both produce the same boolean result (both are 0.0), so distinct node IDs for `0.0` and `-0.0` merely occupy two memoization slots that return the identical value — a minor memory cost, not a correctness issue. In HPC resource expressions, `-0.0` is not a meaningful resource value and will not appear in practice, but the bit-pattern comparison ensures CSE correctness for all float representations. `SEL_STR` uses the interned `xstring` pointer — since `makeString()` already canonicalizes strings (libxce's string de-dup mechanism maps identical strings to the same pointer), pointer equality IS string equality. `SEL_ANY`, `SEL_TRUE`, and `SEL_FALSE` are singletons — all instances share the same interned pointer. **SEL_ANY interning requirement:** `SEL_ANY` must always be created via `makeSelectNode(SEL_ANY, 0, 0)` — never by allocating a raw `xce_select_node_t` and setting `type = SEL_ANY` directly. Only `makeSelectNode` assigns a `node_id` and adds the node to the intern table. A raw allocation bypasses interning, producing a node that is invisible to GC (leaked permanently) and has `node_id = 0` (conflicting with any other node at slot 0). The same rule applies to `SEL_TRUE` and `SEL_FALSE` — always use `makeSelectNode`, never construct them by hand. ### 5.3 Intern Table ```c #define SEL_MAX_INTERNED_NODES 1000000 /* hard cap on intern table size — * prevents unbounded memory growth from * malicious or burst job submissions. * * Failure mode: when makeSelectNode is called * after the cap is reached, it must: * 1. Log a warning (not fatal — the scheduler * can still run existing jobs). * 2. Set errno = ENOMEM and return NULL. * 3. The parse_select caller must treat a NULL * return as a parse failure and reject the * incoming job with PREASON_RESREQ_PARSE_FAIL. * * Sizing: 1,000,000 nodes × ~64 B/node ≈ 64 MB. * For typical HPC workloads (≤10 nodes/job, * ≤100,000 concurrent jobs) the effective high-water * mark is ~10,000 nodes post-GC dedup. 1,000,000 * provides a 100× safety margin. Tune via * compile-time override if workload analysis shows * a different bound is appropriate. */ static _Atomic uint32_t sel_node_next_id = 0; /* monotonically increasing, reassigned by recompact; * all writes occur under sel_node_table.lock and use * memory_order_release: atomic_fetch_add_explicit for * increment (§5.4), atomic_fetch_sub_explicit for rollback * (§5.4), and atomic_store_explicit for recompact (§8.5.4). * The pre-session read in select_eval_ctx_init * (§8.5.3) occurs WITHOUT the lock and uses * atomic_load_explicit(..., memory_order_acquire) * to ensure the final makeSelectNode store is * visible even when staging and session-init run * in the same thread but the compiler or CPU would * otherwise reorder the stores past the read. */ static struct { mylock_t lock; var table; /* kvtab keyed by sel_node_key_t — hash lookup only */ zarray_t nodes; /* all interned nodes in insertion order — iterated by GC and recompact */ } sel_node_table; ``` Modelled on `xstring_table` in `lib/xstring.c`. `sel_node_table.nodes` is an embedded `zarray_t` of `xce_select_node_t*` entries (not a pointer — allocated via `zarray_init` on the embedded struct, matching the codebase convention where all `zarray_t` fields are embedded rather than heap-allocated separately). It is the **only** data structure iterated by the mark-and-sweep GC (`gc_sweep_select_nodes`) and by `recompact_node_ids` — neither function requires an iterator over the kvtab. The kvtab exists solely for O(1) intern-key lookup; GC and compaction walk the linear `nodes` array instead. ### 5.4 `makeSelectNode` ```c xce_select_node_t* makeSelectNode(sel_node_type_t type, uintptr_t op1, uintptr_t op2) { /* CSE-disabled fast path (API server, sel_optimize_level == 0): * allocate a fresh node every call, no interning, node_id intentionally * left 0 (not meaningful without an intern table). */ /* ATOMIC READ: must use sel_get_optimize_level() (atomic_load_explicit with * memory_order_acquire), NOT a direct load of sel_optimize_level, which is * _Atomic int. A direct (non-atomic) load of an _Atomic variable is UB under * C11 §5.1.2.4 when the variable can be concurrently written. */ if (sel_get_optimize_level() <= 0) { xce_select_node_t* n = calloc(1, sizeof(*n)); if (n == NULL) { /* OOM in unoptimized mode: release caller's xstring ref (mirrors * the optimized-mode dereference calls at steps 3a/4/5 below). * Contract is symmetric: caller passes ownership of the xstring * refcount; makeSelectNode either stores it in the node (success) * or releases it (failure). Caller must NEVER dereference op1 * after this call regardless of outcome. */ if (type == SEL_STR) dereference((char*)op1); errno = ENOMEM; return NULL; } n->type = type; /* fill n->u.* from op1/op2 by type (same logic as step 4 below) */ /* SEL_STR ownership: node owns the xstring ref stored in n->u.strval. * free_select_tree calls dereference(n->u.strval) when freeing. */ return n; } /* 1. canonicalize commutative operators (see §6) */ switch (type) { case SEL_AND: case SEL_OR: case SEL_EQ: case SEL_NE: if (op1 > op2) { uintptr_t t = op1; op1 = op2; op2 = t; } break; default: break; } /* 2. build lookup key — use sel_make_key() to zero padding bytes */ sel_node_key_t key = sel_make_key(type, op1, op2); /* 3. lookup */ xce_select_node_t* node = NULL; mylock_lock(&sel_node_table.lock); int rc = key_find(sel_node_table.table, &key, (void**)&node); if (rc > 0) { mylock_unlock(&sel_node_table.lock); if (type == SEL_STR) dereference((char*)op1); /* release caller's ref — intern table already holds one */ return node; } /* 3a. intern table size check — must come AFTER the cache-hit path above. * A table at cap still serves already-interned nodes via key_find. Only * reject new insertions when the cap is reached. */ if (sel_node_next_id >= SEL_MAX_INTERNED_NODES) { mylock_unlock(&sel_node_table.lock); if (type == SEL_STR) dereference((char*)op1); /* cap reached — release caller's ref */ errno = ENOMEM; return NULL; } mylock_unlock(&sel_node_table.lock); /* Return value semantics — MUST branch explicitly before proceeding: * rc > 0 → cache hit (handled above — already returned) * rc == -ENOENT → cache miss — proceed to allocate below * rc == -EINVAL → bad arguments (null table, null key, etc.) — log_fatal */ if (rc == -EINVAL) { /* Intern table is in an invalid state; allocation would corrupt it. */ if (type == SEL_STR) dereference((char*)op1); /* release caller's ref */ log_fatal("makeSelectNode: key_find returned -EINVAL — intern table corrupt or uninitialized"); return NULL; } /* rc == -ENOENT: proceed to allocate */ /* 4. alloc and fill (node_id assigned inside lock in step 5) */ node = calloc(1, sizeof(*node)); if (node == NULL) { if (type == SEL_STR) dereference((char*)op1); /* alloc failed — release caller's ref */ errno = ENOMEM; return NULL; } node->type = type; node->hashcode = sel_key_hash(&key); /* fill u.* from op1/op2 by type … */ /* ORDERING INVARIANT: the fill of node->u.* from op1/op2 MUST occur here in * step 4, BEFORE the mylock_lock in step 5. Step 5 calls * dereference(node->u.strval) on the TOCTOU rollback path — this is only * correct if u.strval has already been set to (char*)op1. An implementation * that fills u.* after the step-5 lock acquisition would call * dereference(NULL) on the calloc-zeroed field, corrupting the xstring * refcount of any string that happens to match that refcount slot. */ /* 5. insert — re-check under lock for TOCTOU safety */ mylock_lock(&sel_node_table.lock); xce_select_node_t* existing = NULL; int rc2 = key_find(sel_node_table.table, &key, (void**)&existing); if (rc2 > 0) { /* another thread inserted first — use theirs, free ours */ mylock_unlock(&sel_node_table.lock); if (node->type == SEL_STR) dereference(node->u.strval); /* release xstring ref before free */ free(node); return existing; } /* Re-check cap under lock — another thread may have filled the table * between the first check and this second lock acquisition. */ if (sel_node_next_id >= SEL_MAX_INTERNED_NODES) { mylock_unlock(&sel_node_table.lock); if (node->type == SEL_STR) dereference(node->u.strval); /* release xstring ref before free */ free(node); errno = ENOMEM; return NULL; } int rc3 = key_add(sel_node_table.table, &key, node); if (rc3 < 0) { /* key_add failed — if EEXIST from a race, re-find and return existing */ if (rc3 == -EEXIST) { xce_select_node_t* raced = NULL; int rc4 = key_find(sel_node_table.table, &key, (void**)&raced); mylock_unlock(&sel_node_table.lock); if (node->type == SEL_STR) dereference(node->u.strval); /* release xstring ref before free */ free(node); return (rc4 > 0) ? raced : NULL; /* return existing or NULL on double-failure */ } mylock_unlock(&sel_node_table.lock); if (node->type == SEL_STR) dereference(node->u.strval); /* release xstring ref before free */ free(node); return NULL; /* ENOMEM or other error */ } /* Assign node_id and store in the linear nodes array — both under lock * to prevent concurrent staging-phase callers from producing duplicate IDs. * zarray_store returns the SLOT INDEX (>= 0, 0-based) on success or a * negative errno code on failure. Use `zrc < 0` (not `zrc == 0` or * `!zrc`) to detect failure — index 0 is a valid success return for the * first node stored and must NOT be treated as an error. */ node->node_id = atomic_fetch_add_explicit(&sel_node_next_id, 1, memory_order_release); int zrc = zarray_store(&sel_node_table.nodes, node); if (zrc < 0) { /* zarray_store failed (OOM) — roll back the ID and remove the kvtab entry. * memory_order_release is required on the fetch_sub even though the mutex is * held: select_eval_ctx_init reads sel_node_next_id WITHOUT the mutex (§5.3) * using memory_order_acquire. The release here synchronizes with that * lock-free reader, ensuring the decremented value is visible. The mutex * alone does not establish a happens-before with a lock-free reader, so * memory_order_relaxed would be incorrect. * * Lock-free reader race: if select_eval_ctx_init fires between the * atomic_fetch_add above and this fetch_sub, it may read the briefly- * incremented value (N+1) and size its eval context arrays to N+1. After * the rollback, the next makeSelectNode re-uses ID N for a legitimately * inserted node. An eval context sized to N+1 has one extra unused slot — * harmless (no overflow given the SEL_MAX_INTERNED_NODES cap) and correct. * * Rollback race safety: the freed node briefly has a valid node_id (N) * before sel_node_next_id is decremented back to N. A subsequent * allocation will reuse that ID for a different node. This is safe * because: * (a) makeSelectNode is only called from the allocator thread or staging * phase — never concurrently with eval_select (§8.4 invariant). * ENFORCEMENT REQUIREMENT: staging must be strictly single-threaded * (no thread-pool dispatch of parse_resreq); Phase 6 wires a * runtime sel_alloc_thread_id check to catch violations. Until * Phase 6 is complete, tests must not invoke makeSelectNode from * two threads simultaneously. * (b) No caller ever sees the rolled-back node's pointer (makeSelectNode * returns NULL) so no eval context can hold a pointer to it. * If the allocator-thread-only invariant is ever relaxed, this rollback * protocol must be re-evaluated before making that change. */ atomic_fetch_sub(&sel_node_next_id, 1, memory_order_release); key_del(sel_node_table.table, &key); mylock_unlock(&sel_node_table.lock); if (node->type == SEL_STR) dereference(node->u.strval); /* release xstring refcount */ free(node); errno = ENOMEM; return NULL; } mylock_unlock(&sel_node_table.lock); return node; } ``` **TOCTOU note:** The existing `makeString()` in `xstring.c` uses the same lock/find/unlock/alloc/lock/add pattern without a re-check. The select intern table adds the re-check to prevent node leaks under concurrent construction. An alternative is to hold the lock through the entire sequence (simpler but longer critical section). **makeString() TOCTOU fix (prerequisite sub-task — `src/lib/xstring.c`):** The numeric-to-string coercion in §7 PEG actions calls `makeString(buf)` under concurrent load (API server goroutines). If two threads concurrently coerce the same value, `makeString`'s `key_add` can return `-EEXIST`, and the existing implementation destroys its allocation and returns **NULL** without recovering the existing interned string — a correctness bug that corrupts the interned AST when this function returns NULL and the caller passes it to `makeSelectNode`. The `makeSelectNode` pattern above corrects this with a re-find on `-EEXIST`. This same fix must be applied to `makeString()` in `src/lib/xstring.c` as a **prerequisite** to implementing the §7 coercion path: on `key_add` returning `-EEXIST` **only**, re-find the existing string and return it, rather than returning NULL. If `key_add` returns `-ENOMEM` (kvent allocation failure), nothing was inserted — re-finding would fail and returning NULL is correct. The required restructuring is non-trivial because the existing code unconditionally releases the lock at `xstring.c:166` before checking `rc`. The corrected structure (replacing lines 164-174 of `xstring.c`): ```c /* Save newly-allocated instance as `new_str` to avoid shadowing below */ var new_str = makeInstance(&xstring_type, s); if (new_str == NULL) return NULL; mylock_lock(&(xstring_table.lock)); rc = key_add(xstring_table.xstr_table, (void*)s, new_str); if (rc == -EEXIST) { /* Lost the race: another thread interned first. Re-find while still * holding the lock — releasing before re-find creates a window where * the winner could be concurrently freed (dangling pointer). * key_find cannot fail here: we hold the lock so no concurrent delete * or resize can occur, and -EEXIST guarantees the key is present. * Check the return value defensively nonetheless. */ void* existing = NULL; int rfind = key_find(xstring_table.xstr_table, (void*)s, &existing); if (rfind <= 0 || existing == NULL) { /* key_find returns 1 on success, not 0 */ /* Unreachable under correct locking, but do not proceed with NULL */ mylock_unlock(&(xstring_table.lock)); destroy_(new_str, (obj_destroy_fn_t)xstring_destroy); errno = ENOMEM; return NULL; } reference(existing); /* bump before unlock */ mylock_unlock(&(xstring_table.lock)); destroy_(new_str, (obj_destroy_fn_t)xstring_destroy); /* free our copy */ return (char*)existing; } mylock_unlock(&(xstring_table.lock)); if (rc < 0) { destroy_(new_str, (obj_destroy_fn_t)xstring_destroy); errno = -rc; return NULL; } return (char*)new_str; ``` **Deadlock guard for `reference()` under lock:** The fix calls `reference(existing)` while holding `xstring_table.lock`. Before implementing, verify that `reference()` does not attempt to acquire `xstring_table.lock` internally (a nested lock → deadlock). In the existing xstring RTTI pattern, `reference()` increments a refcount in the object header — this is an atomic increment with no lock, so no deadlock. Confirm this by reading `src/lib/xcealloc.c` or `wrap.c` (whichever defines `reference()`). If the implementation changes to use a mutex for refcount, the fix must move `reference()` outside the lock (bump refcount after unlock, with a separate guard for the use-after-free window). **Scope note:** `src/lib/xstring.c` is outside the select-clause scope of §2. This fix must be tracked as a separate sub-task and completed before the §7 coercion path is activated, whether in the same PR or an immediately preceding one. It protects all `makeString` callers, not just the select coercion path. `kill_primary_resreq()` in `resreq.c` must follow mode-specific teardown: ```c if (primary->select) { xce_select_node_t* root = primary->select->root; primary->select->root = NULL; /* unlink first (GC reads this) */ if (sel_get_optimize_level() <= 0 && root != NULL) free_select_tree(root); /* disabled: free private tree */ /* CSE-enabled: GC reclaims nodes; no free of root pointer here */ FREEUP(primary->select); /* always: free the container struct */ } ``` **Ownership contract:** - `primary->select` (the `xce_select_t` container struct) is **always** owned by `primary` and must always be freed by `kill_primary_resreq`. - `primary->select->root` (the `xce_select_node_t*`) is owned differently by mode: - CSE-enabled: owned by the intern table; GC reclaims — never `free()` it. - CSE-disabled: owned by the job; `free_select_tree(root)` before `FREEUP`. - The NULL store must happen **before** `FREEUP(primary->select)` so that any concurrent GC mark pass (§8.5.3) cannot dereference a dangling pointer through a job whose container has already been freed. ### 5.5 Mark-and-Sweep GC Between Scheduling Sessions Node reclamation is handled by a **mark-and-sweep** GC that runs between scheduling sessions, when no evaluation is in progress. There is no `derefSelectNode` function — when a job completes, its resreq entry is simply unlinked from the job (see §5.4); the next GC sweep reclaims unreachable nodes. **GC idle invariant:** `gc_sweep_select_nodes()` runs **in the pre-session phase**, after the staging buffer has been merged into the live job set and after all job cleanup (`primary->select->root = NULL` for completed jobs) is complete, but before the next scheduling session begins. At that point the scheduler is completely idle: no scheduling session is active and no `eval_select` call is in progress. Running GC after the merge ensures that newly arrived jobs are visible to the mark pass — otherwise, nodes referenced only by incoming jobs could be incorrectly swept. This is an architectural invariant of the session-based scheduler, and **must be enforced with runtime guards (Not just `assert()`, which is compiled out with `NDEBUG`)**: ```c /* Enforce at the gc_sweep_select_nodes call site in the scheduler. * These are runtime checks — not assert() — because assert() is * compiled out in release builds (NDEBUG), and a GC sweep that * runs concurrently with eval_select causes use-after-free. */ if (sched_session_active(sched) || sched_eval_in_progress(sched) || !staging_merge_complete(sched)) { log_fatal("gc_sweep_select_nodes called with invariant violated: " "session_active=%d eval_in_progress=%d staging_merge=%d", sched_session_active(sched), sched_eval_in_progress(sched), staging_merge_complete(sched)); return; /* do not sweep — prevent use-after-free */ } gc_sweep_select_nodes(live_jobs, njobs); ``` Where `sched_session_active`, `sched_eval_in_progress`, and `staging_merge_complete` are scheduler-internal predicate functions. These runtime guards survive release builds and log a fatal error rather than silently proceeding with a corrupted intern table. **Predicate lifecycle (Phase 1 → Phase 6):** In Phases 1–5 (library development and unit testing), these predicates are implemented as thin wrappers around a `fake_scheduler_t` struct declared in `test_select.c` — a test-only scaffold with the same three `_Atomic int` fields. In Phase 6 (scheduler wiring), the wrapper bodies are updated to reference the real `xce_scheduler_t` fields added in that phase. The function signatures do not change between phases; only the struct pointer inside each wrapper body changes. No Phase 1 stub code leaks into the production implementation. **Phase 6 predicate fields in `xce_scheduler_t`:** Three `_Atomic int` fields must be added to `struct xce_scheduler_` (in `src/scheduler/include/scheduler.h`): ```c _Atomic int session_active; /* 1 during scheduling session, 0 between */ _Atomic int eval_in_progress; /* 1 while eval_select loop is running, 0 otherwise */ _Atomic int staging_merge_complete; /* 1 after staging merge completes, 0 before */ ``` These fields are initialized to 0 in the scheduler's init function (Phase 6 wiring) and set/cleared at the appropriate session-loop transitions. The predicate wrappers read them via `atomic_load_explicit(..., memory_order_acquire)` to ensure visibility from the allocator thread's writes. The `fake_scheduler_t` in Phase 1 tests uses the same field names and types, so the predicate wrapper bodies are mechanically identical — only the container struct pointer differs. ``` gc_sweep_select_nodes(const xce_sched_job_t** live_jobs, size_t njobs) 1. Mark pass: a. Iterate sel_node_table.nodes (zarray_t) over slots 0..caps-1: skip NULL slots (vacated by zarray_del from a prior GC sweep); clear gc_mark on every live node. b. For each live job in live_jobs[0..njobs-1]: dispatch on job->resreq->tag and walk all reachable select roots, setting gc_mark on every reachable node. (Shared sub-nodes are marked once — gc_mark is boolean.) - XCE_RESREQ_PRIMARY: mark from resreq->u.primary->select->root. - XCE_RESREQ_COMPOUND: iterate resreq->u.compound->primaries and mark each primary's select->root. - XCE_RESREQ_ALTER: iterate resreq->u.alter->compounds, then each compound's primaries, then each primary's select->root. (xce_sched_job_t has xce_resreq_t* resreq; xce_job_t only has char* resreq and cannot access the parsed AST.) 2. Sweep pass: a. Iterate sel_node_table.nodes (zarray_t) over slots 0..caps-1: skip NULL slots; collect (slot_idx, node*) pairs for unmarked nodes. **Note:** `slot_idx` here is the loop index `i` from the zarray iteration — it is NOT stored in `xce_select_node_t` (the struct has no `slot_idx` field). The GC sweep obtains the correct `zarray_del` argument directly from the forward-iteration index, not from any per-node field. b. For each unmarked node (iterate collected pairs): - Reconstruct the intern key: `sel_node_key_t key = sel_node_to_key(node)` (see §12 Appendix). The commutativity sort must be applied during key reconstruction, identical to the sort applied in `makeSelectNode` step 1, otherwise the reconstructed key hashes to a different bucket and `key_del` silently no-ops, leaking the kvtab entry. - Remove its key from sel_node_table.table via key_del. (sel_node_table is initialized with datakiller=NULL so key_del removes only the hash table entry — the node value is NOT freed by key_del; see discussion below on datakiller.) - If node->type == SEL_STR: call `dereference(node->u.strval)` to release the xstring refcount acquired by `makeString()`. All SEL_STR nodes are created via `makeString()`, so strval is always an xstring-interned pointer — `free()` is incorrect here. - free(node). - zarray_del(&sel_node_table.nodes, slot_idx) — NULLs the slot and decrements count. Does NOT swap-remove; remaining slot indices are stable. NULL-slot skipping in step 1a handles future iterations. ``` The `gc_mark_tree` helper does a recursive post-order mark: ```c /* Mark all nodes reachable from root (recursive DFS). * Leaf nodes (SEL_RESNAME, SEL_NUM, SEL_STR, SEL_ANY, SEL_TRUE, * SEL_FALSE) have no children — they set gc_mark and return immediately. * SEL_NOT has one child (node->u.operand). * SEL_AND/OR/GT/LT/GE/LE/EQ/NE have two children (node->u.binop.left, * node->u.binop.right). * * Recursion depth: bounded by the post-flatten AST depth, which is at most * SELECT_MAX_DEPTH - 1 levels (the parse-time nesting limit). Any increase * to SELECT_MAX_DEPTH must be reviewed against stack-frame size to avoid * stack overflow. Cycles are impossible — interned nodes form a DAG. */ static void gc_mark_tree(xce_select_node_t* node) { if (node == NULL || node->gc_mark) return; /* NULL root or already marked */ node->gc_mark = 1; switch (node->type) { /* leaf types — no children */ case SEL_RESNAME: case SEL_NUM: case SEL_STR: case SEL_ANY: case SEL_TRUE: case SEL_FALSE: break; /* unary */ case SEL_NOT: gc_mark_tree(node->u.operand); break; /* binary */ default: /* SEL_AND, SEL_OR, SEL_GT, SEL_LT, SEL_GE, SEL_LE, SEL_EQ, SEL_NE */ gc_mark_tree(node->u.binop.left); gc_mark_tree(node->u.binop.right); break; } } ``` **COMPOUND/ALTER xlist iteration:** `xce_compound_resreq_t.primaries` and `xce_alter_resreq_t.compounds` are `xlist_t` linked lists. Use the standard `xlist_get` iteration pattern (as used elsewhere in the codebase for `xlist_t` iteration): ```c /* Example: iterate primaries in a COMPOUND resreq */ xlist_t* list = &resreq->u.compound->primaries; for (int i = 0; i < xlist_length(list); i++) { xce_primary_resreq_t* p = (xce_primary_resreq_t*)xlist_get(list, i); if (p && p->select && p->select->root) gc_mark_tree(p->select->root); } ``` Do not use pointer arithmetic or `xlist_head`/`xlist_next` directly — use `xlist_get` with a counted loop, which is the existing pattern in the codebase (e.g., `resource.c`). This approach is simple and correct: - GC iterates `sel_node_table.nodes` (the linear `zarray_t`), not the kvtab. No kvtab iterator interface is required. - `key_del` removes only the hash table entry. `datakiller` is a **table-level** configuration set at `kvtab_init` time — it is not a per-call argument. `sel_node_table` MUST be initialized with `datakiller=NULL` so that `key_del` does not free the node value; the node is freed explicitly afterward. This avoids a double-free. Do not pass a non-NULL `datakiller` when initializing `sel_node_table`. - A node is reachable if and only if it is visited from at least one live job's resreq root. No reference provenance tracking is needed. - `SEL_STR` nodes' `xstring` references are handled by the same mark pass — if the SEL_STR node is reachable, the string stays interned. - **GC does not acquire `sel_node_table.lock`**: The GC sweep runs in the pre-session phase while the scheduler is completely idle (§8.4). No concurrent `makeSelectNode` or `key_find` calls execute during GC, so `key_del` operates on a quiescent hash table without lock contention. The idle invariant is enforced by runtime guards (not just `assert()`, which is compiled out with `NDEBUG`) at the GC call site — see the guard block above. Do not add `mylock_lock`/`mylock_unlock` around GC's `key_del` calls; doing so would add unnecessary overhead and could deadlock if `key_del` itself acquires an internal lock. **Job cleanup timing:** There is no mid-session job deletion. When a job completes, its entry is marked for removal; the select root pointer is unlinked and the job struct freed at the session boundary, before GC runs. This eliminates the risk of shared sub-nodes becoming unreachable mid-session. After GC, node IDs are re-compacted to `0..N-1` (§8.5.4) for two reasons: (1) to keep the evaluation context arrays dense so per-host allocation is bounded by the live node count; (2) to prevent `uint32_t node_id` overflow in long-running scheduler instances — `sel_node_next_id` would otherwise grow monotonically across thousands of GC cycles and eventually wrap. Recompaction resets `sel_node_next_id` to the current live count, keeping it bounded. Recompaction iterates `sel_node_table.nodes` — no kvtab iterator is needed. Like IBM LSF, the XCE scheduler operates in sessions — each session evaluates all pending jobs against candidate hosts, dispatches what it can, then ends. Pre-session work (staging merge, GC, recompaction) runs between sessions while the scheduler is idle, ensuring new jobs are visible to the mark pass before any nodes are reclaimed. **Job submission and scheduling cycle:** The controller accepts job submissions asynchronously and pushes them to the scheduler via a **staging buffer**. The scheduler's main loop is serial, with each cycle following this sequence: 1. **Staging:** New job resreq strings from the staging buffer are parsed (calling `makeSelectNode`), and the resulting interned nodes are merged into the intern table. Completed jobs from the previous session have their select root pointers unlinked (`primary->select->root = NULL`). 2. **GC + recompaction:** With the staging merge complete and all live jobs (old + newly arrived) visible, `gc_sweep_select_nodes` reclaims unreachable nodes, then `recompact_node_ids` reassigns dense 0..N-1 IDs. Running GC after the merge ensures nodes referenced only by incoming jobs are marked live and not incorrectly swept. 3. **Session:** The allocator thread evaluates each job against each candidate host via `eval_select`, dispatches what it can, and defers the rest. All interned nodes are read-only — no `makeSelectNode` calls occur during the session. This serial architecture ensures that no concurrent `makeSelectNode` calls occur during GC sweep. The dual-buffer pattern (staging buffer for incoming jobs + active intern table for the session) is scheduler-internal and distinct from the controller's own persistence mechanism (snapshot + WAL). **Pointer-based canonicalization note:** Sorting by `uintptr_t` produces a deterministic total order within a single process lifetime. Across process restarts, pointer addresses change (ASLR, heap layout), so the canonical form may differ, but this is immaterial — the intern table is per-process and never persisted. The guarantee is: within a session, `A && B` and `B && A` produce the same intern table key. --- ## 6. Commutativity Canonicalization For commutative operators (`&&`, `||`, `==`, `!=`), operand order in the source string is arbitrary. `A && B` and `B && A` express the same constraint but would produce different intern table keys without canonicalization. **Strategy**: before building the CSE key, sort `op1` and `op2` by their `uintptr_t` value. Since both children are already canonical interned pointers, their addresses are stable. Sorting by address gives a deterministic total order with no semantic knowledge required: ```c if (op1 > op2) { uintptr_t t = op1; op1 = op2; op2 = t; } ``` After this swap, `A && B` and `B && A` produce the same `(SEL_AND, min, max)` key and hit the same intern table entry. Non-commutative operators (`>`, `<`, `>=`, `<=`, `!`) are left unchanged. ### 6.2 Associativity Flattening The binary AST naturally produces `(A && B) && C` and `A && (B && C)` as structurally different trees with different CSE keys, even though they are semantically identical. A **flattening transform** normalizes all same-operator chains to left-associative form: ``` flatten(AND, (AND, A, B), C) → (AND, (AND, A, B), C) /* already left-assoc */ flatten(AND, A, (AND, B, C)) → (AND, (AND, A, B), C) /* re-associate right to left */ ``` After flattening, `(A && B) && C` and `A && (B && C)` both produce the same left-associative tree, maximizing CSE sharing. The same transform applies to `SEL_OR`. **Why binary trees, not n-ary nodes?** An alternative design uses n-ary nodes like `(AND, A, B, C)` instead of binary `(AND, (AND, A, B), C)`. N-ary nodes make operand reordering trivial (sort the children array) and eliminate the need for flattening. However, they break the fixed-size `sel_node_key_t` — the CSE key becomes variable-length, requiring a hash over N sorted child pointers per lookup. Binary trees preserve the O(1) fixed-size key: `(SEL_AND, left_ptr, right_ptr)`. For typical HPC workloads where `&&`/`||` chains are 2–5 operands, binary trees with flattening + commutativity sort give canonical form with zero per-lookup cost beyond two pointer compares. N-ary representation is noted as a potential future enhancement if operand reordering within long chains provides measurable CSE gains beyond what binary canonicalization achieves. ### 6.3 Const-Fold Transform A **const-fold pass** simplifies the AST by evaluating expressions that are statically determined at parse time: - `resname == any` → `SEL_TRUE` (always matches, regardless of the resource value). - `resname != any` → `SEL_FALSE` (never matches — `any` matches every value, so nothing can fail to match it). Both `!= any` forms are accepted at parse time; the const-fold fires immediately, yielding the same interned `SEL_FALSE` singleton as `!(resname == any)`. **Implementation note:** after the commutativity swap in `makeSelectNode` (§5.4), the `SEL_ANY` singleton may appear in either the `left` or `right` child slot of `SEL_EQ`/`SEL_NE` — the address-order sort is non-deterministic across process lifetimes (ASLR). The const-fold pass must check **both** `node->u.binop.left` and `node->u.binop.right` for pointer equality with the `SEL_ANY` singleton, not only one side. - Identity laws:`SEL_TRUE && X` → `X`, `X && SEL_TRUE` → `X`; `SEL_FALSE || X` → `X`, `X || SEL_FALSE` → `X` - Annihilation laws: `SEL_FALSE && X` → `SEL_FALSE`, `X && SEL_FALSE` → `SEL_FALSE`; `SEL_TRUE || X` → `SEL_TRUE`, `X || SEL_TRUE` → `SEL_TRUE` - Complement laws: `!SEL_TRUE` → `SEL_FALSE`, `!SEL_FALSE` → `SEL_TRUE` - Boolean equality: `bool_res == true` and `bool_res == false` are **parse errors** (type mismatch — `true`/`false` are not keywords), so no const-fold rules are needed for these patterns. The idiomatic forms `bool_res` and `!bool_res` are the only valid boolean usage. The const-fold pass runs **after** construction and flattening, replacing subtrees with constant leaf nodes. Since all nodes are interned, `SEL_TRUE` and `SEL_FALSE` are singletons — every const-fold reduces to the same pointer. ```c /* SEL_FALSE also added to enum and intern table as a singleton */ ``` ### 6.4 Transform Pipeline AST construction follows a fixed pipeline after the PEG parser produces the raw tree: ``` 1. Parse (PEG → raw binary AST, each PEG action calls `makeSelectNode` which performs commutativity canonicalization as part of CSE key construction) 2. Post-parse tree walk: associativity flattening (§6.2) → re-intern modified subtrees via `makeSelectNode` 3. Post-parse tree walk: const-fold (§6.3) → re-intern modified subtrees via `makeSelectNode` ``` **Flattening termination:** The `select_and` and `select_or` grammar rules use `( op operand )*` repetition. A correctly implemented PEG action that accumulates left-to-right (`$$ = makeSelectNode(AND, $$, next)`) produces a left-associative tree directly from the grammar — flattening is a no-op for unparenthesized chains. Flattening only fires when the user adds explicit parentheses, e.g. `A && (B && C)`. For depth-2 explicit parenthesization (`A && (B && C)`), a single post-order pass suffices: the inner `(B && C)` is processed first and its children are leaves, so after the outer rotation no AND right-child remains. For depth-3+ explicit parenthesization (`A && (B && (C && D))`), a single post-order pass leaves `(A && (B && C)) && D` with `(A && (B && C))` still having an AND right child. **The flatten pass must therefore repeat until no AND-node has an AND right-child**: ```c do { changed = 0; changed |= flatten_pass(root); } while (changed); ``` **Note:** any orphaned-node list maintained by `flatten_pass` is stack-local to that call — it is not accumulated across loop iterations. Each iteration frees the orphans it discovers before returning `changed`. This means the loop is safe to repeat without risk of double-free or accumulated-list confusion across passes. In practice, HPC select expressions do not use deeply nested explicit parenthesization (2–3 levels is the realistic maximum), so the loop terminates in at most 2 iterations for real workloads. Const-fold does not create new flattening opportunities because it replaces subtrees with single leaf nodes (SEL_TRUE/SEL_FALSE), shrinking the tree rather than restructuring it, so const-fold remains single-pass. Note: commutativity canonicalization (operand swap) happens *inside* `makeSelectNode` as part of the CSE key lookup — it is not a separate pipeline step. The PEG action for each binary operator calls `makeSelectNode`, which sorts `op1`/`op2` for commutative operators before building the key. Flattening and const-fold are post-parse tree walks that traverse the raw AST and call `makeSelectNode` to construct new normalized subtrees. Since `makeSelectNode` already handles interning and canonicalization, these walks naturally produce the maximally shared tree. **No interned-node mutation:** `flatten_pass` NEVER modifies an already-interned node's fields. Re-association is done entirely by calling `makeSelectNode` to construct NEW canonical nodes (e.g., `new_inner = makeSelectNode(AND, A, B)`, then `new_outer = makeSelectNode(AND, new_inner, C)`). The old outer node becomes an orphan. This preserves the §5.1 immutability invariant: once a node is inserted into the intern table its content is never mutated. **Orphaned node ownership in CSE-disabled mode:** When `flatten_pass` replaces a subtree, the old (orphaned) node is not tracked by the intern table and will not be reclaimed by GC. In CSE-disabled mode, `flatten_pass` must explicitly free every replaced node — see the full ownership contract in §5.0 ("Flatten-pass ownership in disabled mode"). --- ## 7. Parse-Time Type Validation The resmgr is passed into the select parse context so resource names can be resolved to `(res_idx, res_type)` at parse time: ```c #define SELECT_MAX_INPUT_LEN 8192 /* reject expressions longer than 8KB */ #define SELECT_MAX_DEPTH 64 /* maximum nesting depth for &&/||/! — * Stack analysis: eval_select_inner has ~80–120 B * frame (pointer args + local int + switch + return addr); * at depth 64, peak recursive stack ≈ 64 × 128 B = 8 KB. * Default Linux thread stack is 8 MB; scheduler threads * use the default stack. 8 KB is well within bounds. * If SELECT_MAX_DEPTH is raised above ~500, re-measure * actual frame size with `objdump -d` and verify against * the thread stack size configured in §8.4. */ typedef struct { const char* input; int pos; int err_pos; int has_error; char errmsg[256]; /* fixed-size, no alloc, no free */ int depth; /* current nesting depth, checked against SELECT_MAX_DEPTH */ var resmgr; /* name → (res_idx, res_type) */ } select_context_t; ``` **resmgr locking:** PEG actions that call `key_find(resmgr, ...)` to resolve resource names must acquire the resmgr read lock before the lookup and release it afterward, matching the pattern used throughout the scheduler and API server codebase (`lock(resmgr, RTTI_LOCK_READ)` before lookup, `unlock(resmgr)` after). In CSE-enabled mode (scheduler), the staging phase is single-threaded in practice, but the read lock is still required for correctness — a future staging-thread change would need it, and the lock is not on any hot path. In CSE-disabled mode (API server, concurrent goroutines), the read lock is equally required — without it, a concurrent `key_add` adding a new resource could corrupt the hash table read. PEG actions validate types before calling `makeSelectNode`: ```c /* action for '>' '<' '>=' '<=' — numeric operands only */ if (!operand_is_numeric(lnode) || !operand_is_numeric(rnode)) { auxil->has_error = 1; $$ = NULL; } else { $$ = (char*)makeSelectNode(SEL_GT, (uintptr_t)lnode, (uintptr_t)rnode); } ``` **EQ/NE type coercion:** For `==` and `!=`, operand types must be compatible. When a resource name (SEL_RESNAME) is compared to a literal, the literal is coerced to the resource's type: - `type == linux` — `type` is `SEL_RESNAME(RES_TYPE_STRING)`, `linux` is `SEL_STR` (not a known resource name) — types match. - `type == 5` — `type` is `RES_TYPE_STRING`, `5` is `SEL_NUM` — the numeric literal is converted to `SEL_STR` (`"5"`) at parse time so that eval sees two string operands. - `mem > 100` — `mem` is `RES_TYPE_NUMERIC`, `100` is `SEL_NUM` — types match. - `gpu` — `gpu` is `RES_TYPE_BOOL`, used as a standalone boolean operand (the only valid form). `gpu == 1` is accepted (numeric comparison against a boolean resource stored as 0/1). `gpu == true` is rejected as a type mismatch (`true` is a string literal, not a boolean keyword). This coercion guarantees that at eval time, `EQ`/`NE` operand types are compatible. For all type pairs except `BOOL` × `NUMERIC`, the types match exactly. For `BOOL` × `NUMERIC` (e.g., `gpu == 1`), the eval code coerces the boolean to `0.0`/`1.0` for comparison — see §8.3. ```c /* action for '==' '!=' — type validation and coercion */ /* Reject type mismatches involving boolean resources */ if (lnode->type == SEL_RESNAME && rnode->type == SEL_STR && lnode->u.resname.res_type == RES_TYPE_BOOL) { /* gpu == "true" — reject: boolean vs string */ auxil->has_error = 1; $$ = NULL; } else if (rnode->type == SEL_RESNAME && lnode->type == SEL_STR && rnode->u.resname.res_type == RES_TYPE_BOOL) { /* "true" == gpu — reject: string vs boolean */ auxil->has_error = 1; $$ = NULL; } /* Coerce numeric literal to string when compared with string resource */ if (lnode->type == SEL_RESNAME && rnode->type == SEL_NUM && lnode->u.resname.res_type == RES_TYPE_STRING) { char buf[64]; snprintf(buf, sizeof(buf), "%.17g", rnode->u.numval); /* NULL CHECK REQUIRED: makeString returns NULL on OOM. Passing NULL cast * to uintptr_t (== 0) to makeSelectNode would intern a SEL_STR node with * u.strval=NULL, which silently evaluates to non-match for every host and * also causes dereference(0) in the step-5 TOCTOU rollback path (§5.4). */ char* s = makeString(buf); if (s == NULL) { auxil->has_error = 1; $$ = NULL; } else { /* ORPHAN NOTE: rnode currently points to a SEL_NUM node allocated by * a prior makeSelectNode call. In unoptimized mode, that node has no * owner once rnode is overwritten — it MUST be freed before replacing * the pointer. In optimized mode the intern table holds the only * reference; do NOT free it (freeing an interned node corrupts the * table). Implementation must branch on sel_get_optimize_level(): * if (sel_get_optimize_level() == 0) free(rnode); * (no dereference needed — SEL_NUM has no xstring field). */ rnode = makeSelectNode(SEL_STR, (uintptr_t)s, 0); /* NULL CHECK: makeSelectNode can return NULL (OOM, table cap, zarray fail). * If NULL, xstring ref for s was already released by makeSelectNode. * Do NOT pass NULL as op2 to the outer makeSelectNode — that would intern * a malformed EQ node whose NULL right child silently matches every host. */ if (rnode == NULL) { auxil->has_error = 1; $$ = NULL; } } } /* symmetric: coerce if right is RESNAME and left is numeric literal */ if (rnode->type == SEL_RESNAME && lnode->type == SEL_NUM && rnode->u.resname.res_type == RES_TYPE_STRING) { char buf[64]; snprintf(buf, sizeof(buf), "%.17g", lnode->u.numval); /* Same NULL CHECK and ORPHAN NOTE as above: free lnode (SEL_NUM) in * unoptimized mode before overwriting the pointer. */ char* s = makeString(buf); if (s == NULL) { auxil->has_error = 1; $$ = NULL; } else { lnode = makeSelectNode(SEL_STR, (uintptr_t)s, 0); /* NULL CHECK: same as above — if NULL, s ref already released. */ if (lnode == NULL) { auxil->has_error = 1; $$ = NULL; } } } /* Locale note: snprintf with %.17g may produce locale-dependent output * (e.g., comma decimal separator in European locales). The select parser * must run with C locale (LC_NUMERIC=C) to guarantee '.' as the decimal * separator, ensuring consistent string comparison at eval time. The * scheduler already runs with LC_ALL=C; if the API server path ever uses * numeric-to-string coercion, it must also set LC_NUMERIC=C before calling * parse_resreq. */ /* Boolean vs numeric comparison is accepted: gpu == 1, gpu != 0. * The numeric literal stays as SEL_NUM; at eval time, the boolean value * (0 or 1) is compared as (double)bool_val == num_val. No coercion * is needed at parse time beyond the type-validation check above (which * rejected BOOL vs STRING). */ $$ = (char*)makeSelectNode(SEL_EQ, (uintptr_t)lnode, (uintptr_t)rnode); ``` Where `operand_is_numeric(node)` returns true if: - `node->type == SEL_NUM`, or - `node->type == SEL_RESNAME && node->u.resname.res_type == RES_TYPE_NUMERIC` See §12 for the formal declaration. Unknown resource names fail the parse (rejected at job submission time). In compound (`X * { primary } + Y * { primary }`) and alter (`{ compound } || { compound }`) resreq strings, a parse error in any primary's select clause rejects the entire resreq — there is no partial-accept fallback. **resmgr is mandatory:** `parse_resreq_primary` requires a non-NULL `resmgr`. There is no NULL-fallback mode. This is a new product with no compatibility constraint, so the interface is defined cleanly from the start: every call site must supply the resource manager. Without `resmgr`, the parser cannot resolve a bare word to `SEL_RESNAME` vs `SEL_STR`, cannot validate operand types, and cannot produce a correctly typed AST. Callers that do not have `resmgr` available must obtain it before calling the parser. **resmgr stability invariant (enforced):** Resources are never deleted during cluster operation — they may only be added. This is an architectural guarantee of the XCE scheduler: resmgr entries are append-only for the process lifetime, enforced by the resmgr implementation (no delete API is exposed). Debug builds should include a periodic assertion in the GC mark pass that every `res_idx` stored in an interned `SEL_RESNAME` node still resolves to a valid resmgr entry. This guarantees that `res_idx` values embedded in interned nodes remain valid for the process lifetime. No intern table invalidation is required on resmgr changes. If a new resource is added, existing interned nodes are unaffected; new parse operations will resolve the new resource to a new `res_idx` and create new `SEL_RESNAME` nodes. **resmgr availability:** Both the API server and scheduler have access to the resmgr via the cluster configuration. The resmgr is initialized once at process startup and updated only by addition. Call sites: - Scheduler: `sched->resmgr` is available at job-parse time in `make_sched_job`; thread it into the resreq parse context. - API server: obtain from the cluster configuration context available at request time; pass into `parse_resreq` as a new parameter or via an externally initialized `resreq_context_t`. The **resmgr** is an array of resource definitions — each entry maps a resource name to its type and other metadata. Both the API server (for validation at job submission) and the scheduler (for full CSE parse) have access to this information via the cluster configuration. --- ## 8. Evaluation in Host Context ### 8.1 Typed Value ```c typedef struct { res_type_t type; union { double num; const char* str; int bval; } u; } sel_val_t; ``` ### 8.2 Leaf Resolution ```c static sel_val_t get_leaf_value(const xce_select_node_t* node, select_eval_ctx_t* ctx) { switch (node->type) { case SEL_RESNAME: return host_resval_typed(ctx->host, node->u.resname.res_idx, node->u.resname.res_type); case SEL_NUM: return (sel_val_t){ RES_TYPE_NUMERIC, .u.num = node->u.numval }; case SEL_STR: return (sel_val_t){ RES_TYPE_STRING, .u.str = node->u.strval }; default: return (sel_val_t){ RES_TYPE_BOOL, .u.bval = 0 }; } } ``` ### 8.3 `eval_select` `eval_select` is a post-order traversal of the AST tree with memoization via a per-host evaluation context (§8.5). The outer `eval_select` checks the cache before recursing; `eval_select_inner` contains the actual recursive logic. If anything goes wrong during the visit, `-1` is returned immediately and propagated up — `&&`, `||`, and `!` never silently swallow errors. **Host evaluation context:** `xvec_idval_t` stores per-job resource allocation data, not host resource values. The select evaluator uses a per-host `select_eval_ctx_t` (§8.5) that provides resource values by `res_idx` through its `numvals` and `strvals` members, and caches evaluation results by `node_id` so each node is evaluated at most once per host across all jobs. **Note:** The pseudocode below is the **host-access reference version** — it reads live host data directly on every call for clarity. The Phase 4e implementation substitutes `ctx->num_vals` / `ctx->str_vals` cache writes in leaf branches and cache reads in comparison branches (see §8.5.2 implementation note). This section is the behavioral spec; §8.5.2 is the performance-correct production form. ```c /* Inner eval — called by memoized eval_select (§8.5.2) on cache miss. * Returns: 1 = host matches, 0 = host does not match, -1 = error. * NULL node means no select constraint — all hosts match. */ static int eval_select_inner(const xce_select_node_t* node, select_eval_ctx_t* ctx) { if (node == NULL) return 1; /* no select clause → all hosts match */ int lv, rv; const host_eval_ctx_t* host = ctx->host; /* host snapshot in ctx */ switch (node->type) { /* logical — explicit error propagation, not C boolean operators */ case SEL_AND: lv = eval_select(node->u.binop.left, ctx); if (lv < 0) return -1; if (lv == 0) return 0; rv = eval_select(node->u.binop.right, ctx); if (rv < 0) return -1; return rv; case SEL_OR: lv = eval_select(node->u.binop.left, ctx); if (lv < 0) return -1; if (lv == 1) return 1; rv = eval_select(node->u.binop.right, ctx); if (rv < 0) return -1; return rv; case SEL_NOT: lv = eval_select(node->u.operand, ctx); if (lv < 0) return -1; return !lv; /* SEL_TRUE: const-folded from expressions like `type == any` */ case SEL_TRUE: return 1; /* SEL_FALSE: const-folded from expressions like `!(type == any)` */ case SEL_FALSE: return 0; /* standalone boolean use of a RESNAME node */ case SEL_RESNAME: switch (node->u.resname.res_type) { case RES_TYPE_BOOL: return host_get_bool (host, node->u.resname.res_idx); case RES_TYPE_NUMERIC: return host_get_float(host, node->u.resname.res_idx) != 0.0f; case RES_TYPE_STRING: return -1; /* string standalone rejected at parse time (§7) */ default: return -1; /* defensive: unrecognized res_type */ } /* numeric comparisons — operand types validated at parse time. * IMPLEMENTATION NOTE: Phase 4e must NOT call get_leaf_value() directly * here. get_leaf_value() reads host data without writing to ctx->num_vals. * The correct implementation must: * 1. Call eval_select(left_leaf, ctx) — this writes ctx->num_vals[left->node_id] * 2. Call eval_select(right_leaf, ctx) — this writes ctx->num_vals[right->node_id] * 3. Then read ctx->num_vals[left->node_id] and ctx->num_vals[right->node_id] * If get_leaf_value() is called directly (bypassing the cache write), and the * leaf node was never evaluated as a standalone bool_bits entry, num_vals[] is * 0.0 (the memset-to-zero default), and the comparison silently uses 0.0 for the * actual host value — a correctness bug, not a performance issue. * The code shown here uses get_leaf_value() for conceptual clarity only. */ case SEL_GT: case SEL_LT: case SEL_GE: case SEL_LE: { double l = get_leaf_value(node->u.binop.left, ctx).u.num; double r = get_leaf_value(node->u.binop.right, ctx).u.num; if (node->type == SEL_GT) return l > r; if (node->type == SEL_LT) return l < r; if (node->type == SEL_GE) return l >= r; return l <= r; } /* type-dispatched equality — parse-time validation ensures compatible types. * Allowed pairs: NUM×NUM, STR×STR, BOOL×BOOL, BOOL×NUM, NUM×BOOL. * Rejected at parse time: BOOL×STR, STR×BOOL. * Coerced at parse time: numeric literal compared to string resource → string * literal (§7 snprintf coercion), so eval sees STR×STR, not mixed types. */ case SEL_EQ: case SEL_NE: { /* IMPLEMENTATION NOTE: Phase 4e must NOT call get_leaf_value() directly here. * get_leaf_value() reads host data without writing to ctx->num_vals/str_vals. * The correct implementation must: * 1. Call eval_select(left_leaf, ctx) — this writes ctx->num_vals[left->node_id] * or ctx->str_vals[left->node_id] for RESNAME/SEL_NUM/SEL_STR leaves * 2. Call eval_select(right_leaf, ctx) — same * 3. Then read ctx->num_vals[left->node_id] / ctx->str_vals[left->node_id] * If get_leaf_value() is called directly (bypassing the cache write), and the * leaf node was never evaluated as a standalone bool_bits entry, num_vals[] is * 0.0 (the memset-to-zero default), and the comparison silently uses 0.0/NULL * for the actual host value — a correctness bug, not a performance issue. * The pseudocode here uses get_leaf_value() for conceptual clarity only. */ sel_val_t l = get_leaf_value(node->u.binop.left, ctx); sel_val_t r = get_leaf_value(node->u.binop.right, ctx); int eq; switch (l.type) { case RES_TYPE_NUMERIC: if (r.type == RES_TYPE_BOOL) eq = l.u.num == (double)r.u.bval; /* NUM×BOOL: 1 == gpu */ else if (r.type == RES_TYPE_NUMERIC) eq = l.u.num == r.u.num; /* NUM×NUM */ else return -1; /* defensive: NUM×STR rejected at parse time */ break; case RES_TYPE_STRING: /* Defensive: STR×BOOL and STR×NUM are rejected at parse time (§7), but * a future parse-validation gap could smuggle an incompatible type here. * Guard explicitly to prevent reading r.u.str from an uninitialized union. */ if (r.type != RES_TYPE_STRING) return -1; /* NULL means the resource has no value for this host (absent resource). * Treat absent-resource comparisons as non-matching (eq=0 for ==, 1 for !=). */ if (l.u.str == NULL || r.u.str == NULL) { eq = 0; break; } eq = xce_str_match(l.u.str, r.u.str); /* STR×STR */ break; case RES_TYPE_BOOL: if (r.type == RES_TYPE_BOOL) eq = l.u.bval == r.u.bval; /* BOOL×BOOL: gpu == gpu2 */ else if (r.type == RES_TYPE_NUMERIC) eq = (double)l.u.bval == r.u.num; /* BOOL×NUM: gpu == 1 */ else return -1; /* defensive: BOOL×STR rejected at parse time */ break; default: return -1; } return (node->type == SEL_EQ) ? eq : !eq; } default: /* SEL_ANY must NEVER reach this point — const-fold (§6.3) eliminates * all SEL_ANY nodes before any eval_select call. If SEL_ANY is seen * here, a tree was constructed without running const-fold (e.g. a unit * test that manually builds a tree). Emit a diagnostic to catch this * during development: * if (node->type == SEL_ANY) log_fatal("SEL_ANY reached eval — const-fold not applied"); * The default return -1 still fires, but the log_fatal makes the bug * visible instead of silently returning non-match for every host. */ return -1; } } ``` **Key design decisions:** - **Error propagation:** AND/OR/NOT use explicit error checks instead of C's `&&`/`||` operators. C's `-1` is truthy, so `&&` would swallow errors. The explicit `if (lv < 0) return -1` pattern propagates errors correctly. - **`SEL_TRUE`:** A constant-true leaf node produced by const-folding (e.g., `type == any` reduces to `SEL_TRUE`). See §6.3. - **`xce_str_match`:** `==` on strings dispatches to `xce_str_match` rather than raw `strcmp`, allowing pattern matching extensions (e.g., wildcard support) without changing the eval structure. For the initial implementation, `xce_str_match` is `strcmp` semantics. - **Type compatibility for EQ/NE:** Parse-time validation (§7) ensures both operands of EQ/NE are type-compatible: either both numeric, both string, or a RESNAME paired with a literal whose type matches the resource's declared type. See §7 for type coercion rules. - **Boolean normalization in EQ/NE (intentional semantic):** When a boolean resource (`SEL_RESNAME(BOOL)`) is compared to a numeric literal, `host_get_bool` normalizes the resource's raw float value to 0 or 1 (any nonzero → 1). This means `gpu == 2` compares `1 == 2` (false, even if the underlying float is 2.0). This is intentional: boolean semantics define "presence/absence" (0 or nonzero), not a specific numeric value. If a user writes `gpu == 2`, they intend a boolean check against a value that happens to be 2 — the correct form is `gpu == 1` (or the standalone boolean idiom `gpu`). This behavior is documented here; no runtime warning is emitted for `gpu == 2`. - **Commutativity and mixed-type dispatch:** Commutativity canonicalization in `makeSelectNode` may swap operands so the larger-typed operand is on the left (or whichever ordering the key chooses). The `eval_select_inner` EQ/NE dispatch switches on `l.type` — it **must** handle every mixed-type pair in both orders (`NUM×BOOL` and `BOOL×NUM`, etc.) because the canonical sort order is fixed at intern time and may place either type on the left. Adding a new mixed-type pair (e.g., `STR×NUM`) requires adding symmetric branches for both orderings. - **Runtime type safety:** `eval_select` should guard operand types for `SEL_GT`/`SEL_LT`/`SEL_GE`/`SEL_LE` branches. Use `log_fatal` (not `assert()` — see §5.5) when both operands are not `RES_TYPE_NUMERIC`, since parse-time validation (§7) should prevent this. As a belt-and-suspenders fallback, return `-1` after the `log_fatal` call. ### 8.4 Thread Safety The scheduler runs a **two-thread pipeline**: ``` Job-picker thread Allocator thread ────────────────── ────────────────────────────────────── Find highest-priority → eval_select (per job, per host) job for scheduling Allocation (resource reservation) Pre-session: staging merge, GC, recompaction ``` `eval_select`, allocation, and all pre-session work (staging merge, GC, `recompact_node_ids`) run exclusively in the **allocator thread**. The job-picker thread identifies candidate jobs; it does not call `eval_select` or `makeSelectNode` during a session. **Allocator-thread-only invariant (CSE-enabled mode only):** The lock-free eval reads in §8.4 and the lock-free GC/recompaction in §5.5 both depend on the guarantee that only the allocator thread calls `eval_select`, `makeSelectNode`, `gc_sweep_select_nodes`, and `recompact_node_ids`. This invariant applies **only when `sel_optimize_level > 0`** (CSE-enabled / scheduler). When `sel_optimize_level == 0` (API server), the intern table is bypassed — `makeSelectNode` allocates private (non-shared) node trees — so concurrent API server goroutine calls are safe (§5.0). In CSE-enabled mode, violating the invariant causes use-after-free (concurrent GC + eval) or data races (concurrent makeSelectNode + eval without the intern table lock). Phase 6 must enforce this with a runtime thread-ID check in `eval_select` and `gc_sweep_select_nodes`. **GC single-thread guarantee:** `gc_mark_tree` writes the non-atomic `gc_mark` field on interned nodes. This is safe because `gc_sweep_select_nodes` is called exclusively from the allocator thread (a single-threaded context — see §8.5.3 pre-session lifecycle). All `gc_mark` writes from a single GC sweep are sequential; no second thread accesses `gc_mark` concurrently. If the single-thread guarantee is ever relaxed, `gc_mark` must be made `_Atomic unsigned` to prevent a C11 data race. **Pre-Phase-6 audit required:** Before implementing Phase 6, verify that the scheduler's allocator thread is indeed the sole thread calling the relevant functions. Run: ``` grep -rn 'eval_select\|alloc_thread\|sched_thread\|sched_session' src/scheduler/ ``` Confirm that `sched_meet_host_selection` and any `eval_select` call site are reachable only from the allocator thread's execution path (not from job-submission callbacks, timer threads, or signal handlers). If any other call site is found, the lock-free design assumption is violated and the design must be revised before Phase 6 begins. Document the audit result (either "confirmed single-threaded" or "found unexpected call site at [location]") in the Phase 6 commit message. ```c /* In eval_select (returns int): */ if (sel_alloc_thread_id != 0 && pthread_self() != sel_alloc_thread_id) { log_fatal("eval_select called from non-allocator thread"); return -1; } /* In gc_sweep_select_nodes (returns void): */ if (sel_alloc_thread_id != 0 && pthread_self() != sel_alloc_thread_id) { log_fatal("gc_sweep_select_nodes called from non-allocator thread"); return; } ``` `sel_alloc_thread_id` (declared as `static pthread_t` in `resreq.c`, requires `#include `) is set once when the allocator thread enters its first session loop iteration (Phase 6 wiring). A value of 0 (the zero-initialized default for `pthread_t` on Linux) means the thread has not been registered yet — the check is skipped during Phase 1–5 unit testing where no scheduler threads exist. This mirrors the `sel_optimize_level` write-once pattern (§5.0). - **Intern table lock:** `sel_node_table.lock` protects all insert/remove operations. New job resreq strings are parsed (calling `makeSelectNode` under lock) in the staging phase before the allocator begins a session. During the session itself, the allocator reads interned nodes without holding any lock — interned nodes are read-only once inserted (§5.1). - **Lock-free eval reads:** Within a session, `eval_select` accesses interned nodes without locking. No concurrent writer exists during a session — staging writes go to a separate buffer merged at the session boundary, and the job-picker thread does not write to the intern table. - **GC idle invariant:** `gc_sweep_select_nodes()` runs in the pre-session phase, after staging merge. At that point both the job-picker and allocator threads are idle — no `eval_select` or `makeSelectNode` call is in progress. The use-after-free concern is structurally impossible given this idle guarantee. See §5.5. - **Lock ordering:** `sel_node_table.lock` is the **outer** lock. `zarray_store` and `zarray_del` (called on `sel_node_table.nodes` while `sel_node_table.lock` is held) acquire zarray's internal lock as the **inner** lock. No code may acquire `sel_node_table.lock` while holding a zarray internal lock — that would invert the ordering and risk deadlock. All zarray calls on `sel_node_table.nodes` must be made with `sel_node_table.lock` already held; direct zarray calls on that array outside `makeSelectNode`, GC sweep, and `recompact_node_ids` are prohibited. **Enforcement:** any new function that needs to call a `zarray_*` function on `sel_node_table.nodes` must acquire `sel_node_table.lock` first, OR must be restricted to the pre-session idle phase (GC/recompaction pattern) with a comment citing the idle invariant. Code review must reject any `zarray_*` call on `sel_node_table.nodes` that has neither a surrounding lock nor an idle-phase annotation. **GC sweep exception:** §5.5 states that `gc_sweep_select_nodes` calls `zarray_del` *without* holding `sel_node_table.lock`. This is not a contradiction: the lock ordering rule above applies to concurrent execution paths where mutual exclusion is needed. GC runs in the pre-session idle phase when no concurrent `makeSelectNode` or `key_find` calls exist — the lock is structurally unnecessary. The §5.5 guidance ("do not add `mylock_lock`/`mylock_unlock` around GC's `key_del` calls") is intentional: acquiring `sel_node_table.lock` in GC would add unnecessary overhead and is omitted because the idle invariant makes mutual exclusion unnecessary — `kvtab_del` has no internal lock (verified in `kv.c:kvtab_del`), and while `zarray_del` does acquire its own per-array internal lock (verified in `zarray.c:zarray_del`), it never acquires `sel_node_table.lock`, so calling `zarray_del` without first holding `sel_node_table.lock` is safe and consistent with the outer→inner ordering rule. The runtime guards (§5.5 guard block) enforce the idle invariant that makes lock-free GC correct. The lock-ordering rule ('outer = `sel_node_table.lock`, inner = zarray lock') applies only to concurrent execution paths; idle-phase code holds neither outer lock at entry and may call `zarray_*` directly. The same lock-free exception applies to `recompact_node_ids`: it also runs in the pre-session idle phase, calls `zarray_get` to iterate `sel_node_table.nodes`, and mutates `node->node_id` directly without acquiring `sel_node_table.lock`. The structural reason is identical — no concurrent `makeSelectNode` or `key_find` calls exist during the idle phase, so mutual exclusion is unnecessary. ### 8.5 Evaluation Memoization Many jobs share sub-expressions via CSE (§5). Without memoization, evaluating `type==linux` for job1 and job2 against the same host computes the same node twice. A per-host evaluation context stores cached results indexed by `node_id`, so each node is evaluated at most once per host across all jobs. **Design principle:** The interned AST nodes are **read-only** — they carry no mutable eval state. All ephemeral evaluation state lives in a separate `select_eval_ctx_t` that is allocated per host, used across all jobs for that host within a session, and discarded when scheduling for that host completes. #### 8.5.1 Evaluation Context ```c typedef struct { const host_eval_ctx_t* host; /* host resource snapshot for this evaluation */ /* Boolean result: 2 bits per node * 00 = not evaluated * 01 = match (true) * 10 = no-match (false) * 11 = error */ uint8_t* bool_bits; /* bitmap, 2 bits per node */ uint32_t bool_bits_cap; /* capacity in nodes (not bytes) */ /* node_count: snapshot of sel_node_next_id captured at session start by * select_eval_ctx_init. select_eval_ctx_reset uses this value — NOT a * live re-read of sel_node_next_id — to compute the memset byte count: * memset(bool_bits, 0, (node_count * 2 + 7) / 8) * Invariant: node_count <= bool_bits_cap (enforced by pre-session capacity * check). Stale entries in bool_bits[node_count .. bool_bits_cap-1] are * never accessed during the session because staging is suspended and no * active node has an ID >= node_count. */ uint32_t node_count; /* Numeric leaf cache: only populated for SEL_RESNAME(numeric) and SEL_NUM. * Indexed by node_id. Unused entries are 0.0 (which is a valid numeric * value, but leaf nodes are always evaluated before use, so the slot * is always written before it is read). */ double* num_vals; uint32_t num_vals_cap; /* String leaf cache: only populated for SEL_RESNAME(string) and SEL_STR. * Indexed by node_id. Unused entries are NULL. */ const char** str_vals; uint32_t str_vals_cap; } select_eval_ctx_t; ``` Each interned node carries a `node_id` — a dense 0-based index assigned at intern time. The evaluation context arrays are sized to `max(node_id) + 1`. ```c #define EVAL_NONE 0 /* not yet evaluated */ #define EVAL_TRUE 1 /* node evaluates to true */ #define EVAL_FALSE 2 /* node evaluates to false */ #define EVAL_ERROR 3 /* node evaluates to error */ /* bool_bits reset relies on memset-to-0 == EVAL_NONE; enforce at compile time */ _Static_assert(EVAL_NONE == 0, "bool_bits reset via memset(0) requires EVAL_NONE==0"); static inline int eval_get_bool(const select_eval_ctx_t* ctx, uint32_t node_id) { if (node_id >= ctx->bool_bits_cap) { log_fatal("eval_get_bool: node_id %u >= bool_bits_cap %u", node_id, ctx->bool_bits_cap); return EVAL_ERROR; } uint32_t bit_off = node_id * 2; return (ctx->bool_bits[bit_off / 8] >> (bit_off % 8)) & 0x3; } static inline void eval_set_bool(select_eval_ctx_t* ctx, uint32_t node_id, int val) { if (node_id >= ctx->bool_bits_cap) { log_fatal("eval_set_bool: node_id %u >= bool_bits_cap %u", node_id, ctx->bool_bits_cap); return; } uint32_t bit_off = node_id * 2; uint32_t byte = bit_off / 8; uint32_t shift = bit_off % 8; ctx->bool_bits[byte] = (ctx->bool_bits[byte] & ~(0x3 << shift)) | ((val & 0x3) << shift); } ``` #### 8.5.2 Memoized `eval_select` ```c int eval_select(const xce_select_node_t* node, select_eval_ctx_t* ctx) { if (node == NULL) return 1; /* no select clause — all hosts match */ /* check cache — O(1) bitmap lookup */ int cached = eval_get_bool(ctx, node->node_id); if (cached != EVAL_NONE) return (cached == EVAL_TRUE) ? 1 : (cached == EVAL_FALSE) ? 0 : -1; int result = eval_select_inner(node, ctx); /* recursive eval */ /* write cache — including EVAL_ERROR: errors are node-structural * (type mismatch, absent resource), not job-context-dependent, so the * same node will produce the same error for every host in the session. * Caching EVAL_ERROR avoids re-evaluating a known-bad subtree on each * host. The error code propagates correctly through AND/OR because * eval_select returns -1 for EVAL_ERROR and the callers check lv < 0. */ eval_set_bool(ctx, node->node_id, result == 1 ? EVAL_TRUE : result == 0 ? EVAL_FALSE : EVAL_ERROR); return result; } ``` `eval_select_inner` contains the recursive evaluation logic (§8.3) but without the cache check — it is called only on cache miss. Leaf nodes (`SEL_RESNAME`, `SEL_NUM`, `SEL_STR`, `SEL_TRUE`, `SEL_FALSE`) write their results to the `num_vals` and `str_vals` arrays as well, so comparison operators can read cached leaf values without re-reading host data. **Implementation note:** the `eval_select_inner` code in §8.3 calls `host_get_bool`/`host_get_float` and `get_leaf_value` directly for leaf and comparison cases. The Phase 4e implementation must add explicit writes to `ctx->num_vals[node->node_id]` and `ctx->str_vals[node->node_id]` in the leaf-node branches of `eval_select_inner`, and reads from these cached arrays in the comparison-operator branches, to realize the memoization described here. The code in §8.3 shows the host-access version for clarity; the implementation will substitute cached-array access. **SEL_RESNAME leaf cache write (concrete sketch):** The `SEL_RESNAME` branch in the Phase 4e `eval_select_inner` must write to **both** `bool_bits` (via `eval_set_bool`) AND the typed leaf array (`num_vals` or `str_vals`) so that comparison operators can read the cached value without hitting live host data again: ```c case SEL_RESNAME: { int bool_result; switch (node->u.resname.res_type) { case RES_TYPE_BOOL: bool_result = host_get_bool(ctx->host, node->u.resname.res_idx); /* bool_bits written by eval_select wrapper — no extra write needed */ break; case RES_TYPE_NUMERIC: { double v = host_get_float(ctx->host, node->u.resname.res_idx); ctx->num_vals[node->node_id] = v; /* cache for comparison ops */ bool_result = (v != 0.0); break; } case RES_TYPE_STRING: { const char* s = host_get_str(ctx->host, node->u.resname.res_idx); ctx->str_vals[node->node_id] = s; /* cache for comparison ops */ bool_result = -1; /* string standalone rejected at parse time (§7) */ break; } default: bool_result = -1; } return bool_result; } ``` The comparison cases (`SEL_GT`, `SEL_LT`, `SEL_GE`, `SEL_LE`, `SEL_EQ`, `SEL_NE`) then call `eval_select(left_leaf, ctx)` / `eval_select(right_leaf, ctx)` (which cache-miss into the leaf branch above and write `num_vals`/`str_vals`), then read `ctx->num_vals[left->node_id]` directly — never `get_leaf_value()`. **Design decision — separate `num_vals`/`str_vals` arrays (deliberate):** An alternative is to encode leaf values in `bool_bits` (e.g., store a union or add a `sel_val_t` array). The separate typed arrays are chosen for three reasons: (1) they avoid a union-per-node overhead (24 B/node) for the 80%+ of nodes that are boolean-type comparisons and never need a leaf cache slot — those nodes only use 2 bits in `bool_bits`; (2) they allow the comparison operator to read a typed slot directly without a type switch; (3) they are consistent with the `host_eval_ctx_t.numvals`/ `strvals` naming convention, making the data flow easy to follow. The added complexity (two extra arrays + two cap fields) is justified by the memory savings shown in §8.5.5. #### 8.5.3 Session Lifecycle ``` ┌─ Staging + GC phase ──────────────────────────────────────────────────┐ │ 1. Merge new jobs from staging buffer into live job set │ │ 2. Parse new job resreq strings (calling makeSelectNode) │ │ 3. Unlink completed jobs (primary->select->root = NULL) │ │ At this point, all live jobs (old + newly arrived) are visible, │ │ and no eval_select or makeSelectNode call is in progress. │ │ *** MEMORY FENCE required here *** │ │ atomic_thread_fence(memory_order_seq_cst) must be inserted after │ │ all `primary->select->root = NULL` writes and before gc_sweep. │ │ Without the fence, the CPU or compiler may reorder the NULL store │ │ after the GC mark pass begins, causing gc_mark_tree to observe a │ │ stale non-NULL root and retain a node that should be swept. │ │ In C11: atomic_thread_fence(memory_order_seq_cst); │ │ Equivalent: make `select->root` an _Atomic pointer and use │ │ atomic_store_explicit(..., memory_order_release) for the unlink. │ │ 4. Run gc_sweep_select_nodes(live_jobs, njobs) — mark pass visits │ │ all live jobs including newly arrived ones, so nodes referenced │ │ only by incoming jobs are correctly retained. │ │ 5. Run recompact_node_ids() — reassign dense 0..N-1 IDs │ │ After recompact: current_node_count = sel_node_next_id │ │ = count of live interned nodes with dense IDs 0..N-1. │ └───────────────────────────────────────────────────────────────────────┘ ↓ ┌─ Pre-session capacity check ──────────────────────────────────────────┐ │ Note: this snapshot happens AFTER staging + GC + recompact, so │ │ current_node_count reflects only the live interned nodes. │ │ │ │ 1. Read sel_node_next_id → current_node_count │ │ 2. If current_node_count > bool_bits_cap: │ │ new_cap = next power-of-two >= current_node_count │ │ old_bool_bytes = (bool_bits_cap * 2 + 7) / 8 │ │ realloc bool_bits to (new_cap * 2 + 7) / 8 bytes │ │ memset(bool_bits + old_bool_bytes, 0, new_bytes - old_bool_bytes) │ │ /* REQUIRED: realloc does not zero extended bytes. Without this │ │ * memset, nodes with IDs in old_cap..new_cap-1 have stale (non- │ │ * zero) bool_bits in their first session, causing eval_select to │ │ * return spurious EVAL_TRUE/FALSE cache hits for unvisited nodes. */ │ realloc num_vals, str_vals to new_cap (calloc-style: zero entire │ │ new array, not just the extension, since they are typed pointers/ │ │ doubles — zeroing the extension alone is sufficient but zeroing │ │ all is safer and the arrays are small relative to the bool_bits) │ │ PARTIAL-OOM CONTRACT: all three realloc calls (bool_bits, │ │ num_vals, str_vals) must be treated as an atomic unit. If any │ │ one realloc fails after another has already succeeded, call │ │ log_fatal — do not proceed with mismatched capacities. Proceeding │ │ with bool_bits_cap != num_vals_cap would allow eval_select_inner │ │ to write ctx->num_vals[node_id] beyond the allocated num_vals │ │ array (no bounds guard at the write site; see BOUNDS NOTE below). │ │ update bool_bits_cap, num_vals_cap, str_vals_cap atomically │ │ (only after all three reallocs succeed) │ │ │ │ BOUNDS NOTE: eval_select_inner writes ctx->num_vals[node->node_id] │ │ and ctx->str_vals[node->node_id] without an inline bounds check │ │ (unlike eval_get_bool/eval_set_bool which guard with │ │ node_id >= bool_bits_cap). Correctness depends entirely on the │ │ pre-session capacity check having grown num_vals and str_vals to │ │ at least sel_node_next_id entries before the session loop runs. │ │ Implementation must ensure the capacity check runs and succeeds │ │ before any eval_select_inner call in the session. │ │ 3. If many nodes GC'd and eval context is oversized by 2x: │ │ shrink to next power-of-two >= current_node_count │ │ (optional, avoids unbounded growth) │ └───────────────────────────────────────────────────────────────────────┘ ↓ ┌─ Scheduling session ─────────────────────────────────────────────┐ │ ctx = select_eval_ctx_init(max_node_id) /* allocate once */ │ │ for each worker (xce_sched_worker_t*): │ │ /* build_host_eval_ctx is scheduler-private (src/scheduler/) │ │ * — not declared in resreq.h */ │ │ host_ctx = build_host_eval_ctx(worker, num_res) /* → host_eval_ctx_t* */ │ │ if host_ctx == NULL: log_error, continue │ │ select_eval_ctx_reset(ctx, host_ctx) /* takes host_eval_ctx_t* */ │ │ /* select_eval_ctx_reset clears bool_bits with: │ │ * memset(ctx->bool_bits, 0, │ │ * (ctx->node_count * 2 + 7) / 8) │ │ * where ctx->node_count is a SNAPSHOT captured at session │ │ * start (select_eval_ctx_init stores sel_node_next_id into │ │ * ctx->node_count and MUST NOT re-read sel_node_next_id on │ │ * each host reset). Re-reading would be wrong if any code │ │ * path increments sel_node_next_id during the session (e.g. │ │ * a mis-routed staging call), causing memset byte count to │ │ * exceed bool_bits_cap and overflow the allocation. │ │ * The byte formula accounts for 2 bits/node. │ │ * Must check (not assert — see §5.5): │ │ * if (ctx->bool_bits_cap < current_node_count) │ │ * log_fatal("bool_bits_cap too small"); │ │ * num_vals and str_vals: no need to clear between hosts. │ │ * Correctness proof: (1) bool_bits[comparator_id] is │ │ * EVAL_NONE after per-host reset, forcing eval_select_inner │ │ * to run for every comparator node; (2) eval_select_inner │ │ * evaluates leaf children (SEL_RESNAME/SEL_NUM/SEL_STR) via │ │ * eval_select BEFORE reading str_vals/num_vals — those leaf │ │ * nodes have their own EVAL_NONE bool_bits entries which │ │ * force a fresh host read and write to str_vals/num_vals; │ │ * (3) therefore every str_vals[leaf_id] and num_vals[leaf_id]│ │ * is overwritten for the current host before any comparator │ │ * reads it, regardless of short-circuit paths. Only │ │ * bool_bits needs clearing because EVAL_NONE == 0. */ │ │ for each pending_job: │ │ rc = eval_select(job->resreq->u.primary->select->root, ctx) │ │ if rc < 0: log error, skip host │ │ if rc == 0: skip host (select filter failed) │ │ if rc == 1: proceed to rusage check │ │ /* free_host_eval_ctx is scheduler-private (src/scheduler/) */ │ │ free_host_eval_ctx(host_ctx) /* dereferences str refcounts */ │ │ select_eval_ctx_fini(ctx) /* free session context after last host */ │ └──────────────────────────────────────────────────────────────────┘ ``` **Session-level arena allocation:** `select_eval_ctx_init` allocates the eval context once at session start, sized to `max_node_id + 1`. Between hosts, `select_eval_ctx_reset` clears `bool_bits` and updates the host pointer — `num_vals` and `str_vals` are overwritten per-host before comparators read them, so no clearing is needed. This avoids `malloc`/`free` per host (10K hosts/session would otherwise cost 10K allocation/deallocation pairs). The arena is freed via `select_eval_ctx_fini` at session end and potentially reallocated at the next session boundary after `recompact_node_ids` adjusts the `max_node_id`. **Ownership lifecycle contract:** The `select_eval_ctx_t` and `host_eval_ctx_t` have a strict parent-child ownership relationship within a session: - `select_eval_ctx_t` (session-scoped): allocated by `select_eval_ctx_init` before the host loop, freed by `select_eval_ctx_fini` after the host loop. It owns the `bool_bits`, `num_vals`, and `str_vals` arrays. It does **not** own the `host` pointer — `host` is a borrowed reference that is valid only for the duration of a single per-host eval pass (between `select_eval_ctx_reset` and the next reset or fini). - `host_eval_ctx_t` (per-host scoped): allocated by `build_host_eval_ctx` (**scheduler-private** — not declared in `resreq.h`; lives in `src/scheduler/`) before each host's eval pass, freed by `free_host_eval_ctx` (**scheduler-private**) after each host's eval pass. It owns the `numvals` and `strvals` arrays (and their xstring refcounts). The `select_eval_ctx_t.host` pointer borrows this struct — it must not be accessed after `free_host_eval_ctx` is called. **Dangling pointer prevention:** The session loop pseudocode in §8.5.3 ensures `free_host_eval_ctx` is called after the per-host inner loop, before the next host's `build_host_eval_ctx`. The `select_eval_ctx_t.host` pointer is stale after `free_host_eval_ctx` and must not be dereferenced until `select_eval_ctx_reset` sets it to the new host context. `select_eval_ctx_fini` does not dereference `host` — it only frees the owned arrays. **Refcount semantics of string fields (design decision):** `select_eval_ctx_t.str_vals` and `host_eval_ctx_t.strvals` both hold `const char*` pointers to xstring-interned strings, but their refcount contracts differ deliberately: - `host_eval_ctx_t.strvals[i]`: each non-NULL entry was acquired via `makeString()` in `build_host_eval_ctx`, which increments the xstring refcount. `free_host_eval_ctx` must call `dereference(strvals[i])` for each non-NULL entry to release those refcounts. - `select_eval_ctx_t.str_vals[i]`: these are **non-owning cached pointers** written by `eval_select_inner` and read by comparison operators within the same per-host eval pass. They are overwritten (not dereferenced) at the next `select_eval_ctx_reset` call. No `dereference` is called on `str_vals` entries — the refcount is held by the corresponding `host_eval_ctx_t.strvals` entry, which outlives the eval pass. Calling `dereference` on `str_vals` would double-release the same refcount. This design avoids an extra `makeString`/`dereference` round-trip per leaf evaluation while keeping the xstring reference stable for the full eval pass duration. #### 8.5.4 Node ID Re-compaction After GC After GC removes unreachable nodes, the remaining nodes have sparse IDs. Compacting to `0..N-1` keeps the eval context arrays dense: ```c /* Run after gc_sweep_select_nodes() — iterates the linear zarray, not the kvtab. * * Rationale for recompaction: (1) keeps eval context arrays dense so per-host * allocation is bounded by the live node count; (2) prevents uint32_t node_id * overflow in long-running scheduler instances where sel_node_next_id would * otherwise grow without bound across thousands of GC cycles. * * IMPORTANT: node_id and zarray slot index are intentionally decoupled. * node_id is a dense 0..N-1 index used exclusively for select_eval_ctx_t * arrays (bool_bits, num_vals, str_vals). The zarray stores node pointers in * arbitrary-order slots with possible gaps (NULLs from zarray_del). The * zarray slot index must NOT be used as an index into eval context arrays. */ static void recompact_node_ids(void) { uint32_t next_id = 0; /* Iterate all allocated slots (caps), not just live count. zarray_del * NULLs slots without swapping, so gaps may exist after GC. * * zarray_get contract: returns NULL for both (a) slots vacated by * zarray_del (NULLed in place, index < count) and (b) indices in the * range count..caps-1 (allocated but never written, initialized to zero * by xcealloc/calloc). Both cases are handled by the NULL-continue below. * Verify this contract holds in src/lib/zarray.c before relying on it. */ int caps = sel_node_table.nodes.caps; for (int i = 0; i < caps; i++) { xce_select_node_t* node = (xce_select_node_t*)zarray_get(&sel_node_table.nodes, i); if (node == NULL) continue; /* vacated or unwritten slot — skip */ node->node_id = next_id++; } atomic_store_explicit(&sel_node_next_id, next_id, memory_order_release); /* reset counter for new allocations */ } ``` #### 8.5.5 Memory Cost For a cluster with 10,000 unique select nodes: | Field | Per-node cost | Total 10K nodes | |-------|:---:|:---:| | `bool_bits` (2 bits/node) | 0.25 B | 2.5 KB | | `num_vals` (float64) | 8 B | 80 KB | | `str_vals` (char*) | 8 B | 80 KB | | **Total per host** | | **~163 KB** | By separating the typed arrays instead of using a `sel_val_t` union (24 bytes/node) for every node, the per-host cost drops from ~240 KB to ~163 KB — a 32% reduction. The savings is larger in practice because 80%+ of nodes are boolean-type (AND, OR, NOT, comparisons), which use only 2 bits instead of 24 bytes. #### 8.5.6 Thread Safety for Memoization The `select_eval_ctx_t` is per-host and per-session. It is not shared between hosts or between sessions. The allocator thread processes one host at a time within a session (§8.4), so no concurrent access to the same context occurs. The `bool_bits` bitmap is cleared per host with `memset`, which is safe because the allocator thread is the only writer during a session. --- ## 9. Job-Level Sharing There are two levels of sharing in the resreq system: ### 9.1 Select Sharing is Global (Node-Level CSE) Select expressions are purely host-filtering — `type==linux` means the same thing regardless of which user, queue, or project submitted the job. Node-level CSE (§5) makes this sharing **global**: all jobs that reference `type==linux` share the same interned `SEL_EQ` subtree, regardless of their user, queue, or project. This is one of the key benefits of CSE at the AST node level rather than string-level interning. ### 9.2 Resreq Sharing is Scoped to (user, queue, project) > **Future work — not part of the current implementation plan (§11).** A second-level intern table for full `xce_resreq_t` objects (scoped to `(user, queue, project, resreq_string)`) is a natural follow-on once select CSE is validated in production; design it when the need is confirmed. ### 9.3 Composability of the Two Levels > **Future work — depends on §9.2.** Node-level CSE (§5) and resreq-level sharing (§9.2) compose naturally: `makeSelectNode` interns individual AST nodes so jobs sharing only a subset of their resreq still share the common select subtree — design details belong in the §9.2 follow-on work. --- ## 10. Scheduling Session Semantics Within a single scheduling session: ``` ctx = select_eval_ctx_init(max_node_id) /* allocate once per session */ for each candidate_host: select_eval_ctx_reset(ctx, host) /* clear bool_bits, update host ptr */ for each pending_job: 1. rc = eval_select(job->resreq->u.primary->select->root, ctx) → 1 (true): proceed to rusage check → 0 (false): skip host (select filter failed) → -1 (error): log error, skip host — do NOT silently treat as false 2. can_satisfy_rusage(job.resreq.primary.rusages, host.available) → false: skip host (insufficient resources) 3. allocate(job, host) → mark job as dispatched, update host.available if no_host_matched_for_job: job remains unsatisfied, re-evaluated next session select_eval_ctx_fini(ctx) /* free session context after last host */ ``` **Error handling:** `eval_select` returns `-1` for error (§8.3). The scheduler must treat `-1` distinctly from `0` (no match): log the error and skip the host rather than silently treating the error as "no match." The updated `eval_select` (§8.3) propagates `-1` through AND/OR/NOT explicitly — C's boolean operators are not used, so `-1` is never silently coerced to truthy. Key properties: - **select evaluates against a host snapshot**: `eval_select` uses `host_eval_ctx_t`, which contains the host's static and semi-static properties (type, architecture, boolean features, current load). These do not change during a session, so `eval_select` on the same `(select_node, host)` pair always returns the same result within a session. The `host_eval_ctx_t` does **not** contain allocation or booking data — it is a pure resource snapshot. - **rusage allocation uses snapshot + booked**: `can_satisfy_rusage` checks `host.available = host.total - host.booked`, where `host.booked` accumulates as jobs are dispatched within the session. This is a separate mechanism from select evaluation — the select filter runs first, and only hosts that pass the select filter are considered for rusage allocation. - **rusage is stateful**: as jobs are dispatched, `host.available` shrinks. Later jobs in the same session see reduced available resources. - **unsatisfied jobs persist**: a job that cannot be placed stays in the queue and is re-evaluated in the next scheduling session. The separation of concerns is deliberate: select filtering is a pure boolean check against host properties (no side effects, fully memoizable per host), while rusage allocation is a resource consumption check that mutates the host's available resources as dispatch decisions are made. --- ## 11. Implementation Plan ### C Library (Phases 1–5) #### Phase 1: TDD test cases (write first) - `lib/testsuite/test_select.c` - Parse: valid expressions, unknown resource names (expect error), type mismatches (numeric op on string resource, expect error). - Parse: input exceeding `SELECT_MAX_INPUT_LEN` (expect error), nesting exceeding `SELECT_MAX_DEPTH` (expect error). - CSE: `A && B` and `B && A` produce the same `xce_select_node_t*`. - CSE: two calls to `makeSelectNode` with same args return same pointer (same interned node). - Eval: numeric `>`, `<`, `==`; string `==`; bool standalone; `!`, `&&`, `||`; short-circuit evaluation. - GC: after unlinking a job's select root, `gc_sweep_select_nodes` frees the now-unreachable node and removes it from the intern table; a subsequent `makeSelectNode` with the same args re-interns a fresh node. - GC predicate functions: `sched_session_active` returns 1 while a session is running and 0 otherwise; `sched_eval_in_progress` returns 1 only inside the eval-select loop; `staging_merge_complete` returns 1 after staging merge and 0 before. **Note:** Phase 1 tests the *interface contract* of these predicates using a test-only scaffold, not the real scheduler wiring. In Phase 1, these predicates are backed by a **test-only `fake_scheduler_t` struct** (declared in `test_select.c`) with the same three `_Atomic int` fields that Phase 6 will add to the real `xce_scheduler_t` (see §5.5 "Predicate lifecycle" and Phase 6 "Add GC predicate fields"). The Phase 1 stub functions wrap the fake struct; Phase 6 replaces them with wrappers around the real struct. No Phase 1 stub code leaks into the production implementation — the predicate functions are thin wrappers whose bodies change in Phase 6, not the test fixtures. The predicate *signatures* and *semantics* are stable across all phases; only the backing struct changes. *Tests are written as skeletons/stubs in Phase 1. They compile but fail until Phase 4 provides the implementation. The TDD contract is: test signatures and assertions are defined before implementation code.* *Note on sequencing: Phase 3 (PEG grammar) defines the exact syntax used in test input strings. Implementers should review or draft Phase 3 before finalizing Phase 1 test cases, or develop Phase 1 and Phase 3 concurrently. The grammar is the test oracle for all parse-level assertions.* #### Phase 2: Header definitions - `include/resreq.h` - Add `res_type_t`, `sel_node_type_t`, `xce_select_node_t` typedef + struct. - Fill `struct xce_resreq_select_` with `xce_select_node_t* root`. - Declare `makeSelectNode`, `eval_select`, `eval_select_inner`, `select_eval_ctx_init`, `select_eval_ctx_reset`, `select_eval_ctx_fini`, `sel_node_table_init`, `sel_node_table_fini`, `gc_sweep_select_nodes`, `recompact_node_ids`. (`gc_mark_tree` is a `static` helper in `resreq.c` — §5.5 — and must NOT be declared in the public header.) - `gc_sweep_select_nodes` signature: `void gc_sweep_select_nodes(const xce_sched_job_t** live_jobs, size_t njobs);` (`xce_sched_job_t` has `xce_resreq_t* resreq`; `xce_job_t` has only `char* resreq` and cannot reach the parsed AST.) Caller: scheduler pre-session, after staging merge. Called after all job cleanup (select root unlinks) and before `recompact_node_ids`. Precondition: no `eval_select` call is in progress (GC idle invariant, §5.5). - Add `var resmgr` to `resreq_context_t` (the PEG auxil struct declared in `include/resreq.h`): this field threads the resmgr through the outer resreq parse context so the select sub-parser receives it via `auxil->resmgr`. (`select_context_t`, the auxil for `xce_select.peg`, already carries `var resmgr` — see §12 Appendix.) - Add `valtype_to_res_type` mapping helper. **Scope decision:** `valtype_to_res_type` is used in the PEG `resname` action to convert the resmgr's `XCE_VAL_*` value into `res_type_t` when populating each `SEL_RESNAME` node — include it. The inverse function `res_type_to_valtype` has no current consumer (the core eval path never re-derives a valtype from `res_type_t`) — **omit it** to avoid dead code. If a future diagnostics or type-error-reporting feature needs it, add it then (YAGNI). The `res_type_to_valtype` implementation at §4 line 326 serves as documentation of the mapping semantics for reference but should not be compiled into the production build without a consumer. - Add `xce_str_match` as a `static inline` helper: ```c static inline int xce_str_match(const char* a, const char* b) { return strcmp(a, b) == 0; } ``` **Scope decision:** wrapping `strcmp` rather than calling it directly is a forward-looking choice — it lets wildcard or glob semantics be added later by changing one function body, without touching every call site in `eval_select_inner`. IBM LSF's select clause supports `model==sun4*` glob patterns, and XCE may need the same. This is a deliberate abstraction, not speculative generality. - Update `parse_resreq_primary` to accept a `resmgr` parameter (required, never NULL). This is a new product with no prior release, so the interface is updated directly — no backward-compatibility shim or separate entry point is needed. **CGO binding audit (mandatory before changing the signature):** The Go API server must NOT have a CGO binding to `parse_resreq` — if it does, a signature change here breaks the Go build silently at link time (CGO type errors are not always caught at compile time). Verify before proceeding: ``` grep -rn 'parse_resreq\|C\.parse_resreq' src/frontend/ ``` Expected result: no matches. If matches exist, update the CGO wrapper in the same PR. Before updating, find all C callers: ``` grep -rn 'parse_resreq\b\|parse_resreq_primary\b' src/ ``` Known call sites to update (verify against grep output — there may be more): 1. `src/lib/xce_resreq.peg` PEG action — add `resmgr` field to `resreq_context_t` (the auxil type) and pass `auxil->resmgr` to `parse_resreq_primary`. The resmgr must also propagate through the outer `parse_resreq()` entry point and `parse_section()` — the full call chain is: `make_sched_job → parse_resreq(resmgr, ...) → parse_section → parse_select(ctx_with_resmgr) → makeSelectNode`. 2. `src/scheduler/src/schedobj.c` — `make_sched_job` line 171 calls `parse_resreq(job->resreq, j->submit->resreq, &errmsg)` with two args; this must be updated to the new form that includes `resmgr`. `make_sched_job` receives a `void*` pointer castable to `xce_scheduler_t*`; access `sched->resmgr` and thread it into the resreq parse context. 3. `src/lib/testsuite/test_select.c` (Phase 1) — unit tests must construct a minimal resmgr fixture (a small in-memory resource table mapping test resource names to types) before calling any parse function. **Shared helper location:** Do NOT define `make_test_resmgr()` inside `test_select.c` — both `test_select.c` and `test_resreq.c` need it, and duplication leads to drift. Instead, create a new shared helper: - **`src/lib/testsuite/test_resmgr_fixture.h`** — declares `var make_test_resmgr(void)` and `void free_test_resmgr(var)` - **`src/lib/testsuite/test_resmgr_fixture.c`** — implements them (creates an in-memory resmgr with a small fixed set of test resources: one bool, one numeric, one string) Both `test_select.c` and `test_resreq.c` `#include` the header and link against the `.c` file. Add `test_resmgr_fixture.c` to the testsuite `executable()` sources in `lib/meson.build`. 4. `src/lib/testsuite/test_resreq.c` — existing test call sites that reach `parse_resreq_primary` must be updated to pass a resmgr. The current signatures and their updated forms: ```c /* Current (before Phase 2): */ int parse_resreq_primary(xce_primary_resreq_t* primary, char* resreqString, char** errmsg); int parse_resreq(xce_resreq_t* resreq, char* data, char** errmsg); /* Updated (Phase 2 + 4b): */ int parse_resreq_primary(xce_primary_resreq_t* primary, char* resreqString, var resmgr, char** errmsg); int parse_resreq(xce_resreq_t* resreq, char* data, var resmgr, char** errmsg); ``` `resmgr` is placed into `resreq_context_t` before invoking the packcc parse runner: ```c resreq_context_t ctx = { 0 }; ctx.resmgr = resmgr; /* thread resmgr into PEG auxil */ /* ... pxc_xce_resreq_create(&ctx) / pxc_xce_resreq_parse() ... */ ``` The three call sites in `test_resreq.c` (current lines 244, 301, 330) each call `parse_resreq(&resreq, resreqString, &errmsg)` and must become `parse_resreq(&resreq, resreqString, test_resmgr, &errmsg)`, where `test_resmgr` is the fixture from the shared `test_resmgr_fixture.h` helper (see item 3 above). #### Phase 3: PEG grammar - `lib/xce_select.peg` - Grammar as in §3.1 with packcc preamble (§12). - `select_context_t` auxil with `resmgr` pointer. - Each action calls `makeSelectNode` with canonicalized operands. - Type validation in comparison actions. - **Clause-keyword conflict check in `resname` action (§3.1 runtime enforcement):** After the identifier is captured, the `resname` PEG action must check it against the clause-keyword list (`select`, `rusage`, `span`, `same`, `affinity`) and emit a parse error if there is a match — see §3.1 for the full code snippet. This is a Phase 3 deliverable, not deferred to Phase 4b; the check lives in the PEG action, not in `resreq.c`. - **Implicit vs explicit select dispatch:** `xce_select.peg` handles only the inner expression grammar. The implicit/explicit dispatch happens in the existing `section.peg`, which already extracts `selectString` from both forms (§2: `select_clause ::= 'select' '[' select_expr ']' | select_expr`). `parse_select()` receives the extracted `selectString` regardless of which form was used — the PEG grammar does not need to handle the `select[...]` wrapper or the clause-keyword boundary detection. The implicit form's boundary detection (recognizing `rusage`, `span`, etc. as clause boundaries) is implemented in `section.peg`. **Audit complete (bug confirmed and fixed):** `section.peg:50` (the second alternative of `select_statement`) previously read: ```peg / (!("span[" "]" / "]") .)* {$$ = strdup($0); } ``` This stopped on the two-token sequence `span[]` (empty span), NOT on `span[` alone — unlike the correct pattern in `usage_statement` (line 58): `(!("span[" / "]") .)*`. The bug caused a select statement preceding a non-empty `span[...]` clause to consume the span content as part of `selectString`, silently corrupting `spanString`. The fix has been applied to `section.peg:50`: ```peg / (!("span[" / "]") .)* {$$ = strdup($0); } ``` This is a confirmed correctness fix; no further audit is required. #### Phase 4: Implementation - `lib/resreq.c` **ABI audit (before starting 4a):** `struct xce_resreq_select_` changes from zero-size to `{ xce_select_node_t* root; }`. Before writing any Phase 4 code, run both: ``` grep -rn 'xce_select_t\|xce_resreq_select_' src/ grep -rn 'sizeof.*xce_select_t' src/ ``` Verify no caller holds it by value (embedded struct, stack copy, serialized). The `sizeof` search catches size-dependent allocation or serialization that the name search alone would miss. Convert any by-value sites to pointer-indirect access before proceeding. **Zero-size-to-nonzero-size transition risk:** The current `struct xce_resreq_select_` has size 0 in C99 (implementation-defined; may be 0 or 1 depending on compiler). Any call to `XCEMALLOC(sizeof(xce_select_t))` in the current codebase that allocates the struct by value will under-allocate after the struct gains an 8-byte pointer field. The ABI grep above must find and fix these sites before the struct definition is changed. Most callers use a pointer-to-struct pattern (`primary->select` is a `xce_select_t*`) so the risk is confined to explicit `sizeof` allocations — but these must be found and updated. **4a. Core node allocation and interning (§5.2–5.4)** - Add `#include "kv.h"` (required for `kv_key_ops_t`; not transitively included via `kvmgr.h`). - `sel_node_table` static with `mylock_t`. - `sel_key_ops_t` for `sel_node_key_t` (hash, cmp, assign, destroy). - `sel_make_key()` constructor (§12 Appendix). - `sel_node_table_init(int nslots)` / `sel_node_table_fini()`. `sel_node_table_init` allocates `sel_node_table.table` via `makeInstance(&kvtab_type, ¶ms)` (the same public API used by `xstring.c`; `kvtab_init` is an RTTI-internal callback, not a public constructor) and initializes `sel_node_table.nodes` via `zarray_init(&sel_node_table.nodes, delta, NULL)` (embedded linear array for GC/recompaction iteration — all existing `zarray_t` usage in the codebase uses embedded structs initialized with `zarray_init`, not `zarray_new()`), sets `datakiller=NULL` on the kvtab (§5.5 sweep frees nodes explicitly, not via kvtab callback), and initializes `sel_node_next_id = 0`. **`sel_node_next_id` memory model:** declare as `static _Atomic uint32_t` (per §5.3). All writes use `atomic_store_explicit(..., memory_order_release)` and the pre-session read in `select_eval_ctx_init` uses `atomic_load_explicit(..., memory_order_acquire)` to prevent the compiler or CPU from reordering the final `makeSelectNode` store past the `select_eval_ctx_init` read. **`datakiller=NULL` invariant:** `kv.c` fires `val_killer` unconditionally on `key_del` if set, which would double-free nodes freed explicitly by GC sweep. This is enforced by code convention — there is no runtime assertion because `kvtab_t` is defined only inside `kv.c` (an opaque type not exported in `kv.h`), so `sel_node_table.table->datakiller` is not accessible outside `kv.c`. The `params` struct passed to `makeInstance(&kvtab_type, ¶ms)` MUST either zero-initialize `datakiller` or set it explicitly to NULL — a non-NULL `datakiller` in `params` is a latent double-free bug that no runtime check can catch. The params literal MUST carry a prominent comment to prevent future readers from accidentally adding a datakiller callback: ```c kv_params_t params = { .nslots = nslots, .keyops = &sel_node_key_ops, /* field is 'keyops', not 'key_ops' — see kv.h */ /* datakiller MUST remain NULL. GC sweep frees nodes explicitly via * free_select_node() after key_del. A non-NULL datakiller would * cause key_del to free the node a second time — silent double-free. */ .datakiller = NULL, }; sel_node_table.table = makeInstance(&kvtab_type, ¶ms); ``` When `sel_optimize_level == 0`, both initializations are skipped (§5.0). - `makeSelectNode` (§5.4) — with `sel_make_key()`, key_add return code check for TOCTOU safety, and node_id assignment. - `sel_key_hash` helper (§12 Appendix). - Fix `makeString()` TOCTOU in `src/lib/xstring.c` (§5.4): on `key_add` returning `-EEXIST` **only**, re-find the existing string **while still holding the intern table lock** and return it, rather than returning NULL. The re-find must happen inside the lock (before `mylock_unlock`) — releasing the lock before re-finding creates a window where the existing string could be dereferenced and freed by a concurrent caller, yielding a dangling pointer. This matches the `makeSelectNode` TOCTOU pattern in §5.4 step 5 exactly. If `key_add` returns `-ENOMEM`, nothing was inserted — re-finding would fail and returning NULL is correct. This protects the numeric-to-string coercion in §7 under concurrent API server goroutines, and all other `makeString` callers. *(Scope note: this change touches `src/lib/xstring.c`, which is outside the select-specific scope of §2. It is included here as an explicit Phase 4a deliverable — a silent NULL return from `makeString` in the §7 coercion path corrupts the interned AST and must be fixed before Phase 4b begins. If change-management policy requires isolated file ownership, track it as a numbered sub-task with its own PR, but do not treat it as optional or defer it beyond Phase 4a. Completion criterion: the concurrent stress test for `makeString` under TSan passes with no races.)* **4b. Parse-time type validation (§7)** - **Phase gate: `makeString()` TOCTOU fix (4a) must be merged and tested before any 4b code is written.** The §7 PEG coercion path calls `makeString(buf)` under concurrent API server load; if `makeString` returns NULL on `-EEXIST`, it corrupts the interned AST silently. **Gate verification:** before starting 4b, confirm that `src/lib/xstring.c:makeString` handles `-EEXIST` from `key_add` by re-finding and returning the existing interned string (rather than returning NULL), and that the existing `test_xstring.c` suite passes with this change. The 4a fix must land in the same PR or an immediately preceding PR — do not begin 4b implementation until the gate is verified. - Update `parse_resreq_primary` to call `parse_select` with resmgr. **Critical ordering:** `parse_select(data.selectString, resmgr, &primary->select, errmsg)` MUST be called **before** `kill_resreq_data(&data)`. `kill_resreq_data` frees `data.selectString` (via `FREEUP(data->selectString)` at `resreq.c:49`). A `parse_select` call after `kill_resreq_data` passes a freed pointer — a use-after-free bug. The correct insertion point is after `parse_rusage` (and all other `parse_*` calls that populate `data`) and immediately before `kill_resreq_data(&data)`: ```c /* After all parse_* calls: */ if (data.selectString) { rc = parse_select(data.selectString, resmgr, &primary->select, errmsg); if (rc != 0) { kill_resreq_data(&data); return rc; } } kill_resreq_data(&data); /* frees data.selectString — must come after */ ``` - Update `kill_primary_resreq` to handle CSE mode conditionally: - CSE-enabled (`sel_optimize_level > 0`): unlink select root only (`primary->select->root = NULL`). GC between sessions reclaims unreachable nodes. - CSE-disabled (`sel_optimize_level == 0`): call `free_select_tree(root)` to walk and free the node tree, then `FREEUP(primary->select)` to free the `xce_select_t` container struct (§5.0). - **`kill_primary_resreq` location:** `include/resreq.h` declares `kill_resreq` (line 128) but does not currently export `kill_primary_resreq` as a separate symbol. Before modifying, locate the primary-cleanup call in `src/lib/xce_resreq.c` (inside `kill_resreq`'s dispatch on `XCE_RESREQ_PRIMARY`) and update it there, or extract it as a named static helper if the logic warrants it. **4b-locale. LC_NUMERIC enforcement (§7 locale note)** - `resreq.c` must define `_XOPEN_SOURCE 700` (or a higher value) before any system headers to expose `newlocale`, `uselocale`, and `freelocale` from ``. These POSIX.1-2008 extensions are not visible without this feature-test macro. - In `parse_select`, wrap the PEG parser call (which executes the `snprintf` numeric-to-string coercion in §7 PEG actions) with an explicit C-locale guard. The API server (`sel_optimize_level == 0`) calls `parse_resreq` from goroutines that may have non-C locale settings; without this guard, `snprintf("%.17g", val)` produces a comma decimal separator in European locales, corrupting string comparisons at eval time. The scheduler already runs `LC_ALL=C`, but the guard is still correct there (a no-op in practice): ```c locale_t cloc = newlocale(LC_NUMERIC_MASK, "C", NULL); if (cloc == (locale_t)0) { /* newlocale() failed. This must never happen on the scheduler: * a locale-sensitive strtod/snprintf in the resreq parser silently * produces wrong numeric values (e.g. "1,5" instead of "1.5" in * European locales), corrupting job scheduling decisions. Abort. */ if (sel_optimize_level > 0) { log_fatal("newlocale(LC_NUMERIC_MASK, C) failed: %s — scheduler" " cannot safely parse numeric resreq values", strerror(errno)); return -1; /* unreachable: log_fatal terminates */ } /* API server: fail hard — a corrupt locale causes parse_select to accept * '1,5' as a valid numeric literal in European locales, then store the * wrong string in the interned node. eval_select_simple would later * compare "1,5" against "1.5" and silently report no match for every * host. Newlocale failure is not recoverable at request time. */ log_error("newlocale(LC_NUMERIC_MASK, C) failed: %s — rejecting resreq " "parse to prevent locale-corrupted numeric comparisons", strerror(errno)); return -1; } locale_t old = uselocale(cloc); /* ... invoke PEG parser ... */ uselocale(old); freelocale(cloc); /* cloc is non-NULL here; never pass (locale_t)0 to freelocale */ ``` Do NOT call `freelocale((locale_t)0)` — passing 0/NULL to `freelocale` is undefined behavior per POSIX. - **strtod locale coverage (mandatory verification):** The packcc-generated PEG scanner may call `strtod` internally to parse number literals at scanner level — before any PEG action fires and before the `uselocale` scope begins. Before closing Phase 4b, verify by inspecting the generated `lib/xce_select.c`: ```bash grep -n 'strtod\|atof\|strtof' build/lib/xce_select.c ``` If packcc inserts a `strtod` call in the scanner (tokenizer), the `uselocale` guard in `parse_select` does NOT cover it. In that case, either: (a) set the C locale at process startup (acceptable for scheduler; risky for API server CGO goroutines), or (b) store the raw matched number string from the PEG `%0` capture and call `strtod` explicitly inside the PEG action where the `uselocale` guard is already active, discarding any packcc-generated `strtod` call. Option (b) is preferred for correctness. If `grep` returns no matches, document the result as a confirmed safe finding and close this item. **4c. Commutativity canonicalization and associativity flattening (§6.2)** - Commutativity sort in `makeSelectNode` (already integrated in §5.4). - Post-parse flatten pass for `&&`/`||` chains. - Bottom-up re-intern via `makeSelectNode` after flattening. **4d. Const-fold transform (§6.3)** - `type==any` → SEL_TRUE, `!(type==any)` → SEL_FALSE, and related constant-propagation cases. **4e. Evaluation with memoization (§8.3/§8.5)** - `eval_select` / `eval_select_inner` — memoized evaluation with per-host `select_eval_ctx_t`, explicit error propagation, NULL root guard. - `select_eval_ctx_init`, `select_eval_ctx_reset`, `select_eval_ctx_fini` — session-level context allocation, per-host reset, session-end teardown (§8.5.3). - `host_resval_typed`, `host_get_bool`, `host_get_float`, `host_get_string` accessors. **4f. Mark-and-sweep GC and recompaction (§5.5/§8.5.4)** - `gc_sweep_select_nodes()` with resreq->tag dispatch for COMPOUND/ALTER. - `gc_mark_tree()` helper. - `recompact_node_ids()` — reassign dense 0..N-1 IDs after GC (§8.5.4). #### Phase 5: Build wiring - `lib/meson.build` — add a `custom_target` block for `xce_select.peg` matching the `section_parser`, `rusage_parser`, and `xce_resreq_parser` pattern already in the file (each invokes packcc to produce a `.c`/`.h` pair), then add `xce_select_parser[0]` to `libxce_src`. Simply listing `xce_select.peg` in the source array is insufficient — the PEG file must be code-generated by packcc before it can be compiled. Example: ```meson xce_select_parser = custom_target( 'xce select parser', output: ['xce_select.c', 'xce_select.h'], input: ['xce_select.peg'], command: [packcc, '-l', '-o', 'lib/xce_select', '@INPUT@'], build_by_default: true) ``` **`-o` path verification:** The `-o 'lib/xce_select'` argument mirrors the established pattern in `lib/meson.build` for `section_parser` (`-o 'lib/section'`), `rusage_parser` (`-o 'lib/rusage'`), and `xce_resreq_parser` (`-o 'lib/xce_resreq'`). Before adding the new block, verify the pattern has not changed: ``` grep -A8 'custom_target' src/lib/meson.build | grep '\-o' ``` All three existing entries must show `'lib/'` — if the path format has changed (e.g. to an absolute path or `@OUTDIR@` form), update the `xce_select` entry to match. Add `xce_select_parser[0]` (the generated `.c` file) to `libxce_src`. ### Scheduler Integration (Phase 6) #### Phase 6: C scheduler wiring The scheduler is a C process. No CGO bridge is required. Phase 6 wires the select evaluator into the scheduler's existing session loop: - **Restructure the scheduler session loop** — the cross-job CSE memoization benefit depends on a **single `select_eval_ctx_t` being reused across all jobs for a given host** within a session. The existing `sched_meet_host_selection(xce_scheduler_t*, xce_sched_worker_t*, xce_select_t*)` is called once per (job, host) pair, which means each invocation would construct and discard a fresh eval context, silently losing the memoization. Phase 6 must restructure the allocator thread's session loop to an **outer-host / inner-job** pattern: ``` /* select_eval_ctx_init takes only max_node_id — no host parameter. * select_eval_ctx_reset sets the host pointer before every host's eval * loop, so no initial host is needed at construction time. * Consistent with the Appendix declaration and §8.5.3. */ ctx = select_eval_ctx_init(max_node_id) /* once per session */ for each worker (xce_sched_worker_t*): host_ctx = build_host_eval_ctx(worker, num_res) if host_ctx == NULL: log_error("OOM in build_host_eval_ctx — skipping host") continue /* skip this host; other hosts unaffected */ select_eval_ctx_reset(ctx, host_ctx) /* reset bitmap, update host ptr */ for each pending_job: rc = eval_select(job->resreq->u.primary->select->root, ctx) /* rc: 1 = pass, 0 = filter out, -1 = error */ if rc == 1: proceed to rusage check (can_satisfy_rusage) if rc == 0: skip job for this host if rc < 0: log error, skip job for this host free_host_eval_ctx(host_ctx) select_eval_ctx_fini(ctx) /* once per session */ ``` **`sched_meet_host_selection` role after restructuring:** Retain the function as the **per-host setup entry point** (not per-job): it calls `build_host_eval_ctx`, `select_eval_ctx_reset`, then drives the inner-job eval loop above. Update its signature to accept the session-level `select_eval_ctx_t*` as a parameter so the context is allocated once per session by the caller (the session loop), not once per host call: **Host iteration source compatibility:** The pseudo-code above uses "for each worker" as a shorthand. The actual scheduler may iterate hosts via a cbuf, linked list, or other container — the select evaluation code does not depend on the host container type. The only requirement is that the allocator thread can produce a `xce_sched_worker_t*` for each candidate host in sequence, which is already the case for the existing `sched_meet_host_selection` call pattern. If the scheduler uses a cbuf with wraparound semantics, the host loop iterates until the cbuf is empty (or a maximum-iteration guard prevents infinite loops on cbuf corruption). The select eval code is container-agnostic — it receives a `host_eval_ctx_t*` per host and does not care how the host was obtained. ```c /* Updated signature: ctx is session-level, allocated by caller */ void sched_meet_host_selection(xce_scheduler_t* sched, xce_sched_worker_t* worker, select_eval_ctx_t* ctx); ``` **Current implementation status:** `sched_meet_host_selection` in `src/scheduler/src/resource.c` is currently a one-line stub: `return 1;` (FIXME comment). Phase 6 is a **from-scratch implementation**, not a signature migration — the entire evaluation logic must be written. The Phase 6 integration test (`parity test`) cannot compare against a meaningful baseline since the stub always passes; the test must explicitly validate against known-good resreq/host pairs from the test suite. Identify all callers of `sched_meet_host_selection` in the allocator thread's session loop and update them to pass the session-level `ctx` and call `select_eval_ctx_init`/`select_eval_ctx_fini` around the host loop. **MANDATORY pre-condition (blocking gate):** Before changing the `sched_meet_host_selection` signature, run: ``` grep -rn 'sched_meet_host_selection\|meet_host_selection' src/ ``` This grep is a **required blocking step** — Phase 6 cannot proceed until it is completed and all results are accounted for. A missed call site that still uses the old `int`-returning signature will compile (C allows implicit casts on function pointers in some contexts) but invoke the function with the wrong ABI — causing undefined behavior at runtime where the missing `select_eval_ctx_t*` parameter causes the function to read garbage from the stack. **PASS criterion:** all grep hits are either: (a) the definition itself, (b) direct call sites that have been updated, or (c) the forward declaration in `scheduler.h` that has been updated. Any typedef, function pointer, or callback table entry must also be updated in the same change. Do not mark Phase 6 as complete until all hits pass this criterion. **Return type migration:** the existing signature returns `int`; callers (e.g., `resource.c:368`) check the return value to gate host-skip and reason-recording logic: ```c /* resource.c:368 — current caller pattern */ rc = sched_meet_host_selection(sched, w, primary->select); if (rc == 0) { add_job_preason(..., PREASON_MATCH_FAIL, w->name, NULL); return 0; /* skip to next host */ } ``` The updated signature returns `void` — this control flow must move inside the restructured function. **The restructured `sched_meet_host_selection` must call `add_job_preason` (for each non-matching job) and handle the skip-to-next-host logic internally**, driving the inner per-job loop to completion before returning. The caller no longer inspects a return value; it simply calls the function and proceeds to the next host. There is no out-parameter or shared field to check after the call — the function owns all per-host decision logic. Before changing the signature: 1. Run `grep -rn 'sched_meet_host_selection' src/` to find all call sites. 2. Remove return-value checks at each caller. **Critical:** the `add_job_preason(schedobj->reason->reasons, PREASON_MATCH_FAIL, w->name, NULL)` call currently at `resource.c:369–374` must migrate into the inner per-job loop body inside the restructured `sched_meet_host_selection` — do not simply delete it. Removing the call without moving it would cause the scheduler to silently skip jobs with no pending-reason record, making those jobs appear to hang with no explanation. 3. Update the forward declaration at `src/scheduler/include/scheduler.h:111` (`extern int sched_meet_host_selection(...)`) to match the new `void` return. - **`build_host_eval_ctx`** — a **scheduler-private** function (lives in `src/scheduler/src/`, not in `lib/resreq.c`) that constructs a `host_eval_ctx_t` for each candidate host. It flattens `xce_sched_worker_t.resvals` (a `zarray_t` indexed by slot, where the slot index is the `res_idx`) into the dense `numvals`/`strvals` arrays expected by `eval_select`. `host_eval_ctx_t` itself is declared in `include/resreq.h` (accessible to both lib and scheduler); `build_host_eval_ctx` is declared only in a scheduler-local header. ```c /* Signature (scheduler-private, not in resreq.h): * host_eval_ctx_t* build_host_eval_ctx(const xce_sched_worker_t* worker, * int num_res); * num_res: total resource count from resmgr, used to size flat arrays. * * Field mapping for xce_val_t (src/include/xce/types.h): * slot index in resvals zarray → res_idx (NOT a field in xce_val_t) * v->which == Value_u_fvalue → numeric/bool: numvals[slot] = v->u.fvalue * v->which == Value_u_svalue → string: strvals[slot] = makeString(v->u.svalue) * v->which == Value_u_bvalue → bool as 0.0/1.0: numvals[slot] = (double)v->u.bvalue * (Current scheduler code in resource.c uses Value_u_fvalue for both numeric * and boolean resources. If a future scheduler change stores booleans with * Value_u_bvalue, this branch handles the conversion correctly.) * Header location for Value_u_fvalue / Value_u_bvalue / Value_u_svalue: * These enum constants are generated by capnp from src/schema/controller.capnp. * The generated header is controller.capnp.h (produced by the capnp build target * in src/lib/meson.build). Do NOT search types.h — they are not defined there. * In the scheduler build, confirm the generated header is on the include path via: * grep -rn 'Value_u_fvalue' src/scheduler/ * to locate the actual #include directive used by resource.c. * Skip NULL slots (zarray_del vacated them after resource removal). */ host_eval_ctx_t* build_host_eval_ctx(const xce_sched_worker_t* worker, int num_res) { /* 1. malloc sizeof(host_eval_ctx_t) */ /* 2. malloc numvals[num_res] (double) and strvals[num_res] (char*), * zero-initialize both. */ /* 3. Iterate worker->resvals over all caps slots (0..caps-1): * - slot_idx is the res_idx for the flat arrays. * - Skip NULL slots. * - v->which == Value_u_fvalue: numvals[slot_idx] = v->u.fvalue. * - v->which == Value_u_svalue: strvals[slot_idx] = makeString(v->u.svalue). * - v->which == Value_u_bvalue: numvals[slot_idx] = (double)v->u.bvalue. */ /* 4. Set ctx->num_res = num_res, ctx->numvals = numvals, ctx->strvals = strvals. */ /* 5. Return the populated struct. */ } ``` Pre-session sequencing: the size snapshot (`sel_node_next_id` read for eval context sizing, §8.5.3 step 1) must happen **after** all staging-phase `makeSelectNode` calls have completed, so that no node added during staging falls outside the sized eval context arrays. After construction, the allocator thread calls `select_eval_ctx_init()` once per session, then `select_eval_ctx_reset()` per host, evaluates each job via `eval_select()`, and calls `select_eval_ctx_fini()` at session end. **`host_eval_ctx_t` lifetime:** `host_eval_ctx_t` is constructed per candidate host by `build_host_eval_ctx` and freed after evaluation of that host completes. It is **not** reused across hosts — each host has different resource values, and the `makeString` calls in `strvals` increment the xstring refcount per host. When freeing `host_eval_ctx_t`, iterate `strvals[0..num_res-1]` and call `dereference(strvals[i])` for each non-NULL entry to release the xstring refcount acquired by `makeString()`, then `free(strvals)`, `free(numvals)`, and `free(ctx)`. This per-host lifecycle is distinct from `select_eval_ctx_t`, which is session-level and reused across hosts via `reset` (only `bool_bits` is cleared between hosts; `num_vals` and `str_vals` are overwritten before being read, so no clearing or `dereference` is needed between hosts). - **GC trigger:** In the pre-session phase, after staging merge and job cleanup (unlinking select roots for completed jobs), while both job-picker and allocator threads are idle (§8.4): **GC phase (§8.5.3 "Staging + GC phase"):** 1. Merge new jobs from staging buffer, parse their resreq strings (calling `makeSelectNode`), and unlink completed jobs. 2. `gc_sweep_select_nodes(live_jobs, njobs)` — reclaim unreachable nodes. Running GC after the staging merge ensures nodes referenced only by newly arrived jobs are marked live and retained. 3. `recompact_node_ids()` — reassign dense 0..N-1 IDs (§8.5.4). **Pre-session capacity check (§8.5.3 "Pre-session capacity check"):** 4. Check whether eval context arrays need to grow or shrink to accommodate the new `max_node_id` (§8.5.3 step 1). - **Add GC predicate fields to `xce_scheduler_t`** in `src/scheduler/include/scheduler.h`: add `_Atomic int session_active`, `_Atomic int eval_in_progress`, and `_Atomic int staging_merge_complete` to the struct. These back the predicate functions below; the struct declaration must be updated before the functions can reference them. Use `_Atomic int` (not `volatile int`) to match the `_Atomic uint32_t sel_node_next_id` pattern (§5.3) and to provide correct atomicity on ARM where `volatile` alone does not guarantee atomic reads/writes. - **GC predicate implementation:** The `sched_session_active`, `sched_eval_in_progress`, and `staging_merge_complete` predicates (§5.5) wrap the new fields above added to `xce_scheduler_t` in Phase 6. Lifecycle of each flag (all use `_Atomic int` — use `atomic_store_explicit(..., memory_order_release)` for writes and `atomic_load_explicit(..., memory_order_acquire)` for reads): - `session_active`: set to 1 in `sched_begin_session`; cleared to 0 in `sched_end_session`. - `eval_in_progress`: set to 1 immediately before the per-host eval loop begins; cleared to 0 after the loop completes. - `staging_merge_complete`: set to 1 after staging merge finishes **and after all completed-job root-unlink writes have been committed**; cleared to 0 at the top of the next iteration's staging phase, *before* any new `makeSelectNode` call. The release/acquire pair on this flag provides the cross-thread visibility that the root-unlink stores (`primary->select->root = NULL`) are observable to the GC guard check on the other side. The `atomic_thread_fence(memory_order_seq_cst)` required by §8.5.3 ensures all root-unlink stores are visible before GC reads them; the `staging_merge_complete` release write then propagates that guarantee to any thread reading the flag. **Clearing timing:** clear `staging_merge_complete` *after* `gc_sweep_select_nodes` returns and *before* the next staging phase begins — never clear it while GC is still running. These flags enforce the GC idle invariant with assertions; the scheduler's serial main loop already guarantees the invariants structurally. - **`sel_optimize_level == 0` in the scheduler:** The scheduler is always expected to run with `sel_optimize_set(1)` (set at startup before any job parsing). Phase 6 wiring code must assert this at the start of the session loop: ```c if (sel_get_optimize_level() <= 0) { log_fatal("scheduler must run with CSE enabled; set sel_optimize_level=1 at startup"); return; } ``` If `sel_optimize_level == 0` were reached (e.g., a mis-configured test build), the eval context would not be initialized and `eval_select` would access uninitialized `node_id` values. This guard uses `log_fatal` + `return` rather than `assert()` because §5.5 explicitly requires runtime guards for safety-critical invariants — `assert()` is compiled out with `NDEBUG` and must not be relied upon here. - **live_jobs materialization:** `gc_sweep_select_nodes` takes a flat `const xce_sched_job_t**` array, but the scheduler stores jobs in `sched->jobs` as a `var`/kvmgr keyed by jobid. Phase 6 must materialize this array using a plain heap-allocated C array to avoid the fragile cast assumption of `(const xce_sched_job_t**)zarray_t.data`: ```c size_t live_cap = 64, n = 0; xce_sched_job_t** live_arr = malloc(live_cap * sizeof(*live_arr)); if (live_arr == NULL) { log_error("OOM: live_arr malloc failed"); return; } iterator_t it; int irc = iter_make(&it, sched->jobs); if (irc < 0) { free(live_arr); log_error("iter_make failed"); return; } xce_sched_job_t* job; for (;;) { irc = iter_next(&it, (void**)&job); if (irc < 0 || irc == RTTI_ITER_NO_DATA) break; if (n == live_cap) { live_cap *= 2; xce_sched_job_t** tmp = realloc(live_arr, live_cap * sizeof(*live_arr)); if (tmp == NULL) { free(live_arr); log_error("OOM: live_arr realloc failed"); return; } live_arr = tmp; } live_arr[n++] = job; } gc_sweep_select_nodes(live_arr, n); free(live_arr); ``` The kvmgr iteration uses the `iter_make` / `iter_next` pattern — the same pattern used at `src/scheduler/src/scheduler.c:709`. `kvmgr_first`/`kvmgr_next` do not exist in this codebase. **Phase 6 integration test plan:** Phase 6 is the highest-risk phase — it restructures the live scheduler loop. The following regression tests must pass before Phase 6 is merged: 1. **Scheduler smoke test:** Submit 5 jobs with distinct select expressions (`type==linux`, `gpu`, `mem > 100`, `type==linux && gpu`, no select clause). Verify all jobs reach RUNNING or PENDING state without scheduler crash. Run with TSan enabled. 2. **CSE deduplication test:** Submit 1000 jobs with the same select expression. Verify `sel_node_next_id` equals the expected number of unique nodes (not 1000×nodes_per_job) after staging. 3. **GC correctness test:** Submit 500 jobs, complete 250, trigger a GC cycle. Verify the remaining 250 jobs still evaluate correctly on a subsequent session (no use-after-free, no missing preason records). 4. **Thread-safety test:** Run the scheduler under TSan for at least one complete session with > 1000 jobs. No data races permitted. 5. **`sched_meet_host_selection` parity test:** For each of 10 test jobs with known expected host-match results, verify that Phase 6 produces the same match/no-match outcomes as the unmodified Phase 5 baseline. This catches any `add_job_preason` migration regression. These tests must be added to the scheduler's own testsuite (`src/scheduler/testsuite/` or equivalent) and wired into the meson build under the scheduler test target — **not** `src/frontend/Makefile`, which is the Go API server test harness. Phase 6 cannot be submitted for review until all five tests pass under TSan. --- ## 12. Appendix: Data Structures ### Complete `xce_select_node_t` ```c typedef enum { RES_TYPE_NUMERIC = 0, RES_TYPE_STRING, RES_TYPE_BOOL } res_type_t; typedef enum { SEL_AND, SEL_OR, SEL_NOT, SEL_GT, SEL_LT, SEL_GE, SEL_LE, SEL_EQ, SEL_NE, SEL_RESNAME, SEL_NUM, SEL_STR, SEL_ANY, SEL_TRUE, SEL_FALSE, } sel_node_type_t; typedef struct xce_select_node_ xce_select_node_t; struct xce_select_node_ { sel_node_type_t type; uint32_t hashcode; uint32_t node_id; /* dense index into eval context arrays (§8.5) */ unsigned gc_mark; /* set during mark-and-sweep GC (§5.5) */ union { struct { xce_select_node_t *left, *right; } binop; xce_select_node_t* operand; struct { int res_idx; res_type_t res_type; } resname; double numval; char* strval; } u; }; ``` ### CSE Key ```c typedef struct { sel_node_type_t type; uintptr_t op1; uintptr_t op2; } sel_node_key_t; ``` ### Typed Eval Value ```c typedef struct { res_type_t type; union { double num; const char* str; int bval; } u; } sel_val_t; ``` ### Host Evaluation Context ```c typedef struct { int num_res; /* total resource count from resmgr — used for bounds checks */ const double* numvals; /* numeric (and boolean as 0.0/1.0) resource values, dense array indexed by res_idx; malloc'd by scheduler */ const char** strvals; /* string resource values, dense array indexed by res_idx; each pointer is makeString()-interned; malloc'd by scheduler */ } host_eval_ctx_t; ``` Note: `numvals` is a plain `double*` array indexed by `res_idx`, **not** `xvec_idval_t*`. `xvec_idval_t` is used for per-job resource allocation data elsewhere in the scheduler; it is not used for host resource snapshots passed to `eval_select`. ### Select Evaluation Context (per-host, §8.5) ```c typedef struct { const host_eval_ctx_t* host; /* host resource snapshot */ uint8_t* bool_bits; /* bitmap, 2 bits per node (eval result cache) */ uint32_t bool_bits_cap; /* capacity in nodes */ uint32_t node_count; /* session-start snapshot of sel_node_next_id; * select_eval_ctx_reset clears * (node_count * 2 + 7) / 8 bytes of bool_bits — * must NOT re-read sel_node_next_id (see §8.5.3) */ double* num_vals; /* numeric leaf cache by node_id */ uint32_t num_vals_cap; const char** str_vals; /* string leaf cache by node_id */ uint32_t str_vals_cap; } select_eval_ctx_t; /* Initialize eval context for a session (sized to max_node_id + 1). * NOT thread-safe — allocator thread processes one host at a time. * Reused across all hosts within a session; only bool_bits is cleared between hosts. * * Integer overflow guard: if max_node_id == UINT32_MAX, adding 1 wraps to 0 * and malloc(0) succeeds but returns a minimal allocation — subsequent array * accesses at index 0..max_node_id are then out-of-bounds. The implementation * must reject this before computing n = max_node_id + 1: * if (max_node_id == UINT32_MAX) { log_fatal(...); return NULL; } * In practice, SEL_MAX_INTERNED_NODES (1,000,000) << UINT32_MAX, so this * branch is unreachable under correct operation, but the guard is required * as a belt-and-suspenders defence against counter corruption. */ select_eval_ctx_t* select_eval_ctx_init(uint32_t max_node_id); /* Reset eval context for the next host (clears bool_bits bitmap only; * num_vals and str_vals are overwritten before being read by comparators). * str_vals entries are non-owning pointers — do NOT call dereference() * on them. The xstring refcount is held by host_eval_ctx_t.strvals, * which outlives this eval pass (§8.5.2 refcount semantics). */ void select_eval_ctx_reset(select_eval_ctx_t* ctx, const host_eval_ctx_t* host); /* Free eval context at session end. * Frees the owned arrays (bool_bits, num_vals, str_vals) via free(). * str_vals is freed as a raw array — no dereference() calls on entries * because they are non-owning (§8.5.2). Does not dereference ctx->host. */ void select_eval_ctx_fini(select_eval_ctx_t* ctx); ``` ### Select Parse Context ```c typedef struct { const char* input; int pos; int err_pos; int has_error; char errmsg[256]; /* fixed-size, no alloc, no free */ int depth; /* current nesting depth, checked against SELECT_MAX_DEPTH */ var resmgr; /* name → (res_idx, res_type) */ } select_context_t; ``` ### Helper: `operand_is_numeric` ```c static int operand_is_numeric(const xce_select_node_t* node) { return node->type == SEL_NUM || (node->type == SEL_RESNAME && node->u.resname.res_type == RES_TYPE_NUMERIC); } ``` ### CSE Key Operations The `sel_node_table` uses `kvtab_type` keyed by `sel_node_key_t`. A custom `kv_key_ops_t` is required since `sel_node_key_t` is a struct with potential padding between `type` (int, 4 bytes) and `op1`/`op2` (uintptr_t, 8 bytes each) — `sel_key_hash` and `sel_key_cmp` MUST operate on the three fields individually. Using `memcmp` or `memhash` on the entire struct would include undefined padding bytes and produce incorrect or non-deterministic results: Always construct `sel_node_key_t` values through `sel_make_key()` to guarantee padding bytes are zeroed before assignment: ```c static inline sel_node_key_t sel_make_key(sel_node_type_t type, uintptr_t op1, uintptr_t op2) { sel_node_key_t k; memset(&k, 0, sizeof k); k.type = type; k.op1 = op1; /* SEL_RESNAME: res_idx (op1) uniquely determines res_type (op2), so zero * op2 in the key — two nodes with the same res_idx are always the same * resource regardless of the res_type value passed by the caller. * makeSelectNode stores the original op2 (res_type) in node->u.resname.res_type * BEFORE calling sel_make_key, so the fill data is not lost. */ k.op2 = (type == SEL_RESNAME) ? 0 : op2; return k; } ``` ```c static int sel_key_cmp(const void* a, const void* b) { const sel_node_key_t* ka = (const sel_node_key_t*)a; const sel_node_key_t* kb = (const sel_node_key_t*)b; /* Compare fields individually — do NOT memcmp the whole struct. */ if (ka->type != kb->type) return ka->type < kb->type ? -1 : 1; if (ka->op1 != kb->op1) return ka->op1 < kb->op1 ? -1 : 1; if (ka->op2 != kb->op2) return ka->op2 < kb->op2 ? -1 : 1; return 0; } static kv_key_ops_t sel_node_key_ops = { .hash = sel_key_hash, .cmp = sel_key_cmp, .assign = sel_key_assign, /* deep-copy: malloc + memcpy of all bytes */ .destroy = sel_key_destroy, /* free */ }; ``` The `assign` and `destroy` callbacks handle struct-sized keys through the kvtab's `void**`/`void*` interface. Since `sel_make_key()` zeroes padding before field assignment, `memcpy` of the full struct is safe for `assign`: ```c /* Deep-copy a sel_node_key_t for kvtab storage. * The kvtab assign callback receives (void** dst, void* src). * Allocate a new sel_node_key_t and copy all bytes — this is safe * because sel_make_key() zeros padding before field assignment. */ static int sel_key_assign(void** dst, void* src) { sel_node_key_t* copy = malloc(sizeof(sel_node_key_t)); if (copy == NULL) return -ENOMEM; memcpy(copy, src, sizeof(sel_node_key_t)); *dst = copy; return 1; /* success — matches assign_string_key / assign_int_key convention in kv.c */ } /* Free a kvtab-stored sel_node_key_t. */ static void sel_key_destroy(void* key) { free(key); } ``` The same field-by-field rule applies to `sel_key_hash`: do NOT hash the struct as raw bytes (undefined padding bytes would produce non-deterministic hash values across builds/platforms). Instead, hash the three fields individually: ```c /* Example: FNV-style field-by-field hash for sel_node_key_t */ static uint32_t sel_key_hash(const void* key) { const sel_node_key_t* k = (const sel_node_key_t*)key; uint32_t h = 2166136261u; h ^= (uint32_t)k->type; h *= 16777619u; h ^= (uint32_t)(k->op1 & 0xffffffffu); h *= 16777619u; #if UINTPTR_MAX > 0xffffffffu h ^= (uint32_t)(k->op1 >> 32); h *= 16777619u; #endif h ^= (uint32_t)(k->op2 & 0xffffffffu); h *= 16777619u; #if UINTPTR_MAX > 0xffffffffu h ^= (uint32_t)(k->op2 >> 32); h *= 16777619u; #endif return h; } ``` These must be defined in `resreq.c` alongside the intern table. ### GC Key Reconstruction Helper `gc_sweep_select_nodes` calls `key_del` with the original CSE key for each swept node. The key must be reconstructed from the node's fields with the **same commutativity sort** applied in `makeSelectNode` step 1 — otherwise the reconstructed key hashes to a different bucket and `key_del` silently no-ops, leaking the kvtab entry: ```c /* Reconstruct the canonicalized CSE key for an interned node. * Used by GC sweep to call key_del with the correct sorted key. * Must match the sort applied in makeSelectNode step 1. */ static inline sel_node_key_t sel_node_to_key(const xce_select_node_t* node) { uintptr_t op1 = 0, op2 = 0; switch (node->type) { case SEL_AND: case SEL_OR: case SEL_EQ: case SEL_NE: { /* commutative: op1 = min(left, right), op2 = max(left, right) */ uintptr_t a = (uintptr_t)node->u.binop.left; uintptr_t b = (uintptr_t)node->u.binop.right; op1 = (a < b) ? a : b; op2 = (a < b) ? b : a; break; } case SEL_GT: case SEL_LT: case SEL_GE: case SEL_LE: op1 = (uintptr_t)node->u.binop.left; op2 = (uintptr_t)node->u.binop.right; break; case SEL_NOT: op1 = (uintptr_t)node->u.operand; break; case SEL_RESNAME: op1 = (uintptr_t)node->u.resname.res_idx; break; case SEL_NUM: { /* Bit-cast double → uintptr_t via memcpy (avoids strict-aliasing UB). * A static_assert(sizeof(uintptr_t) == sizeof(double)) must appear * before this function — equality (not >=) is required so that * sizeof(op1) copies all 8 bytes of the double without truncation. * On 32-bit builds where the sizes differ, the assert fires at * compile time rather than silently producing hash collisions. */ memcpy(&op1, &node->u.numval, sizeof(op1)); break; } case SEL_STR: op1 = (uintptr_t)node->u.strval; break; default: /* SEL_ANY, SEL_TRUE, SEL_FALSE: op1=op2=0 */ break; } return sel_make_key(node->type, op1, op2); } ``` ### Host Resource Value Accessors `eval_select` calls accessor functions that use the `host_eval_ctx_t` context (not `xvec_idval_t` directly). `xvec_idval_t` stores per-job resource allocation data; host resource values are retrieved through the evaluation context's `numvals` and `strvals` members: ```c /* Retrieve typed value from host evaluation context. * Precondition: 0 <= res_idx < host->num_res (checked in debug builds). */ static sel_val_t host_resval_typed(const host_eval_ctx_t* host, int res_idx, res_type_t res_type); /* Boolean resource: nonzero → true */ static int host_get_bool(const host_eval_ctx_t* host, int res_idx) { if (!host || res_idx < 0 || res_idx >= host->num_res) { log_fatal("host_get_bool: invalid host=%p res_idx=%d", (void*)host, res_idx); return 0; } return host->numvals[res_idx] != 0.0; } /* Numeric resource: return float value */ static double host_get_float(const host_eval_ctx_t* host, int res_idx) { if (!host || res_idx < 0 || res_idx >= host->num_res) { log_fatal("host_get_float: invalid host=%p res_idx=%d", (void*)host, res_idx); return 0.0; } return host->numvals[res_idx]; } /* String resource: return interned string pointer. * Returns NULL if the resource has no value for this host (absent resources are * zero-initialized in build_host_eval_ctx). Callers MUST check for NULL before * passing the pointer to xce_str_match; absent string resources mean "no value", * not an empty string — treat a NULL return as a filter-out (non-match). */ static const char* host_get_string(const host_eval_ctx_t* host, int res_idx) { if (!host || res_idx < 0 || res_idx >= host->num_res) { log_fatal("host_get_string: invalid host=%p res_idx=%d", (void*)host, res_idx); return NULL; } return host->strvals[res_idx]; /* may be NULL for absent resources */ } ``` **String resource storage:** `xvec_idval_t` stores per-job resource allocation data (not host values). The `host_eval_ctx_t` bundles host resource data: `numvals` provides numeric and boolean resource values indexed by `res_idx`, and `strvals` provides string resource values indexed by `res_idx`. When host data is loaded into the scheduler, both the numeric vector and the string mapping are populated from the cluster configuration. `libxce` already employs a de-duplication mechanism that maps identical strings to the same pointer (via `makeString`), so `host_get_string` returns an interned pointer — `xce_str_match` (which starts as `strcmp`) can be replaced with pointer equality when both operands are known-interned. ### PEG Grammar Preamble The existing PEG parsers use `%value "char *"`. The select parser produces `xce_select_node_t*` values. The packcc preamble: ```c %prefix "xce_select" %value "char *" /* packcc returns char*; actions cast */ %auxil "select_context_t *" ``` PEG actions cast node pointers through `char*` return values: ```c $$ = (char*)makeSelectNode(SEL_AND, (uintptr_t)lnode, (uintptr_t)rnode); ``` **`parse_select` input length guard (security requirement):** The `strlen` check against `SELECT_MAX_INPUT_LEN` must occur in `parse_select` **before** constructing the `select_context_t` or calling `xce_select_parse`. Placing the check inside the PEG grammar (as a grammar rule or action) is not sufficient — a malicious caller can pass a string that causes the PEG engine to allocate a packet-cache proportional to the input length before the grammar can reject it. The correct structure is: ```c int parse_select(const char* input, ...) { if (strlen(input) > SELECT_MAX_INPUT_LEN) { snprintf(errmsg, ERRMSG_MAX, "select expression exceeds maximum length (%zu > %d bytes)", strlen(input), SELECT_MAX_INPUT_LEN); return -1; /* reject before PEG context is allocated */ } select_context_t ctx = { .input = input, .resmgr = resmgr, ... }; /* ... newlocale guard, then PEG call ... */ } ``` The `parse_select` function unwraps the final return: ```c xce_select_node_t* root = (xce_select_node_t*)xce_select_parse(ctx, input, ctx); ```