Max
Max

Reputation: 1213

is my jQuery script performing bad?

I build a web tool for a photo lab to start print orders to be processed by the printers.

On older Mac's I am experiencing bad performance with my scripts. I was wondering whether there are some low performing parts in my scripts.

Do you guys see such low performing parts?

thx, Max

$(function() {

/* when start button is clicked */
$('.start_btn').click(function() {

    /* remove all previous input fields */
    $("#dialog_fieldset").children().remove();

    /* save id */
    id = $(this).attr('name');

    /* get data from db */
    $.post("inc/get_start_order_infos.inc.php", { id: id },

        /* Callback */
        function(data){

            /* make data globally available */
            daten = data;

            /* build the var 'infos_html' */
            var infos_html = '<label for="kunden_nr">Kunden-Nr.</label><input type="text" name="kunden_nr" id="kunden_nr" ';
            infos_html += 'value="' + data.kunden_nr + '" class="text ui-widget-content ui-corner-all" />';
            infos_html += '<label for="sort">Sort</label><input type="text" name="sort" id="sort" value="';
            infos_html += data.sort_nr + '" class="text ui-widget-content ui-corner-all" />';

            /* append infos_html to the fieldset */
            $('#dialog_fieldset').append(infos_html);

            /* Code Index */
            anzahlCodes = 1;

            /* For each element within the sub array codes */
            for(e in data.codes){

                /* build the var 'code_html' */
                var code_html = '<label for="code' + anzahlCodes + '">Code ' + anzahlCodes;
                code_html += '</label><input type="text" name="code' + anzahlCodes + '" id="code' + anzahlCodes;
                code_html += '" value="' + data.codes[e] + '" class="text ui-widget-content ui-corner-all" />';

                /* append code_html to the fieldset */
                $('#dialog_fieldset').append(code_html);
                anzahlCodes++;
            }

            $('#dialog').dialog('open');
            $('#kunden_nr').select();

    }, "json");

    return false;
});

$("#dialog").dialog({
    bgiframe: false,
    autoOpen: false,
    height: 350,
    modal: true,
    buttons: {
        'Auftrag starten': function() {

            /* close dialog */
            $(this).dialog('close');

            /* create the info array to be submitted */
            var arr_infos = new Array();
            arr_infos[0] = id;
            arr_infos[1] = $('#kunden_nr').attr('value');
            arr_infos[2] = $('#sort').attr('value');

            /* write inputs into the str_codes */
            var str_codes = "";
            for (var i=1; i < anzahlCodes; i++){
                var cde = '#code' + i;
                if(i == 1){
                    str_codes += $(cde).attr('value');
                }
                else{
                    str_codes += "<--->" + $(cde).attr('value');
                }
            }


            /* execute start */
            $.post("inc/start_orders.inc.php", { 'corrected_infos[]':arr_infos, 'corrected_codes': str_codes },

                /* Callback */
                function(data){
                    $('#notice').remove();

                    /* if start was successful */
                    if (data.mstype == 'success'){

                        /* the pressed button */
                        btn = ".start_btn[name=" + id + "]";
                        /* the tr where the button is inside */
                        tr = $(btn).parent().parent();
                        /* remove red */
                        $(tr).removeClass('rot');

                        /* set text of Kunden-Nr. td */
                        $(tr).children().eq(3).text(arr_infos[1]);

                        /* set text of Sort td */
                        $(tr).children().eq(4).text(arr_infos[2]);

                        /* set text of Code td */
                        $(tr).children().eq(5).text(str_codes);

                        /* set text of start td */
                        $(tr).children().eq(8).text('ja');

                        /* prepend notice */
                        $("#outline").prepend("<div id='notice'>" + data.ms + "</div>");
                    }

                    /* if not successful */
                    else {
                        $("#outline").prepend("<div id='notice' class='notice_err'>" + data.ms + "</div>");
                    }
                }, "json");
        },
        'Abbrechen': function() {
            $(this).dialog('close');
        }
    }
});

Upvotes: 1

Views: 320

Answers (4)

pixeline
pixeline

Reputation: 17974

note that this dialog_fieldset_node.children().remove(); would be more aptly done via empty(); dialog_fieldset_node.empty();

Upvotes: 0

Justin Johnson
Justin Johnson

Reputation: 31300

My suggestions are in the code as comments indicated by //++. Generally, JavaScript is faster when you search or alter the DOM as little as possible.

The main points of concern are in your loops and your repeat usage of $(tr).

I haven't tested this code.

$(function() {
    //++ jQuery doesn't cache this call, so it has to search the DOM each time.  
    //++ I've moved any node that is requested more than once here
    var dialog_fieldset_node = $("#dialog_fieldset"),
        dialog_node          = $("#dialog"),
        kunden_nr_node       = $("#kunden_nr"),
        outline_node         = $("#outline");
    //++ variables that you're using as globals I've moved here so that they can be shared 
    //++ between the event handlers, but aren't poluting the global scope.  They are accessible
    //++ to each event handler via the closure property of this annonymous function.
    var id = null;
    //++ Since you're creating the code <INPUT> nodes, store a refernce to them at that time
    //++ instead of having to find them in the DOM again later.  Now, anzahlCodes doesn't need
    //++ to be available to both handlers.
    var code_input_nodes = [];

    /* when start button is clicked */
    $('.start_btn').click(function() {
        /* remove all previous input fields */
        dialog_fieldset_node.children().remove();

        /* save id */
        id = $(this).attr('name');

        /* get data from db */
        $.post("inc/get_start_order_infos.inc.php", { id: id },
            /* Callback */
            function(data){
                /* make data globally available */
                daten = data;

                /* append infos_html to the fieldset */
                //++ No need to build a string in a variable that you're only going to use once.  You might want 
                //++ to build this out using DOM methods as I did below.  Since its only done once, there might 
                //++ not be a difference performancy wise
                dialog_fieldset_node.append(
                    '<label for="kunden_nr">Kunden-Nr.</label><input type="text" name="kunden_nr" id="kunden_nr" ' +
                    'value="' + data.kunden_nr + '" class="text ui-widget-content ui-corner-all" />' +
                    '<label for="sort">Sort</label><input type="text" name="sort" id="sort" value="' +
                    data.sort_nr + '" class="text ui-widget-content ui-corner-all" />'
                );


                //++ 1) `var e` to keep `e` from begin global.  If you want this side effect, you should be explicit about it.
                //++ 2) Create nodes via DOM methods to avoid running the HTML parser. node variables are defined outside of the
                //++ loop to avoid overhead of instantiation and scope-chain modification (minimal, but effective for large sets
                //++ of iterations.
                //++ 3) Append created nodes to a document fragment and then append the fragment to the `dialog_fieldset_node` to 
                //++ avoid multiple, unnecessary DOM tree reflows (which are slow).
                var fragment     = document.createDocumentFragment(),
                    label_node   = null, 
                    input_node   = null;
                    anzahlCodes  = 0;
                //++ Seems this needs to be reset everytime
                code_input_nodes = [];

                /* For each element within the sub array codes */
                for( var e in data.codes){
                    label_node = document.createElement("LABEL");
                    label_node.setAttribute("for", anzahlCodes);
                    label_node.innerHTML = "Code " + anzahlCodes;

                    input_node = document.createElement("INPUT");
                    input_node.setAttribute("type",  "text");
                    input_node.setAttribute("name",  "code" + anzahlCodes);
                    input_node.setAttribute("id",    "code" + anzahlCodes);
                    input_node.setAttribute("class", "text ui-widget-content ui-corner-all");
                    input_node.setAttribute("value", data.codes[e]);

                    //++ store a reference for later use
                    code_input_nodes.push(input_node);

                    /* append code_html to the fieldset */
                    fragment.appendChild(label_node);
                    fragment.appendChild(input_node);
                    anzahlCodes++;
                }
                dialog_fieldset_node.append(fragment);

                dialog_node.dialog('open');
                kunden_nr_node = $("#kunden_nr");
                kunden_nr_node.select();

            }, 
            "json"
        );

        return false;
    });

    dialog_node.dialog({
        bgiframe: false,
        autoOpen: false,
        height:   350,
        modal:    true,
        buttons:  {
            'Auftrag starten': function() {
                /* close dialog */
                $(this).dialog('close');

                /* create the info array to be submitted */
                var arr_infos = [id, kunden_nr_node.attr('value'), $('#sort').attr('value')];

                /* write inputs into the str_codes */
                var str_codes = "";
                for ( var i in code_input_nodes ) {
                    str_codes += (i ? "" : "<--->") + code_input_nodes[i].attr('value');
                }


                /* execute start */
                $.post("inc/start_orders.inc.php", { 'corrected_infos[]':arr_infos, 'corrected_codes': str_codes },
                    /* Callback */
                    function(data){
                        $('#notice').remove();

                        /* if start was successful */
                        if (data.mstype == 'success'){
                            /* the tr where the button is inside */
                            //++ 1) Was this intentionally global?  Global variables are the slowest to access because they
                            //++ are at the end of the scope-chain (which *sometimes* makes a difference, depending on depth).
                            //++ 2) Instead of wrapping `tr` in `$()` every time you use it, just do it once.
                            var tr = $(
                                    $(".start_btn[name=" + id + "]").parent().parent()
                                );
                            //++ Avoid calling `.children()` multiple times, just do it once.
                            var tr_children = tr.children();

                            /* remove red */
                            tr.removeClass('rot');

                            /* set text of Kunden-Nr. td */
                            tr_children.eq(3).text(arr_infos[1]);

                            /* set text of Sort td */
                            tr_children.eq(4).text(arr_infos[2]);

                            /* set text of Code td */
                            tr_children.eq(5).text(str_codes);

                            /* set text of start td */
                            tr_children.eq(8).text('ja');

                            /* prepend notice */
                            outline_node.prepend("<div id='notice'>" + data.ms + "</div>");
                        }

                        /* if not successful */
                        else {
                            outline_node.prepend("<div id='notice' class='notice_err'>" + data.ms + "</div>");
                        }
                    },
                    "json"
                );
            },

            'Abbrechen': function() {
                $(this).dialog('close');
            }
        }
    });
});

Hope that helped.

Upvotes: 6

Darko
Darko

Reputation: 38860

Well just from glancing over your code the biggest mistake you seem to be making is in your second callback (the bottom one) where you keep reselecting the row children elements. You should really assign that common query result to a variable and just use that. I would imagine youd see an improvement in performance then. This is how i'd change it:

/* beginning of callback code */

if (data.mstype == 'success') {

    /* the pressed button */
    btn = $(".start_btn[name=" + id + "]");
    /* the tr where the button is inside */
    tr = $(btn).parent().parent();
    /* remove red */
    tr.removeClass('rot');

    /***** This is the change *****/
    var children = $(tr).children();

    /* set text of Kunden-Nr. td */
    children.eq(3).text(arr_infos[1]);

    /* set text of Sort td */
    children.eq(4).text(arr_infos[2]);

    /* set text of Code td */
    children.eq(5).text(str_codes);

    /* set text of start td */
    children.eq(8).text('ja');
    /* prepend notice */
    $("#outline").prepend("<div id='notice'>" + data.ms + "</div>");
}

/* end of callback code */

Other than that you'd need to be more specific as to WHERE exactly it is that the bad performance is coming from and for that you can use Firebug as others have pointed out.

Upvotes: 1

hobodave
hobodave

Reputation: 29293

Hey Max. I suggest you install Firebug for Firefox if you haven't already. You can then use the Javascript Profiling functionality to find out where exactly in your code the slowdown is.

Here's a good tutorial on profiling with Firebug.

Best of luck.

Upvotes: 2

Related Questions