chrono icon indicating copy to clipboard operation
chrono copied to clipboard

Make `DelayedFormat` hold a generic offset

Open pitdicker opened this issue 1 year ago • 1 comments

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

pitdicker avatar Apr 06 '24 14:04 pitdicker

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
@@            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.

codecov[bot] avatar Apr 06 '24 14:04 codecov[bot]