Lazily initialize the time zone cache by KristofferC · Pull Request #457 · JuliaTime/TimeZones.jl

KristofferC · 2024-04-11T15:21:09Z

This avoids having to compile a bunch of code in __init__ and improves the load time of the package (on Julia 1.11) from:

julia> @time_imports import TimeZones
...
              ┌ 2.6 ms TimeZones.TZData.__init__() 92.83% compilation time
               ├ 237.9 ms TimeZones.__init__() 94.11% compilation time (89% recompilation)
    276.3 ms  TimeZones 81.90% compilation time (88% recompilation)

julia> @time_imports import TimeZones
...
     17.5 ms  TimeZones 16.03% compilation time

The change in performance of the functions is (in my opinion) acceptable.

After:

julia> using BenchmarkTools, TimeZones

julia> @btime istimezone("Europe/Warsaw")
  48.068 ns (1 allocation: 16 bytes)
true

Before:

julia> @btime istimezone("Europe/Warsaw")
  45.356 ns (1 allocation: 16 bytes)
true

src/types/timezone.jl

KristofferC · 2024-04-11T15:30:41Z

src/types/timezone.jl

+   _COMPILED_DIR[] = if desired_version == TZJData.TZDATA_VERSION
+       TZJData.ARTIFACT_DIR
+   else
+       # @info "Loading tzdata $desired_version"


I commented out this because otherwise it just logs randomly when you call a function and the cache initializes. It feels a bit pointless in the first place as well.

The point was to alert users that the environmental JULIA_TZ_VERSION was specified to a different version than TimeZones.jl would use by default. I'm not opposed to dropping this but it was informative

Okay but it will just appear at some random point now without any context to the user.

I think a reasonable compromise is to display a message during the __init__ and perform the loading like you propose

omus

Overall I'm good with this. Just a couple of things to clean up

src/TimeZones.jl

src/types/timezone.jl

this avoids having to compile a bunch of code in `__init__` and improves the load time of the package (on Julia 1.11) from: ```julia julia> @time_imports import TimeZones ... ┌ 2.6 ms TimeZones.TZData.__init__() 92.83% compilation time ├ 237.9 ms TimeZones.__init__() 94.11% compilation time (89% recompilation) 276.3 ms TimeZones 81.90% compilation time (88% recompilation) ``` ```julia julia> @time_imports import TimeZones ... 17.5 ms TimeZones 16.03% compilation time ``` The performance of the functions seem more or less unaffected: Before: ```julia julia> using BenchmarkTools, TimeZones julia> @Btime istimezone("Europe/Warsaw") 48.068 ns (1 allocation: 16 bytes) true julia> @Btime TimeZone("Europe/Warsaw") 28.591 ns (2 allocations: 64 bytes) Europe/Warsaw (UTC+1/UTC+2) ``` After: ```julia julia> @Btime istimezone("Europe/Warsaw") 45.356 ns (1 allocation: 16 bytes) true julia> @Btime TimeZone("Europe/Warsaw") 28.687 ns (2 allocations: 64 bytes) Europe/Warsaw (UTC+1/UTC+2) ```

omus · 2024-05-17T20:23:34Z

Rebased this PR now that #456 has been merged

omus · 2024-05-17T21:08:13Z

Posting some benchmarks from my machine so I can do some refactoring without losing the performance gains.

Before this PR with Julia 1.11.0-beta1 on an M1 MacBook

julia> @time_imports import TimeZones
...
               ┌ 4.4 ms TimeZones.TZData.__init__() 53.17% compilation time
               ├ 318.6 ms TimeZones.__init__() 85.10% compilation time (93% recompilation)
    690.8 ms  TimeZones 59.42% compilation time (85% recompilation)

julia> using BenchmarkTools, TimeZones

julia> @btime istimezone("Europe/Warsaw");
  77.314 ns (1 allocation: 16 bytes)

With this PR with Julia 1.11.0-beta1 on an M1 MacBook

