From 33b76d9a7775c1e30d9138c3f0787a183b8e125e Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Mon, 20 Apr 2026 09:43:07 +0000 Subject: [PATCH] docs: add select expression AST with CSE/hash-consing design Design document for implementing the select clause parser with: - PEG grammar with LSF-compatible resname/string disambiguation - Hash-consing CSE via intern table with TOCTOU-safe insertion - Mark-and-sweep GC between scheduling sessions - Explicit error propagation in eval_select (-1/0/1 tri-state) - Parse-time type coercion for EQ/NE (e.g., type==5 converts 5 to "5") - AST transform pipeline: commutativity canonicalization, associativity flattening, const-fold (type==any -> SEL_TRUE) - Special keywords: any, SEL_TRUE, SEL_FALSE - String host resource accessor (parallel res_idx->char* lookup) - res_type_t <-> XCE_VAL_* bidirectional mapping - Single-goroutine CGO bridge invariant --- src/docs/resreq-ast-cse-design.md | 1022 +++++++++++++++++++++++++++++ 1 file changed, 1022 insertions(+) create mode 100644 src/docs/resreq-ast-cse-design.md diff --git a/src/docs/resreq-ast-cse-design.md b/src/docs/resreq-ast-cse-design.md new file mode 100644 index 00000000..cf1fd653 --- /dev/null +++ b/src/docs/resreq-ast-cse-design.md @@ -0,0 +1,1022 @@ +# XCE Resource Requirement AST & CSE Design + +> **Status**: Draft +> **Last Updated**: 2026-04-19 +> **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) +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 +(`selectString`, `rusageString`, `spanString`, …). `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`. + +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==true`). 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. +- 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.6). + +--- + +## 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. + +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[...] rusage[...] span[...] same[...] affinity[...] +``` + +--- + +## 3. Select Expression AST + +### 3.1 Grammar (new `lib/xce_select.peg`) + +``` +select_expr ::= or_expr EOF +or_expr ::= and_expr ( '||' and_expr )* +and_expr ::= not_expr ( '&&' not_expr )* +not_expr ::= '!' atom_expr | atom_expr +atom_expr ::= '(' or_expr ')' | comparison | resname +comparison ::= resname cmp_op value +cmp_op ::= '>=' | '<=' | '!=' | '==' | '>' | '<' +resname ::= [a-zA-Z_][a-zA-Z_0-9]* +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') +``` + +**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). + +**`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. + +### 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 (const-folded to SEL_TRUE) */ + SEL_TRUE, /* leaf: constant true (from const fold) */ + SEL_FALSE, /* leaf: constant false (from const fold) */ +} sel_node_type_t; +``` + +### 3.3 Node Structure + +```c +struct xce_select_node_ { + sel_node_type_t type; + atomic_ulong refcount; /* intern table reference count */ + uint32_t hashcode; /* cached at construction */ + unsigned gc_mark; /* set during mark-and-sweep GC (§5.6) */ + 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 | +| `==` `!=` | ✓ | ✓ | ✓ | +| standalone (no op) | ✓ (non-zero) | parse error | ✓ | + +The `res_type` field is stored in the `SEL_RESNAME` node for eval-time +dispatch. It is **not** part of the CSE hash key — `res_idx` alone +identifies the resource uniquely, and a resource has exactly one type. + +**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 */ +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]; +} + +/* existing valtype → res_type_t inverse mapping */ +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: return RES_TYPE_NUMERIC; /* safe default */ + } +} +``` + +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. + +**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.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. + +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. + +### 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` | +| `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` | + +`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. + +### 5.3 Intern Table + +```c +static struct { + mylock_t lock; + var table; /* kvtab keyed by sel_node_key_t */ +} sel_node_table; +``` + +Modelled on `xstring_table` in `lib/xstring.c`. + +### 5.4 `makeSelectNode` + +```c +xce_select_node_t* makeSelectNode(sel_node_type_t type, + uintptr_t op1, uintptr_t op2) +{ + /* 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 */ + sel_node_key_t 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) reference(node); + mylock_unlock(&sel_node_table.lock); + if (rc > 0) return node; /* cache hit */ + if (rc < 0) return NULL; /* key_find error (not ENOENT) */ + + /* 4. alloc and fill */ + node = calloc(1, sizeof(*node)); + if (node == NULL) { errno = ENOMEM; return NULL; } + node->type = type; + node->hashcode = sel_key_hash(&key); + atomic_store(&node->refcount, 1); + /* fill u.* from op1/op2 by type … */ + + /* 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 */ + reference(existing); + mylock_unlock(&sel_node_table.lock); + free(node); + return existing; + } + key_add(sel_node_table.table, &key, node); + if (rc2 < 0) { + /* key_add failed (EEXIST check already passed, likely ENOMEM) */ + mylock_unlock(&sel_node_table.lock); + free(node); + 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). + +```c +void derefSelectNode(xce_select_node_t* node) +{ + if (node == NULL) return; + atomic_fetch_sub(&node->refcount, 1); + /* Reclamation is handled by mark-and-sweep GC (§5.6). + When a job completes, kill_primary_resreq calls derefSelectNode + on the root to decrement refcounts. The GC sweep between + sessions frees nodes that are no longer reachable from any + live job's resreq root. */ +} +``` + +`kill_primary_resreq()` in `resreq.c` must call +`derefSelectNode(primary->select->root)` before freeing the select struct. + +### 5.6 Mark-and-Sweep GC Between Scheduling Sessions + +`derefSelectNode` only decrements refcounts — it does **not** free nodes +or remove them from the intern table. Reclamation is handled by a +**mark-and-sweep** GC that runs between scheduling sessions, when no +evaluation is in progress: + +``` +gc_sweep_select_nodes() + 1. Mark pass: + a. Clear the GC mark flag on every node in sel_node_table. + b. For each live job's resreq, visit the AST tree starting from + the root node and set the GC mark flag on every reachable node. + (If a node is referenced by multiple AST trees, it is marked + only once — the flag is boolean.) + 2. Sweep pass: + a. Iterate sel_node_table. + b. For each node whose GC mark flag is NOT set: + - remove the node from the intern table (key_del) + - free the node + c. Clear all GC mark flags. +``` + +This approach is simple and correct: + +- 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 — the + mark flag is a single boolean per node. +- `derefSelectNode` is removed — when a job completes, its resreq entry + is simply unlinked from the job; the next GC sweep reclaims unreachable + nodes. +- The sweep runs only between sessions (no concurrent eval), so no + read-after-free risk exists. +- `SEL_STR` nodes' `xstring` references are handled by the same mark + pass — if the SEL_STR node is reachable, the string stays interned. + +**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`. + +### 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). The `any` keyword (§3.1) is only valid on the + right-hand side of `==` or `!=`. For `!=`, `resname != any` is a + parse error (nothing would not match any). +- `SEL_TRUE && X` → `X`, `X && SEL_TRUE` → `X` (identity elimination) +- `SEL_FALSE || X` → `X`, `X || SEL_FALSE` → `X` (identity elimination) +- `!SEL_TRUE` → `SEL_FALSE`, `!SEL_FALSE` → `SEL_TRUE` + +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) +2. Commutativity canonicalization (operand swap, §6) +3. Associativity flattening (§6.2) +4. Const-fold (§6.3) +5. CSE intern (makeSelectNode for each node bottom-up) +``` + +Steps 2–4 operate on the tree before interning (they construct new +nodes via `makeSelectNode` which itself does the commutativity swap). +Step 5 re-interns any modified subtrees, ensuring CSE maximizes sharing +on the fully normalized tree. + +--- + +## 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 +typedef struct { + const char* input; + int pos; + int err_pos; + int has_error; + char* errmsg; + var resmgr; /* name → (res_idx, res_type) */ +} select_context_t; +``` + +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 `RES_TYPE_STRING`, `"linux"` is already + `SEL_STR` — 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 == true` — `gpu` is `RES_TYPE_BOOL`, `true` remains as a boolean + value. + +This coercion guarantees that at eval time, both operands of `EQ`/`NE` +always have matching types — the `switch (lv.type)` dispatch in +`eval_select` never reads the wrong union member. + +```c +/* action for '==' '!=' — coerce literal to resource type */ +if (lnode->type == SEL_RESNAME && rnode->type == SEL_NUM + && lnode->u.resname.res_type == RES_TYPE_STRING) { + /* convert numeric literal to string: 5 → "5" */ + char buf[64]; + snprintf(buf, sizeof(buf), "%g", rnode->u.numval); + rnode = makeSelectNode(SEL_STR, (uintptr_t)makeString(buf), 0); +} +/* 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), "%g", lnode->u.numval); + lnode = makeSelectNode(SEL_STR, (uintptr_t)makeString(buf), 0); +} +$$ = (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). + +The **resmgr** (resource manager) is simply an array of resource +definitions — each entry maps a resource name to its type and other +metadata. The API server has access to this information via the cluster +configuration and can pass it to the parser at job submission time. + +--- + +## 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, + const xvec_idval_t* host_resvals) +{ + switch (node->type) { + case SEL_RESNAME: + return host_resval_typed(host_resvals, + 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. If anything +goes wrong during the visit, `-1` is returned immediately and propagated +up — `&&`, `||`, and `!` never silently swallow errors. + +```c +/* Returns: 1 = host matches, 0 = host does not match, -1 = error */ +int eval_select(const xce_select_node_t* node, const xvec_idval_t* host_resvals) +{ + int lv, rv; + + switch (node->type) { + /* logical — explicit error propagation, not C boolean operators */ + case SEL_AND: + lv = eval_select(node->u.binop.left, host_resvals); + if (lv < 0) return -1; + if (lv == 0) return 0; + rv = eval_select(node->u.binop.right, host_resvals); + if (rv < 0) return -1; + return rv; + + case SEL_OR: + lv = eval_select(node->u.binop.left, host_resvals); + if (lv < 0) return -1; + if (lv == 1) return 1; + rv = eval_select(node->u.binop.right, host_resvals); + if (rv < 0) return -1; + return rv; + + case SEL_NOT: + lv = eval_select(node->u.operand, host_resvals); + if (lv < 0) return -1; + return !lv; + + /* SEL_TRUE: const-folded from expressions like `type == any` */ + case SEL_TRUE: + return 1; + + /* 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_resvals, node->u.resname.res_idx); + case RES_TYPE_NUMERIC: return host_get_float(host_resvals, node->u.resname.res_idx) != 0.0f; + default: return -1; /* string standalone rejected at parse time */ + } + + /* numeric comparisons — operand types validated at parse time */ + case SEL_GT: case SEL_LT: case SEL_GE: case SEL_LE: { + double l = get_leaf_value(node->u.binop.left, host_resvals).u.num; + double r = get_leaf_value(node->u.binop.right, host_resvals).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 — type guaranteed compatible at parse time */ + case SEL_EQ: case SEL_NE: { + sel_val_t l = get_leaf_value(node->u.binop.left, host_resvals); + sel_val_t r = get_leaf_value(node->u.binop.right, host_resvals); + int eq; + switch (l.type) { + case RES_TYPE_NUMERIC: eq = l.u.num == r.u.num; break; + case RES_TYPE_STRING: eq = xce_str_match(l.u.str, r.u.str); break; + case RES_TYPE_BOOL: eq = l.u.bval == r.u.bval; break; + default: return -1; + } + return (node->type == SEL_EQ) ? eq : !eq; + } + + default: 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. + +### 8.4 Thread Safety + +- The intern table is protected by `sel_node_table.lock` for all + insert/remove operations. +- Once a node is in the table, its content is **read-only**. Concurrent + `eval_select` calls on the same node require no locking. +- **The Go CGO bridge must call `eval_select` from a single, pinned + goroutine** (the scheduler goroutine). `eval_select` accesses interned + nodes without holding references — if a concurrent goroutine triggered + GC sweep while another was evaluating, a use-after-free would occur. + The mark-and-sweep GC (§5.6) runs only between sessions when the + scheduler goroutine is not evaluating, so single-goroutine eval + guarantees safety. If concurrent evaluation is needed in the future, + an eval-time reference count (increment before eval, decrement after) + must be added. + +--- + +## 9. Job-Level Sharing + +Two jobs share the same `xce_resreq_t*` pointer if and only if they were +submitted with the same user, same queue, same project, and the same resreq +string. The sharing mechanism is a second intern table at the resreq level, +modelled on `xstring.c`: + +``` +makeResreq(canonical_resreq_str) + → lookup in resreq_intern_table + → cache hit: addref, return existing xce_resreq_t* + → cache miss: parse_resreq() → intern each select node (CSE) → insert → return +``` + +Because `makeSelectNode` interns individual AST nodes, jobs that share only a +**subset** of their resreq (e.g., same `select[...]` but different `rusage[...]`) +still share the common select subtree. The sharing is structural, not just +per-string. + +The resreq intern table key is the **canonical resreq string** (raw, after +whitespace normalization). A future enhancement could normalize clause order +(e.g., always `select[...]` before `rusage[...]`) to improve hit rates. + +--- + +## 10. Scheduling Session Semantics + +Within a single scheduling session: + +``` +for each pending_job: + for each candidate_host: + 1. rc = eval_select(job.resreq.primary.select, host.resvals) + → 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: + job remains unsatisfied, re-evaluated next session +``` + +**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 is stateless**: host properties (type, architecture, boolean + features) do not change during a session. `eval_select` on the same + `(select_node, host)` pair always returns the same result within a session. +- **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. + +--- + +## 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). + - CSE: `A && B` and `B && A` produce the same `xce_select_node_t*`. + - CSE: two calls to `makeSelectNode` with same args return same pointer, + refcount increases. + - Eval: numeric `>`, `<`, `==`; string `==`; bool standalone; `!`, `&&`, + `||`; short-circuit evaluation. + +*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.* + +#### 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`, `derefSelectNode`, `eval_select`, + `sel_node_table_init`, `sel_node_table_fini`. + - Add `res_type_to_valtype` / `valtype_to_res_type` mapping helpers. + - Add `parse_resreq_primary_with_resmgr(primary, resreqString, errmsg, resmgr)` + as a new entry point. The existing `parse_resreq_primary` calls it + with `resmgr=NULL` (no previous release, so backward compatibility is + not a concern — the existing function can be updated directly if + preferred). + +#### 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. + +#### Phase 4: Implementation + +- `lib/resreq.c` + - `sel_node_table` static with `mylock_t`. + - `sel_key_ops_t` for `sel_node_key_t` (hash, cmp, assign, destroy). + - `sel_node_table_init(int nslots)` / `sel_node_table_fini()`. + - `makeSelectNode` (§5.4) — with key_add return code check for TOCTOU + safety. + - `derefSelectNode` (§5.5). + - `sel_key_of`, `sel_key_hash` helpers. + - `eval_select` (§8.3) — explicit error propagation (no C boolean ops). + - `host_resval_typed`, `host_get_bool`, `host_get_float`, + `host_get_string` accessors. + - Mark-and-sweep GC: `gc_sweep_select_nodes()`, `gc_mark_tree()`. + - AST transform pipeline: flatten, const-fold (§6.2–6.4). + - `xce_str_match` wrapper (initially `strcmp` semantics). + - Update `kill_primary_resreq` to call `derefSelectNode`. + - Update `parse_resreq_primary` to call `parse_select` with resmgr. + +#### Phase 5: Build wiring + +- `lib/meson.build` — add `xce_select.peg` to the source list alongside + `xce_resreq.peg`, `section.peg`, `rusage.peg`. + +### Go Integration (Phase 6) + +#### Phase 6: Go bridge + +- `backend/clib.go` — replace `EvalSelectForHost` stub with real CGO call. + Must marshal host resource values (`xvec_idval_t*`) from Go to C. +- `controllers/hosts.go` — wire `FilterHostByResReqSelect` into + `preCheckHostRequest`. +- **CGO concurrency:** `FilterHostByResReqSelect` must be called from a + single, pinned scheduler goroutine (§8.4). This is a hard invariant: + `eval_select` accesses interned nodes without holding references, so + concurrent evaluation while GC sweep runs would cause use-after-free. + If concurrent scheduling is needed in the future, eval-time reference + counting (increment before eval, decrement after) must be added to the + C API. + +--- + +## 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; + atomic_ulong refcount; + uint32_t hashcode; + unsigned gc_mark; /* set during mark-and-sweep GC (§5.6) */ + 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; +``` + +### Select Parse Context + +```c +typedef struct { + const char* input; + int pos; + int err_pos; + int has_error; + char* errmsg; + 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 packed +struct, not a string or int: + +```c +static kv_key_ops_t sel_node_key_ops = { + .hash = sel_key_hash, + .cmp = sel_key_cmp, + .assign = sel_key_assign, /* malloc + memcpy */ + .destroy = sel_key_destroy, /* free */ +}; +``` + +These must be defined in `resreq.c` alongside the intern table. + +### Host Resource Value Accessors + +`eval_select` calls three accessor functions that must be defined: + +```c +/* Retrieve typed value from host resource vector */ +static sel_val_t host_resval_typed(const xvec_idval_t* host_resvals, + int res_idx, res_type_t res_type); + +/* Boolean resource: nonzero → true */ +static int host_get_bool(const xvec_idval_t* host_resvals, int res_idx); + +/* Numeric resource: return float value */ +static double host_get_float(const xvec_idval_t* host_resvals, int res_idx); + +/* String resource: return interned string pointer */ +static const char* host_get_string(const xvec_idval_t* host_resvals, + int res_idx); +``` + +**String resource storage:** `xvec_idval_t` stores only `(idx, float)` +pairs for numeric resources. String host resources (e.g., `type`, +`arch`) are stored in a parallel lookup structure alongside each host's +`xvec_idval_t`. When host data is loaded into the scheduler, both the +numeric `xvec_idval_t` vector and a `(res_idx → const char*)` 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); +``` + +The `parse_select` function unwraps the final return: + +```c +xce_select_node_t* root = (xce_select_node_t*)xce_select_parse(ctx, input, ctx); +```