-
Notifications
You must be signed in to change notification settings - Fork 61
Reduce memory & redundant work for concurrent TimeZones construction #356
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
base: master
Are you sure you want to change the base?
Changes from 2 commits
5bffed5
6c0d3ea
beacaaa
96252ab
4f444eb
0258929
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,12 @@ | |
| # to the cache, while still being thread-safe. | ||
| const THREAD_TZ_CACHES = Vector{Dict{String,Tuple{TimeZone,Class}}}() | ||
|
|
||
| # Holding a lock during construction of a specific TimeZone prevents multiple Tasks (on the | ||
| # same or different threads) from attempting to construct the same TimeZone object, and | ||
| # allows them all to share the result. | ||
| const tz_cache_mutex = ReentrantLock() | ||
| const TZ_CACHE_FUTURES = Dict{String,Channel{Tuple{TimeZone,Class}}}() # Guarded by: tz_cache_mutex | ||
|
|
||
| # Based upon the thread-safe Global RNG implementation in the Random stdlib: | ||
| # https://github.com/JuliaLang/julia/blob/e4fcdf5b04fd9751ce48b0afc700330475b42443/stdlib/Random/src/RNGs.jl#L369-L385 | ||
| @inline _tz_cache() = _tz_cache(Threads.threadid()) | ||
|
|
@@ -19,10 +25,22 @@ const THREAD_TZ_CACHES = Vector{Dict{String,Tuple{TimeZone,Class}}}() | |
| end | ||
| @noinline _tz_cache_length_assert() = @assert false "0 < tid <= length(THREAD_TZ_CACHES)" | ||
|
|
||
| function _reset_tz_cache() | ||
| # ensures that we didn't save a bad object | ||
| function _tz_cache_init() | ||
|
NHDaly marked this conversation as resolved.
Outdated
|
||
| resize!(empty!(THREAD_TZ_CACHES), Threads.nthreads()) | ||
| end | ||
| # ensures that we didn't save a bad object | ||
| function _reset_tz_cache() | ||
| # Since we use thread-local caches, we spawn a task on _each thread_ to clear that | ||
| # thread's local cache. | ||
| Threads.@threads for i in 1:Threads.nthreads() | ||
| @assert Threads.threadid() === i "TimeZones.TZData.compile() must be called from the main, top-level Task." | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is this guaranteed when calling
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't remember / understand the reason, but the The behavior of help?> Threads.@threads
Threads.@threads [schedule] for ... end
A macro to parallelize a for loop to run with multiple threads. Splits the iteration space among multiple tasks and runs those tasks on threads according
to a scheduling policy. A barrier is placed at the end of the loop which waits for all tasks to finish execution.
The schedule argument can be used to request a particular scheduling policy. The only currently supported value is :static, which creates one task per
thread and divides the iterations equally among them. Specifying :static is an error if used from inside another @threads loop or from a thread other than
1.
The default schedule (used when no schedule argument is present) is subject to change.
│ Julia 1.5
│
│ The schedule argument is available as of Julia 1.5.It only works from thread 1, for reasons i can't quite remember, but so this basically means you have to start it from the main Task. (I didn't remember that it was a "thread 1" requirement - i thought it was actually a "main Task" requirement.. i can consider changing the assertion message, perhaps? But i think it's easier guidance to say "don't call this concurrently, dude") |
||
| empty!(_tz_cache()) | ||
| end | ||
| @lock tz_cache_mutex begin | ||
|
NHDaly marked this conversation as resolved.
Outdated
|
||
| empty!(TZ_CACHE_FUTURES) | ||
| end | ||
| return nothing | ||
| end | ||
|
|
||
| """ | ||
| TimeZone(str::AbstractString) -> TimeZone | ||
|
|
@@ -68,20 +86,40 @@ 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!(_tz_cache(), str) do | ||
| tz_path = joinpath(TZData.COMPILED_DIR, split(str, "/")...) | ||
|
|
||
| if isfile(tz_path) | ||
| open(deserialize, tz_path, "r") | ||
| elseif occursin(FIXED_TIME_ZONE_REGEX, str) | ||
| FixedTimeZone(str), Class(:FIXED) | ||
| elseif !isdir(TZData.COMPILED_DIR) || isempty(readdir(TZData.COMPILED_DIR)) | ||
| # Note: Julia 1.0 supresses the build logs which can hide issues in time zone | ||
| # compliation which result in no tzdata time zones being available. | ||
| throw(ArgumentError( | ||
| "Unable to find time zone \"$str\". Try running `TimeZones.build()`." | ||
| )) | ||
| # Even though we're using Thread-local caches, we still need to lock during | ||
| # construction to prevent multiple tasks redundantly constructing the same object, | ||
| # and potential thread safety violations due to Tasks migrating threads. | ||
| # NOTE that we only grab the lock if the TZ doesn't exist, so the mutex contention | ||
| # is not on the critical path for most constructors. :) | ||
| constructing = false | ||
| # We lock the mutex, but for only a short, *constant time* duration, to grab the | ||
| # future for this TimeZone, or create the future if it doesn't exist. | ||
| future = @lock tz_cache_mutex begin | ||
| get!(TZ_CACHE_FUTURES, str) do | ||
| constructing = true | ||
| Channel{Tuple{TimeZone,Class}}(1) | ||
| end | ||
| end | ||
| if constructing | ||
| tz_path = joinpath(TZData.COMPILED_DIR, split(str, "/")...) | ||
|
|
||
| tz = if isfile(tz_path) | ||
|
NHDaly marked this conversation as resolved.
Outdated
|
||
| open(deserialize, tz_path, "r") | ||
| elseif occursin(FIXED_TIME_ZONE_REGEX, str) | ||
| FixedTimeZone(str), Class(:FIXED) | ||
| elseif !isdir(TZData.COMPILED_DIR) || isempty(readdir(TZData.COMPILED_DIR)) | ||
| # Note: Julia 1.0 supresses the build logs which can hide issues in time zone | ||
| # compliation which result in no tzdata time zones being available. | ||
| throw(ArgumentError( | ||
| "Unable to find time zone \"$str\". Try running `TimeZones.build()`." | ||
| )) | ||
| else | ||
| throw(ArgumentError("Unknown time zone \"$str\"")) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exceptions while constructing will cause threads to be blocked upon waiting for a channel that will never be populated |
||
| end | ||
|
|
||
| put!(future, tz) | ||
| else | ||
| throw(ArgumentError("Unknown time zone \"$str\"")) | ||
| fetch(future) | ||
| end | ||
| end | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.