julia> @time_imports import TimeZones
...
               ┌ 3.7 ms TimeZones.TZData.__init__() 56.80% compilation time
               ├ 0.0 ms TimeZones.__init__()
    107.0 ms  TimeZones 83.43% compilation time (53% recompilation)

julia> using BenchmarkTools, TimeZones

julia> @btime istimezone("Europe/Warsaw");
  80.966 ns (1 allocation: 16 bytes)

omus · 2024-05-17T21:28:55Z

Iterating on the `istimezone` internals

Iteration 1

function istimezone(str::AbstractString, mask::Class=Class(:DEFAULT))
    # Start by performing quick FIXED class test
    if mask & Class(:FIXED) != Class(:NONE) && occursin(FIXED_TIME_ZONE_REGEX, str)
        return true
    end

    # Checks against pre-compiled time zones
    tz, class = get(get_tz_cache(), str, (nothing, Class(:NONE)))
    return tz !== nothing && mask & class != Class(:NONE)
end

julia> @time_imports import TimeZones
...
               ┌ 2.7 ms TimeZones.TZData.__init__() 77.28% compilation time
               ├ 0.0 ms TimeZones.__init__()
    106.6 ms  TimeZones 84.49% compilation time (53% recompilation)

julia> using BenchmarkTools, TimeZones

julia> @btime istimezone("Europe/Warsaw");
  95.032 ns (2 allocations: 64 bytes)

Iteration 2

function istimezone(str::AbstractString, mask::Class=Class(:DEFAULT))
    # Start by performing quick FIXED class test
    if mask & Class(:FIXED) != Class(:NONE) && occursin(FIXED_TIME_ZONE_REGEX, str)
        return true
    end

    # Checks against pre-compiled time zones
    tz_class = get(get_tz_cache(), str, nothing)
    tz_class === nothing && return false
    tz, class = tz_class
    return mask & class != Class(:NONE)
end

julia> @time_imports import TimeZones
...
               ┌ 2.5 ms TimeZones.TZData.__init__() 78.26% compilation time
               ├ 0.0 ms TimeZones.__init__()
    103.5 ms  TimeZones 83.75% compilation time (52% recompilation)

julia> using BenchmarkTools, TimeZones

julia> @btime istimezone("Europe/Warsaw");
  75.789 ns (1 allocation: 16 bytes)

Iteration 3

function istimezone(str::AbstractString, mask::Class=Class(:DEFAULT))
    # Start by performing quick FIXED class test
    if mask & Class(:FIXED) != Class(:NONE) && occursin(FIXED_TIME_ZONE_REGEX, str)
        return true
    end

    # Checks against pre-compiled time zones
    _, class = get(get_tz_cache(), str, (UTC_ZERO, Class(:NONE)))
    return mask & class != Class(:NONE)
end

julia> @time_imports import TimeZones
...
               ┌ 3.1 ms TimeZones.TZData.__init__() 66.09% compilation time
               ├ 0.0 ms TimeZones.__init__()
    103.3 ms  TimeZones 83.89% compilation time (52% recompilation)

julia> using BenchmarkTools, TimeZones

julia> @btime istimezone("Europe/Warsaw");
  80.492 ns (1 allocation: 16 bytes)

Iteration 4

function istimezone(str::AbstractString, mask::Class=Class(:DEFAULT))
    # Start by performing quick FIXED class test
    if mask & Class(:FIXED) != Class(:NONE) && occursin(FIXED_TIME_ZONE_REGEX, str)
        return true
    end

    # Checks against pre-compiled time zones
    class = get(get_tz_cache(), str, (UTC_ZERO, Class(:NONE)))[2]
    return mask & class != Class(:NONE)
end

julia> @time_imports import TimeZones
...
               ┌ 2.8 ms TimeZones.TZData.__init__() 79.89% compilation time
               ├ 0.0 ms TimeZones.__init__()
    102.1 ms  TimeZones 83.99% compilation time (54% recompilation)

julia> using BenchmarkTools, TimeZones

julia> @btime istimezone("Europe/Warsaw");
  75.610 ns (1 allocation: 16 bytes)

omus · 2024-05-17T21:58:29Z

Commit 5701f3a

