From e0b16e9c5385bbe7a741c93cf78bdeccc4b57fbb Mon Sep 17 00:00:00 2001 From: Eric Sheppard Date: Sat, 22 Oct 2022 22:13:05 +1100 Subject: [PATCH 1/4] add bench for Local::now() --- benches/chrono.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/benches/chrono.rs b/benches/chrono.rs index c0704e33cb..1cb17a1927 100644 --- a/benches/chrono.rs +++ b/benches/chrono.rs @@ -4,7 +4,7 @@ use criterion::{black_box, criterion_group, criterion_main, BenchmarkId, Criterion}; use chrono::prelude::*; -use chrono::{DateTime, FixedOffset, Utc, __BenchYearFlags}; +use chrono::{DateTime, FixedOffset, Local, Utc, __BenchYearFlags}; fn bench_datetime_parse_from_rfc2822(c: &mut Criterion) { c.bench_function("bench_datetime_parse_from_rfc2822", |b| { @@ -56,6 +56,14 @@ fn bench_year_flags_from_year(c: &mut Criterion) { }); } +fn bench_get_local_time(c: &mut Criterion) { + c.bench_function("bench_get_local_time", |b| { + b.iter(|| { + let _ = Local::now(); + }) + }); +} + /// 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 @@ -109,6 +117,7 @@ criterion_group!( bench_datetime_to_rfc3339, bench_year_flags_from_year, bench_num_days_from_ce, + bench_get_local_time, ); criterion_main!(benches); From e46e357ca5c623652d3b71fa7da865ac586ff184 Mon Sep 17 00:00:00 2001 From: Eric Sheppard Date: Sun, 30 Oct 2022 22:30:27 +1100 Subject: [PATCH 2/4] move last_changed to the Cache --- src/offset/local/unix.rs | 65 +++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/src/offset/local/unix.rs b/src/offset/local/unix.rs index 398f07ec2f..1a1bdf888a 100644 --- a/src/offset/local/unix.rs +++ b/src/offset/local/unix.rs @@ -32,7 +32,7 @@ thread_local! { } enum Source { - LocalTime { mtime: SystemTime, last_checked: SystemTime }, + LocalTime { mtime: SystemTime }, // we don't bother storing the contents of the environment variable in this case. // changing the environment while the process is running is generally not reccomended Environment, @@ -53,47 +53,21 @@ impl Default for Source { // by picking SystemTime::now() we raise the probability of // the cache being invalidated if/when the mtime starts working mtime: data.modified().unwrap_or_else(|_| SystemTime::now()), - last_checked: SystemTime::now(), }, Err(_) => { // as above, now() should be a better default than some constant // TODO: see if we can improve caching in the case where the fallback is a valid timezone - Source::LocalTime { mtime: SystemTime::now(), last_checked: SystemTime::now() } + Source::LocalTime { mtime: SystemTime::now() } } }, } } } -impl Source { - fn out_of_date(&mut self) -> bool { - let now = SystemTime::now(); - let prev = match self { - Source::LocalTime { mtime, last_checked } => match now.duration_since(*last_checked) { - Ok(d) if d.as_secs() < 1 => return false, - Ok(_) | Err(_) => *mtime, - }, - Source::Environment => return false, - }; - - match Source::default() { - Source::LocalTime { mtime, .. } => { - *self = Source::LocalTime { mtime, last_checked: now }; - prev != mtime - } - // will only reach here if TZ has been set while - // the process is running - Source::Environment => { - *self = Source::Environment; - true - } - } - } -} - struct Cache { zone: TimeZone, source: Source, + last_checked: SystemTime, } #[cfg(target_os = "android")] @@ -118,14 +92,43 @@ impl Default for Cache { Cache { zone: TimeZone::local().ok().or_else(fallback_timezone).unwrap_or_else(TimeZone::utc), source: Source::default(), + last_checked: SystemTime::now(), } } } impl Cache { fn offset(&mut self, d: NaiveDateTime, local: bool) -> LocalResult> { - if self.source.out_of_date() { - *self = Cache::default(); + let now = SystemTime::now(); + + match now.duration_since(self.last_checked) { + // If the cache has been around for less than a second then we reuse it + // unconditionally. This is a reasonable tradeoff because the timezone + // generally won't be changing _that_ often, but if the time zone does + // change, it will reflect sufficiently quickly from an application + // user's perspective. + Ok(d) if d.as_secs() < 1 => (), + Ok(_) | Err(_) => { + let new_source = Source::default(); + + let out_of_date = match (&self.source, &new_source) { + // change from env to file or file to env, must recreate the zone + (Source::Environment, Source::LocalTime { .. }) + | (Source::LocalTime { .. }, Source::Environment) => true, + // stay as file, but mtime has changed + (Source::LocalTime { mtime: old_mtime }, Source::LocalTime { mtime }) + if old_mtime != mtime => + { + true + } + // cache can be reused + _ => false, + }; + + if out_of_date { + *self = Cache::default(); + } + } } if !local { From 1b2a9136e07286ee96c5ab6a7a93abb7c49e09b8 Mon Sep 17 00:00:00 2001 From: Eric Sheppard Date: Sun, 30 Oct 2022 22:44:54 +1100 Subject: [PATCH 3/4] allow sharing of the allocated environment variable --- src/offset/local/tz_info/timezone.rs | 11 +++++------ src/offset/local/unix.rs | 29 +++++++++++++++++++--------- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/offset/local/tz_info/timezone.rs b/src/offset/local/tz_info/timezone.rs index 085b25c117..ae6fb5f848 100644 --- a/src/offset/local/tz_info/timezone.rs +++ b/src/offset/local/tz_info/timezone.rs @@ -26,11 +26,10 @@ impl TimeZone { /// /// This method in not supported on non-UNIX platforms, and returns the UTC time zone instead. /// - pub(crate) fn local() -> Result { - if let Ok(tz) = std::env::var("TZ") { - Self::from_posix_tz(&tz) - } else { - Self::from_posix_tz("localtime") + pub(crate) fn local(env_tz: Option<&str>) -> Result { + match env_tz { + Some(tz) => Self::from_posix_tz(tz), + None => Self::from_posix_tz("localtime"), } } @@ -813,7 +812,7 @@ mod tests { // so just ensure that ::local() acts as expected // in this case if let Ok(tz) = std::env::var("TZ") { - let time_zone_local = TimeZone::local()?; + let time_zone_local = TimeZone::local(Some(tz.as_str()))?; let time_zone_local_1 = TimeZone::from_posix_tz(&tz)?; assert_eq!(time_zone_local, time_zone_local_1); } diff --git a/src/offset/local/unix.rs b/src/offset/local/unix.rs index 1a1bdf888a..e71c16ebee 100644 --- a/src/offset/local/unix.rs +++ b/src/offset/local/unix.rs @@ -38,16 +38,16 @@ enum Source { Environment, } -impl Default for Source { - fn default() -> Source { +impl Source { + fn new(env_tz: Option<&str>) -> Source { // use of var_os avoids allocating, which is nice // as we are only going to discard the string anyway // but we must ensure the contents are valid unicode // otherwise the behaivour here would be different // to that in `naive_to_local` - match env::var_os("TZ") { - Some(ref s) if s.to_str().is_some() => Source::Environment, - Some(_) | None => match fs::symlink_metadata("/etc/localtime") { + match env_tz { + Some(_) => Source::Environment, + None => match fs::symlink_metadata("/etc/localtime") { Ok(data) => Source::LocalTime { // we have to pick a sensible default when the mtime fails // by picking SystemTime::now() we raise the probability of @@ -89,14 +89,20 @@ fn fallback_timezone() -> Option { impl Default for Cache { fn default() -> Cache { // default to UTC if no local timezone can be found + let env_tz = env::var("TZ").ok(); + let env_ref = env_tz.as_ref().map(|s| s.as_str()); Cache { - zone: TimeZone::local().ok().or_else(fallback_timezone).unwrap_or_else(TimeZone::utc), - source: Source::default(), last_checked: SystemTime::now(), + source: Source::new(env_ref), + zone: current_zone(env_ref), } } } +fn current_zone(var: Option<&str>) -> TimeZone { + TimeZone::local(var).ok().or_else(fallback_timezone).unwrap_or_else(TimeZone::utc) +} + impl Cache { fn offset(&mut self, d: NaiveDateTime, local: bool) -> LocalResult> { let now = SystemTime::now(); @@ -109,7 +115,9 @@ impl Cache { // user's perspective. Ok(d) if d.as_secs() < 1 => (), Ok(_) | Err(_) => { - let new_source = Source::default(); + let env_tz = env::var("TZ").ok(); + let env_ref = env_tz.as_ref().map(|s| s.as_str()); + let new_source = Source::new(env_ref); let out_of_date = match (&self.source, &new_source) { // change from env to file or file to env, must recreate the zone @@ -126,8 +134,11 @@ impl Cache { }; if out_of_date { - *self = Cache::default(); + self.zone = current_zone(env_ref); } + + self.last_checked = now; + self.source = new_source; } } From e785e6748558605baa06dd5b9e1772be7cf3ae83 Mon Sep 17 00:00:00 2001 From: Eric Sheppard Date: Sun, 30 Oct 2022 22:50:15 +1100 Subject: [PATCH 4/4] store hash of environment variable --- src/offset/local/unix.rs | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/offset/local/unix.rs b/src/offset/local/unix.rs index e71c16ebee..32aa31618b 100644 --- a/src/offset/local/unix.rs +++ b/src/offset/local/unix.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use std::{cell::RefCell, env, fs, time::SystemTime}; +use std::{cell::RefCell, collections::hash_map, env, fs, hash::Hasher, time::SystemTime}; use super::tz_info::TimeZone; use super::{DateTime, FixedOffset, Local, NaiveDateTime}; @@ -33,20 +33,18 @@ thread_local! { enum Source { LocalTime { mtime: SystemTime }, - // we don't bother storing the contents of the environment variable in this case. - // changing the environment while the process is running is generally not reccomended - Environment, + Environment { hash: u64 }, } impl Source { fn new(env_tz: Option<&str>) -> Source { - // use of var_os avoids allocating, which is nice - // as we are only going to discard the string anyway - // but we must ensure the contents are valid unicode - // otherwise the behaivour here would be different - // to that in `naive_to_local` match env_tz { - Some(_) => Source::Environment, + Some(tz) => { + let mut hasher = hash_map::DefaultHasher::new(); + hasher.write(tz.as_bytes()); + let hash = hasher.finish(); + Source::Environment { hash } + } None => match fs::symlink_metadata("/etc/localtime") { Ok(data) => Source::LocalTime { // we have to pick a sensible default when the mtime fails @@ -121,14 +119,20 @@ impl Cache { let out_of_date = match (&self.source, &new_source) { // change from env to file or file to env, must recreate the zone - (Source::Environment, Source::LocalTime { .. }) - | (Source::LocalTime { .. }, Source::Environment) => true, + (Source::Environment { .. }, Source::LocalTime { .. }) + | (Source::LocalTime { .. }, Source::Environment { .. }) => true, // stay as file, but mtime has changed (Source::LocalTime { mtime: old_mtime }, Source::LocalTime { mtime }) if old_mtime != mtime => { true } + // stay as env, but hash of variable has changed + (Source::Environment { hash: old_hash }, Source::Environment { hash }) + if old_hash != hash => + { + true + } // cache can be reused _ => false, };