Displaying dynamic fields for Trac tickets

Question

TLDR: Rendering is too slow when having lots of custom fields and rules. So I need help to improve this javascript code.

#9606: Trac ticket about issue

I have a trac 1.0.8 installed on a ubuntu machine with DynamicFields 1.2.6 installed (compiled from latest revisions 0.11 folder). I have more than 100 custom fields with 30 ticket types. Each field has its own rule.

• It takes too long to get response from js.
• Disabling Rules doesn't seem to speed up the page loading.

I mainly think there are some inefficiencies in the js code.

/*
 * Layout 'class'
 */
var Layout = function (name) {
 this.name = name;
 // Selector for all field tds/ths
 this.selector = '';
 // Return the given field name's td/th element
 this.get_tx = function (field) {};
 // Return the given td/th element's field name
 this.get_field = function (tx) {};
 // Move a field's tds and ths to slot i
 this.move_field = function (field, i) {};
 // Returns true of the field needs its own row
 this.needs_own_row = function (field) {
 var $field = jQuery('#field-' + field);
 if ($field.length)
 return $field.is('TEXTAREA');
 return false;
 };
 // Update the field layout given a spec
 this.update = function (spec) {
 var this_ = this;
 // save original field order
 if (window.dynfields_orig_field_order == undefined)
 window.dynfields_orig_field_order = Object();
 if (window.dynfields_orig_field_order[this.name] == undefined) {
 window.dynfields_orig_field_order[this.name] = [];
 jQuery(this.selector).each(function (i, e) {
 var field = this_.get_field($(this));
 if (field)
 window.dynfields_orig_field_order[this_.name].push(field);
 });
 }
 // get visible and hidden fields
 var visible = [];
 var hidden = [];
 jQuery.each(window.dynfields_orig_field_order[this.name], function (i, field) {
 var tx = this_.get_tx(field);
 if (tx.hasClass('dynfields-hide')) {
 hidden.push(field);
 } else {
 visible.push(field);
 }
 });
 // get new field order
 var new_fields = jQuery.merge(visible, hidden); // warning: side-effects!
 // order the fields
 this.order_fields(new_fields);
 };
 this.order_fields = function (new_fields) {
 var this_ = this;
 var skip_slot = 0;
 // determine which fields need to move and move 'em!
 jQuery(this.selector).each(function (i, e) {
 var old_field = this_.get_field($(this));
 var old_slot = -1;
 if (old_field.length)
 old_slot = jQuery.inArray(old_field, new_fields);
 var new_field = new_fields[i];
 // check to allow *this* field be in its own row
 if (i % 2 == 1 && old_field.length && this_.needs_own_row(new_field))
 skip_slot += 1;
 var new_slot = i + skip_slot;
 // check if field is in the correct slot in the new order
 if (new_slot != old_slot && i < new_fields.length) {
 // wrong order!
 this_.move_field(new_field, new_slot);
 }
 // check to move *next* field to its own row
 if (old_field.length && this_.needs_own_row(new_field))
 skip_slot += 1;
 });
 }
};
/*
 * Inputs Layout implementation
 */
var inputs_layout = new Layout('inputs');
// selector
inputs_layout.selector = '#properties td[class!=fullrow]:parent';
// get_tx
inputs_layout.get_tx = function (field) {
 return jQuery('#field-' + field).closest('td');
};
// get_field
inputs_layout.get_field = function (td) {
 var input = td.find(':input:first');
 if (!input.length) return '';
 return input.attr('id').slice(6);
};
// move_field
inputs_layout.move_field = function (field, i) {
 var td = this.get_tx(field);
 var th = td.prev('th');
 // find correct row (tr) to insert field
 var row = Math.round(i / 2 - 0.5); // round down
 var $properties = jQuery('#properties');
 row += $properties.find('td.fullrow').length; // skip fullrows
 var tr = $properties.find('tr:eq(' + row + ')');
 // find correct column (tx) to insert field
 var col = 'col' + ((i % 2) + 1);
 if (tr.find('th').length) {
 if (col == 'col1') {
 var old_th = tr.find('th:first');
 if (old_th.get(0) != th.get(0)) { // don't move self to self
 old_th.before(th);
 old_th.before(td);
 }
 } else {
 var old_td = tr.find('td:has(:input):last');
 if (old_td.get(0) != td.get(0)) { // don't move self to self
 old_td.after(td);
 old_td.after(th);
 }
 }
 } else {
 // no columns so just insert
 tr.append(th);
 tr.append(td);
 }
 // let's set col
 td.removeClass('col1 col2');
 th.removeClass('col1 col2');
 td.addClass(col);
 th.addClass(col);
};
/*
 * Header Layout implementation
 */
