Skip to content

Commit

Permalink
tz/posix: remove last bits of reliance on alloc
Browse files Browse the repository at this point in the history
We no longer keep around the "original" string we parsed for POSIX time
zones. Instead, we implement `Display` for our POSIX TZ types, and
that's how we'll show it in places that need it. This does mean that we
show the parsed form instead of what the user actually provided, but I
think this should be fine in the vast majority of cases.

Notice again how much more annoying this is. A couple of small allocs
where it didn't really matter for perf saved us a lot of work.

This module does still allocate a `Box<str>` in one place, but it's in
code that's only used when `std` is enabled (code for reading the `TZ`
environment variable), so we're fine with that for now.
  • Loading branch information
BurntSushi committed Dec 28, 2024
1 parent a8795c3 commit 30dcb95
Show file tree
Hide file tree
Showing 7 changed files with 271 additions and 68 deletions.
27 changes: 18 additions & 9 deletions src/tz/db/bundled/enabled.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ impl BundledZoneInfo {
}
};
#[cfg(feature = "std")]
self::global::add(&tz);
self::global::add(canonical_name, &tz);
Some(tz)
}

Expand Down Expand Up @@ -80,7 +80,7 @@ fn lookup(name: &str) -> Option<(&'static str, &'static [u8])> {
/// `std` is supported.
#[cfg(feature = "std")]
mod global {
use std::{sync::RwLock, vec::Vec};
use std::{string::String, string::ToString, sync::RwLock, vec::Vec};

use crate::tz::TimeZone;

Expand All @@ -101,10 +101,13 @@ mod global {
///
/// The only way a time zone can be remove from the cache is if it's
/// overwritten or if the cache is cleared entirely.
pub(super) fn add(tz: &TimeZone) {
pub(super) fn add(name: &str, tz: &TimeZone) {
let mut cache = CACHED_ZONES.write().unwrap();
if let Err(i) = cache.get_zone_index(tz.diagnostic_name()) {
cache.zones.insert(i, tz.clone());
if let Err(i) = cache.get_zone_index(name) {
cache.zones.insert(
i,
CachedZone { name: name.to_string(), tz: tz.clone() },
);
}
}

Expand All @@ -115,17 +118,17 @@ mod global {

#[derive(Debug)]
struct CachedZones {
zones: Vec<TimeZone>,
zones: Vec<CachedZone>,
}

impl CachedZones {
fn get(&self, query: &str) -> Option<&TimeZone> {
self.get_zone_index(query).ok().map(|i| &self.zones[i])
self.get_zone_index(query).ok().map(|i| &self.zones[i].tz)
}

fn get_zone_index(&self, query: &str) -> Result<usize, usize> {
self.zones.binary_search_by(|tz| {
cmp_ignore_ascii_case(tz.diagnostic_name(), query)
self.zones.binary_search_by(|entry| {
cmp_ignore_ascii_case(&entry.name, query)
})
}

Expand All @@ -134,6 +137,12 @@ mod global {
}
}

#[derive(Debug)]
struct CachedZone {
name: String,
tz: TimeZone,
}

/// Like std's `eq_ignore_ascii_case`, but returns a full `Ordering`.
fn cmp_ignore_ascii_case(s1: &str, s2: &str) -> core::cmp::Ordering {
let it1 = s1.as_bytes().iter().map(|&b| b.to_ascii_lowercase());
Expand Down
19 changes: 17 additions & 2 deletions src/tz/db/zoneinfo/enabled.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,14 @@ impl ZoneInfo {
let zones = self.zones.read().unwrap();
if let Some(czone) = zones.get(query) {
if !czone.is_expired() {
trace!(
"for time zone query `{query}`, \
found cached zone `{}` \
(expiration={}, last_modified={:?})",
czone.tz.diagnostic_name(),
czone.expiration,
czone.last_modified,
);
return Some(czone.tz.clone());
}
}
Expand Down Expand Up @@ -208,7 +216,7 @@ impl CachedZones {

fn get_zone_index(&self, query: &str) -> Result<usize, usize> {
self.zones.binary_search_by(|zone| {
cmp_ignore_ascii_case(zone.tz.diagnostic_name(), query)
cmp_ignore_ascii_case(zone.name.lower(), query)
})
}

Expand All @@ -220,6 +228,7 @@ impl CachedZones {
#[derive(Clone, Debug)]
struct CachedTimeZone {
tz: TimeZone,
name: ZoneInfoName,
expiration: Expiration,
last_modified: Option<Timestamp>,
}
Expand All @@ -240,9 +249,10 @@ impl CachedTimeZone {
file.read_to_end(&mut data).map_err(|e| Error::fs(path, e))?;
let tz = TimeZone::tzif(&info.inner.original, &data)
.map_err(|e| e.path(path))?;
let name = info.clone();
let last_modified = last_modified_from_file(path, &file);
let expiration = Expiration::after(ttl);
Ok(CachedTimeZone { tz, expiration, last_modified })
Ok(CachedTimeZone { tz, name, expiration, last_modified })
}

/// Returns true if this time zone has gone stale and should, at minimum,
Expand Down Expand Up @@ -506,6 +516,11 @@ impl ZoneInfoName {
ZoneInfoNameInner { full, original: original.to_string(), lower };
Ok(ZoneInfoName { inner: Arc::new(inner) })
}

/// Returns the lowercase name of this time zone.
fn lower(&self) -> &str {
&self.inner.lower
}
}

impl Eq for ZoneInfoName {}
Expand Down
50 changes: 34 additions & 16 deletions src/tz/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -601,13 +601,8 @@ impl TimeZone {
}

#[inline]
pub(crate) fn diagnostic_name(&self) -> &str {
let Some(ref kind) = self.kind else { return "UTC" };
match **kind {
TimeZoneKind::Fixed(ref tz) => tz.name(),
TimeZoneKind::Posix(ref tz) => tz.name(),
TimeZoneKind::Tzif(ref tz) => tz.name().unwrap_or("Local"),
}
pub(crate) fn diagnostic_name(&self) -> DiagnosticName<'_> {
DiagnosticName(self)
}

/// When this time zone was loaded from an IANA time zone database entry,
Expand Down Expand Up @@ -1130,7 +1125,6 @@ impl PartialEq for TimeZoneFixed {

#[derive(Eq, PartialEq)]
struct TimeZonePosix {
name: Box<str>,
posix: ReasonablePosixTimeZone,
}

Expand All @@ -1141,11 +1135,6 @@ impl TimeZonePosix {
Ok(TimeZonePosix::from(iana_tz.into_tz()))
}

#[inline]
fn name(&self) -> &str {
&self.name
}

#[inline]
fn to_offset(&self, timestamp: Timestamp) -> (Offset, Dst, &str) {
self.posix.to_offset(timestamp)
Expand All @@ -1170,8 +1159,7 @@ impl TimeZonePosix {
impl From<ReasonablePosixTimeZone> for TimeZonePosix {
#[inline]
fn from(posix: ReasonablePosixTimeZone) -> TimeZonePosix {
let name = posix.as_str().to_string().into();
TimeZonePosix { name, posix }
TimeZonePosix { posix }
}
}

Expand All @@ -1180,7 +1168,14 @@ impl From<ReasonablePosixTimeZone> for TimeZonePosix {
impl core::fmt::Debug for TimeZonePosix {
#[inline]
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
f.debug_tuple("Posix").field(&self.posix.as_str()).finish()
write!(f, "Posix({})", self.posix)
}
}

impl core::fmt::Display for TimeZonePosix {
#[inline]
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
core::fmt::Display::fmt(&self.posix, f)
}
}

Expand Down Expand Up @@ -1231,6 +1226,29 @@ impl core::fmt::Debug for TimeZoneTzif {
}
}

/// A helper type for converting a `TimeZone` to a succinct human readable
/// description.
///
/// This is principally used in error messages in various places.
///
/// A previous iteration of this was just an `as_str() -> &str` method on
/// `TimeZone`, but that's difficult to do without relying on dynamic memory
/// allocation (or chunky arrays).
pub(crate) struct DiagnosticName<'a>(&'a TimeZone);

impl<'a> core::fmt::Display for DiagnosticName<'a> {
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
let Some(ref kind) = self.0.kind else { return write!(f, "UTC") };
match **kind {
TimeZoneKind::Fixed(ref tz) => write!(f, "{}", tz.name()),
TimeZoneKind::Posix(ref tz) => write!(f, "{}", tz),
TimeZoneKind::Tzif(ref tz) => {
write!(f, "{}", tz.name().unwrap_or("Local"))
}
}
}
}

/// Configuration for resolving ambiguous datetimes in a particular time zone.
///
/// This is useful for specifying how to disambiguate ambiguous datetimes at
Expand Down
Loading

0 comments on commit 30dcb95

Please sign in to comment.