performance regression introduced by fix for #84

[jkryl]

By profiling my application which uses cron module I have discovered that a lot of time is spent in _getNextDateFrom() method. Further investigation showed that it is a regression of fix for "Update for fixing node-cron to better handle DST. #84". Number of iterations to determine next run of a cron job in _getNextDateFrom() can be ~3000 with fix for #84 and here is the reason for it:

start date: Wed Jul 16 2014 09:58:35 GMT+0000 (UTC)
this object: { source: '0 0 1 1 *',
   zone: undefined,
   second: { '0': true },
   minute: { '0': true },
   hour: { '0': true },
   dayOfWeek: 
    { '0': true,
      '1': true,
      '2': true,
      '3': true,
      '4': true,
      '5': true,
      '6': true },
   dayOfMonth: { '1': true },
   month: { '1': true } }

snapping stdout: date 1 2014-07-16T09:58:35.943Z
snapping stdout: date 2 2014-08-01T00:00:35.943Z
snapping stdout: date 3 2014-09-01T00:00:35.943Z
snapping stdout: date 4 2014-10-01T00:00:35.943Z
snapping stdout: date 5 2014-11-01T00:00:35.943Z
snapping stdout: date 6 2014-12-01T00:00:35.943Z
snapping stdout: date 7 2015-01-01T00:00:35.943Z
snapping stdout: date 8 2015-02-01T00:00:35.943Z
snapping stdout: date 9 2015-02-01T00:00:36.943Z
snapping stdout: date 10 2015-02-01T00:00:37.943Z
snapping stdout: date 11 2015-02-01T00:00:38.943Z
snapping stdout: date 12 2015-02-01T00:00:39.943Z
snapping stdout: date 13 2015-02-01T00:00:40.943Z
snapping stdout: date 14 2015-02-01T00:00:41.943Z
snapping stdout: date 15 2015-02-01T00:00:42.943Z
snapping stdout: date 16 2015-02-01T00:00:43.943Z
snapping stdout: date 17 2015-02-01T00:00:44.943Z
snapping stdout: date 18 2015-02-01T00:00:45.943Z
snapping stdout: date 19 2015-02-01T00:00:46.943Z
snapping stdout: date 20 2015-02-01T00:00:47.943Z
snapping stdout: date 21 2015-02-01T00:00:48.943Z
snapping stdout: date 22 2015-02-01T00:00:49.943Z
snapping stdout: date 23 2015-02-01T00:00:50.943Z
snapping stdout: date 24 2015-02-01T00:00:51.943Z
snapping stdout: date 25 2015-02-01T00:00:52.943Z
snapping stdout: date 26 2015-02-01T00:00:53.943Z
snapping stdout: date 27 2015-02-01T00:00:54.943Z
snapping stdout: date 28 2015-02-01T00:00:55.943Z
snapping stdout: date 29 2015-02-01T00:00:56.943Z
snapping stdout: date 30 2015-02-01T00:00:57.943Z
snapping stdout: date 31 2015-02-01T00:00:58.943Z
snapping stdout: date 32 2015-02-01T00:00:59.943Z
snapping stdout: date 33 2015-02-01T00:01:00.943Z
snapping stdout: date 34 2015-02-01T00:02:00.943Z
snapping stdout: date 35 2015-02-01T00:03:00.943Z
...

as can be seen after the second goes 59 -> 00, the minute is incremented by one, which is wrong. Search then continues by searching for minute again and so on. Also the code in _findDST() seems to be quite inefficient, incrementing seconds by one until we step over DST shift can result in 60*60 extra cycles if I'm not mistaken. Also I don't see a reason why this function should be called when adjusting day of a week. In general it would be nice if function had an explanatory comment describing how it works.

[ncb000gt]

Sorry, I must have missed this issue in my email. I'll take a look. Thanks.

[ncb000gt]

Sorry, I must have missed this issue in my email. I'll take a look. Thanks.

[Marak]

I can confirm that there is a performance issue in _getNextDateFrom method.

I have a browserify version of the package and when putting in any dates that are somewhat far into the future it melts the browser. Did a trace and saw the while loop in _getNextDateFrom executing way too much.

[Marak]

I can confirm that there is a performance issue in _getNextDateFrom method.

I have a browserify version of the package and when putting in any dates that are somewhat far into the future it melts the browser. Did a trace and saw the while loop in _getNextDateFrom executing way too much.

[Marak]

Confirmed that reverting back to v1.0.3 fixes the issue.

[Marak]

Confirmed that reverting back to v1.0.3 fixes the issue.

[ncb000gt]

Thanks for confirming. I'm making my way through the tickets and this is definitely one I'll be looking into, albeit slowly. I'll post more when I have more info.

[ncb000gt]

Thanks for confirming. I'm making my way through the tickets and this is definitely one I'll be looking into, albeit slowly. I'll post more when I have more info.

[ncb000gt]

Closing due to age. Can create a new ticket if it comes up again.

[ncb000gt]

Closing due to age. Can create a new ticket if it comes up again.