Skip to content
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

proposal: time: direct support for Unix millis and micros #18935

Closed
aebrahim opened this issue Feb 4, 2017 · 25 comments
Closed

proposal: time: direct support for Unix millis and micros #18935

aebrahim opened this issue Feb 4, 2017 · 25 comments

Comments

@aebrahim
Copy link

aebrahim commented Feb 4, 2017

Initial tl;dr: Go could make timestamp handling much easier by providing the following functions in its time library:

func (t Time) UnixMicro() int64 { ... }
func (t Time) UnixMilli() int64 { ... }
func FromUnixMicro(us int64) Time { ... }
func FromUnixMilli(ms int64) Time { ... }

Golang has time.Time as the idiomatic way of representing a time. However, not every API we encounter has this, and lots of them return Unix-style timestamps as int64 (sometimes uint64) in units of ms, us, or ns. For example, this will always occur when using API's that operate on protocol buffers, which use integers for timestamps instead of a native time type.

If we happen to have nanosecond timestamps, go is pretty nice. However, if we're using microseconds, then it gets fairly annoying to use time.Time, as you'll have to do error-prone type conversions everywhere, or rely on a timeutil library you write yourself for something which feels like it should be built in.

This is the type of stuff we have to do right now when dealing with niceapi that uses time.Time and otherapi that uses microseconds:

ts := otherapi.GetTimestamp()
t := time.Unix(0, ts * 1000) // one unit conversion, potential cause for bugs
niceapi.Use(t)

// elsewhere
t := niceApi.WhatTime()
ts := t.UnixNano() / 1000 // second unit conversion, potential cause for bugs
otherApi.Use(ts)

But why not

t := time.FromUnixMicro(otherapi.GetTimestamp()) // seems way less error prone to me
niceapi.Use(t)

// elsewhere
ts := niceApi.WhatTime().UnixMicro() // so easy!
otherApi.Use(ts)
@bradfitz bradfitz added this to the Proposal milestone Feb 4, 2017
@cespare
Copy link
Contributor

cespare commented Feb 6, 2017

I'm in favor of this. At $WORK we have an auxiliary time package that defines

func FromUnixMilli(ms int64) time.Time
func UnixMilli(t time.Time) int64

and these see a fair amount of use since ms-since-epoch is a common timestamp standard (as one example, you see it a lot in Javascript/web contexts since Date.now() returns ms).

@rsc
Copy link
Contributor

rsc commented Feb 6, 2017

The fact that Date.now() returns ms in Javascript is good context.

What common systems use micros?

The getters on time are unobjectionable; the new constructor names are not as nice. It would be easier to accept this proposal if we had a better idea for the names.

@rsc rsc changed the title Proposal: More int64 Unix time conversion functions proposal: time: direct support for Unix millis and micros Feb 6, 2017
@aebrahim
Copy link
Author

aebrahim commented Feb 6, 2017

@rsc what do you think would be nicer for the constructor names? Is it better to name them UnixMicro and UnixMilli (to match the identically named getter and constructor style already being used)?

@aebrahim
Copy link
Author

aebrahim commented Feb 7, 2017

@rsc
Copy link
Contributor

rsc commented Feb 7, 2017

@aebrahim I didn't look at all of those, but the ones I did all seemed to be about turning (in Go terms) a time.Duration into microseconds, not about a time stamp expressed in microseconds since 1970. Go already does have time.Microsecond for computing with durations.

@aebrahim
Copy link
Author

aebrahim commented Feb 7, 2017

OK, I see what you mean. Here's a list of functions that use seconds, microseconds % seconds since the unix epoch:

@rsc
Copy link
Contributor

rsc commented Feb 7, 2017

@aebrahim OK, but that's still not really motivation for adding t.UnixMicros() int64. What I'm hoping for is some evidence that this is a common serialized time format, used in file formats, network protocols, or other places where times are written down.

@mattalbr
Copy link

mattalbr commented Feb 7, 2017

So a bunch of thoughts on this:

  1. Supporting ns and s is arbitrary, ms and us seem logica extensions
  2. time.Microsecond isn't good enough. One of the main arguments for passing around a Time instead of an int is that you don't have those pesky unit mismatch bugs. But because the time library uses constants instead of functions, you end up with conversion bugs ("do I multiply or divide by time.Microsecond....?").
  3. If you're looking for how ugly things are with the current library, just read through http://stackoverflow.com/questions/24122821/go-golang-time-now-unixnano-convert-to-milliseconds and try not to scream at your monitor. If you succeed, you have more willpower than I do.

@ianlancetaylor
Copy link
Contributor

Personally I don't see anything very wrong with

t.UnixNano() * int64(time.Nanosecond) / int64(time.Microsecond)

Not that I object strongly to t.UnixMicro() but it seems easy enough to convert from nanoseconds to microseconds.

I do agree with Russ: we should add UnixMicro if there is demand. Simply saying that it is a logical extension is not enough. Every new feature makes the package harder to understand. We should only add features that carry their weight.

