chronotope / chrono

Date and time library for Rust
Other
3.3k stars 523 forks source link

Make `DelayedFormat` hold a generic offset #1559

Open pitdicker opened 6 months ago

pitdicker commented 6 months ago

Just learned that adding a type parameter is a backwards-compatible change if it has a default type, thanks to RFC 0213.

That means we can fix the biggest remaining performance problem with DelayedFormat, and make our formatting work without allocations! :tada:

In this PR I only added an extra generic parameter to DelayedFormat. Making our formatting work without allocating needs one more tricky thing that is best for another PR.

To quote from #1163:

Our current formatting code has noticeable overhead (see #94). This is mostly caused by one design choice: if DelayedFormat::new is given an offset, it first formats a timezone name into a String. In case of DateTime<FixedOffset> and DateTime<Local> this involves formatting the offset, as there is no name available. This is responsible for 20~25% of the ca. 40% overhead in the format_with_items benchmark.

By taking the offset as a generic we can delay formatting the timezone name until it is needed, which often is never.

To keep the deprecated format::{format, format_items} functions working (which expect a String and FixedOffset instead of a generic offset) I had to add an OffsetWrapper type.

Benchmarks

     Running benches/chrono.rs (target/release/deps/chrono-413c84d256ba35f5)
bench_format            time:   [467.24 ns 470.63 ns 475.26 ns]
                        change: [-16.270% -15.552% -14.816%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
  5 (5.00%) high mild
  5 (5.00%) high severe

bench_format_with_items time:   [323.83 ns 325.30 ns 326.94 ns]
                        change: [-18.296% -17.524% -16.769%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
  4 (4.00%) high mild
  4 (4.00%) high severe
codecov[bot] commented 6 months ago

Codecov Report

Attention: Patch coverage is 65.85366% with 14 lines in your changes are missing coverage. Please review.

Project coverage is 91.77%. Comparing base (0cfc405) to head (30735ee).

Files Patch % Lines
src/format/formatting.rs 69.69% 10 Missing :warning:
src/date.rs 0.00% 4 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1559 +/- ## ========================================== - Coverage 91.80% 91.77% -0.04% ========================================== Files 37 37 Lines 18148 18149 +1 ========================================== - Hits 16661 16656 -5 - Misses 1487 1493 +6 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.