From 3e592d45658c98acdfdd85b1464fdcbdde6290af Mon Sep 17 00:00:00 2001 From: Gwaihir Thorondorsen Date: Sun, 6 Oct 2019 20:09:23 +0200 Subject: [PATCH 1/2] Test `Datelike::num_days_from_ce` against an alternative implementation --- src/lib.rs | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 12d24f7..c832281 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -968,6 +968,9 @@ pub trait Datelike: Sized { /// assert_eq!(NaiveDate::from_ymd(0, 1, 1).num_days_from_ce(), -365); /// ``` fn num_days_from_ce(&self) -> i32 { + // See test_num_days_from_ce_against_alternative_impl below for a more straightforward + // implementation. + // we know this wouldn't overflow since year is limited to 1/2^13 of i32's full range. let mut year = self.year() - 1; let mut ndays = 0; @@ -1069,3 +1072,52 @@ fn test_readme_doomsday() { assert!(other_dates.iter().all(|d| d.weekday() == weekday)); } } + +/// Tests `Datelike::num_days_from_ce` against an alternative implementation. +/// +/// The alternative implementation is not as short as the current one but it is simpler to +/// understand, with less unexplained magic constants. +#[test] +fn test_num_days_from_ce_against_alternative_impl() { + /// Returns the number of multiples of `div` in the range `start..end`. + /// + /// If the range `start..end` is back-to-front, i.e. `start` is greater than `end`, the + /// behaviour is defined by the following equation: + /// `in_between(start, end, div) == - in_between(end, start, div)`. + /// + /// When `div` is 1, this is equivalent to `end - start`, i.e. the length of `start..end`. + /// + /// # Panics + /// + /// Panics if `div` is not positive. + fn in_between(start: i32, end: i32, div: i32) -> i32 { + assert!(div > 0, "in_between: nonpositive div = {}", div); + let start = (start.div_euclid(div), start.rem_euclid(div)); + let end = ( end.div_euclid(div), end.rem_euclid(div)); + // The lowest multiple of `div` greater than or equal to `start`, divided. + let start = start.0 + (start.1 != 0) as i32; + // The lowest multiple of `div` greater than or equal to `end`, divided. + let end = end.0 + ( end.1 != 0) as i32; + end - start + } + + /// Alternative implementation to `Datelike::num_days_from_ce` + fn num_days_from_ce(date: &Date) -> i32 { + let year = date.year(); + let diff = move |div| in_between(1, year, div); + // 365 days a year, one more in leap years. In the gregorian calendar, leap years are all + // the multiples of 4 except multiples of 100 but including multiples of 400. + date.ordinal() as i32 + 365 * diff(1) + diff(4) - diff(100) + diff(400) + } + + use num_iter::range_inclusive; + + for year in range_inclusive(naive::MIN_DATE.year(), naive::MAX_DATE.year()) { + let jan1_year = NaiveDate::from_ymd(year, 1, 1); + assert_eq!(jan1_year.num_days_from_ce(), num_days_from_ce(&jan1_year), + "on {:?}", jan1_year); + let mid_year = jan1_year + Duration::days(133); + assert_eq!(mid_year.num_days_from_ce(), num_days_from_ce(&mid_year), + "on {:?}", mid_year); + } +} From 536ac8651190e4ff57d2cd86ef7c2035bbbc1ce9 Mon Sep 17 00:00:00 2001 From: Brandon W Maister Date: Mon, 23 Dec 2019 13:46:22 -0500 Subject: [PATCH 2/2] Add bench for new implementation Unfortunately it seems to be slightly more than twice as slow. On the plus side, that is some justification for the existing optimizations. --- benches/chrono.rs | 50 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/benches/chrono.rs b/benches/chrono.rs index 1bd66af..7f1f2b5 100644 --- a/benches/chrono.rs +++ b/benches/chrono.rs @@ -3,7 +3,7 @@ extern crate chrono; extern crate criterion; -use criterion::{black_box, criterion_group, criterion_main, Criterion}; +use criterion::{black_box, criterion_group, criterion_main, Criterion, BenchmarkId}; use chrono::prelude::*; use chrono::{Utc, FixedOffset, DateTime, YearFlags}; @@ -59,6 +59,53 @@ fn bench_year_flags_from_year(c: &mut Criterion) { YearFlags::from_year(year); } })); + +/// Returns the number of multiples of `div` in the range `start..end`. +/// +/// If the range `start..end` is back-to-front, i.e. `start` is greater than `end`, the +/// behaviour is defined by the following equation: +/// `in_between(start, end, div) == - in_between(end, start, div)`. +/// +/// When `div` is 1, this is equivalent to `end - start`, i.e. the length of `start..end`. +/// +/// # Panics +/// +/// Panics if `div` is not positive. +fn in_between(start: i32, end: i32, div: i32) -> i32 { + assert!(div > 0, "in_between: nonpositive div = {}", div); + let start = (start.div_euclid(div), start.rem_euclid(div)); + let end = (end.div_euclid(div), end.rem_euclid(div)); + // The lowest multiple of `div` greater than or equal to `start`, divided. + let start = start.0 + (start.1 != 0) as i32; + // The lowest multiple of `div` greater than or equal to `end`, divided. + let end = end.0 + (end.1 != 0) as i32; + end - start +} + +/// Alternative implementation to `Datelike::num_days_from_ce` +fn num_days_from_ce_alt(date: &Date) -> i32 { + let year = date.year(); + let diff = move |div| in_between(1, year, div); + // 365 days a year, one more in leap years. In the gregorian calendar, leap years are all + // the multiples of 4 except multiples of 100 but including multiples of 400. + date.ordinal() as i32 + 365 * diff(1) + diff(4) - diff(100) + diff(400) +} + +fn bench_num_days_from_ce(c: &mut Criterion) { + let mut group = c.benchmark_group("num_days_from_ce"); + for year in &[1, 500, 2000, 2019] { + let d = NaiveDate::from_ymd(*year, 1, 1); + group.bench_with_input( + BenchmarkId::new("new", year), + &d, + |b, y| b.iter(|| num_days_from_ce_alt(y)), + ); + group.bench_with_input( + BenchmarkId::new("classic", year), + &d, + |b, y| b.iter(|| y.num_days_from_ce()), + ); + } } criterion_group!( @@ -69,6 +116,7 @@ criterion_group!( bench_datetime_to_rfc2822, bench_datetime_to_rfc3339, bench_year_flags_from_year, + bench_num_days_from_ce, ); criterion_main!(benches);