@aebrahim
Copy link
Author

aebrahim commented Feb 8, 2017

My main issue with this is that it's the opposite of what you'd expect from unit conversions.

t.UnixNano() * int64(time.Nanosecond) / int64(time.Microsecond)

However, this is how you would do a unit conversion, with microseconds in the numerator instead of denominator:
40000 nanoseconds * 1 microsecond / 1000 nanoseconds.

This seems makes an already error prone operation even worse. This is way more error prone than just using microseconds everywhere to start with instead of using go's time.

@minux
Copy link
Member

minux commented Feb 8, 2017 via email

@aebrahim
Copy link
Author

aebrahim commented Feb 8, 2017

I personally think that the math working out only with UNIT/ns really violates the principle of least astonishment. Especially when in other places, we do things like 3 * time.Microsecond

@ianlancetaylor
Copy link
Contributor

OK, but that is an argument for a NanoToMicro function. It's not an argument for a Time.UnixMicro method.

@mattalbr
Copy link

mattalbr commented Feb 8, 2017

I for one would be ecstatic with a function like

func (d Duration) ToUnit(u Duration) {
return d / u
}

and then it would look like:
t.UnixNano().ToUnit(time.Microsecond)

(Warning I am not a golang nor naming expert)

@aebrahim
Copy link
Author

aebrahim commented Feb 8, 2017

@ianlancetaylor that's a good point. I just think there has to be a better way to to convert unix timestamps without doing unit conversions. It may not necessarily be what I proposed, and I'd be happy to change it with whatever idea we think is best here

Another potential idea I'd like to throw into the mix is:

func (d Duration) Microseconds() int64 { ... }
func (d Duration) Milliseconds() int64 { ... }
func (t Time) UnixDuration() Duration { ... }

This would let you do

time.Now().UnixDuration().Microseconds()

However, we'd still need a way to construct the time from a timestamp.

@aebrahim
Copy link
Author

aebrahim commented Feb 8, 2017

Here is another proposal which goes both directions (to/from timestamps) and can be done with only 3 additions. We could add

var UnixEpoch = time.Unix(0, 0) // a constant of the time at the start of the unix epoch
func (d Duration) Milliseconds() float64 { ... }
func (d Duration) Microseconds() float64 { ... }

This would allow the following

// int64 timestamp to time
var tsMicro int64 = 1000000
t := time.UnixEpoch.Add(time.Duration(tsMicro) * time.Microsecond)
// time to int64 timestamp
ts = t.Sub(time.UnixEpoch).Microseconds()
now := time.Since(time.UnixEpoch).Microseconds()

The one thing I don't like about this is the kind of clunky time.Duration(tsMicro) * time.Microsecond. However, this also allows the general conversion of Duration to microseconds and milliseconds independent of Unix timestamps.

@rsc
Copy link
Contributor

rsc commented Feb 8, 2017

@mattalbr Supporting s and ns is not arbitrary at all. The full precision stored in the time.Time is nanoseconds, hence t.UnixNano() returning nanoseconds. And then it turns out that many common systems and formats - among them tar and gzip in the standard library - store Unix time in seconds (aka time_t), hence t.Unix() returning seconds.

What's missing from this proposal is a strong motivation for adding t.UnixMicro and t.UnixMilli and the reverse converters. The best motivation would be easier interoperability or less worry about overflow (since the time package could do a better job than Nano/1000) when using some existing systems, formats, or wire protocols that represent time in these formats. It's true that many Unix-based operating systems use Unix millis and micros in system calls, but Go abstracts those system calls away.

At Google we have a variety of systems that store time in Unix micros, and we too have a local conversion library to make that easier. But importance to Google alone is not a compelling argument for adding the API surface to package time, so we haven't. And more recently, the standard proto buffer Timestamp now uses nanoseconds.

@cespare mentioned that systems at his work use Unix millis internally, because Java System.currentTimeMillis and Javascript Date.now both return Unix millis, and they have a local conversion library to make that easier.

What's needed to move forward is more evidence that this is a widespread need. We have not been able to find existing systems, formats, or wire protocols that use this representation. Those would still be nice, but if there is enough evidence of internal systems at various places that need these formats, that might be enough.

@mattalbr
Copy link

mattalbr commented Feb 8, 2017

@rsc I think it's pretty clear this is a pain point between stack overflow, Google code internals, and other testimonials. We're trying to solicit suggestions to alleviate that pain point. It doesn't have to be UnixNano() and UnixMicro(); we would love if you proposed something different that you think more generalizable/useful/idomatic/what have you.

I really like @aebrahim's first suggestion of UnixDuration() and the two methods on Duration.

Re: going from int64 (micros) to Time, I still don't love that you have to multiply by time.Microsecond. What would people think about a constructor for Duration that takes the units? So:
tusec := 1000000
t := time.Unix(0, newDuration(tusec, time.Microsecond).Nanoseconds())

