-
-
Notifications
You must be signed in to change notification settings - Fork 709
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
Performance Issue: setTimeout/setInterval #244
Comments
I see this as a genuine performance issue on a page which uses timeago on many elements. |
Here's a version (based on a slightly older base) which does this, and also doesn't keep the timer running if the element is no longer in the DOM.
|
@edwh, I only took a quick look at your code, but I think it has a bug.
|
Also, it might not work for future dates.
|
@theonlypwner Even with the / 2? I would expect that to cause it to check again after a further 0.5 years. But even then it should really update sooner than that if it was to preserve the switchover at 1.5 years, so you're right it's bugged. For my immediate purposes that's ok - I'm only interested in something rough for past dates only, so if it's a minute or two out I don't much mind. The performance hit of having a zillion timers running was a killer - particularly using setInterval, because once you had more than a certain number of timers running it wouldn't be able to keep up and so would flatline in timer processing. I did wonder about trying to avoid having a per-usage timer at all, but, y'know, other fish to fry. I may come back to this and do a less hasty fix, but don't hold your breath :-) |
All |
Unless you can tolerate error of up to 1 year, disregard what I said about grouping timers, as The timeago.org plugin updates the timer /**
* nextInterval: calculate the next interval time.
* - diff: the diff sec between now and date to be formated.
*
* What's the meaning?
* diff = 61 then return 59
* diff = 3601 (an hour + 1 second), then return 3599
* make the interval with high performace.
**/
function nextInterval(diff) {
var rst = 1, i = 0, d = Math.abs(diff);
for (; diff >= SEC_ARRAY[i] && i < SEC_ARRAY_LEN; i++) {
diff /= SEC_ARRAY[i];
rst *= SEC_ARRAY[i];
}
// return leftSec(d, rst);
d = d % rst;
d = d ? rst - d : rst;
return Math.ceil(d);
} |
It makes more sense to use setTimeout instead of setInterval.
If this is done, the function can determine when the text is actually going to change and not waste calls that don't do anything. If the text says
2 years ago
, it's probably not going to change in a minute, so it makes more sense to calculate the time delay.Also, this would be more efficient for a modified version that shows
%d seconds ago
and needs to update at 1000 ms intervals only before it becomes a minute. Then it can change the interval to 60000 ms.The text was updated successfully, but these errors were encountered: