4
\$\begingroup\$

I have a file on my server which contains a running output of a script. This file is constantly changing and fluctuates in size between 0 - 1Mb. Now I want to take this file and make it viewable from the web. Currently I am using jQuery to get the file then I set a textarea's value to be the file content. In order to make this a "live" window I repeat the code every 5 seconds.

This was working meh at first but now is causing high cpu usage and essentially crashing my webpage.

Here is the code I am currently using.

function getUtilLog(){
    $("#livewindow").resizable();
    $.ajaxSetup({
        // Disable caching of AJAX responses
        cache: false
    });

    readUtilLog();
    setInterval(function(){readUtilLog();}, 5000);

    function readUtilLog(){
        $.get("UtilRun.log", function(data){
            if (data.length < 256000){
                $.get("UtilRun.log.1", function(data1){
                    d=data1 + data;
                    $('textarea#livewindow').val(d);
                });
            }
            else{
                d=data;
                $('textarea#livewindow').val(d);
            }
        });
    }
}

I want to know a better way of doing this which will be less cpu intensive. What is the correct way of accomplishing this?

\$\endgroup\$
  • 1
    \$\begingroup\$ Be careful with setInterval, what if the AJAX call takes 5 or more seconds? \$\endgroup\$ – Juan Mendes Aug 21 '15 at 18:54
  • \$\begingroup\$ Great point I know this has happened before. I just removed the setInterval and replaced it with setTimeout inside readUtilLog function. So now 5 seconds after finishing the AJAX call I reload the file. \$\endgroup\$ – preezzzy Aug 21 '15 at 19:20
3
\$\begingroup\$

Global variables

In side your function readUtilLog, in both cases of your if conditional, you have this:

d=data1 + data;

or this:

d=data;

Why are you using the d variable here? No where else in your code did you write var d, so when one of these conditional cases first runs, you are going to create a global d variable, which is just bad practice.

Since all you are doing with that d variable is setting an element's value to it in the very next line, then why not just set the element's value immediately?

That would look like this:

$('textarea#livewindow').val(data1 + data);

and:

$('textarea#livewindow').val(data);

OC$ (obsessive compulsive $)

Every 5 seconds, no matter what conditional passes, you do this:

$('textarea#livewindow')...

This magical $ function does quite a lot work under the hood to find an element. Therefore, this can be a pretty huge performance impact, especially if you are calling it every 5 seconds.

Your code would benefit a lot speed and efficiency wise if you store the this element in a variable, and then access it's .val through the variable.

var liveWindow = $("textarea#livewindow");

Now, when you are accessing the variable(for example):

liveWindow.val(data);

setTimeout vs setInterval

I have two thoughts on why this is bad.


Possible bad way 1

It's quite dangerous that you are using setInterval as a timer for calling readUtilLog every 5 seconds, and here's why:

After you have created an interval with setInterval, the interval now is going to create a new thread running the readUtilLog every 5 seconds even if the last readUtilLog has not yet completed.

And that's the problem: completing the last readUtilLog. The readUtilLog function has two very intensive and performance reducing tasks: XHR requests.

XHR requests can take quite a while. That means that, as these requests are taking their time, the interval could be spawning new threads background, which will eventually run your browser dry of threads and possibly cause it to crash.


Possible bad way 2

The same thing as above, only a thought on $.get.

In vanilla JavaScript, XHR requests occur asynchronously. That means that, every time you launch an XHR request, you are creating a new thread for the return.

Now you have two sources spawning threads: your interval, and the function that your interval runs.

That means that every 5 seconds, your interval is going to create a thread running readUtilLog, which is going to create a thread to handle the XHR request, which could potentially create another thread for another XHR request.

That is just complete madness, and could easily run your browser dry of threads potentially crashing it.


Solution

Use setTimeout.

As an example, this is what your code would look like if you were to switch to setTimeout:

function foo() {
    ...
    setTimeout(foo, 5000);
}

foo();

Why is this better? This will wait for the last thread to complete before starting a new one.

If there is an intensive function running in foo that will take longer than 5 seconds, who cares? That setTimeout isn't going to run until the end of the function.

This prevents thread build-up.


Nit-picks

There are just small things in your code that I'd like to point out.


setInterval(function(){readUtilLog();}, 5000);

Since you aren't trying to pass any parameters to readUtilLog, wrapping it an anonymous function is unnecessary:

setInterval(readUtilLog, 5000);

if (data.length < 256000){

Where did the 256000 come from?. You should create a constant at the top of your code and use that constant instead of just the straight-up value 256000.

var CONSTANT_NAME = 256000;

CONSTANT_NAME is not a good name; I could not derive a good name out of context.


| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.