TooTallNate / node-time

"time.h" bindings for Node.js
MIT License
382 stars 84 forks source link

Very, Very Slow! #42

Open ghost opened 11 years ago

ghost commented 11 years ago

Hi! We use node-time in a little weather service we built (http://forecast.io/). We think it's really great and we couldn't do what we do without it. (And we love you for it!)

However, it's far, far too slow! No, I mean really really too slow. Here's a quick benchmark of creating a 1000 Date() objects and setting their hours, minutes, seconds, and milliseconds to zero:

native: 10ms
time: 428ms
time w/ tz: 9113ms

Yes, that's nine hundred times slower: it takes 9ms to do a simple operation on a time.Date() object.

Native is simply doing these operations on a Date() object in the current timezone. Time is doing these operations on a time.Date() object, setting the timezone to the current system timezone (in my case, "America/New_York"). Time w/ TZ is doing these operations on a time.Date() object, and setting the timezone to any other timezone (in the case above, "America/Chicago").

We have to do such operations in our API several hundred times in order to fulfill a request. We're working on decreasing that number, but either way the latency is ridiculous.

(Incidentally, what we currently do is just look up the timezone offset once per API request, and run with it from there, doing all of the math on native Date() objects. Sadly, since our API returns data forecasted over the next week, if the timezone offset changes during the next week--such as for Daylight Savings Time--then our timestamps are off by an hour and bad things happen! This is what we're currently dealing with, and why we're trying to move to making heavier use of node-time.)

I'm happy to spend some time working on this, but I wanted to write you first:

I've included my benchmark program below, it's really trivial.

Thanks!


var time = require("time"),
    i, now;

console.time("native");
for(i = 1000; i--; ) {
  now = new Date(1365187331709);
  now.setHours(0);
  now.setMinutes(0);
  now.setSeconds(0);
  now.setMilliseconds(0);
}
console.timeEnd("native");

console.time("time");
for(i = 1000; i--; ) {
  now = new time.Date(1365187331709);
  now.setTimezone("America/New_York");
  now.setHours(0);
  now.setMinutes(0);
  now.setSeconds(0);
  now.setMilliseconds(0);
}
console.timeEnd("time");

console.time("time w/ tz");
for(i = 1000; i--; ) {
  now = new time.Date(1365187331709);
  now.setTimezone("America/Chicago");
  now.setHours(0);
  now.setMinutes(0);
  now.setSeconds(0);
  now.setMilliseconds(0);
}
console.timeEnd("time w/ tz");
moll commented 11 years ago

Any update on this?

ghost commented 11 years ago

We're still waiting for one. :(

On our end, we've moved to using a binary search to check for DST differences; this is a horrible solution but generally keeps that maximum latency down to 100ms at the worst case. (And since it's summer time, we havn't had to think about it more, though it might give us some trouble in another month or two...)

moll commented 11 years ago

Thanks for the prompt answer, J.T.L. I'm also skimming the other alternatives to find something that translates UTC to local time. I'm guessing the alternatives to this one are even worse that you didn't go with those?

ghost commented 11 years ago

Yes, we couldn't find any that were even worth checking into, though we last checked five months ago.

Perhaps the ideal solution (for us, at any rate) would be a pure-JavaScript module for reading up the tz-info database and responding to queries, but that would likely require a significant time investment.

moll commented 11 years ago

Aah, gotcha. Thanks. I'll have to look into this more seriously then as time zones are a fairly important component of the Monday Calendar app that I'm doing. 8-)

fatso83 commented 10 years ago

I can see that node-time actually interfaces with a binary, and I am guessing it does this for each new time.Date that is being created. If so, this would be the root cause of the performance degredation we are seeing. I guess a pretty basic fix for this should be to simply cache the results in the time module when first requested. At least it would save you 999 exec()s out of 1000 in the above benchmark.

ghost commented 10 years ago

We actually can't cache the results, since we look up the results for a different time each time, over the course of a week or two. If we cache it, then we might miss a Daylight Saving Time shift over that period, which causes all sorts of bugs and angry emails. (Actually, DST is the cause of much consternation over here.) :)

We actually gave up waiting on this and switched to the moment-timezone module. It's also slow (though not quite as bad as node-time), but with sufficient cleverness we were able to reduce the number of times we need to call into it for a request from around 1000 to around 6, which was good enough.

fatso83 commented 10 years ago

Well I meant as an optimization in the node-time module internally. The DST only affects time zones two times per year (dates differ depending on tz) so the offset would only need to be refreshed every six months or so (per timezone). So the TZ and DST table could be created at startup and only periodically updated, removing the need to call the binary every time.

There is unfortunately no cross-platform way of creating a DST table {TZ, DST_start, DST_end}, but the linked solution works on Mac, Linux and (with a conversion) on Windows.