Reputation: 1213
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
Reputation: 17974
note that this dialog_fieldset_node.children().remove();
would be more aptly done via empty();
dialog_fieldset_node.empty();
Upvotes: 0
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
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