and the reverse
t := time.Now()
tusec := t.UnixDuration().Microseconds()

@rsc
Copy link
Contributor

rsc commented Feb 8, 2017

@mattalbr:

I think it's pretty clear this is a pain point between stack overflow, Google code internals, and other testimonials.

As I pointed out in my previous message, Google's internal systems don't count by themselves, unless there is reason to believe that experience is shared by others. That's why I'm asking for examples of systems that would need these conversions. Concrete details are great. Vague references to stack overflow and "other testimonials" not so much.

Let's focus on understanding the justification / motivation before we start proposing or evaluating alternative APIs.

@aebrahim
Copy link
Author

aebrahim commented Feb 8, 2017

I think what Matt and I are trying to say is that time conversions in go are generally problematic. Unix microsecond timestamp conversions are a manifestation of that. I'm not looking for a FromUnixMicro function; I'm looking for any way to achieve that without doing unit conversions.

Because of the pushback on a Unix-specific microseconds function, I'm instead suggesting we change this proposal to make it simpler to just convert units for duration (which is a very common operation in many different languages), and then provide a canonical way to convert from time to Duration since the Unix epoch.

The initial list I sent out (I believe) clearly establishes that converting between the equivalent of a Duration and micro/milli seconds is common in many languages. Can we at least agree on these 2 functions?

func (d Duration) Milliseconds() float64 { ... }
func (d Duration) Microseconds() float64 { ... }

@rsc
Copy link
Contributor

rsc commented Feb 8, 2017

A Time is an instant. A Duration is a delta between two instants. They have the same units, but it is important not to confuse them. A Duration can be meaningfully added to a Time. A Time cannot. Adding a t.UnixDuration() method violates that very helpful separation. We are not going to add that method nor in any other way encourage converting Times to Durations.

If there is a common need for UnixMicro and UnixMilli, we can add them. I am still waiting for evidence of that. @aebrahim and @mattalbr, you work at Google, where lots of internal systems store Unix micros as a time format. You're generalizing from that setting to the world outside Google, but there's no evidence here that the generalization is valid.


Regarding time.Duration conversions, the idioms are:

d := time.Duration(ms) * time.Millisecond
ms := int64(d / time.Millisecond)

I'm sorry, but that doesn't seem to me “generally problematic.” In any event, if you want to discuss the Duration API, best to start a new issue. Let's keep this one about whether to add t.UnixMicro and t.UnixMilli.

@rsc
Copy link
Contributor

rsc commented Feb 13, 2017

Because I was unable to find examples of this being a common need, I am leaning toward not doing this. I did web searches and asked on Twitter and got nothing back except for one or two internal uses.

@rsc rsc closed this as completed Feb 13, 2017
x8x referenced this issue in red/red Jul 3, 2017
Only integer! <=> date! is allowed, the integer value is Unix epoch time in seconds.
@laahs
Copy link

laahs commented Oct 13, 2017

https://play.golang.org/p/UaXMVCQ6pW

`package main

import (
"fmt"
"time"
)

func main() {
fmt.Println("Hello, playground")
n:=int32(1000000000)
t:=time.Now().AddDate(int(n),0,0)
fmt.Println(t)
fmt.Println(t.UnixNano())
t2:=time.Unix(0,t.UnixNano())
fmt.Println(t2)
t3:=time.Unix(t.Unix(),0)
fmt.Println(t3)
}`

prints

Hello, playground
102008-11-10 23:00:00 +0000 UTC
2528321395666673664
2050-02-12 23:29:55.666673664 +0000 UTC
102008-11-10 23:00:00 +0000 UTC

shows that due to int overflow, we have some issues with time representation. Consideration the buzz around the blockchain technology for example and what it can accomplish, and because this is an immutable distributed ledger ... if it becomes very popular maybe those entries would have to still be accessible later than year 2262 (with a consistent timestamp representation).

What if we could get a decomposed version of a time (with a Timestamp() int64,int64 method), with a seconds representation, and the additional nanoseconds precision. We already have time.Unix(sec,nanosec) that constructs time from a count of sec and nanosecond. It could help store dates as n1(sec)+n2(nanosecond) .

Then a "common" timestamp would be seconds based and we could store some additional precision (to the nanosecond) if required without overflowing. Reducing precision would just be an integer division of the nanosecond count to the precision level (for instance ns/time.Millisecond).

The first part count (seconds) of the timestamp is already returned by t.Unix(), the t.UnixNano() is the one that gets overflowed because returning the whole nanoseconds count, instead of only the precision that can not be returned by the t.Unix()

@laahs
Copy link

laahs commented Oct 13, 2017

just discovered that this basically is how the timestamp is represented in protobuf: seconds + nanoseconds. So this decomposition seems relevant.

@ianlancetaylor
Copy link
Contributor

@laahs This issue is closed and you seem to be talking about a different problem in any case. Please use a forum (https://golang.org/wiki/Questions) or open a new issue. Thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants