perf: optimize date_trunc#23542
Conversation
date_trunc
|
run benchmarks date_trunc |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing auto-opt/date_trunc-datafusion-20260713-184055 (bce729e) to 3a29d6b (merge-base) diff using: date_trunc File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagedate_trunc — base (merge-base)
date_trunc — branch
File an issue against this benchmark runner |
|
🤔 As main does not have the benchmarks, the bot cannot perform the comparison. I run the benches myself and can replicate locally, so it's all good 👍 |
| fn civil_from_days(days: i64) -> (i64, i64, i64) { | ||
| let z = days + DAYS_EPOCH_SHIFT; | ||
| let era = z.div_euclid(DAYS_PER_ERA); | ||
| let day_of_era = z.rem_euclid(DAYS_PER_ERA); | ||
| let year_of_era = (day_of_era - day_of_era / 1460 + day_of_era / 36524 | ||
| - day_of_era / 146_096) | ||
| / 365; | ||
| let day_of_year = | ||
| day_of_era - (365 * year_of_era + year_of_era / 4 - year_of_era / 100); | ||
| // Month index with March as 0, so that the leap day falls at the end of the year. | ||
| let month_index = (5 * day_of_year + 2) / 153; | ||
| let day = day_of_year - (153 * month_index + 2) / 5 + 1; | ||
| let month = if month_index < 10 { | ||
| month_index + 3 | ||
| } else { | ||
| _date_trunc_coarse::<NaiveDateTime>(granularity, None)?; | ||
| Ok(None) | ||
| } | ||
| month_index - 9 | ||
| }; | ||
| let year = year_of_era + era * 400 + i64::from(month <= 2); | ||
| (year, month, day) | ||
| } | ||
|
|
There was a problem hiding this comment.
This math looks correct. Is there any link we can put in the comment so that people and robots can have a reference in case this code needs some maintenance?
For example, I found these:
|
Nice, thanks @andygrove! |
|
Thanks for the review @gabotechs! |
Which issue does this PR close?
N/A
Rationale for this change
Optimize existing expression.
What changes are included in this PR?
Replaced the per-row chrono NaiveDateTime round trip in the untimezoned coarse path of date_trunc (week/month/quarter/year) with integer civil-calendar arithmetic, cutting several allocationless-but-validating chrono field setters down to a few integer ops per row.
Are these changes tested?
Existing tests.
Benchmark (criterion):
Full criterion output:
Are there any user-facing changes?