Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Time Zone Cache thread-safe with lazily-allocated thread-local caches #344

Merged
merged 15 commits into from
Aug 19, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 30 additions & 4 deletions src/types/timezone.jl
Original file line number Diff line number Diff line change
@@ -1,4 +1,30 @@
const TIME_ZONE_CACHE = Dict{String,Tuple{TimeZone,Class}}()
# ---------- Time Zone Cache setup ----------------------------------------------------------------
# Thread-local timezone caches to support multithreaded tasks simultaneously constructing TimeZones.
# See https://github.com/JuliaTime/TimeZones.jl/issues/342 for more information.

const THREADLOCAL_TIME_ZONE_CACHES = Dict{String,Tuple{TimeZone,Class}}[]

@inline default_tz_cache() = default_tz_cache(Threads.threadid())
@noinline function default_tz_cache(tid::Int)
omus marked this conversation as resolved.
Show resolved Hide resolved
0 < tid <= length(THREADLOCAL_TIME_ZONE_CACHES) || _tz_cache_length_assert()
if @inbounds isassigned(THREADLOCAL_TIME_ZONE_CACHES, tid)
@inbounds TZ_CACHE = THREADLOCAL_TIME_ZONE_CACHES[tid]
else
TZ_CACHE = eltype(THREADLOCAL_TIME_ZONE_CACHES)()
@inbounds THREADLOCAL_TIME_ZONE_CACHES[tid] = TZ_CACHE
end
return TZ_CACHE
end
@noinline _tz_cache_length_assert() = @assert false "0 < tid <= length(THREADLOCAL_TIME_ZONE_CACHES)"
omus marked this conversation as resolved.
Show resolved Hide resolved

module __InitThreadLocalCaches # This module is only here to house the __init__() function.
using ..TimeZones: THREADLOCAL_TIME_ZONE_CACHES
function __init__()
resize!(empty!(THREADLOCAL_TIME_ZONE_CACHES), Threads.nthreads()) # ensures that we didn't save a bad object
omus marked this conversation as resolved.
Show resolved Hide resolved
end
end
NHDaly marked this conversation as resolved.
Show resolved Hide resolved

# ---------- End Time Zone Cache setup ------------------------------------------------------------

"""
TimeZone(str::AbstractString) -> TimeZone
Expand Down Expand Up @@ -43,7 +69,7 @@ TimeZone(::AbstractString, ::Class)
function TimeZone(str::AbstractString, mask::Class=Class(:DEFAULT))
# Note: If the class `mask` does not match the time zone we'll still load the
# information into the cache to ensure the result is consistent.
tz, class = get!(TIME_ZONE_CACHE, str) do
tz, class = get!(default_tz_cache(), str) do
tz_path = joinpath(TZData.COMPILED_DIR, split(str, "/")...)

if isfile(tz_path)
Expand Down Expand Up @@ -98,12 +124,12 @@ function istimezone(str::AbstractString, mask::Class=Class(:DEFAULT))
end

# Perform more expensive checks against pre-compiled time zones
tz, class = get(TIME_ZONE_CACHE, str) do
tz, class = get(default_tz_cache(), str) do
tz_path = joinpath(TZData.COMPILED_DIR, split(str, "/")...)

if isfile(tz_path)
# Cache the data since we're already performing the deserialization
TIME_ZONE_CACHE[str] = open(deserialize, tz_path, "r")
default_tz_cache()[str] = open(deserialize, tz_path, "r")
else
nothing, Class(:NONE)
end
Expand Down
4 changes: 2 additions & 2 deletions src/tzdata/compile.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ using Dates
using Serialization
using Dates: parse_components

using ...TimeZones: TIME_ZONE_CACHE
using ...TimeZones: default_tz_cache
using ...TimeZones: TimeZones, TimeZone, FixedTimeZone, VariableTimeZone, Transition, Class
using ...TimeZones: rename
using ..TZData: TimeOffset, ZERO, MIN_GMT_OFFSET, MAX_GMT_OFFSET, MIN_SAVE, MAX_SAVE,
Expand Down Expand Up @@ -694,7 +694,7 @@ function compile(tz_source::TZSource, dest_dir::AbstractString; kwargs...)
results = compile(tz_source; kwargs...)

isdir(dest_dir) || error("Destination directory doesn't exist")
empty!(TIME_ZONE_CACHE)
empty!(default_tz_cache())
NHDaly marked this conversation as resolved.
Show resolved Hide resolved

for (tz, class) in results
parts = split(TimeZones.name(tz), '/')
Expand Down
2 changes: 1 addition & 1 deletion test/helpers.jl
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,6 @@ show_compact = (io, args...) -> show(IOContext(io, :compact => true), args...)
# not be used and only should be required if the test tzdata version and built tzdata
# version do not match.
function cache_tz((tz, class)::Tuple{TimeZone, TimeZones.Class})
TimeZones.TIME_ZONE_CACHE[TimeZones.name(tz)] = (tz, class)
TimeZones.default_tz_cache()[TimeZones.name(tz)] = (tz, class)
return tz
end
2 changes: 1 addition & 1 deletion test/types/timezone.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ using TimeZones: Class

@testset "istimezone" begin
# Invalidate the cache to ensure that `istimezone` works for non-loaded time zones.
empty!(TimeZones.TIME_ZONE_CACHE)
empty!(TimeZones.default_tz_cache())
NHDaly marked this conversation as resolved.
Show resolved Hide resolved

@test istimezone("Europe/Warsaw")
@test istimezone("UTC+02")
Expand Down