Skip to content

LoadLocation Call in time package #64

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.

Sign up for GitHub

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

Jump to bottom
Closed
rusco opened this issue Jul 4, 2014 · 15 comments
Closed

LoadLocation Call in time package #64

rusco opened this issue Jul 4, 2014 · 15 comments

Comments

@rusco
Copy link
Member

rusco commented Jul 4, 2014 

package main

import (
    "fmt"
    "math"
    "time"
)

func calcDuration(hrStart, minStart, hrEnd, minEnd int) (hrDiff, minDiff int) {

    // next line crashes on Win7/go1.3, systemcalls not supported:
    //pt, _ := time.LoadLocation("Europe/Lisbon")

    //this works fine:
    pt := time.UTC

    yr, mo, dy := time.Now().Date()

    t1 := time.Date(yr, mo, dy, hrStart, minStart, 0, 0, pt)
    t2 := time.Date(yr, mo, dy, hrEnd, minEnd, 0, 0, pt)

    overallDiff := t2.Sub(t1).Minutes()
    hrDiff = int(math.Floor(overallDiff / 60))
    minDiff = int(math.Mod(overallDiff, 60))

    return
}

func main() {

    hr, mi := calcDuration(8, 15, 19, 35)
    fmt.Printf("Duration: %d hours and %d minutes.\n", hr, mi)
}

the "time.LoadLocation" call causes a System call, at least on Windows.
I don't know how hard it is to implement this, but could you at least
make a hint in the compatibility doc ?
@dmitshur
Copy link
Member

dmitshur commented Jul 4, 2014

I believe there was a similar issue in the past:

#32

@neelance
Copy link
Member

neelance commented Jul 5, 2014

Yes, the timezone information is read from disk on all platforms. This means that in the current state, it will never work in a browser. Having timezone information would be quite useful, right?

Theoretically we could take the content of $GOROOT/lib/time/zoneinfo.zip and turn it into a virtual file system stored in a Go file, but the compressed size is 351K, so this would be quite big.

Another option is to use http://momentjs.com/timezone/ . It seems to include most of the timezone data in a very compressed format. So we need a wrapper that loads the data from this library into the time package's data structures. Or we just look how the library achieved this good compression and write our own decompressor in pure Go. Anyone interested in looking into it?

neelance added a commit that referenced this issue Jul 5, 2014
@neelance
fetch local timezone from JS environment (#64)
031c884
neelance added a commit that referenced this issue Jul 5, 2014
@neelance
add hint about timezones (#64)
baa96e9
@neelance
Copy link
Member

neelance commented Jul 5, 2014

I just added support for the local timezone (so time.Now().String() will return the local time) and added a hint to the compatibility table. Further improvement as described above would require some more work. A pull request would be great or someone needs to tell me why he/she needs more timezone support, then I may do it myself. :-)

@ajhager
Copy link

ajhager commented Jul 5, 2014

I am currently working on this. moment.js' data compression process is well documented, and it should be simple to embed it inline.

@neelance
Copy link
Member

@ajhager How is the progress on this issue? Does the moment.js approach work?

@neelance
Copy link
Member

Closing this issue for now, since there is nothing for me to do about it right now. Please reopen if required.

@FlorianUekermann
Copy link

Since nothing has changed on this front in more than 2 years, maybe it is time to just include zoneinfo.zip. Maybe only if time is imported or even only if LoadLocation is called. I am not sure if the latter is doable, but the former should be. The compilation results are already pretty large (even a small demo is typically >100k compressed), so conditionally adding 300k doesn't sound completely crazy to me.
This issue is quite debilitating given that there is currently no obvious way to manually include timezone
information besides some really awkward "unsafe" hackery.

@flimzy
Copy link
Member

flimzy commented Jun 9, 2017

It might be that by using an alternate compression algorithm might allow us to reduce the size burden somewhat. On my system, zoneinfo.zip is 360k. By recompressing with 'zip -9', it is reduced to 300k. By using gzip (on the original. zip), it is reduced to 96k.

I'm not sure what the best combination would be, for size/performance, but I don't think we need to be stuck with adding 360k to a bundle just to use timezones.

@FlorianUekermann
Copy link

I didn't think of that. I tried a bunch of combinations (with the decompressed data, then tar, then compression). Looks like around 100k is the lower end with the stuff that is supported directly in go (gzip -9). With xz or 7z it goes down to 50k. Compressing the zip instead of a tar adds 10% for xz, but maybe that is worth it to avoid touching the internals of LoadLocation.

tldr: Using xz on the zip gives 55k.

Leaving this to transport compression would be convenient, but is not an option I guess, since that'll be bad for https.

@flimzy
Copy link
Member

flimzy commented Jun 9, 2017

I'm not sure how lightweight the decompressor for xz is compared to gz, zip, etc. If it's possible to use a browser's built-in gzip decompressor, then using gz offers a huge advantage. But I'm not sure how realistic that is.

@FlorianUekermann FlorianUekermann mentioned this issue Jun 9, 2017
Closed
@dmitshur
Copy link
Member

I'll reopen this issue for better visibility since there's renewed interest in resolving this in some way. But we still need to find an acceptable solution, and I'm not sure if adding 100 KB or more (if it needs to also include decompression code) is acceptable.

@dmitshur dmitshur reopened this Jun 10, 2017
@pjebs
Copy link
Contributor

pjebs commented Jun 14, 2018

my 2 cents: utilise moment.js and it's timezone package. https://momentjs.com/timezone/

@flimzy flimzy mentioned this issue Jun 2, 2019
Closed
@duckbrain
Copy link

Go now has built-in support for embedding the time zone database statically from within in the standard library.

https://golang.org/pkg/time/tzdata/

This might just work as-is (haven't checked), or could be used as a trigger the same way in GopherJS to tell it to include the optimized moment db.

Browser support for IANA timezones has also gotten better. I'm not sure if the browser exposed APIs now cover the needs of Location.

https://caniuse.com/?search=Timezone

@duckbrain
Copy link

Related: #989

@nevkontakte
Copy link
Member

I believe that tzdata package in introduced in Go 1.16 resolves this issue sufficiently: users who need complete time zone support can include the package an the cost of bigger output size, others can use the "Local" time zone that's provided by default. This is on par with what upstream wasm implementation does.

I'm going to close this issue now, but if anyone thinks this is a mistake — please let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
9 participants
@neelance @ajhager @rusco @nevkontakte @duckbrain @FlorianUekermann @dmitshur @pjebs @flimzy