Skip to content

chore: Add benches for datetime funcs - #5767

Merged
coderfender merged 4 commits into
apache:mainfrom
coderfender:bench_datetime_funcs
Sep 8, 2026
Merged

chore: Add benches for datetime funcs#5767
coderfender merged 4 commits into
apache:mainfrom
coderfender:bench_datetime_funcs

Conversation

@coderfender

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Part of #5396

Rationale for this change

Adding comprehensive benches

What changes are included in this PR?

All comet custom implemented date time related benches

How are these changes tested?

Local compilation , format, cargo check and runs

@coderfender coderfender changed the title Bench datetime funcs chore: Add benches for datetime funcs Sep 7, 2026
@coderfender

Copy link
Copy Markdown
Contributor Author

Flexible in terms of consolidating all the benches into a single file as well

@sunchao sunchao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correctness

Reviewed head 921623f0eb121322d03f67d65d985bfd68fd5407 against base 7f1e00189b1ed86f1cb5acd872d97fce694482b1. No verified correctness issue requiring a change before merge. The patch adds ten benchmark targets and three typed input builders; production datetime implementations, Spark serialization and dependency versions are unchanged.

The fixtures use the types required by the existing native implementations: Date32 for date operations, microsecond timestamps for extraction, Decimal128(16,6) seconds for make_time, and six Int32 components plus Float64 seconds for make_interval. The extraction cases distinguish UTC-tagged timestamps converted to America/Los_Angeles from TimestampNTZ inputs. The interval fixture matches the existing native adapter, including its documented compatibility restrictions; it does not establish parity with Spark's default execution path.

The inspected benchmark check compiled the benches and linted all targets. Rust CI passed 1,183 tests with four skipped. Both logs identify merge checkout 23b9d4b4, whose parents are this base/head and whose source tree equals the reviewed head. Local formatting and diff checks passed. No local benchmark or Spark/JNI execution was performed; the CI jobs above did not run the 171 benchmark cases. Canonical Spark context was checked on maintained 3.5/4.0 branches; maintained 3.4/4.1 sources were unavailable.

Performance

This supplies 171 synthetic cases across three batch sizes and three null ratios, with extra variants for date parts, timestamp timezone metadata and numeric input types. Input arrays and UDF instances are constructed outside the timed loop. The UDF cases time invocation together with argument cloning, return-field/configuration construction and output lifecycle, so their results include wrapper overhead. In particular, date_from_unix_date uses an existing zero-copy cast; its batch-size labels do not imply row-by-row computation.

The PR provides no timing table, and the inspected CI establishes compilation and existing-test success only. The fixtures cover ordinary valid values and correlated nulls; they do not measure scalar, error, overflow or fractional-input workloads. These limits are consistent with adding initial native benchmarks, but they provide no evidence of a Spark performance gain.

Design

The change fills a benchmark-coverage gap by exposing existing datetime entry points through independently selectable Criterion targets. Each target keeps its input generation and return type visible, while the existing shared row-count/null-ratio matrix gives repeatable comparisons. Grouping day/month names and hour/minute/second extraction reflects their shared implementations. No production behavior, fallback policy or default setting changes.

Abstraction & complexity

The new Float32, Int32 and Date32 builders follow the existing small array-helper pattern. The benchmark bodies remain straightforward, including the decimal construction needed by make_time; there is no new general runner or dispatch framework. Keeping the targets separate is reasonable despite the author's offer to consolidate them, and I found no abstraction change needed before merge.

@coderfender
coderfender merged commit cb4a261 into apache:main Sep 8, 2026
73 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants