-
Notifications
You must be signed in to change notification settings - Fork 54
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
ZonedDateTime not isbits #271
Comments
I have briefly looked into this and I think it's a feasible thing to do. My main concern is how this will work with serialization specifically if a user had defined a custom time zone instead of a predefined time zone. |
Could we fix that by defining custom serialization, so that it still serializes the same as it does now, even though it just stores a integery identifier both pre-serialization and after deserialization ? |
https://github.com/kcajf/TimeZones.jl kcajf@4caacd3#diff-5c5b3616b976774dc6e6c77d00227ba3 |
I'll be working on this next week. |
Interface-wise I like the idea of a parametric |
I thought worth an update on some more thoughts i have had on using ShortStrings.jl I did some checked, and if we represented timezone name not as a single (short) string, So updates: since: #287 (comment)
Would not be a problem as
This has now been resolved.
This has now been resolved, pretty much all operations now don't require constructing a
Nothing has changed with this AFAIK. So ShortString is more viable than before, but still might not be viable. Though it might be worth considering, is making |
Yet another idea for this, from @iamed2 is to just solve it for UTC. It doesn't solve the whole problem but it does for one special and important case. |
Making If the change is made, I still believe that the Rust-inspired enum approach that I demonstrated in my branch is a good way to go. The timezone database is limited in size and is almost static, so storing it in the DateTime objects themselves feels wasteful. As a reminder, here is what the code generated in the build step looks like:
The rest of the logic is implemented in terms of these functions and lookup tables. The code compiles down to fast jump tables. It is entirely allocation-free, and the types themselves take up very little memory (they basically just contain a 16bit enum, which is pretty much as good as it can get). |
For reference, this made |
@oxinabox and I were discussing some performance problems in the presence of large numbers of
ZonedDateTime
objects and I noted that them not beingisbits
is a problem if you put lots of them in a vector, because the GC time taken to mark such vectors will beO(number of elements)
notO(number of vectors)
. Now, arguably this is a prime use case for a partially pooled vector that just pools the TimeZone objects, but keeps the raw DateTime inline, but I'm also wondering ifZonedDateTime
itself couldn't be made to beisbits
and not have this issue in the first place. In particular, it seems like there's probably only ever gonna be a small handful of TimeZones globally in the system, so we could just globally intern them and only refer to them by small integer identifiers (reconstituting them into the full object on access). Does that sound like a feasible thing to do? I think it would dramatically increase the performance ofZonedDateTime
.The text was updated successfully, but these errors were encountered: