Conversation
- Fixed-offset timezones (UTC and +HH[:MM]) truncate by shifting by the offset and reusing the integer calendar arithmetic instead of per-value chrono conversions (~30x on hour granularity). Fine granularities and hour/day are handled with vectorized arithmetic; coarse granularities (week/month/quarter/year) truncate the shifted value per value. Named IANA timezones keep the chrono path. - Fix eager ok_or error allocation in the per-value chrono path. - Extend the criterion bench to cover fixed offsets, DST zones and 1M rows.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #25326 +/- ##
========================================
Coverage 81.92% 81.93%
========================================
Files 1134 1134
Lines 426000 426111 +111
Branches 426000 426111 +111
========================================
+ Hits 349000 349117 +117
+ Misses 56304 56289 -15
- Partials 20696 20705 +9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
namanjain24-sudo
left a comment
namanjain24-sudo
left a comment
There was a problem hiding this comment.
Nice speedup. I checked the fixed-offset path against the existing chrono path on 672k random values (offsets +05:45, -09:30, +14:00, -12:00, +00:00, UTC, -03, +0530; minute through year; ns/ms/s) and got no differences.
One overflow issue inline. Also, the branch now conflicts with main in date_trunc.rs (#25430 and #25409 changed it since), and my #25498 moves the fine-granularity unit table into a shared helper, so whichever of us lands second will need a small rebase.
| maybe_underflow |= | ||
| *value < add_lower || *value > add_upper || *value < underflow_bound; | ||
| let shifted = value.wrapping_add(offset_in_unit); | ||
| shifted.wrapping_sub(shifted.rem_euclid(unit)) - offset_in_unit |
There was a problem hiding this comment.
This - offset_in_unit isn't wrapping, and it runs for every value before maybe_underflow is checked, so it panics in debug builds near the ends of the range. On this branch, date_trunc('hour', ...) over a Timestamp(Second, "+05:00") column holding i64::MIN or i64::MAX panics here with attempt to subtract with overflow; on main the same input returns Timestamp ... out of range.
Since those values already set maybe_underflow and go through the checked path, .wrapping_sub(offset_in_unit) should be enough. Could be worth adding them to the underflow tests too.
3 participants
What I did
After the optimization:
date_trunc_hour_notz_nanos_100000 time: [525.81 µs 527.52 µs 528.95 µs] change: [−2.8681% −2.6375% −2.3945%] (p = 0.00 < 0.05) Performance has improved. date_trunc_hour_utc_nanos_100000 time: [530.68 µs 530.85 µs 531.06 µs] change: [−96.516% −96.512% −96.507%] (p = 0.00 < 0.05) Performance has improved. Found 8 outliers among 100 measurements (8.00%) 6 (6.00%) high mild 2 (2.00%) high severe Benchmarking date_trunc_hour_fixed_offset_nanos_100000: Collecting 100 samples in estimated 5.3867 s (10k itera date_trunc_hour_fixed_offset_nanos_100000 time: [521.82 µs 523.58 µs 525.67 µs] change: [−96.560% −96.549% −96.540%] (p = 0.00 < 0.05) Performance has improved. Benchmarking date_trunc_hour_dst_zone_nanos_100000: Collecting 100 samples in estimated 5.7956 s (700 iteration date_trunc_hour_dst_zone_nanos_100000 time: [8.3345 ms 8.3677 ms 8.4027 ms] change: [−60.520% −60.351% −60.178%] (p = 0.00 < 0.05) Performance has improved. date_trunc_day_utc_nanos_100000 time: [466.83 µs 468.50 µs 470.41 µs] change: [−96.919% −96.911% −96.903%] (p = 0.00 < 0.05) Performance has improved. Found 22 outliers among 100 measurements (22.00%) 19 (19.00%) low severe 3 (3.00%) high mild Benchmarking date_trunc_day_fixed_offset_nanos_100000: Collecting 100 samples in estimated 7.1836 s (15k iterat date_trunc_day_fixed_offset_nanos_100000 time: [472.41 µs 472.57 µs 472.75 µs] change: [−96.953% −96.949% −96.944%] (p = 0.00 < 0.05) Performance has improved. Found 7 outliers among 100 measurements (7.00%) 7 (7.00%) high mild date_trunc_minute_notz_nanos_100000 time: [602.10 µs 602.50 µs 602.94 µs] change: [−1.3404% −1.2577% −1.1705%] (p = 0.00 < 0.05) Performance has improved. Found 1 outliers among 100 measurements (1.00%) 1 (1.00%) high mild date_trunc_minute_utc_nanos_100000 time: [601.23 µs 601.58 µs 601.98 µs] change: [−1.6199% −1.5341% −1.4394%] (p = 0.00 < 0.05) Performance has improved. Found 9 outliers among 100 measurements (9.00%) 2 (2.00%) low mild 6 (6.00%) high mild 1 (1.00%) high severe Benchmarking date_trunc_minute_dst_zone_nanos_100000: Collecting 100 samples in estimated 6.0929 s (10k iterati date_trunc_minute_dst_zone_nanos_100000 time: [601.72 µs 601.99 µs 602.31 µs] change: [−1.5949% −1.5300% −1.4665%] (p = 0.00 < 0.05) Performance has improved. Found 7 outliers among 100 measurements (7.00%) 2 (2.00%) high mild 5 (5.00%) high severe date_trunc_week_notz_nanos_100000 time: [702.79 µs 703.29 µs 703.94 µs] change: [+1.3167% +1.4981% +1.6598%] (p = 0.00 < 0.05) Performance has regressed. Found 11 outliers among 100 measurements (11.00%) 2 (2.00%) low mild 3 (3.00%) high mild 6 (6.00%) high severe Benchmarking date_trunc_month_notz_nanos_100000: Warming up for 3.0000 s Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 6.9s, enable flat sampling, or reduce sample count to 50. date_trunc_month_notz_nanos_100000 time: [1.3708 ms 1.3716 ms 1.3724 ms] change: [+2.7066% +2.8029% +2.8938%] (p = 0.00 < 0.05) Performance has regressed. Found 6 outliers among 100 measurements (6.00%) 4 (4.00%) high mild 2 (2.00%) high severe Benchmarking date_trunc_quarter_notz_nanos_100000: Collecting 100 samples in estimated 5.0390 s (2300 iteration date_trunc_quarter_notz_nanos_100000 time: [2.1888 ms 2.1895 ms 2.1902 ms] change: [−0.2713% −0.2060% −0.1445%] (p = 0.00 < 0.05) Change within noise threshold. Found 8 outliers among 100 measurements (8.00%) 2 (2.00%) low mild 4 (4.00%) high mild 2 (2.00%) high severe Benchmarking date_trunc_year_notz_nanos_100000: Warming up for 3.0000 s Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 8.4s, enable flat sampling, or reduce sample count to 50. date_trunc_year_notz_nanos_100000 time: [1.6574 ms 1.6583 ms 1.6595 ms] change: [+0.8459% +1.0083% +1.1853%] (p = 0.00 < 0.05) Change within noise threshold. Found 11 outliers among 100 measurements (11.00%) 1 (1.00%) low mild 4 (4.00%) high mild 6 (6.00%) high severe Benchmarking date_trunc_month_fixed_offset_nanos_100000: Warming up for 3.0000 s Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 7.0s, enable flat sampling, or reduce sample count to 50. Benchmarking date_trunc_month_fixed_offset_nanos_100000: Collecting 100 samples in estimated 6.9700 s (5050 ite date_trunc_month_fixed_offset_nanos_100000 time: [1.3531 ms 1.3578 ms 1.3627 ms] change: [−91.250% −91.217% −91.180%] (p = 0.00 < 0.05) Performance has improved. Found 20 outliers among 100 measurements (20.00%) 7 (7.00%) low severe 4 (4.00%) low mild 6 (6.00%) high mild 3 (3.00%) high severe Benchmarking date_trunc_month_dst_zone_nanos_100000: Collecting 100 samples in estimated 5.0534 s (600 iteratio date_trunc_month_dst_zone_nanos_100000 time: [8.3957 ms 8.4031 ms 8.4111 ms] change: [−60.986% −60.940% −60.897%] (p = 0.00 < 0.05) Performance has improved. Found 7 outliers among 100 measurements (7.00%) 4 (4.00%) high mild 3 (3.00%) high severeWhich issue does this PR close?
Rationale for this change
What changes are included in this PR?
What is the testing strategy for this PR?
Added an unit test which compares the fast-path results against the results of the normal path.
Are there any user-facing changes?
No.