refactor: Clean up DSL#270
Conversation
There was a problem hiding this comment.
Pull request overview
This PR consolidates duplicated per-KernelRole fan-outs across the four DSL backends (JIT, AoT, WASM, native) by introducing a shared NativeKernels table on NativeExecutionArtifact, reusing CompiledKernelAvailability in the WASM backend, and routing role lookups through helper functions. It also dedupes two small numeric-label helpers in the DSL frontend by moving them into name_match. The behavior is intended to be unchanged; only internal constructor signatures (from_jit_module, from_library, WasmKernelSession::new) are tightened.
Changes:
- Introduce
NativeKernels(withget/has/Debug) and re-routeNativeExecutionArtifactand itsKernelSession/RuntimeArtifactimpls through it; update JIT/AoT call sites to construct it. - Replace
WasmKernelAvailabilitywith the existingCompiledKernelAvailability, factor amaybe_typed_funchelper, and promotekernel_output_lento a sharedpub(super)helper incompiled_backend_abi. - Move
bare_numeric_label/canonical_numeric_suffixfromauthoring.rsandsemantic.rsintoname_match.rsand import them from there.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/dsl/native.rs | Extract per-role kernel fields into a new NativeKernels struct with get/has/Debug impls; route KernelSession::invoke_raw and has_kernel through it. |
| src/dsl/jit.rs | Compile kernels into a fixed-size [Option<FuncId>; 8] indexed by role_index, then build a NativeKernels for the artifact; update tests to access artifact.kernels.*. |
| src/dsl/aot.rs | Build a NativeKernels literal and pass it into from_library instead of eight positional arguments. |
| src/dsl/wasm.rs | Drop WasmKernelAvailability in favor of CompiledKernelAvailability; add maybe_typed_func helper; remove local kernel_output_len and import the shared one. |
| src/dsl/compiled_backend_abi.rs | Promote kernel_output_len from #[cfg(test)] to pub(super) for cross-module reuse. |
| pharmsol-dsl/src/name_match.rs | Add bare_numeric_label and canonical_numeric_suffix as pub(crate) helpers. |
| pharmsol-dsl/src/semantic.rs | Remove local copies of the numeric-label helpers and import them from name_match. |
| pharmsol-dsl/src/authoring.rs | Same dedup as semantic.rs: import from name_match instead of redefining. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -119,6 +123,49 @@ pub struct NativeExecutionArtifact { | |||
| pub diffusion: Option<DenseKernelFn>, | |||
| pub route_lag: Option<DenseKernelFn>, | |||
| pub route_bioavailability: Option<DenseKernelFn>, | |||
|
Bencher Report| Branch | dsl-cleanup |
| Testbed | mhovd-pgx |
🐰 View full continuous benchmarking report in Bencher
⚠️ WARNING: Truncated view!The full continuous benchmarking report exceeds the maximum length allowed on this platform.
This PR consolidates duplicated per-role kernel boilerplate across the four DSL backends (JIT, AoT, WASM, native) and dedupes a few small helpers in the DSL frontend. Behavior is unchanged; signatures on internal constructors were tightened.
The four backends each carried near-identical 8-way fan-outs over KernelRole — one per kernel field, one per role symbol, one per match arm. That made adding or renaming a role a multi-file, multi-block edit and obscured the actual per-backend logic (Cranelift module ownership in JIT, libloading symbol resolution in AoT, wasmtime typed-func lookup in WASM). Grouping the per-role state into one struct and routing role lookups through a single helper makes the per-role surface explicit and shrinks each backend to the part that's actually backend-specific.
No public API changes outside the DSL backend modules. Constructor signature changes (from_jit_module, from_library, WasmKernelSession::new) are internal to src/dsl.
Written by Claude Opus 4.7, verified by myself.