julia> @time_imports import TimeZones
...
               ┌ 3.1 ms TimeZones.TZData.__init__() 77.56% compilation time
               ├ 0.0 ms TimeZones.__init__()
    106.5 ms  TimeZones 84.26% compilation time (54% recompilation)

julia> using BenchmarkTools, TimeZones

julia> @btime istimezone("Europe/Warsaw");
  78.689 ns (1 allocation: 16 bytes)

Commit b5948fc

julia> @time_imports import TimeZones
...
      0.6 ms  Mocking
               ┌ 2.5 ms TimeZones.TZData.__init__() 81.56% compilation time
               ├ 0.0 ms TimeZones.__init__()
    108.0 ms  TimeZones 79.75% compilation time (53% recompilation)

julia> using BenchmarkTools, TimeZones

julia> @btime istimezone("Europe/Warsaw");
  75.703 ns (1 allocation: 16 bytes)

Looks like I broke some tests in the process

codecov-commenter · 2024-05-17T22:05:14Z

Codecov Report

Attention: Patch coverage is 91.66667% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 92.67%. Comparing base (2bc8f50) to head (d900457).
Report is 3 commits behind head on master.

Files	Patch %	Lines
src/build.jl	0.00%	1 Missing ⚠️
src/types/timezone.jl	95.00%	1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #457      +/-   ##
==========================================
- Coverage   92.79%   92.67%   -0.12%     
==========================================
  Files          39       39              
  Lines        1818     1830      +12     
==========================================
+ Hits         1687     1696       +9     
- Misses        131      134       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

omus · 2024-05-17T22:11:01Z

Close to the finish line on this one. I'd like to recheck the performance with the fixed tests yet.

KristofferC commented Apr 11, 2024

View reviewed changes

src/types/timezone.jl Outdated Show resolved Hide resolved

KristofferC commented Apr 11, 2024

View reviewed changes

omus reviewed Apr 26, 2024

View reviewed changes

omus mentioned this pull request Apr 26, 2024

Make ISOZonedDateTimeFormat a const #456

Merged

visr mentioned this pull request Apr 26, 2024

move timezone support to extension apache/arrow-julia#482

Open

1 task

KristofferC added 2 commits May 17, 2024 15:21

remove info printing when tzdata is loaded

2d96f95

omus force-pushed the kc/lazy2 branch from 60674b3 to 2d96f95 Compare May 17, 2024 20:23

omus added 3 commits May 17, 2024 16:29

Refactor istimezone internals

3772fac

Internal refactor of *_tz_cache functions

5701f3a

Show tzdata version used when JULIA_TZ_VERSION set

b5948fc

Fix tests

d900457

omus changed the title ~~lazily initialize the time zone cache~~ Lazily initialize the time zone cache May 21, 2024

omus merged commit 1ddfa59 into JuliaTime:master May 21, 2024

omus mentioned this pull request May 23, 2024

Minimize allocations when unpacking TimeZones from cache (updated) #451

Merged

omus mentioned this pull request Oct 30, 2024

Make TimeZones.jl relocatable #479

Merged

KristofferC mentioned this pull request Nov 6, 2024

Use Once in Julia 1.12 for initialization #480

Open

Conversation

KristofferC commented Apr 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

KristofferC Apr 11, 2024

Choose a reason for hiding this comment

Uh oh!

omus Apr 26, 2024

Choose a reason for hiding this comment

Uh oh!

KristofferC May 6, 2024

Choose a reason for hiding this comment

Uh oh!

omus May 17, 2024

Choose a reason for hiding this comment

Uh oh!

omus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

omus commented May 17, 2024

Uh oh!

omus commented May 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

omus commented May 17, 2024

Iteration 1

Iteration 2

Iteration 3

Iteration 4

Uh oh!

omus commented May 17, 2024

Uh oh!

codecov-commenter commented May 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

omus commented May 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

KristofferC commented Apr 11, 2024 •

edited

Loading

omus commented May 17, 2024 •

edited

Loading

codecov-commenter commented May 17, 2024 •

edited

Loading