Conversation
Proposes an additive refactor introducing ExprV2 (immutable node), ExprRuntimeState (per-node mutable state), EvalFrame (per-call state), and ExprEvaluatorV2 (stateless orchestrator). Behavior-preserving; ships behind a feature flag alongside the existing Expr until parity is verified.
Introduce the additive ExprV2 / EvalFrame / ExprRuntimeState / ExprEvaluatorV2 / ExprSetV2 surface described in the design doc as empty skeletons. All entry points are VELOX_NYI stubs; nothing routes through them yet. The five new types compile cleanly and link into velox_expression. V1 (Expr, ExprSet) remains the only evaluation path until later steps. See velox/docs/designs/expr-evaluation-refactor.md.
Implement the Expr -> ExprV2 adapter that walks a compiled Expr tree and produces a parallel ExprV2 tree. Function-call and special-form nodes are both supported; special-form nodes keep a shared_ptr to the source Expr for delegated evaluation in later steps. Each ExprV2 retains its sourceExpr to ensure raw FieldReference* pointers in distinctFields_ and multiplyReferencedFields_ remain valid for the lifetime of the V2 node. ExprSetV2 construction now wires up: 1. Adapting every root from an existing ExprSet. 2. Building an ExprRuntimeStateTree covering all unique nodes. Adds three public accessors on Expr (multiplyReferencedFields, listeners, trackCpuUsage) needed by the adapter. These are pure non-behavioral additions. Adds ExprV2AdapterTest covering function-call shape, special-form tagging, FieldReference pointer preservation, and ExprSetV2 multi-root construction.
Wire up the staged pipeline end-to-end for a restricted set of
expressions: no-null flat inputs, no shared sub-expressions, no
encoding peeling, no lazy vectors. V2 produces bit-identical output
to V1 on this subset.
What's implemented:
- ExprEvaluatorV2::evaluateFrame entry guards (fast path delegated
to V1's evalFlatNoNulls; empty rows; special-form fork).
- evaluateWithFieldPeeling / evaluateWithNullPruning /
evaluateWithSharedSubexpr as pass-through layers. Each marked
with a TODO for the step that fills it in.
- evaluateFunctionCall: preserve-null arg evaluation, then
applyFunction (bare vectorFunction->apply, no listeners /
tracer / stats yet).
- evaluateSpecialForm: delegates to sourceExpr->evalSpecialForm.
- ExprSetV2::eval drives the pipeline per root.
What's deferred (each with a TODO):
- Exception-context wiring (step 10).
- Lazy loading decision tree (step 5+).
- FieldPeeling, DictionaryMemo, NullPruning, SharedSubexprCache
(steps 5-8).
- ArgPeeling, default-null arg eval, error masking (step 9).
- Listener / tracer / stats / adaptive sampling (step 10).
EvalFrame gains a 'runtimeStates' reference so child frames can look
up per-child runtime state via the tree. nodeRuntime is now derived
from runtimeStates.at(expr) at frame construction.
Adds ExprV2ParityTest: an A/B harness comparing V1 and V2 vector
outputs. Covers field references, simple plus, nested arithmetic,
constant operands, comparison, and empty selectivity.
Step 4 missed that callers of ExprSetV2::eval still need to construct an EvalCtx, which requires a non-null ExprSet pointer. Add a sourceSet() accessor returning the V1 ExprSet the V2 set wraps, and update the parity test to pass it through. All 10 V2 tests now pass (ExprV2AdapterTest x4 + ExprV2ParityTest x6). Full velox_expression_test suite: 1639 pass, 1 pre-existing failure (CastExprTest.intervalDayTimeToVarchar) unrelated to this work.
Port Expr::evalEncodings + Expr::peelEncodings (Expr.cpp:1025, 1101)
into ExprEvaluatorV2::evaluateWithFieldPeeling. Gating, peel logic,
and result wrapping are mirror-images of V1.
On successful peel, construct an inner EvalFrame on the translated
inner rows and recurse to evaluateWithNullPruning -- skipping
evaluateWithFieldPeeling since peeling has already happened. The
DictionaryMemo branch (mayCache=true) is detected and tagged but not
yet wired; it lands in step 6.
Also fills two gaps from earlier steps:
- ExprV2 was missing skipFieldDependentOptimizations. Adapter and
EvalFrame now propagate it.
- ExprV2 was missing #include for LazyVector (needed by isFlat()).
Parity tests added:
- dictionaryEncodedInput (peels a dict-encoded input)
- constantEncodedInput (constant-encoded input)
- twoDictionariesSameBase (multi-field peel with shared base)
Port Expr::evalWithMemo (Expr.cpp:1246) into ExprEvaluatorV2::evaluateDictionaryMemo. Reached only from FieldPeeling on the peeled+mayCache path. Caches results by base dictionary identity and reuses them across batches. Per-Expr memo state moved from Expr member fields onto ExprRuntimeState::dictMemo, keyed in the parallel runtime-state tree owned by ExprSetV2. This is the change that lets the same ExprSetV2 be evaluated concurrently (separate runtime-state trees) without sharing mutable cache state. One deferred piece: V1 calls context.exprSet()->addToMemo(this) so ExprSet::clearMemo can later reset the cache. V2 doesn't have its own memo registry yet; documented as TODO for step 14. Natural invalidation via the weak_ptr identity check still works. Parity test added: dictionaryMemoAcrossBatches evaluates the same ExprSetV2 across three batches sharing a base dictionary and asserts bit-identical output vs V1 each time.
Port the null-pruning branch of Expr::evalWithNulls (Expr.cpp:1201) plus the Expr::removeSureNulls helper (Expr.cpp:1157) into ExprEvaluatorV2::evaluateWithNullPruning. For propagatesNulls expressions whose distinct input fields may have nulls, compute the non-null row subset, recurse on a child EvalFrame in that subset, then null-fill the removed rows. Pruning is gated by the same conditions as V1 (propagatesNulls && !skipFieldDependent). Parity tests added: - propagatesNullsWithNullInput (single input has nulls) - propagatesNullsBothInputs (multi-field null pruning) - allNullInput (all rows pruned)
Port Expr::evaluateSharedSubexpr (Expr.cpp:899) plus its
shouldEvaluateSharedSubexp gate (Expr.h:592) into
ExprEvaluatorV2::evaluateWithSharedSubexpr.
Cache state (InputForSharedResults key, SharedResults value) moves
from Expr private members onto ExprRuntimeState::sharedCache. Cache
keyed by the identity of all distinct input field vectors; entries
self-evict on weak_ptr expiration.
Handles all four shared-subexpr paths:
1. Cache full -> evaluate without caching.
2. First insertion -> compute, store rows + values; skip caching if
TRY masked every row.
3. Full hit -> copy cached output for the requested rows.
4. Partial hit -> compute missing rows via child frame on top of
the cached buffer, with finalSelection fixed
at outer rows; deselect error rows on return.
ExprV2 gains isMultiplyReferenced field + getter, mirrored by the
adapter.
Parity tests:
- repeatedSubexpression (single-batch (a+b)*(a+b))
- sharedSubexprAcrossBatches (two-batch (a+b)+(a+b)+(a+b))
Port the function-call evaluation leaf: child evaluation under
either the default-null or preserve-null strategy, argument-level
peeling, function apply, and post-apply null fill.
ArgEval:
- evalArgsDefaultNull mirrors Expr::evalArgsDefaultNulls
(Expr.cpp:380): saves pre-existing errors aside, scopes
throwOnError, evaluates each child, deselects rows that are null
without an error (null + error keeps the error), merges argument
errors back, falls through to setAllNulls if remainingRows
empties.
- evalArgsPreserveNull mirrors Expr::evalArgsWithNulls
(Expr.cpp:455): evaluates each child, deselects error rows only.
- Strategy selected per-frame from f.defaultNulls (cached from
VectorFunctionMetadata).
ArgPeeling (tryApplyWithPeeling) mirrors
Expr::applyFunctionWithPeeling (Expr.cpp:1534):
- Peeling-suppressed small-batch path: flatten single-dictionary
inputs, else fall through.
- PeeledEncoding::peel on inputValues; on success, applies on
inner rows, wraps result, moveOrCopyResult back to outer space.
evaluateFunctionCall now follows the canonical layout: arg eval
(may shrink remainingRows) -> try arg peeling -> applyFunction ->
addNulls for any pruned rows. releaseGuard ensures inputValues is
cleared on every exit path.
Still deferred to step 10:
- VectorFunctionListeners invocation around apply().
- CpuWallTimer / adaptive sampling.
- Tracer hooks.
Replace the swap-and-restore of frame.result with a local peeledResult variable that mirrors V1's structure (Expr.cpp:1598-1614). Behavior unchanged; readability improved.
Port Expr::maybeSetupTracer / Expr::finishTracer / Expr::traceOutput (Expr.cpp:2081, 2115, 2131) plus the ExprSet wrappers (Expr.cpp:2070, 2105) into V2. Per-Expr tracer state moves from Expr::outputTracer_ onto ExprRuntimeState::outputTracer. ExprSetV2 gains maybeSetupTracers and finishTracers mirroring the V1 API; the actual tree walks live in anonymous-namespace helpers in ExprSetV2.cpp. ExprEvaluatorV2::applyFunction now calls outputTracer->write after the bare vectorFunction->apply, mirroring V1's call site at Expr.cpp:1768. No new parity tests: tracer output is only emitted when callers explicitly install a TraceCtx, which existing parity tests don't. The 19 V2 tests continue to pass.
…p 10b)
Port the rest of Expr::applyFunction (Expr.cpp:1753) and
Expr::invokeApplyWithListeners (Expr.cpp:1700) plus the adaptive
sampling state machine (Expr.cpp:1619, 1650).
ExprEvaluatorV2::applyFunction now:
1. Increments numProcessedVectors / numProcessedRows on
ExprRuntimeState::stats.
2. Acquires a (possibly nullptr) CpuWallTimer via cpuWallTimer().
3. Calls computeIsAsciiForInputs and captures isAscii for varchar
results.
4. Invokes pre-listeners, calls vectorFunction->apply with
VeloxException/std::exception trapping, invokes post-listeners
with any captured applyError, rethrows on failure.
5. Writes tracer output (added in 10a).
6. Handles the "function returned no result and no error" case by
recording a synthetic VELOX_USER_FAIL and null-filling.
7. Propagates isAscii onto the result vector.
8. Calls finalizeAdaptiveCalibration while still in warmup or
calibrating.
AdaptiveSamplingState in ExprRuntimeState.h is now fully fleshed out
(was an empty placeholder): phase machine, calibration stopwatch,
accumulated wall nanos, batch count, sampling rate + counter.
V1's adaptive math is preserved exactly (sampling rate computation,
counter initialization to rate-1, zero-cost-function fallback to
rate=100).
All 19 V2 tests pass. Full velox_expression_test: 1648/1649 pass,
same single pre-existing failure as before (intervalDayTimeToVarchar).
ExprSetV2 reshaped as a subclass of ExprSet so the existing factory
(makeExprSetFromFlag) and operator callers route to V2 via
polymorphic dispatch — same pattern ExprSetSimplified uses. A new
QueryConfig flag expression.eval_v2 (default false) gates the V2
path so it can be canaried per-query before the cutover.
Pieces:
- QueryConfig: add expression.eval_v2 flag.
- Expr.h: expose outputTracer() accessor so V2 can delegate trace
writes to the V1 tracer state.
- Expr.cpp: makeExprSetFromFlag branches on exprEvalV2() to
construct ExprSetV2 instead of ExprSet.
- ExprSetV2: inherits ExprSet, calls the base constructor (runs
ExprCompiler), then builds the V2 mirror tree + runtime-state tree.
Overrides eval(begin, end, initialize, ...). Drops the previous
sourceSet_ field and V2-specific tracer setup; both come from the
inherited V1 surface now.
- ExpressionVerifier: also exercises the V2 evaluator alongside V1
common + simplified so the existing fuzzer harness covers V2.
Adds "Copyright (c) 2026 IBM Corporation." alongside the existing
Facebook copyright line in the V2 evaluator files touched here.
All V2 tests pass.
CastExprV2 is a near-verbatim copy of V1's CastExpr (and CastExpr-inl.h)
with the V1 surface kept untouched so V2 cast evaluation can be
canaried independently. V2's tree builder substitutes a CastExprV2
in place of V1's CastExpr; V1's CastExpr still lives in the V1 set's
exprs_ so raw FieldReference* pointers held by the V2 mirror remain
valid.
Routing:
- CastExprV2 gains a static HooksFactory mechanism. The default
factory produces PrestoCastHooks (same hooks V1 uses); callers
register their own at startup to plug in a vectorized variant.
- ExprV2::from now takes core::ExecCtx&. For kCast nodes it builds
a CastExprV2 via CastExprV2::hooksFactory() and uses it as
sourceExpr_ instead of V1's CastExpr.
- V1's CastCallToSpecialForm and TryCastCallToSpecialForm are NOT
touched; the V2 routing happens entirely in ExprV2::from.
- Expr.cpp dispatches V2 special forms via SpecialFormKind tag
(kCast, kConstant, kFieldAccess) instead of dynamic_cast against
concrete V2 subclasses, so Expr stays decoupled from V2 types.
Polish:
- Standardize cast helper naming inside CastExprV2 (parallel to
CastExpr) so it's easy to diff the two.
- Tidy V2-only comments and mirror V1's documentation where the
semantics carry over.
New CastExprV2 files (CastExprV2.h, CastExprV2.cpp, CastExprV2-inl.h)
include the IBM Corporation copyright header from creation alongside
the existing Facebook one.
All 22 V2 tests + CastExpr tests pass.
Introduces a V2 cast hooks interface that exposes whole-column entry
points alongside the inherited scalar ones from V1's CastHooks, plus
a concrete header-only PrestoCastHooksV2 that owns its own conversion
logic so the parser is visible to the compiler inside vector loops.
V2's CastExprV2 dispatches the hook-served primitive paths through
the new vector surface; everything else continues to flow through the
util::Converter fallback in castKernel.
Surface:
- CastHooksV2 inherits CastHooks (so shared_ptr<CastHooksV2> remains
compatible with V1's CastOperator::castTo(..., hooks)) and adds
pure-virtual whole-column entry points for VARCHAR -> {Timestamp,
Date, Real, Double}, DATE -> Timestamp, the bulk local->GMT shift,
and the four casts Presto disallows (Int/Bool/Double -> Timestamp,
Timestamp -> Int).
- PrestoCastHooksV2 is header-only. Conversion logic is inlined as
private helpers (doStringToTimestamp, doStringToDate,
doStringToFloating<T>) so the parser body is visible at every
vector-hook call site; no composition of PrestoCastHooks or
virtual scalar dispatch per row. Disallowed-cast Status objects
are cached as static-inline members so bulk setStatuses reuses one
Status per category.
Pluggability:
- Spark CAST factories (SparkCastCallToSpecialForm,
SparkTryCastCallToSpecialForm) accept a SparkCastHooksFactory at
construction so a CastHooks subclass — existing SparkCastHooks or a
future vectorized variant — can be plugged in without further
subclassing. V1 Presto's CastCallToSpecialForm is left untouched.
- CastExprV2 holds shared_ptr<CastHooksV2>; the V2 factory's default
implementation produces PrestoCastHooksV2.
Migration:
- castPrimitives dispatches INT/BOOL/DOUBLE/REAL -> TIMESTAMP and
TIMESTAMP -> INT to the vector hooks (Presto's bulk error
handling) and VARCHAR -> {Timestamp, Real, Double} to the
whole-column string parsers; the V1-style castKernel per-row loop
keeps only the util::Converter fallback for VARCHAR -> int kinds
and the other non-hook conversions.
- castFromDate (DATE -> TIMESTAMP) and castToDate (VARCHAR -> DATE)
call castDateToTimestampVector / castStringToDateVector directly,
replacing the per-row applyToSelectedNoThrowLocal pattern.
Tests + benchmark:
- PrestoCastHooksV2Test exercises each vector hook directly against
an EvalCtx (success, error reporting, empty-string handling,
deselected rows, timezone shift, disallowed casts).
- CastHooksBenchmark contrasts V1 (per-row PrestoCastHooks) and V2
(whole-column PrestoCastHooksV2) for the five migrated directions;
wires the previously-orphaned velox/expression/benchmarks
subdirectory into the velox_expression CMakeLists when
VELOX_ENABLE_BENCHMARKS_BASIC is on.
New V2 hook + test + benchmark files include "Copyright (c) 2026 IBM
Corporation." from creation alongside the existing Facebook copyright.
92/92 ExprV2 + CastExpr + PrestoCastHooksV2 tests pass.
…snprintf This is a fix for a pre-existing CastExprTest.intervalDayTimeToVarchar failure that surfaces on macOS arm64 while continuing to pass on CentOS / Linux x86_64. The bug was introduced by 6bde69d ("Fix cast(IntervalDayTime as Varchar) for negative intervals (facebookincubator#9871)") - the cast-performance stack on this branch happens to also exercise the failing test, so the fix is bundled here for convenience. IntervalDayTimeType::valueToString built its string with snprintf(buf, sizeof(buf), "%s%lld %02d:%02d:%02d.%03d", sign.c_str(), days, hours, minutes, seconds, remainMillis); where the four arguments matched to %d / %02d / %03d were not ints: - hours, minutes, seconds were int64_t - remainMillis was int128_t (widened by 6bde69d to safely negate std::numeric_limits<int64_t>::min()) Passing a wider integer through varargs to a %d conversion is undefined behavior. The int64_t -> %d path happens to work on every common ABI because int64_t occupies one 8-byte varargs slot and %d reads its low 4 bytes (which on little-endian contain the small value for hours / minutes / seconds < 64). The int128_t -> %d path is where macOS arm64 and Linux x86_64 diverge: * Linux x86_64 (SysV, CentOS CI): the first six integer/pointer varargs go in registers (RDI..R9). For our snprintf, buf/size/ format/sign/days/hours fill those, so minutes, seconds and remainMillis spill to the stack. __int128 is laid out with its low 64 bits at the lower address, and the byte where printf reads %03d happens to coincide with the low 4 bytes of remainMillis (which holds a value < 1000, so the low 4 bytes are the correct value). The test passes by coincidence. * macOS arm64 (Apple variadic ABI): variadic arguments are passed exclusively on the stack regardless of available registers, with strict natural alignment. The stack layout for the six varargs is: off 0: sign (char*, 8 bytes) off 8: days (int64_t, 8 bytes) off 16: hours (int64_t, 8 bytes) off 24: minutes (int64_t, 8 bytes) off 32: seconds (int64_t, 8 bytes) off 40: 8 bytes of padding (int128 needs 16-byte alignment) off 48: remainMillis (int128, 16 bytes) va_arg(list, int) on this ABI reads 4 bytes and advances the cursor by 8 (the minimum slot size). After processing %s, %lld, %02d, %02d, %02d the cursor sits at offset 40 - exactly the alignment padding. %03d reads 4 bytes of uninitialised stack and prints whatever happens to be there. The visible symptom: expected "1 00:00:00.000" got "1 00:00:00.1803785600" Day, hours, minutes, seconds render correctly (int64_t -> %d still works); only the milliseconds field, which reads from the padding, is junk. After the day divmod each of hours, minutes, seconds, milliseconds is bounded (<24, <60, <60, <1000), so narrowing to int is safe. Compute them as int locals and pass int to snprintf - eliminates the UB on every ABI. remainMillis stays int128_t for the intermediate arithmetic so the INT64_MIN negation introduced by 6bde69d still works. CastExprTest.intervalDayTimeToVarchar now passes on macOS arm64 without regressing Linux x86_64; 56/56 CastExprTest tests pass.
…ast evaluation
Adds a folly benchmark harness for CAST(...) over a stable dictionary
base, used to evaluate evalWithMemo improvements on real param
matrices without rebuilding test scaffolding ad-hoc.
The benchmark sweeps the dimensions that drive cast performance:
- numVectors : input vectors evaluated per iteration
(fixed at 1000 in the matrix below;
not encoded in entry names)
- rowsPerVector : rows in each input vector
- distinctValueCount : dictionary base cardinality (DICT only)
- newIndicesPerVector : new dictionary indices each input vector
introduces relative to the previous one -
drives the memoization cache miss rate
(DICT only)
- nullPct : percentage of positions marked null in
{0, 50, 100}. For DICT, nulls live on the
dictionary wrap (reach
PeeledEncoding::translateToInnerRows via
wrapNulls_); for FLAT, on the flat input
itself. nullPct=100 short-circuits the cast
to a null constant and surfaces the absolute
measurement floor.
For each cast pair the matrix registers 27 dictionary-input
combinations (rv x dvc x nipv) and 3 flat baselines (rv). Each
source line through the DICT_NULLS / FLAT_NULLS wrapper macros
expands into three benchmark entries - one per nullPct value - so
the source stays the same length but every existing combination has
three null variants registered.
Cast pairs covered:
- BIGINT -> VARCHAR
- INT -> BIGINT
- DATE -> VARCHAR
- DATE -> TIMESTAMP
- REAL -> DOUBLE
Entry names read:
DICT_<from>To<to>(rowsPerVector_distinctValueCount_newIndicesPerVector_nullPct)
FLAT_<from>To<to>(rowsPerVector_nullPct)
Wires velox/expression/benchmarks into the build under the existing
VELOX_ENABLE_BENCHMARKS gate (it was never added before) and lands
the velox_benchmark_widening_cast target.
Sample at FLAT_IntToBigint shows how nullPct affects per-row time:
FLAT_IntToBigint(100_0) 12.84 ns/row no nulls
FLAT_IntToBigint(100_50) 16.70 ns/row half rows null
FLAT_IntToBigint(100_100) 8.30 ns/row all-nulls short-circuit
A short investigation triggered by entries like
DICT_IntToBigint(10000_5_1_0) printing "0.00fs Infinity":
These are not measurement bugs. Folly's runBenchmarkGetNSPerIteration
subtracts a globally-measured baseline (cost of an empty BENCHMARK
loop, ~0.5 ns/iter) and floors the result at zero
(Benchmark.cpp:207). For configurations where the dictionary cache
covers the whole base in the first couple of batches - small dvc
relative to rowsPerVector, so the peelEncodings bypass kicks in for
~998 of 1000 batches - the per-row cast cost falls below the
baseline. "0.00fs" reads as "below folly's resolution after baseline
subtraction", not actual zero. The corresponding nullPct=100 entry
(which doesn't hit the bypass: translateToInnerRows returns an empty
inner-rows set, so dictionaryCache_ never populates) still measures
around 400-500 ps/iter, showing the resolution floor folly can
report. Added the explanation to the file's header comment so the
next reader doesn't have to re-derive it.
While here, also pin the cast result with folly::doNotOptimizeAway
before reading ->size(), so the compiler can't drop any of the
intermediate state from the cast call. The numbers didn't change -
the side effects on dictionaryCache_ already prevented DCE - but
the pin makes the intent explicit and is a defensible default for
any cast benchmark.
Verified: with the suspender excluding setup, runDictionary's cast
loop runs in ~500 us per batch-1000 (measured via direct
steady_clock during investigation), which is 50 ps/row for the
10000-row config. That's below folly's baseline (~500 ps) so the
display floors to 0.00fs.
… date_format to WideningCastBenchmark
Extends the existing single-base, single-thread benchmark with three
dimensions that the original was missing - each models a real
behavior production sees but the original could not measure.
Multi-base alternation. The original always reuses one dictionary
base FlatVector across all 1000 batches per iteration. That measures
only the steady-state cache-hit cost; it never exercises the
numMemoBaseChange path of evalWithMemo, which a real scan operator
hits every time its underlying storage chunk advances. Add a
batchesPerBase parameter and build ceil(numVectors /
batchesPerBase) distinct base FlatVectors with distinct content; the
i-th batch wraps base [i / batchesPerBase]. Sweep {1, 10, 100}
across the higher-signal shapes per type pair via DICT_ALT_*
macros. Entry names get a _bpb<n> suffix; entries without that
suffix retain the original single-base semantics (= 1000, matching
numVectors).
Multi-thread. The production refcount cost on the source Buffer is
a cross-driver effect: many threads each constructing a
DictionaryVector that wraps the same file-cache-shared Buffer
issues 2 atomics per batch per thread on one cache line, and that
line bounces between L1 caches. The original benchmark cannot
reproduce this. Add runDictionaryMultiThread that spawns N worker
threads, each with its own ExecCtx and ExprSet (because Expr
evaluation is not thread-safe) but all evaluating against the
shared base FlatVectors built on the calling thread. Entry name
format: `MT_<funcName>(threads<n>_<rv>_<dvc>_<nipv>_<nullPct>
_bpb<n>)`. Sweep numThreads in {4, 16} and batchesPerBase in
{1, 100, 1000} for the 3 type pairs and the production
expression where the cross-driver pattern matters in prod
(BigintToVarchar, DateToVarchar, DateToTimestamp, DateFormatProd).
Production expression. Add DICT_DateFormatProd /
FLAT_DateFormatProd / MT_DICT_DateFormatProd that exercise the
exact shape from the slow query that motivated this work:
date_format(CAST(date_trunc('day',
date_add('day', 0 - mod(((day_of_week(c0) % 7) - 1) + 7, 7),
c0)) AS timestamp), '%Y-%m-%d')
This is a DATE -> VARCHAR chain dominated by the date_format
result's stringBuffers_ refcount churn under evalWithMemo. Sweep
the same (rv, dvc) shapes as the alt entries; include both
single-base and _bpb sweeps so the cache-hit floor and the
realistic mix are both observable.
The original ~430 entries (single-base sweep across 5 type pairs +
flat baselines + nullPcts) are preserved unchanged so existing
measurements remain comparable.
Build and ran a representative subset of new entries
(DICT_BigintToVarchar/DICT_DateFormatProd at bpb=10, and
MT_DICT_DateFormatProd at threads=4/16) at --bm_min_iters=1 to
confirm correct registration and execution.
…rk + CastHooksBenchmark
Every DICT/DICT_BPB/FLAT/MT entry in WideningCastBenchmark now expands
to a paired set: a _v1 BENCHMARK_NAMED_PARAM_MULTI that routes through
V1's ExprSet, followed by a _v2 BENCHMARK_RELATIVE_NAMED_PARAM_MULTI
that routes through ExprSetV2 + CastExprV2 + PrestoCastHooksV2. V1
reports absolute time/iter; V2 reports the V1-vs-V2 ratio in folly's
relative column, so pairs are visible row-adjacent in the output.
Wiring:
- WideningCastBenchmark::useV2Evaluator(bool) flips
expression.eval_v2 on the shared queryCtx_ and stashes the
selection so the MT path can propagate it to each per-thread
queryCtx at setup time.
- WideningCastBenchmark::compileExprSet routes through
exec::makeExprSetFromFlag and returns a polymorphic
unique_ptr<exec::ExprSet> -- V1 ExprSet or V2 ExprSetV2 depending
on the per-execCtx config. Replaces the calls into
FunctionBenchmarkBase::compileExpression (which constructs
exec::ExprSet directly and ignores the flag). Used by
runDictionary, runFlat, and runDictionaryMultiThread alike.
- Each free benchmark function (DICT_*, FLAT_*, MT_DICT_*) takes a
`bool v2` parameter and calls useV2Evaluator before compile so
the V1 and V2 entries share identical input data and only differ
on the evaluator.
- CastHooksBenchmark::runCast inlines the same construction shape
against execCtx_ so its paired BENCHMARK_MULTI / BENCHMARK_
RELATIVE_MULTI entries actually compare evaluators.
With the toggle wired through, output now shows real ratios:
DICT_DateToTimestamp(1000_500_1_0_v1) 43.58ns 22.95M
DICT_DateToTimestamp(1000_500_1_0_v2) 115.91% 37.60ns 26.60M
MT_DICT_DateToTimestamp(threads4_1000_500_1_0_b 16.50ns 60.62M
MT_DICT_DateToTimestamp(threads4_1000_500_1_0_b 112.59% 14.65ns 68.25M
The startup legend documents the new _v{1,2} suffix.
…xecCtx's VectorPool
FilterProject::project and FilterProject::filter both declared the
results vector as a stack local:
std::vector<VectorPtr> results;
exprs_->eval(..., results);
So every getOutput() handed exprs_->eval a fresh empty vector. Inside
Expr::eval the per-expression result starts null, BaseVector::ensure-
Writable consults the ExecCtx's VectorPool, finds it empty (nothing
ever released into it), and falls through to BaseVector::create -
malloc through MemoryPoolImpl::allocate, which on a production cast
workload accounts for ~1-2% of total Driver CPU per a Presto worker
profile (FilterProject::project -> ExprSet::eval -> ... -> Cast-
Expr::apply -> ensureWritable -> createInternal -> AlignedBuffer::
allocate -> MmapAllocator::allocateContiguousImpl).
EvalCtx::releaseVector / releaseVectors already feed the ExecCtx's
pool, but the top-level result vectors handed out by getOutput() are
owned by the downstream consumer - we don't get an automatic release.
The pool stays empty unless someone explicitly returns vectors to it.
Promote the results vectors to FilterProject members
(filterResults_, projectResults_) and release them back to the pool
at the top of each filter() / project() call. By the next call:
- The downstream consumer has typically dropped the prior output's
RowVector, so each VectorPtr in our member has use_count == 1.
- releaseVectors moves singly-referenced flat vectors into the pool
(per VectorPool::release: silently skips multi-owned vectors, so
consumers that still hold a reference cause a missed reuse but
no correctness issue).
- exprs_->eval gets the now-null entries, ensureWritable finds the
recycled vectors in the pool, writes into them in place.
For project() the return signature changes to
'const std::vector<VectorPtr>&' so the getOutput() caller sees the
member directly without a per-call copy. filter() is unchanged
externally (still returns vector_size_t).
processFilterResults reads filterResults_[0] inline and does not
retain it, so the boolean vector is free to recycle on the next call.
17/17 FilterProjectTest pass.
…ed flats can be reused in place The previous commit pushed FilterProject's prior result vectors back to ExecCtx's VectorPool at the top of filter() / project(). That recovers the FlatVector allocation cost for workloads where the projection produces a top-level flat vector. For the workload that prompted this stack - CAST(INT -> BIGINT) over a dictionary-encoded column behind a hash-join probe - the outputs are DictionaryVector wrappers (Expr::evalEncodings re-wraps the cast result with the input dictionary's indices), and VectorPool::release rejects non-flat encodings (VectorPool.cpp:96, maybePushBack requires isFlatEncoding()). The previous commit therefore left those Dictionary wrappers in projectResults_ on the path back into exprs_->eval. Two consequences: 1. The wrapped flat - which for a stable base is the same FlatVector held by Expr::evalWithMemo's dictionaryCache_ - kept a live reference from projectResults_[i] through the wrapper. So dictionaryCache_.use_count() stayed >= 2 entering the next call, and BaseVector::ensureWritable at Expr.cpp:1328 hit the copy-on-write branch in BaseVector::ensureWritable (BaseVector.cpp:818-838): allocate a new flat from the pool (or create), copy the already-cached rows over, swap. The 0.78% "direct ensureWritable" line of the production profile lived here. 2. exprs_->eval was invoked with non-null DictionaryVector entries in the result slot, which BaseVector::ensureWritable can't reuse in place (the switch at BaseVector.cpp:802-815 only covers FLAT, ROW, ARRAY, MAP, FLAT_MAP, FUNCTION) - it falls straight through to allocate-and-copy as well. Add a projectResults_.clear() / filterResults_.clear() right after releaseVectors. The pool keeps whatever it accepted (flat outputs of non-encoded projections); everything else - notably the Dictionary wrappers from peeled-encoding cast paths - is dropped. The wrapper's destructor decrements its base's use_count; when the Expr's dictionaryCache_ is now the sole remaining owner, the next call's ensureWritable takes the in-place branch and skips the COW copy entirely. 17/17 FilterProjectTest pass.
…Project over dict-encoded input
The two preceding commits target an allocation hot spot visible in
production profiles of int->bigint casts behind a hash-join probe -
FilterProject's filter/project result vectors are recreated empty
on every getOutput, and the dictionary-wrapped cast output never
reaches EvalCtx's VectorPool, so Expr::evalWithMemo's
dictionaryCache_ keeps hitting the copy-on-write branch in
BaseVector::ensureWritable. WideningCastBenchmark exercises
CastExpr directly via FunctionBenchmarkBase::evaluate and skips the
operator pipeline entirely, so it can't measure these changes.
Add a small operator-level benchmark that builds the exact shape:
Values(dictionary-encoded input) -> Project(cast(c0 as <type>))
run via AssertQueryBuilder. Sweeps rowsPerVector, distinctValue-
Count, and nullPct for int->bigint (the production case),
real->double (another fast upcast), and bigint->varchar (a
string-producing cast that exercises the StringView allocation
path). numVectors is fixed at 1000 so the run amortises one-off
startup, and every input vector wraps the same shared flat base in
a DictionaryVector with rotating indices - so the
second-and-subsequent vectors hit evalWithMemo's cache path and
exercise the in-place / COW branches selected by
dictionaryCache_'s use_count.
nullPct sweeps {0, 50, 100}:
- 0 : nullptr wrap nulls. Pure no-nulls path through
velox::translateToInnerRows.
- 50 : every other dictionary position marked null on the wrap.
Reaches translateToInnerRows with a non-null bitmap; about
half the cast work still runs (non-null rows go through
evalWithMemo / cast).
- 100 : every position null. translateToInnerRows produces an
empty inner-rows set; downstream cast work short-circuits
to null constants. Surfaces the absolute floor for "cast
over all-nulls".
Reading the output: the printed time/iter is per row (folly
divides by the row count returned). At rowsPerVector=100 it is
dominated by the per-call fixed cost in FilterProject + Expr::eval -
exactly the shape the recycle path is meant to flatten.
Entry names:
DICT_<from>To<to>(rowsPerVector_distinctValueCount_nullPct)
Sample at 1000 rows / dvc=500 across nullPct values:
DICT_IntToBigint(1000_500_0) 28.16 ns/row no-nulls
DICT_IntToBigint(1000_500_50) 29.09 ns/row null-aware loop
DICT_IntToBigint(1000_500_100) 17.24 ns/row all-nulls short-circuit
…rBenchmark Adds BM_translateToInnerRows measuring the free function velox::translateToInnerRows directly, isolated from the rest of the expression evaluation pipeline. Sweeps numEntries (1'000, 1'000'000) and nullPct (0, 50, 100): - 0 : nullptr nulls, hits the recently-hoisted no-nulls fast path. - 50 : null-aware loop with alternating null/non-null pattern. - 100 : every position null, branch always skips setValid. The free function is what PeeledEncoding::translateToInnerRows delegates to and what the prior 'perf(vector): Hoist null-presence branch out of translateToInnerRows loop' commit targets. Direct measurement removes the noise that expressed itself when measuring through the cast benchmark - there the function is only one of many contributors per iteration and the hoist signal got buried. Sample numbers on this build: BM_translateToInnerRows(1000_0) 2.05 us/iter (2.05 ns/row) BM_translateToInnerRows(1000_50) 1.01 us (null skips half the setValid) BM_translateToInnerRows(1000_100) 335 ns (null skips all setValid) BM_translateToInnerRows(1000000_0) 2.07 ms (~2.07 ns/row, holds at scale)
… rows are null For a dictionary peel whose wrap nulls cover every selected outer row (e.g. CAST(... AS ...) over a column where every value is null in the current batch), velox::translateToInnerRows walked all selected rows calling bits::isBitNull on each one, found them all null, set no inner row - and then evalEncodings saw newRows->hasSelections() == false, skipped evalWithMemo, and wrap() produced a NullConstant (PeeledEncoding.cpp:251). The per-row null walk was pure overhead. Add a vectorised early-exit before the per-row loop: when nulls is non-null and outerRows is contiguous (the common case after the caller has built remainingRows by deselecting errors), one bits::countBits over the wrap-nulls bitmap tells us if any selected position is non-null. If countBits returns 0 the entire range is null, no inner row can be set, and we can return immediately. countBits is a vectorised popcount - roughly ~150 word reads + 150 popcnts for a 10K-row bitmap (~200 ns), vs ~10K per-row branch+test in the existing loop (~4 us in measurements). Effect on WideningCastBenchmark, CAST(INT AS BIGINT) with nullPct=100 (every dictionary position null, so the bypass added in 'perf(expression): Skip translateToInnerRows when dictionaryCache_ covers the peeled base' never fires - the cache never populates because newRows is always empty): DICT_IntToBigint(1000_5_1_100) 974 ps -> 456 ps (-53%) DICT_IntToBigint(10000_5_1_100) 345 ps -> 0.00fs (now below folly resolution) DICT_IntToBigint(10000_500_100_100) 344 ps -> 0.00fs (now below folly resolution) Sparse outerRows (begin > 0 or non-contiguous selections) keep the existing per-row loop - the fast path requires the contiguous range to make countBits valid. Sparse case is rarer in production and the correctness path is unchanged. 1626/1626 velox_expression_test pass. 17/17 FilterProjectTest pass.
…ps (V2) V2 port of the V1 hoist optimization. CastExprV2::castFromTime (TIME -> VARCHAR) evaluated a per-row `if (timeZone)` check inside the row loop even though the timezone is fixed for the whole call; split into two specialized loops chosen once by timezone presence so the inner body is straight-line and more inlinable. CastExprV2's DATE -> TIMESTAMP path is already hoisted on the V2 side via PrestoCastHooksV2::castDateToTimestampVector, which keys its two specialized inner loops off the same timezone pointer.
V2 port of the V1 optimization. CastExprV2::castFromDate's DATE -> VARCHAR kernel was doing three allocations per row (intermediate std::string, 32KB result-owned buffer, finalize) to write 10 bytes; the StringWriter buffer was held by the vector but never referenced because the output fits StringView's 12-byte inline storage. Format directly into a 17-byte stack array via Timestamp::tmToStringView and construct the StringView inline. For the common case (year in [-9999, 999999]) the StringView ctor copies the bytes into its own inline storage and the stack buffer is unreferenced after the row; only when the formatted output exceeds 12 bytes do we fall back to a result-owned buffer for correctness on edge-case large years.
…e (V2)
V2 port of the V1 caching optimization. CastExprV2::castPeeled was
looking up the custom cast operator for the input and output types
from a static F14FastMap on every per-vector call. Both lookups are
pure hash table reads, but they sit on the hot path of every
evaluate() call and the (fromType, toType) pair never changes for
the lifetime of a CastExprV2. hooks_->getPolicy() resolves a
constant per CastExprV2 too even though the V2 hooks devirtualize
the call, the function body still isn't inlinable across the TU
boundary.
Cache both on the CastExprV2 instance with lazy first-call population:
- topLevelCastFromOperator_ / topLevelCastToOperator_, populated by
ensureTopLevelCastOperators(fromType, toType) on the first
castPeeled call. castPeeled is only ever called from apply()
with the top-level (inputs_[0]->type(), type_) pair, so caching
is safe -- recursive complex-type element casts go through
castArray / castMap / castRow, not through castPeeled with
element types.
- cachedPolicy_, populated by policy() on first call. Used inside
castPrimitives' switch instead of dispatching through
hooks_->getPolicy() per per-vector entry.
Wins are most visible for "cheap" per-row casts where the fixed
per-call cost dominates -- small input vectors and trivial kernels.
…xprV2::apply (V2) V2 port of the V1 fast path. CastExprV2::apply was unconditionally wrapping the input in a LocalDecodedVector and calling decode, even when the input was already flat. For a flat input the decode is effectively a no-op: isIdentityMapping=true, base()==&input, nulls()==input->rawNulls(). Add a fast path that runs when input->isFlatEncoding(): - Use input->rawNulls() directly instead of decoded->nulls(rows). - Skip the LocalDecodedVector entirely. - Call castPeeled with *input. The existing decoded-and-maybe-peel path stays untouched and still handles every other encoding shape (constant, dictionary, lazy).
V2 port of the V1 `final` annotation. Lets the compiler/LTO devirtualize calls made through a statically-known PrestoCastHooksV2* when the receiver type is visible. V2's vector hooks already keep per-row dispatch out of CastExprV2's hot loops, so this closes the same hole when callers reach the hooks through the base CastHooks or CastHooksV2 surface (e.g. via the inherited CastOperator::castTo).
yingsu00
requested review from
jinchengchenghh,
majetideepak and
rui-mo
as code owners
July 29, 2026 02:22
yingsu00
requested review from
aditi-pandit
and removed request for
jinchengchenghh,
majetideepak and
rui-mo
July 29, 2026 02:23
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Expr.cpp/Expr.h(~3600 lines) mix tree shape, immutable metadata, per-callstate, per-node caches, stats, adaptive sampling, tracer wiring, and the
evaluator algorithm. The pipeline is implicit — each layer is named after what
its predecessor did not do (
eval→evalEncodings→evalWithMemo→evalWithNulls→evalAll→evalAllImpl).This PR adds a parallel evaluator (V2) in new files, leaving
Exprintact:ExprV2(immutable node),ExprRuntimeState(per-node mutable state),EvalFrame(per-call state, on the stack),ExprEvaluatorV2(statelessorchestrator, one function per phase), and
ExprSetV2(owner). Becauseper-call state moves off the tree node, the same
ExprSetV2can be evaluatedfrom multiple threads — impossible today with
Expr::inputValues_.The refactor is behavior-preserving and ships behind a new query config
expression.eval_v2(defaultfalse), dispatched through the existingmakeExprSetFromFlag. V1 stays the default; cutover and V1 deletion areseparate changes.
Over 10 optimizations were then added on the new ExprV2 evaluator. The new
V2 evaluator shows over 20x improvement in CPU time, over 80x improvement
in wall time on FilterProject operator in Bolt's workload.
Design doc:
velox/docs/designs/expr-evaluation-refactor.md(added here).What are included
Expr→ExprV2adapter, then one commit perpipeline phase — FieldPeeling, DictionaryMemo, NullPruning,
SharedSubexprCache, ArgEval/ArgPeeling, tracer hooks, listeners + CPU timing
ExprSetV2subclassesExprSet(same pattern asExprSetSimplified), so operator call sites reach V2 by polymorphicdispatch with no changes.
CastExprV2starts as a near-verbatim copy ofCastExprso the two can be diffed.
CastHooksV2adds whole-column entry points nextto V1's scalar hooks; header-only
PrestoCastHooksV2inlines its conversionlogic so the parser is visible inside the vector loops. Migrated:
VARCHAR → {Timestamp, Date, Real, Double},DATE → Timestamp, the bulklocal→GMT shift, and the four casts Presto disallows; everything else keeps
the
util::Converterfallback. Spark's CAST special forms gain ahooks-factory parameter so a vectorized variant can be plugged in.
WideningCastBenchmark(sweeps rows/vector, dictionarycardinality, cache-miss rate, nulls, base alternation, thread count; every
entry expands into a
_v1/_v2pair),CastHooksBenchmark,CastInFilterProjectBenchmark(operator-level), and atranslateToInnerRowsentry inSelectivityVectorBenchmark.FilterProjectrecycles itsfilter/project result vectors through
ExecCtx'sVectorPooland clearsthem after release — previously they were stack locals recreated per
getOutput(), soensureWritablealways fell through tomalloc(~1–2% ofDriver CPU on a production
CAST(int → bigint)profile behind a hash join)and
evalWithMemo'sdictionaryCache_kept taking the copy-on-writebranch.
translateToInnerRowsearly-exits via one vectorizedcountBitswhen wrap nulls cover the whole selected range: 974 ps → 456 ps per row on
DICT_IntToBigint(1000_5_1_100).branches, no per-row allocation in
DATE → VARCHAR, cast operators andpolicy cached on the instance, decode skipped for flat input,
PrestoCastHooksV2markedfinal.Testing
ExprV2ParityTest— A/B harness asserting V1 and V2 produce identicalvectors across field references, arithmetic, constants, empty selectivity,
dictionary/constant encodings, shared bases, memo across batches, null
pruning, and shared subexpressions.
ExprV2AdapterTest— adapter shape, special-form tagging,FieldReference*pointer preservation, multi-root construction.PrestoCastHooksV2Test— every vector hook against a realEvalCtx:success, error reporting, empty strings, deselected rows, timezone shift,
disallowed casts.
ExpressionVerifieralso runs V2, so the expression fuzzer covers it withno new harness.
velox_expression_test,CastExprTest, andFilterProjectTestpass at each step of the stack.Not in this PR
No default flip and no V1 deletion. Special forms (
CaseExpr,ConjunctExpr,CoalesceExpr, …) still delegate to V1 viaevaluateSpecialForm;ExprSetSimplifiedis not subsumed; V2 has no memo registry of its own, soclearMemo-style explicit invalidation isn't mirrored yet (naturalweak_ptridentity invalidation still works). No compiler or function-registry changes.
Changes visible on the V1 path are limited to four new
Expraccessors(
multiplyReferencedFields,listeners,trackCpuUsage,outputTracer), aSpecialFormKind-tag dispatch inExpr.cpp, theFilterProjectrecycling,the
translateToInnerRowsearly exit, and the Spark CAST hooks factory.