Skip to content

Commit c0d0101

Browse files
authored
remove unneeded clone in TzdbResolver::get (#628)
I noticed that every time when a timezone is accessed via `ResolvedId` in the cache, the full `Tzif` struct is cloned, which contains multiple allocations. Just from poking around in the code, this seems to happen quite frequently, for example when a new `ZonedDateTime` is created. In the future the dx can be improved once `RwLockReadGuard::map` is stabilized, see rust-lang/rust#117108, though I think it will take some time.
1 parent cb81c9e commit c0d0101

1 file changed

Lines changed: 24 additions & 12 deletions

File tree

provider/src/tzif.rs

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ use alloc::vec::Vec;
3939
use core::cmp::Ordering;
4040
use core::ops::Range;
4141
use core::str;
42-
use std::sync::RwLock;
42+
use std::sync::{RwLock, RwLockReadGuard};
4343

4444
use combine::Parser;
4545

@@ -1385,16 +1385,20 @@ impl TzdbResolverBackend for FsTzdbResolver {
13851385
}
13861386

13871387
impl<Kind> TzdbResolver<Kind> {
1388-
/// Get timezone data for a single identifier
1389-
fn get(&self, id: ResolvedId) -> TimeZoneProviderResult<Tzif> {
1388+
/// Read access to the internal cache
1389+
pub fn read_cache(&self) -> TimeZoneProviderResult<RwLockReadGuard<'_, Vec<Tzif>>> {
13901390
self.cache
13911391
.read()
1392-
.map_err(|_| TimeZoneProviderError::Assert("poisoned RWLock"))?
1393-
.get(id.0)
1394-
.cloned()
1395-
.ok_or(TimeZoneProviderError::Assert(
1396-
"Time zone identifier does not exist.",
1397-
))
1392+
.map_err(|_| TimeZoneProviderError::Assert("poisoned RWLock"))
1393+
}
1394+
1395+
/// Get timezone data for a single identifier
1396+
pub fn get(cache: &[Tzif], id: ResolvedId) -> TimeZoneProviderResult<&Tzif> {
1397+
// TODO(shark): ideally this should get an read lock of the cache and use `RwLockReadGuard::map`, so we don't need to rely on an external lock
1398+
// see: https://github.com/rust-lang/rust/issues/117108
1399+
cache.get(id.0).ok_or(TimeZoneProviderError::Assert(
1400+
"Time zone identifier does not exist.",
1401+
))
13981402
}
13991403
}
14001404

@@ -1430,7 +1434,9 @@ impl<Kind: TzdbResolverBackend> TimeZoneResolver for TzdbResolver<Kind> {
14301434
identifier: ResolvedId,
14311435
local_datetime: IsoDateTime,
14321436
) -> TimeZoneProviderResult<CandidateEpochNanoseconds> {
1433-
self.get(identifier)?
1437+
let cache = self.read_cache()?;
1438+
1439+
Self::get(&cache, identifier)?
14341440
.candidate_nanoseconds_for_local_epoch_nanoseconds(local_datetime)
14351441
}
14361442

@@ -1439,17 +1445,23 @@ impl<Kind: TzdbResolverBackend> TimeZoneResolver for TzdbResolver<Kind> {
14391445
identifier: ResolvedId,
14401446
epoch_nanoseconds: i128,
14411447
) -> TimeZoneProviderResult<UtcOffsetSeconds> {
1442-
self.get(identifier)?
1448+
let cache = self.read_cache()?;
1449+
1450+
Self::get(&cache, identifier)?
14431451
.transition_nanoseconds_for_utc_epoch_nanoseconds(epoch_nanoseconds)
14441452
}
14451453

1454+
#[inline(always)]
14461455
fn get_time_zone_transition(
14471456
&self,
14481457
identifier: ResolvedId,
14491458
epoch_nanoseconds: i128,
14501459
direction: TransitionDirection,
14511460
) -> TimeZoneProviderResult<Option<EpochNanoseconds>> {
1452-
let tzif = self.get(identifier)?;
1461+
let cache = self.read_cache()?;
1462+
1463+
let tzif = Self::get(&cache, identifier)?;
1464+
14531465
tzif.get_time_zone_transition(epoch_nanoseconds, direction)
14541466
}
14551467
}

0 commit comments

Comments
 (0)