Time: CMOS + NTP

[facekapow]

C++ sets CMOS as the base time, and then JS synchronizes the base time with NTP time. Probably has 2 or 3 issues (one is that the CMOS time is sometimes in the future).

[iefserge]

This looks great, nice to have a real time support in runtime!

Few issues I found:

• cmos-time class is arch dependent, can we move it to x64/cmos-time-x64?
• can we split real time from hpet and just add the counter to the platform (platform.cc/platform.h)? There is probably no need to use volatile or pointer for it, would simple uint64_t work?
• why string argument in SetTime native instead of a number?
• what's the use of int SetTime(uint64_t newtime) return value?
• CMOS time result is in milliseconds, should we multiply it by 1000 to convert to microseconds?
• can we initialize all fields in constructors? This could be one of those crazy bugs because QEMU sets all memory to zeros and it works, but can crash on a real hardware
• runtime.net.IP4Address constructor accepts numbers as well like IP4Address(127,0,0,1)

[facekapow]

1: Sure. 2: Yes, a plain uint64_t would work. 3: V8 has terrible 64-bit number support (from what i've read), so a string seems like a good way of transporting it from JS to CPP. 4: Debugging at first, now it's useless, so i'll just void it. 5: For some reason that wasn't giving me the right time, i'll try again but i'm not so sure it'll work. 6: Ok. 7: I'll fix it.

[piranna]

3: V8 has terrible 64-bit number support (from what i've read), so a string seems like a good way of transporting it from JS to CPP.

If you talk about the 64 bits ints support, it's inherent to the language. By design, internally the Number type is a 64 bits double, so the "safe integer limit" is at 53 bits, from there the granularity between integers increase.

[facekapow]

Well, either way, 64-bit integers lose precision, so a string (which can be 512MB on plain V8 or 1.9GB on Node, I believe? I don't think the number will be that long) is the best way to safely send it across JS and into C++.

[facekapow]

Plus, there's a Uint32 class, but no Uint64 class, no way to read the 64-bit integer from the function's arguments (unless there is a way).

[iefserge]

We can use a js number, a double which can store a 53 bit integer. date.getTime() will not be able to handle a 64 bit int either. Btw, we can multiply date.getTime() result by 1000 in c++.

[facekapow]

I feel really stupid right now, i'd forgotten about date.getTime()! Yeah, i'll use a Number.

[facekapow]

OK, the C++-to-JS setTime function uses a Number argument now, CMOS time is more accurate and no long in the future any more, I moved the time variable + functions to platform.h, everything is initialized in the constructor, and I made the C++-only SetTime function void.

[facekapow]

Should I take down this PR? Because no one has said anything since my last change to it (it's been 3 days), so... (it's just that this is on the roadmap, so I figured it'd be good to have it done), sorry if this isn't something I should comment about.

[iefserge]

Sorry, I've seen the PR and it looks good, but I haven't had time to integrate it into runtime. There are a few issues in TCP stack, it relies on Date.now() being monotonic and needs to be fixed first. Few minor issues with this PR (I can take care of it for you):

• we shouldn't use Node buffers in core
• CMOSTime doesn't have initializer list
• Platform constructor could use initializer list as well
• using V8 deprecated API ->NumberValue()

Great work anyway, I'll merge as soon as I can!

[iefserge]

@ArielAbreu Btw, we can chat in gitter or irc if you'd like :)

[facekapow]

I changed the buffer in order to use the U8 directly, and I use the USEARG(0); ... As<v8::Number> ... method instead of ->NumberValue(). I'm not sure what there is to initialize, though. CMOSTime doesn't even have (or need) a constructor, and Platform doesn't accept any parameters for it's constructor.

[iefserge]

@ArielAbreu there are uninitialized private fields

[facekapow]

I added a constructor that initializes CMOSTime's private variables, removes last_century (it's unused) and makes the private year variable a regular int in order to avoid a compile-time warning.

[iefserge]

CMOSTime* time_; allocation leaks memory, probably can change it to CMOSTime time_. Very minor issue, but let's rename set_time.js to set-time.js to be consistent.

[facekapow]

Fixed all this.

[iefserge]

@ArielAbreu going to do some tests on real hardware

[kesla]

Very excited about this!

[facekapow]

Only problem I noticed is that CMOS time returns July 18 as the day, might be an issue.

[iefserge]

Yep, same issue

Sat Jul 18 2015 22:34:36 GMT+0000 (GMT)
Sat Jul 18 2015 22:34:37 GMT+0000 (GMT)
Tue Jul 21 2015 22:16:53 GMT+0000 (GMT)
Tue Jul 21 2015 22:16:54 GMT+0000 (GMT)
Tue Jul 21 2015 22:16:55 GMT+0000 (GMT)
Tue Jul 21 2015 22:16:56 GMT+0000 (GMT)
``

[iefserge]

Looks like CMOS logic works fine on hardware, we need to fix the date though.

[iefserge]

One note, is that sometimes 1000ms timeout is not enough for NTP, it should probably try to send the request a few times with a couple of seconds interval each time. Or maybe we can even turn it into daemon(or background service) that's responsible for timekeeping (that's probably an idea for a separate feature).

[facekapow]

Theads = daemons, just sayin'... Tomorrow i'll work out the issues with CMOS.

[iefserge]

The beauty of non-blocking code is that there is no need to use threads:)

[facekapow]

Moved CMOS into JavaScript, plus I fixed it (finally).

[iefserge]

@ArielAbreu great work, thanks! CMOS works nicely, I fixed rounding because time was a bit off sometimes (and c++ version operates on ints) https://github.com/runtimejs/runtime/tree/real-time can you confirm this works for you?

[facekapow]

Yeah, it works!