Skip to content

Commit 0187a79

Browse files
authored
Improve the performance of ArithmeticDate.until() (unicode-org#7739)
Improve the performnace of the calendar `until()` function. - Tighten the estimation of the number of months between two dates in order to reduce the number of incremental `surpasses()` tests. This is done by introducing a new DateFieldsResolver function, `min_months_from()`, which the Coptic, Ethiopic, and Hebrew calendar implementations customize. - Eliminate the `surpasses()` loop for the final days delta calculation. Instead compute an intermediate date, which is the start date plus the years, months, and weeks computed so far. Subtract the Rata Die of the intermediate date from that of the target date to get the days field of the DateDuration. - Fix minor typos in function names and comments. Performance, as measured using criterion, is improved for Years and Months intervals. Improvements on the order of 20% to 80%, or 100s of ns to >1 ms, are shown, for base times of 500 ns to 5 ms. For Weeks and Days, which are already computed efficiently using RD subtraction, criterion measures a slight slowdown (on the order of 1 ns, for a 15-20 ns operation). This is most likely due to a change in compiler inlining, optimization, register usage, etc., since the code for Days/Weeks has not changed. <!-- Thank you for your pull request to ICU4X! Reminder: try to use [Conventional Comments](https://conventionalcomments.org/) to make comments clearer. Please see https://github.com/unicode-org/icu4x/blob/main/CONTRIBUTING.md for general information on contributing to ICU4X. -->
1 parent 6cb032d commit 0187a79

File tree

8 files changed

+49
-12
lines changed

8 files changed

+49
-12
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ Fully filled in up to 30c187f4b7
2424
- Deprecate `Date::new_from_iso`/`Date::to_iso` (unicode-org#7287)
2525
- Optimize Hebrew and Julian calendars (unicode-org#7213)
2626
- Optimize day/week diffing to use RDs (unicode-org#7308)
27+
- Optimize `until` month and day calculation performance
2728
- `icu_casemap`
2829
- General changes only
2930
- `icu_collections`

components/calendar/benches/until.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ fn bench_calendar<C: Copy + Calendar>(
3737
}
3838
}
3939

40-
fn convert_benches(c: &mut Criterion) {
40+
fn until_benches(c: &mut Criterion) {
4141
let mut group = c.benchmark_group("until/years");
4242
bench_all_calendars!(group, bench_calendar, DateDurationUnit::Years);
4343
group.finish();
@@ -55,5 +55,5 @@ fn convert_benches(c: &mut Criterion) {
5555
group.finish();
5656
}
5757

58-
criterion_group!(benches, convert_benches);
58+
criterion_group!(benches, until_benches);
5959
criterion_main!(benches);

components/calendar/src/cal/coptic.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,11 @@ impl DateFieldsResolver for Coptic {
6767
13
6868
}
6969

70+
#[inline]
71+
fn min_months_from_inner(_start: Self::YearInfo, years: i64) -> i64 {
72+
13 * years
73+
}
74+
7075
#[inline]
7176
fn extended_year_from_era_year_unchecked(
7277
&self,

components/calendar/src/cal/east_asian_traditional.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,13 @@ impl<R: Rules> DateFieldsResolver for EastAsianTraditional<R> {
587587
12 + year.packed.leap_month().is_some() as u8
588588
}
589589

590+
#[inline]
591+
fn min_months_from_inner(_start: Self::YearInfo, years: i64) -> i64 {
592+
// A lunisolar leap month is inserted at least every 3 years. Reingold denies
593+
// that the Chinese calendar determines leap years using the 19-year Metonic cycle.
594+
12 * years + (years / 3)
595+
}
596+
590597
#[inline]
591598
fn extended_year_from_era_year_unchecked(
592599
&self,

components/calendar/src/cal/ethiopian.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,11 @@ impl DateFieldsResolver for Ethiopian {
9494
Coptic::months_in_provided_year(year)
9595
}
9696

97+
#[inline]
98+
fn min_months_from_inner(start: Self::YearInfo, years: i64) -> i64 {
99+
Coptic::min_months_from_inner(start, years)
100+
}
101+
97102
#[inline]
98103
fn extended_year_from_era_year_unchecked(
99104
&self,

components/calendar/src/cal/hebrew.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,12 @@ impl DateFieldsResolver for Hebrew {
134134
12 + year.keviyah.is_leap() as u8
135135
}
136136

137+
#[inline]
138+
fn min_months_from_inner(_start: HebrewYear, years: i64) -> i64 {
139+
// There are 7 leap years in every 19-year Metonic cycle.
140+
235 * years / 19
141+
}
142+
137143
#[inline]
138144
fn extended_year_from_era_year_unchecked(
139145
&self,

components/calendar/src/calendar_arithmetic.rs

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,14 @@ pub(crate) trait DateFieldsResolver: Calendar {
235235
12
236236
}
237237

238+
/// The minimum number of months over `years` years, starting from the given year.
239+
///
240+
/// The default impl is for non-lunisolar calendars with 12 months!
241+
#[inline]
242+
fn min_months_from_inner(_start: Self::YearInfo, years: i64) -> i64 {
243+
12 * years
244+
}
245+
238246
/// Calculates the ordinal month for the given year and month code.
239247
///
240248
/// The default impl is for non-lunisolar calendars!
@@ -902,7 +910,7 @@ impl<C: DateFieldsResolver> ArithmeticDate<C> {
902910

903911
/// Implements the Temporal abstract operation `NonISODateUntil`.
904912
///
905-
/// This takes a duration (`self`) and a date (`other`), then returns a duration that, when
913+
/// This takes two dates (`self` and `other`), then returns a duration that, when
906914
/// added to `self`, results in `other`, with largest unit according to `options`.
907915
pub(crate) fn until(
908916
&self,
@@ -992,14 +1000,12 @@ impl<C: DateFieldsResolver> ArithmeticDate<C> {
9921000
let mut candidate_months = sign;
9931001

9941002
if options.largest_unit == Some(DateDurationUnit::Months) && min_years != 0 {
995-
// Optimization: No current calendar supports years with month length < 12.
996-
// If something is at least N full years away, it is also at least 12*N full months away.
997-
//
998-
// In the future we can introduce per-calendar routines that are better at estimating a month count.
999-
//
1000-
// We only need to apply this optimization for largest_unit = Months. If the largest_unit is years then
1001-
// our candidate date is already pretty close and won't need more than 12 iterations to get there.
1002-
let min_months = min_years * 12;
1003+
// If largest_unit = Months, then compute the calendar-specific minimum number of
1004+
// months corresponding to min_years. For solar calendars, this is 12 * min_years.
1005+
// For the Hebrew calendar, a leap month is added for 7 out of 19 years. East Asian
1006+
// Calendars do not provide a specialized implementation of `min_months_from()`
1007+
// because it would be too expensive to calculate; they default to 12 * min_years.
1008+
let min_months = self.min_months_from(min_years);
10031009
debug_assert!(!self.surpasses(
10041010
other,
10051011
DateDuration::from_signed_ymwd(years, min_months, 0, 0),
@@ -1038,6 +1044,7 @@ impl<C: DateFieldsResolver> ArithmeticDate<C> {
10381044
candidate_weeks += sign;
10391045
}
10401046
}
1047+
10411048
// 1. Let _days_ be 0.
10421049
// 1. Let _candidateDays_ be _sign_.
10431050
// 1. Repeat, while NonISODateSurpasses(_calendar_, _sign_, _one_, _two_, _years_, _months_, _weeks_, _candidateDays_) is *false*,
@@ -1057,9 +1064,15 @@ impl<C: DateFieldsResolver> ArithmeticDate<C> {
10571064
days = candidate_days;
10581065
candidate_days += sign;
10591066
}
1067+
10601068
// 1. Return ! CreateDateDurationRecord(_years_, _months_, _weeks_, _days_).
10611069
DateDuration::from_signed_ymwd(years, months, weeks, days)
10621070
}
1071+
1072+
/// The minimum number of months over `years` years, starting from `self.year()`.
1073+
pub(crate) fn min_months_from(self, years: i64) -> i64 {
1074+
C::min_months_from_inner(self.year(), years)
1075+
}
10631076
}
10641077

10651078
#[cfg(test)]

components/calendar/src/options.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ mod unstable {
202202
/// let mut options_days = options_default;
203203
/// options_days.largest_unit = Some(DateDurationUnit::Days);
204204
/// assert_eq!(
205-
/// d1.try_until_with_options(&d2, options_default).unwrap(),
205+
/// d1.try_until_with_options(&d2, options_days).unwrap(),
206206
/// DateDuration::for_days(410)
207207
/// );
208208
///

0 commit comments

Comments
 (0)