You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Follow-up to #5363. The Scala end-to-end microbenchmarks conflate expression cost with scan and columnar-to-row transfer, so a Rust-side Criterion bench is the right tool for actually attributing per-row time to an expression. We already have a bench harness under native/spark-expr/benches/, but coverage is ad hoc.
This issue is the catalog: what native expressions exist, which have Criterion benches, which don't, and a proposal for filling the gap deliberately (not one bench per one expression — there are hundreds of thin wrappers around identical shapes).
Scope: what counts as a "native expression"
Two sources reach native code from Comet:
Comet-owned kernels in native/spark-expr/src/. These have their own Rust implementations that Comet is responsible for.
DataFusion / datafusion-spark passthroughs dispatched by function name via CometScalarFunction(...) in spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala. Comet's responsibility here is limited to argument massaging and correctness gating — the kernel itself is upstream.
For (2), a bench inside Comet's tree is redundant with upstream benches for the kernel itself, but is useful when Comet wraps the passthrough with pre/post processing (input coercion, timezone handling, NULL semantics) or when the compat gate (allow_incompat) selects a different path than the plain DF call. The catalog below treats those distinctly.
Plus native/core/benches/array_element_append.rs and perf.rs.
Coverage matrix — Comet-owned kernels
Categories mirror the directory layout under native/spark-expr/src/. [x] = bench exists, [ ] = gap. Passthrough entries are shown separately at the end.
cast_string_to_numeric (there is cast_from_string but the string-to-numeric path should be exercised across all target widths)
cast_timestamp_to_string, cast_date_to_string
cast_timestamp_to_date, cast_date_to_timestamp
cast_decimal_to_decimal (rescale is covered, but full cast between decimal widths is not)
trim variants that route through conversion_funcs/trim.rs (leading/trailing/both, custom trim strings) — passthrough today, but Comet's serde does the coercion
csv_funcs
to_csv
csv_to_structs (from_csv) — the more expensive direction, unbenched
hours / hour / minute / second / day_of_month / day_of_week / day_of_year / week_of_year / week_day / quarter / year / month — most are thin, but extract_date_part.rs centralizes them and is worth one parameterized bench across the field list
next_day, last_day, add_months, months_between
from_utc_timestamp, to_utc_timestamp, convert_timezone — timezone handling has been a repeat perf hotspot
xpath family (XPathBoolean, XPathShort, XPathInt, XPathLong, XPathFloat, XPathDouble, XPathString, XPathList) — 8 entries, likely one parameterized bench
bloom_filter_might_contain (probe side; agg side is covered)
char_varchar_utils (static_invoke) — pad/trim for CHAR/VARCHAR semantics
Coverage matrix — DataFusion passthroughs
40 expressions dispatched by name via CometScalarFunction. The kernel itself is upstream. Comet-owned coverage here should focus on the passthroughs whose serde does non-trivial work or where Comet has a compat gate. Everything else can be left to upstream benches.
Passthroughs listed for completeness (from QueryPlanSerde.scala):
Math (trig / transcendentals):acos, acosh, asin, asinh, atan, atanh, cbrt, cos, cosh, cot, csc, degrees, exp, expm1, factorial, greatest, least, pi, radians, rint, sec, signum, sin, sinh, sqrt, tan, tanh, bin. Skip individually. Add one parameterized "trig fastpath" bench that runs the whole set on Float64 with and without nulls, so any regression across the whole family surfaces at once.
Hash:crc32, md5. Bench both. Small kernels but on the hot path for many workloads.
Strings:ascii, char, instr, space, trim, ltrim, rtrim. Bench trim variants (Comet's serde does whitespace / custom-trim-char routing); others can go in the parameterized string-fastpath bench.
Arrays:array_distinct, array_repeat. Bench both — non-trivial per-row work.
Bitwise:shiftrightunsigned. Skip; trivial.
Proposal
Not one bench per expression. Concretely:
A shared harness — native/spark-expr/benches/common.rs (file already exists but is empty) with helpers for building RecordBatches with deterministic seeded data at a standard set of row counts (e.g. 8k / 64k / 512k) and typical null ratios (0%, 10%, 100%). Every new bench uses those builders. Fixes the reproducibility problem the Scala benches have.
Parameterized "family" benches for expressions that share a shape. Trig fastpath is one. Extract-date-part fields is another (14 date-part accessors → one bench with BenchmarkId per field). String-fastpath (ascii, chr, space, length, octet_length, bit_length) is another. Cuts total bench code by ~5×.
Individual benches for the expressions listed with [ ] above where perf is non-trivial, has a compat gate, or has been a historical hotspot. Prioritize in this order:
P0 (hot path for typical Spark workloads):murmur3, xxhash64, abs, upper/lower/init_cap (both compat modes), contains/starts_with/ends_with, regexp_replace, substring, timezone conversions (from_utc_timestamp, to_utc_timestamp, convert_timezone), unix_timestamp, timestamp_trunc, is_nan, rlike (both native and JVM modes), the remaining aggregates in agg_funcs.
P1 (correctness-gated / has known variants):from_json, csv_to_structs, cast timestamp/date crossings, decimal-to-decimal cast, unbase64, mask, to_number / try_to_number.
P2 (thin wrappers): everything else — most array_funcs, map_funcs, struct_funcs, xpath (one parameterized bench for the family), format_number/format_string, sound_ex, find_in_set, string_locate, elt, reverse, empty2null, misc math.
A coverage script in CI — native/spark-expr/benches/coverage.sh (or a small Rust test) that reads the list of expression classes registered in QueryPlanSerde.scala and asserts each one either has a bench file, appears in a parameterized-family allowlist, or is on an explicit skip-list with a reason. This is what prevents the gap from reopening. Failing this gate should be a merge blocker.
A README section in native/spark-expr/benches/ documenting the convention (harness, naming, parameterization rules, skip-list) so contributors adding a new expression know a bench is expected.
Rough size estimate: ~40 new bench files needed to cover the P0+P1 list, if each is ~150 lines and reuses the shared harness. That is one focused sprint of work, or drip-fed alongside the individual expression changes as they come up. The coverage script is what makes the drip-feed viable.
Explicitly out of scope: the Scala end-to-end microbenchmarks. Those have separate problems (see #5363) and Criterion benches don't replace them because they don't exercise the JNI or serde boundary.
Background
Follow-up to #5363. The Scala end-to-end microbenchmarks conflate expression cost with scan and columnar-to-row transfer, so a Rust-side Criterion bench is the right tool for actually attributing per-row time to an expression. We already have a bench harness under
native/spark-expr/benches/, but coverage is ad hoc.This issue is the catalog: what native expressions exist, which have Criterion benches, which don't, and a proposal for filling the gap deliberately (not one bench per one expression — there are hundreds of thin wrappers around identical shapes).
Scope: what counts as a "native expression"
Two sources reach native code from Comet:
native/spark-expr/src/. These have their own Rust implementations that Comet is responsible for.CometScalarFunction(...)inspark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala. Comet's responsibility here is limited to argument massaging and correctness gating — the kernel itself is upstream.For (2), a bench inside Comet's tree is redundant with upstream benches for the kernel itself, but is useful when Comet wraps the passthrough with pre/post processing (input coercion, timezone handling, NULL semantics) or when the compat gate (
allow_incompat) selects a different path than the plain DF call. The catalog below treats those distinctly.Existing Criterion benches
40 bench files under
native/spark-expr/benches/:aggregate,array_size,arrays_overlap,base64,bloom_filter_agg,cast_binary_to_string,cast_decimal_to_string,cast_float_to_decimal,cast_float_to_string,cast_from_boolean,cast_from_string,cast_int_to_decimal,cast_int_to_timestamp,cast_non_int_numeric_timestamp,cast_numeric,cast_string_to_date,cast_string_to_timestamp,ceil,check_overflow,checked_arithmetic,conditional,date_trunc,date_trunc_array_fmt,decimal_div,decimal_rescale,floor,get_json_object,make_decimal,map_sort,normalize_nan,padding,parse_url,regexp_extract,round,to_csv,to_json,to_time,unhex,unscaled_value,wide_decimalPlus
native/core/benches/array_element_append.rsandperf.rs.Coverage matrix — Comet-owned kernels
Categories mirror the directory layout under
native/spark-expr/src/.[x]= bench exists,[ ]= gap. Passthrough entries are shown separately at the end.agg_funcsavg_decimal— covered byaggregate.rssum_decimal— covered byaggregate.rssum_intavg(non-decimal)approx_percentilepercentilecorrelationcovariance(pop, samp)variance(pop, samp) /stddev(pop, samp) — the Welford pathhll_plus_plusbloom_filter_aggfirst,lastcollect_list,collect_setbit_and_agg,bit_or_agg,bit_xor_aggarray_funcsarrays_overlaparray_size(size)array_insertarray_positionarray_slicearrays_zipflattenget_array_struct_fieldslist_extractsort_arrayarray_join,array_max,array_min,array_remove,array_intersect,array_union,array_except,array_contains,array_transform,array_exists,array_forall,array_aggregate,array_sort,zip_with,sequence,shuffle,create_array,get_array_item,element_at(all reachable via serde entries inarrayExpressions)conditional_funcsif_expr— covered byconditional.rscase_when— covered byconditional.rsconversion_funcs(cast)Well-covered already:
cast_numeric,cast_from_string,cast_from_boolean,cast_int_to_decimal,cast_float_to_decimal,cast_int_to_timestamp,cast_string_to_date,cast_string_to_timestamp,cast_non_int_numeric_timestamp,cast_float_to_string,cast_binary_to_string,cast_decimal_to_stringGaps:
cast_string_to_numeric(there iscast_from_stringbut the string-to-numeric path should be exercised across all target widths)cast_timestamp_to_string,cast_date_to_stringcast_timestamp_to_date,cast_date_to_timestampcast_decimal_to_decimal(rescale is covered, but full cast between decimal widths is not)trimvariants that route throughconversion_funcs/trim.rs(leading/trailing/both, custom trim strings) — passthrough today, but Comet's serde does the coercioncsv_funcsto_csvcsv_to_structs(from_csv) — the more expensive direction, unbencheddatetime_funcsdate_trunc,date_trunc_array_fmt,to_timetimestamp_truncunix_timestamp(timestamp input, date input)from_unix_timedate_add,date_sub,date_diffdate_from_unix_date,unix_datemake_date,make_time,make_interval,make_timestamp,make_ym_interval,make_dt_interval,multiply_dt_intervalhours/hour/minute/second/day_of_month/day_of_week/day_of_year/week_of_year/week_day/quarter/year/month— most are thin, butextract_date_part.rscentralizes them and is worth one parameterized bench across the field listnext_day,last_day,add_months,months_betweenfrom_utc_timestamp,to_utc_timestamp,convert_timezone— timezone handling has been a repeat perf hotspotseconds_to_timestamp,micros_to_timestamp,millis_to_timestamptimestamp_add,timestamp_diffhash_funcsmurmur3— used per row on every hash-partitioned shuffle key; regressions here hit shuffle throughput directlyxxhash64json_funcsto_json,get_json_objectfrom_jsonjson_array_lengthjson_object_keys,schema_of_jsonmap_funcsmap_sortmap_extract(get_map_value),map_keys,map_values,map_entries,map_from_arrays,map_from_entries,map_concat,str_to_map,map_filter,transform_keys,transform_values,map_zip_with,create_mapmath_funcsceil,floor,round,unhex,checked_arithmetic,decimal_div,decimal_rescale,unscaled_value,wide_decimal,check_overflow,normalize_nan,make_decimalabslog,log10,log2,log1p,logarithmpow,hypotmodulo(pmod,remainder)negative/unary_minuswidth_bucketconv,hexbroundnanvlnondetermenistic_funcsTiming is meaningful for the RNG path even though the values are random:
rand,randn,rand_struuidbernoulli_cell_sampler,shufflemonotonically_increasing_idpredicate_funcsis_nanrlike— critical because it has native Rust and JVM fallback modes; regression here shows up in every LIKE-heavy querystring_funcsbase64,regexp_extract,padding(lpad/rpad)unbase64— has its own kernelcontains,starts_with,ends_withlength,octet_length,bit_length(thin, but common)levenshteinregexp_extract_all,regexp_in_str,regexp_replacesplitupper,lower,init_cap— these areallow_incompatgated; the two paths (built-in ICU vs Comet's) should both be benchedsubstring,substring_index,left,rightoverlaymask,sound_ex,format_number,format_stringfind_in_set,string_locateeltto_number,try_to_number,to_characterreverse(string variant; array variant separately)empty2nullstruct_funcscreate_named_structget_struct_fieldurl_funcsparse_urlMisc /
static_invoke/ kernelsxpathfamily (XPathBoolean,XPathShort,XPathInt,XPathLong,XPathFloat,XPathDouble,XPathString,XPathList) — 8 entries, likely one parameterized benchbloom_filter_might_contain(probe side; agg side is covered)char_varchar_utils(static_invoke) — pad/trim for CHAR/VARCHAR semanticsCoverage matrix — DataFusion passthroughs
40 expressions dispatched by name via
CometScalarFunction. The kernel itself is upstream. Comet-owned coverage here should focus on the passthroughs whose serde does non-trivial work or where Comet has a compat gate. Everything else can be left to upstream benches.Passthroughs listed for completeness (from
QueryPlanSerde.scala):acos,acosh,asin,asinh,atan,atanh,cbrt,cos,cosh,cot,csc,degrees,exp,expm1,factorial,greatest,least,pi,radians,rint,sec,signum,sin,sinh,sqrt,tan,tanh,bin. Skip individually. Add one parameterized "trig fastpath" bench that runs the whole set on Float64 with and without nulls, so any regression across the whole family surfaces at once.crc32,md5. Bench both. Small kernels but on the hot path for many workloads.ascii,char,instr,space,trim,ltrim,rtrim. Benchtrimvariants (Comet's serde does whitespace / custom-trim-char routing); others can go in the parameterized string-fastpath bench.array_distinct,array_repeat. Bench both — non-trivial per-row work.shiftrightunsigned. Skip; trivial.Proposal
Not one bench per expression. Concretely:
A shared harness —
native/spark-expr/benches/common.rs(file already exists but is empty) with helpers for buildingRecordBatches with deterministic seeded data at a standard set of row counts (e.g. 8k / 64k / 512k) and typical null ratios (0%, 10%, 100%). Every new bench uses those builders. Fixes the reproducibility problem the Scala benches have.Parameterized "family" benches for expressions that share a shape. Trig fastpath is one. Extract-date-part fields is another (14 date-part accessors → one bench with
BenchmarkIdper field). String-fastpath (ascii,chr,space,length,octet_length,bit_length) is another. Cuts total bench code by ~5×.Individual benches for the expressions listed with
[ ]above where perf is non-trivial, has a compat gate, or has been a historical hotspot. Prioritize in this order:murmur3,xxhash64,abs,upper/lower/init_cap(both compat modes),contains/starts_with/ends_with,regexp_replace,substring, timezone conversions (from_utc_timestamp,to_utc_timestamp,convert_timezone),unix_timestamp,timestamp_trunc,is_nan,rlike(both native and JVM modes), the remaining aggregates inagg_funcs.from_json,csv_to_structs, cast timestamp/date crossings, decimal-to-decimal cast,unbase64,mask,to_number/try_to_number.array_funcs,map_funcs,struct_funcs,xpath(one parameterized bench for the family),format_number/format_string,sound_ex,find_in_set,string_locate,elt,reverse,empty2null, misc math.A coverage script in CI —
native/spark-expr/benches/coverage.sh(or a small Rust test) that reads the list of expression classes registered inQueryPlanSerde.scalaand asserts each one either has a bench file, appears in a parameterized-family allowlist, or is on an explicit skip-list with a reason. This is what prevents the gap from reopening. Failing this gate should be a merge blocker.A README section in
native/spark-expr/benches/documenting the convention (harness, naming, parameterization rules, skip-list) so contributors adding a new expression know a bench is expected.Rough size estimate: ~40 new bench files needed to cover the P0+P1 list, if each is ~150 lines and reuses the shared harness. That is one focused sprint of work, or drip-fed alongside the individual expression changes as they come up. The coverage script is what makes the drip-feed viable.
Explicitly out of scope: the Scala end-to-end microbenchmarks. Those have separate problems (see #5363) and Criterion benches don't replace them because they don't exercise the JNI or serde boundary.