I built a custom HTML5 video control bar, and for each second, it updates two readings:
Time Elapsed
Time Remaining
in hh:mm:ss notation, like so:
enter image description here
Because of how often this function runs, I want it to be as efficient as possible.
I created this JS Fiddle to test benchmarks. It runs the timer normally, until you un-comment the benchmark and comment the casual running func. The commented function call runs this function 1,000,000 times and logs the speed via the console's timing utility.
I'm looking for best practices, efficiency, and general improvements.
Here's the code. For testing, it's been modified to run in a loop or at a set interval rather than being connected to a video.
/* Video Time Processing Benchmark Snippet */
var timeProgress = $("#time-progress"),
timeRemaining = $("#time-remaining");
// Set mock video duration for testing
var duration = 5000;
// Convert numbers to double+ digit format (09, 99, 999)
function digitize(n) {
return n > 9 ? "" + n : "0" + n;
}
// Convert seconds to hh:mm:ss
function secondsToHMS(seconds) {
var numhours = digitize(Math.floor(((seconds % 31536000) % 86400) / 3600));
var numminutes = digitize(Math.floor((((seconds % 31536000) % 86400) % 3600) / 60));
var numseconds = digitize(Math.floor((((seconds % 31536000) % 86400) % 3600) % 60));
return numhours + ":" + numminutes + ":" + numseconds;
}
// Process the elapsed and remaining times to two hh:mm:ss format
function processTime(currentTime) {
var value = (100 / duration) * currentTime;
var progressHMS = secondsToHMS(currentTime);
var remainingHMS = "-" + secondsToHMS(duration - currentTime);
timeProgress.html(progressHMS);
timeRemaining.html(remainingHMS);
}
// Test time via interval or loop
function testTime(type, number) {
if (type == "interval") {
// If a number isn't provided, the default is every second (1000)
if (!number) var number = 1000;
var i = 0;
// Set interval to run function
setInterval(function(){
currentTime = i;
processTime(currentTime);
i++;
// Clear the interval when duration is reached
if (i > duration) clearInterval(Int);
}, number);
}
else if (type == "loop"){
for (var i = 0; i < duration; i++){
currentTime = i;
processTime(currentTime);
}
}
}
// Use Int to set time for clearInterval() targetng
var Int = testTime("interval");
//Get benchamark on functon, vs optimized versions
/*
console.time("Benchmark");
testTime("loop", 1000000);
console.timeEnd("Benchmark");
*/
3 Answers 3
If you're worried about performance, this should raise a red flag:
someJQueryObject = $("#some-id");
...
someJQueryObject.html(aPlainOldString);
.html(foo)
is almost certainly slower than .innerHTML = foo
, because .html()
does a bunch of extra stuff, like checking if it can use innerHTML, trying to use innerHTML, catching errors if that fails and using a fallback, etc. Also, using innerHTML is probably slower than simply creating a Text node and updating its data
property.
I'd suggest replacing timeProgress = $("#time-progress")
with something like this:
var timeProgress = document.createTextNode('');
document.getElementById('time-progress').appendChild(timeProgress);
And replacing timeProgress.html(progressHMS);
with this:
timeProgress.data = progressHMS;
This will be a significant performance improvement and will also eliminate the jQuery dependency. Using a large GP library like jQuery to select two elements and update their text content seems like overkill anyway.
Here's a jsPerf demonstrating the performance difference.
-
-
\$\begingroup\$ @Schism good catch, updated with the empty string. Interesting perf. :) \$\endgroup\$Dagg– Dagg2014年07月30日 20:45:23 +00:00Commented Jul 30, 2014 at 20:45
-
\$\begingroup\$ Whoooops, the property is
data
, notvalue
. No wonder it went so fast. I just perfed it and it's still way faster though. \$\endgroup\$Dagg– Dagg2014年07月30日 21:11:48 +00:00Commented Jul 30, 2014 at 21:11 -
\$\begingroup\$ I have to admit, while the other answers are valuable, this one mostly clearly demonstrates a performance increase. +1 and check. \$\endgroup\$user73428– user734282014年07月31日 06:03:41 +00:00Commented Jul 31, 2014 at 6:03
-
2\$\begingroup\$ @Falco I'd love to but I can't; comments can only be edited in the first five minutes. I've edited it into the question. \$\endgroup\$Schism– Schism2014年07月31日 13:41:04 +00:00Commented Jul 31, 2014 at 13:41
When converting time values like this I find it much simpler, and more readable to work up from the smallest units. Consider the following:
// Convert seconds to hh:mm:ss
function secondsToHMS(intime) {
var numseconds = digitize(intime % 60);
intime = Math.floor(intime / 60);
var numminutes = digitize(intime % 60);
intime = Math.floor(intime / 60);
var numhours = digitize(intime);
return numhours + ":" + numminutes + ":" + numseconds;
}
The intime variable is modified as the time runs through. It first counts seconds, then minutes, then hours.
as for the performance, I imagine it is faster because the calculations are fewer, and simpler, but I have not benchmarked it. Even if it is not faster, I would still prefer it.
-
-
\$\begingroup\$ Thanks @Schism - on my machine (Firefox), my solution came out ahead by 15% \$\endgroup\$rolfl– rolfl2014年07月30日 21:01:05 +00:00Commented Jul 30, 2014 at 21:01
I (like many others) prefix my jQuery object variables with a
$
to make$timeProgress
and$timeRemaining
. To me, this is a nice explicit reminder that they're jQuery objects, à la Hungarian notation.duration
seems to me like it could be better namedtotalTime
or something.In
digitize()
, I see little point in doing"" + n
. Usereturn (n > 9 ? "" : "0") + n
orreturn n > 9 ? n : "0" + n
.I know that you stripped out your code, but here, you don't use
value
inprocessTime()
.In
testTime()
, you aren't actually passing through the return value ofsetInterval
, so you won't be able to go andclearInterval
withInt
(which, incidentally, should be lowercased renamed to something liketestIntervalHandle
or something).Not that it'll matter in production/release, but you're polluting global namespace with
currentTime
intestTime()
.
Finally, I don't imagine performance is as crucial as you make it out to be; once per second is pretty lenient.
-
1\$\begingroup\$ +1 for pointing out that performance may not be a issue at all. This is basically an animation set to run at 1 fps - that should be doable :) \$\endgroup\$Flambino– Flambino2014年07月31日 01:56:00 +00:00Commented Jul 31, 2014 at 1:56
-
1\$\begingroup\$ +1 for formatting, naming, and namespace, but most of all because I didn't realize that once per second mattered so little. However, perhaps learning the most efficient method in situations like this, where it hardly matters, will prepare me (and maybe even you?) for a situation where a similar thing needs to be done much faster than this. \$\endgroup\$user73428– user734282014年07月31日 06:06:05 +00:00Commented Jul 31, 2014 at 6:06
setInterval
to animate, userequestAnimationFrame
. This allows the browser to decide when to reanimate and sync it with general repainting. \$\endgroup\$