Register parent-reduce rules with the session kernels by robert3005 · Pull Request #11092 · vortex-data/vortex

robert3005 · 2026-11-02T12:06:05Z

Parent-reduce rules move from static PARENT_RULES sets on the vtables into the session kernel registry, so the session controls all rules, as it already does for execute-parent kernels.

Changes

Each encoding registers its parent-reduce rules in its initialize function, through ArrayKernels::register_reduce_parent_rule. The registry keys the rules by the parent and child encoding ids. Rules for any scalar function use AnyScalarFn::id.
The optimizer reads only the registry. It skips the children of a parent for which no rule is registered.
ExecutionCtx takes a snapshot of the parent-reduce kernels when it is created, so optimizing with the context does not read the session again.

Constructors such as slice, filter, take and the array builtins now return lazy arrays. Code that inspects the encoding of the result, stores it, or reads it row by row lost its fast paths or changed its output. Optimize the result at those sites, and add a Dict rule that keeps only the referenced value when the codes reduce to a constant. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Robert Kruszewski <github@robertk.io>

Signed-off-by: Robert Kruszewski <github@robertk.io>

Each encoding now registers its parent-reduce rules with the session's ArrayKernels through register_reduce_parent_rule, keyed by the parent and child encoding ids, in place of a static PARENT_RULES set on its vtable. Rules for any scalar function are keyed by AnyScalarFn::id. The vtable reduce_parent hook and ParentRuleSet are removed. The optimizer consults only the registry and skips the children of a parent that no rule is registered for. An ExecutionCtx snapshots the parent-reduce kernels when it is created, so optimizing with the context does not read the session again. The shared dictionary values of a chunked array are now pulled up by a chunked reduce rule instead of a dict parent rule. The deprecated session-less optimize now uses only the rules vortex-array registers by default, and it does not mark its result as optimized. Signed-off-by: Robert Kruszewski <github@robertk.io>

Create one execution context per sample and mark the input arrays as optimized, so the timed loop measures the slice and its reduction rather than the session lookup and the walk over the unmarked input. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Robert Kruszewski <github@robertk.io>

myrrc · 2026-11-06T16:53:50Z

-    ParentRuleSet::lift(&CastReduceAdaptor(ALP)),
-    ParentRuleSet::lift(&MaskReduceAdaptor(ALP)),
-]);
+/// Registers the parent-reduce rules for [`ALP`] children in the session kernels.


Can we remove this and other comments please?

myrrc · 2026-11-06T16:55:58Z

+/// Registers the parent-reduce rules for [`DecimalByteParts`] children in the session kernels.
+pub(crate) fn register_parent_rules(session: &VortexSession) {
+    let kernels = session.kernels();
+    kernels.register_reduce_parent_rule(


Don't have a strong opinion, but I'd prefer kernels.register_parent_rule(Cast, DecimalByteParts, ... so that the function is parametrized on impl ScalarFnVTable instead of an Id.

myrrc · 2026-11-06T16:58:00Z

+/// Rewrites `Chunked<Dict<codes_i, values>>` into `Dict<Chunked<codes_i>, values>` only when
+/// every chunk is a dictionary over the exact same values array allocation. The rewrite depends
+/// on every chunk at once, so it is a rule of the chunked array rather than a parent rule each
+/// chunk evaluates.


Can we remove this comment?

myrrc · 2026-11-06T17:01:43Z

+//! [`ArrayKernels`] stores the parent-reduce rules and parent-execution kernels that participate in
+//! array optimization and execution without adding them to an encoding vtable. The optimizer
+//! consults it for every parent-reduce rewrite, and the executor consults it for parent execution,
+//! so a session can add support for an extension encoding or register a rule ahead of a built-in
+//! one. When several entries are registered for the same key and kind, they are tried in
+//! registration order until one applies.


If you're changing the description, can you please rewrite this paragraph altogether? This is a hard read for me

myrrc · 2026-11-06T17:02:38Z

+#[derive(Clone, Debug, Default)]
+pub(crate) struct ParentReduceKernels {
+    /// Kernels keyed by hashed `(parent, child)` pair.
+    kernels: Arc<HashMap<ReduceParentFnId, Arc<[ReduceParentKernelRef]>>>,


Do we need an inner Arc?
Do we need an outer Arc?

myrrc · 2026-11-06T17:03:42Z

-pub type ReduceParentFn =
-    fn(child: &ArrayRef, parent: &ArrayRef, child_idx: usize) -> VortexResult<Option<ArrayRef>>;
+/// Type-erased parent-reduce kernel stored in the session registry.
+pub(crate) trait DynReduceParentKernel: Debug + Send + Sync + 'static {


We likely don't need Debug

myrrc · 2026-11-06T17:05:21Z

+        parent: &ArrayRef,
+        child_idx: usize,
+    ) -> VortexResult<Option<ArrayRef>> {
+        let Some(child) = child.as_opt::<V>() else {


Can we match a child and a parent in one match instead of doing this separately?

myrrc · 2026-11-06T17:07:22Z

-/// Registry of [`ReduceParentFn`]s, keyed by hashed `(parent, child)` pair.
-type ReduceParentRegistry = ArcSwapMap<ReduceParentFnId, Arc<[ReduceParentFn]>>;
+/// Registry of [`ReduceParentKernelRef`]s, keyed by hashed `(parent, child)` pair.
+type ReduceParentRegistry = ArcSwapMap<ReduceParentFnId, Arc<[ReduceParentKernelRef]>>;


So we have an ArcSwapMap, then an Arc of slices, and each slice element is an Arc itself.
Can we remove two inner Arcs? We can use a Box of a slice of &'static dyn DynRule trait

myrrc · 2026-11-06T17:07:47Z

+    /// many threads at once. Borrowing this published value does not write to a shared reference
+    /// count, unlike taking a new snapshot of each map.


Please don't document the implementation in the public contract

myrrc · 2026-11-06T17:09:57Z

    execute_parent: ExecuteParentRegistry,
 }

+#[derive(Debug, Default)]


We also have here an Arc of a pair of an ArcSwap of a structure holding Arcs themselves, and also a Mutex.

Can we get rid of Arcs? Can we get rid of mutex and use a semaphore instead?

myrrc · 2026-11-06T17:10:29Z

+    /// [`AnyScalarFn::id`].
+    ///
+    /// Use [`Self::register_scalar_fn_reduce_parent_rule`] for a rule on a scalar function child.
+    pub fn register_reduce_parent_rule<V, R>(&self, parent: Id, child: V, rule: R)


Let's make it taking an impl ScalarFnVTable as first argument

myrrc · 2026-11-06T17:11:14Z

+    /// function array. Rules for the exact ids are tried before those registered under
+    /// [`AnyScalarFn::id`].
+    ///
+    /// Use [`Self::register_scalar_fn_reduce_parent_rule`] for a rule on a scalar function child.


Can we rewrite this comment and/or trim it?

myrrc · 2026-11-06T17:12:58Z

 //! after a child is decoded, new `reduce_parent` rules may match that were previously blocked.
 //!
-//! There are three public entry points on [`ArrayOptimizer`]:
+//! Parent-reduce rules live in the session's [`kernels::ArrayKernels`] registry, keyed by the


Can we please trim this comment?

- register_reduce_parent_rule and register_scalar_fn_reduce_parent_rule take the parent vtable, such as Cast or Filter, instead of its Id. - Remove the doc comments on the register_parent_rules functions. - Keep the parent-reduce and execute-parent kernels each in one ArcSwap of a HashMap with boxed slices. Registration replaces the value with rcu. Signed-off-by: Robert Kruszewski <github@robertk.io>

Signed-off-by: Robert Kruszewski <github@robertk.io>

myrrc · 2026-11-06T17:17:50Z

+    let any_scalar_fn = AnyScalarFn.id();
+    let parent_ids = [
+        Some(parent.encoding_id()),
+        parent.is::<ScalarFn>().then_some(any_scalar_fn),


For the future work: any-any case is imo the wrong path

myrrc · 2026-11-06T17:21:29Z

-        if let Some(new_array) = reduce_parent(child, parent, slot_idx)? {
+    for (_kernel_idx, kernel) in reduce_parent_kernels.iter().enumerate() {
+        if let Some(new_array) = kernel.reduce_parent(child, parent, slot_idx)? {
+            vortex_ensure_eq!(


Please move this to a special #[cold] handler, this will be slow?

myrrc

LGTM. Can we please remove extra comments and make the function take an impl ScalarFnVtable instead of an Id so we'd have typecheck

myrrc · 2026-11-06T17:29:07Z

+    ) -> VortexResult<Option<ArrayRef>>;
+}
+
+pub(crate) type ReduceParentKernelRef = Arc<dyn DynReduceParentKernel>;


I'm not sure we need an Arc here. Yes, we have adapters, but given rules can have 'static lifetime, you can have a &'static dyn DynReduceParentKernel, in which case the adapter can be put in a const {} block which will also propagate 'static.
So a ref will also be a &'static dyn DynReduceParentKernel

Callers pass each rule as a const { &rule } block, and the registry keeps a &'static dyn reference to it instead of an Arc. Signed-off-by: Robert Kruszewski <github@robertk.io>

robert3005 added this pull request to stack #11093 November 2, 2026 12:06

This was referenced Nov 2, 2026

Record whether an array has been optimized #11089

Merged

Optimize arrays top down as execution reaches them, and retry a parent when its children change #11090

Merged

Deprecate the session-less optimize and keep constructors lazy #11091

Open

robert3005 added the changelog/break A breaking API change label Nov 2, 2026

robert3005 force-pushed the rk/session-parent-rules branch 2 times, most recently from 1364ff7 to 99ee78b Compare November 2, 2026 12:28

robert3005 force-pushed the rk/session-parent-rules branch 2 times, most recently from be6bdae to 951db46 Compare November 2, 2026 15:11

robert3005 force-pushed the rk/session-parent-rules branch 2 times, most recently from f6a5894 to a564054 Compare November 2, 2026 19:18

robert3005 force-pushed the rk/session-parent-rules branch from a564054 to cc7b45d Compare November 3, 2026 13:16

robert3005 force-pushed the rk/session-parent-rules branch from cc7b45d to 1d85ea1 Compare November 4, 2026 14:31

robert3005 force-pushed the rk/session-parent-rules branch from 1d85ea1 to d493f1f Compare November 4, 2026 15:29

robert3005 force-pushed the rk/session-parent-rules branch from d493f1f to aa27450 Compare November 4, 2026 18:30

robert3005 force-pushed the rk/session-parent-rules branch from aa27450 to 0adc708 Compare November 4, 2026 18:42

robert3005 force-pushed the rk/session-parent-rules branch from 0adc708 to 3894fcb Compare November 4, 2026 19:36

robert3005 force-pushed the rk/session-parent-rules branch from 3894fcb to 5402a3c Compare November 4, 2026 20:06

robert3005 force-pushed the rk/session-parent-rules branch from 365f883 to d180ffb Compare November 6, 2026 13:35

robert3005 force-pushed the rk/session-parent-rules branch from d180ffb to 4752b50 Compare November 6, 2026 13:56

robert3005 and others added 4 commits November 6, 2026 16:48

Remove comments flagged in review

aeca596

Signed-off-by: Robert Kruszewski <github@robertk.io>

Remove comments that explain why arrays are optimized

14618c1

Signed-off-by: Robert Kruszewski <github@robertk.io>

robert3005 force-pushed the rk/session-parent-rules branch from 4752b50 to 5543bce Compare November 6, 2026 16:51

myrrc reviewed Nov 6, 2026

View reviewed changes

robert3005 added 2 commits November 6, 2026 17:13

Trim optimizer docs and match child and parent together

e73dbca

Signed-off-by: Robert Kruszewski <github@robertk.io>

myrrc reviewed Nov 6, 2026

View reviewed changes

myrrc approved these changes Nov 6, 2026

View reviewed changes

myrrc reviewed Nov 6, 2026

View reviewed changes

Store parent-reduce rules as static references

0b47bf4

Callers pass each rule as a const { &rule } block, and the registry keeps a &'static dyn reference to it instead of an Arc. Signed-off-by: Robert Kruszewski <github@robertk.io>

		/// many threads at once. Borrowing this published value does not write to a shared reference
		/// count, unlike taking a new snapshot of each map.

Conversation

robert3005 commented Nov 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

myrrc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

robert3005 commented Nov 2, 2026 •

edited

Loading