var header_layout = new Layout('header');
// selector
header_layout.selector = '#ticket .properties th:parent';
// get_tx
header_layout.get_tx = function (field) {
 return jQuery('#h_' + field);
};
// get_field
header_layout.get_field = function (th) {
 return (th.attr('id') ? th.attr('id').slice(2) : '');
};
// move_field
header_layout.move_field = function (field, i) {
 var th = this.get_tx(field);
 var td = th.next('td');
 // find correct row (tr) to insert field
 var row = Math.round(i / 2 - 0.5); // round down
 var tr = jQuery('#ticket').find('.properties tr:eq(' + row + ')');
 // find correct column (tx) to insert field
 if (tr.find('th').length) {
 if (i % 2 == 0) {
 var old_th = tr.find('th:first');
 if (old_th.get(0) != th.get(0)) { // don't move self to self
 old_th.before(th);
 old_th.before(td);
 }
 } else {
 var old_td = tr.find('td:last');
 if (old_td.get(0) != td.get(0)) { // don't move self to self
 old_td.after(td);
 old_td.after(th);
 }
 }
 } else {
 // no columns so just insert
 tr.append(th);
 tr.append(td);
 }
};

---

Is there anyone who has knowledge about this subject?

Also here is my inspectation:

enter image description here

• Flambino: "I mainly think there are some inefficiencies in the js code." Are you sure, though? Have you profiled the code to see where the bottlenecks are or is it a guess? I'm not saying your wrong necessarily, but it'd be a huge fool's errand if it turns out the bottleneck is somewhere else entirely
• kfb: You can look especially here: code
• Mario Santini: you could use .toArray() to convert a jQuery object to a plain javascript array, and then you can do a normal for loop or a forEach loop (just revert i and e in the callback). But I think you should take care of the functions that manipulate the DOM, instead.
• kfb: Manipulating DOM is a great idea, but unfortunately my DOM is just short as it can be, I mean it is doing only what it has to do. So, I am stuck with this piece of code. Especially here: code
• woxxom: To avoid layout calculations try processing a detached deep-cloned container var work = src.cloneNode(true); ....... src.parentNode.replaceChild(work, src);. Although I'm not sure it makes any difference in case of jQuery and that layout library, but it might work in vanilla js. Also, building the entire html of the container as string and then using it to overwrite the source could work because there should be just one layout adjustment.

Answer 1

A few thoughts:

• Define all your class methods on the object prototype. This will generally perform better than methods defined on the instance.
• You currently store jQuery selector strings on your objects. Have you considered actually storing the jQUery collection that results from the selectors? If you are not dynamically adding/removing DOM elements from these collections, you can save yourself a lot of overhead by not having to re-query the DOM every time that you want to work with the collection again.
• You spend a decent amount of time rendering and painting. There are ways to optimize CSS such that you reduce time spent in these areas. Take a look at this site - https://csstriggers.com/ - for a good reference on what CSS impacts are for different CSS properties across popular browsers in terms of when a layout, rendering, or painting change is triggered. You don't really show your CSS in this question, but there may be opportunity to improve overall speed by changing the way you specify things.