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 5 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
3 changes: 3 additions & 0 deletions src/TimeZones.jl
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ const DEPS_DIR = joinpath(PKG_DIR, "deps")
abstract type Local <: TimeZone end

function __init__()
# Initialize the thread-local TimeZone cache (issue #342)
_tz_cache_init()

# Base extension needs to happen everytime the module is loaded (issue #24)
Dates.CONVERSION_SPECIFIERS['z'] = TimeZone
Dates.CONVERSION_SPECIFIERS['Z'] = TimeZone
Expand Down
32 changes: 28 additions & 4 deletions src/types/timezone.jl
Original file line number Diff line number Diff line change
@@ -1,4 +1,28 @@
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

# Called from __init__() to initialize the Thread Local caches at runtime.
function _tz_cache_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 Time Zone Cache setup ------------------------------------------------------------
NHDaly marked this conversation as resolved.
Show resolved Hide resolved

"""
TimeZone(str::AbstractString) -> TimeZone
Expand Down Expand Up @@ -43,7 +67,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 +122,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
1 change: 1 addition & 0 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ include("helpers.jl")
include("rounding.jl")
include("parse.jl")
include("plotting.jl")
include("thread-safety.jl")
omus marked this conversation as resolved.
Show resolved Hide resolved

# Note: Run the build tests last to ensure that re-compiling the time zones files
# doesn't interfere with other tests.
Expand Down
51 changes: 51 additions & 0 deletions test/thread-safety.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# Test that TimeZones.jl can be safely used in a multithreaded environment.
# Note that the number of threads being used cannot be changed dynamically, so
# this test file spawns a new julia process running with multiple threads.

using Test

const program = """
using TimeZones
using Test

@assert Threads.nthreads() > 1 "This system does not support multiple threads, so the thread-safety tests cannot be run."

function create_zdt(year, month, day, tz_name)
ZonedDateTime(DateTime(year, month, day), TimeZone(tz_name))
end
function cycle_zdts()
return [
try
create_zdt(year, month, day, tz_name)
catch e
e isa Union{ArgumentError,NonExistentTimeError} || rethrow()
nothing
end
for year in 2000:2020
for month in 1:5
for day in 10:15
for tz_name in timezone_names()
]
end

const outputs = Channel(Inf)
@sync begin
for _ in 1:15
Threads.@spawn begin
put!(outputs, cycle_zdts())
end
end
end
close(outputs)

const tzs = collect(outputs)

# Test that every Task produced the same result
allsame(x) = all(y -> y == first(x), x)
@test allsame(tzs)
"""

@info "Running Thread Safety tests"
@testset "Multithreaded TimeZone construction" begin
run(`$(Base.julia_cmd()) -t8 --proj -E $(program)`)
end
NHDaly marked this conversation as resolved.
Show resolved Hide resolved
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