Conversation
- We're using InlineStrings and a sentinel empty string to retain the isbits property where possible. Unfortunately, we needed to update a bunch of helpers for this. - Since some names in the backward file exceed the InlineString15 size we're using InlineString31. - It seemed a bit verbose so I renamed link_target to just link where appropriate. - We also added "backward" to the test regions and added a compile.jl test that ensure only :LEGACY timezones have links saved (reducing memory usage).
NOTES: - Wrapped `DEFAULT_VERSION` in `tzjfile_version()` which supports a JULIA_TZJ_VERSION feature flag. - Setup expected compiled dir to include the version format (`~/.julia/scratchspaces/f269a46b-ccf7-5d73-abea-4c690281aa53/build/compiled/tzjf/v2/2025b`) - Needed some hacks in __init__ and timezonecache._build to handle the path changes temporarily.
| function __init__() | ||
| # Set at runtime to ensure relocatability | ||
| _COMPILED_DIR[] = @static if isdefined(TZJData, :artifact_dir) | ||
| # Prefer scratch-compiled directory with matching versions, fall back to artifact |
There was a problem hiding this comment.
Part of the version feature flag and path hacks for testing both v1 and v2 behaviour.
| tz, class, link = get(_TZ_CACHE, str) do | ||
| if occursin(FIXED_TIME_ZONE_REGEX, str) | ||
| FixedTimeZone(str), Class(:FIXED) | ||
| FixedTimeZone(str), Class(:FIXED), InlineString31("") |
There was a problem hiding this comment.
Unfortunately, a Union with nothing is not isbits, hence the empty InlineString31.
| "The time zone \"$str\" is deprecated, using \"$link\" instead.", | ||
| :TimeZone | ||
| ) | ||
| return TimeZone(String(link), mask) |
There was a problem hiding this comment.
NOTE: If the backward file had a recursive case of a -> b -> c, then this would still error. The header comments say that's been avoided due to other parsers not handling it.
| if desired_version != TZJData.TZDATA_VERSION | ||
| _COMPILED_DIR[] = TZData.build(desired_version, _scratch_dir()) | ||
| # Also rebuilds if the TZJFile format version doesn't match the expected version | ||
| function _build(tzjf_version::Integer=TZJFile.tzjfile_version()) |
There was a problem hiding this comment.
Another main hack is for switching compiled file formats.
| 1 | ||
| ``` | ||
| """ | ||
| function tzjfile_version() |
There was a problem hiding this comment.
Again, a handy feature for switching compiled file versions, but it could be removed.
|
I'm not surprised about the TZJData system image errors, but the allocation test differences on 1.6 and nightly seem odd. |
1 participant
Related to #469 and #419.
Background
We already have access to links when compiling tzjfiles, but we currently don't retain that information for TimeZones.jl to use. The proposed solution is to:
TimeZoneconstructor to auto-redirect legacy timezones with the link info and throw adepwarn, so errors are limited to running with--depwarn=error.Usage
On Julia v1.12.3
Basic:
Error:
V1 (same behaviour):
Decisions Summary
Union{String, Nothing}in the compiled files for simplicity.backwardfile header.isbits, I'm reusing InlineStrings.jl with an empty string instead ofnothing. Looks likeInlineString31should fit all the names, but it does increase memory usage per cache entry.