Modify

Opened 8 years ago

Closed 3 months ago

#9606 closed defect (fixed)

Rendering slow when having lots of custom fields and rules

Reported by: Jan Beilicke Owned by: Ryan J Ollos
Priority: normal Component: DynamicFieldsPlugin
Severity: normal Keywords: performance
Cc: Trac Release: 0.12

Description

The plugin works like a charm in general. Unfortunately, the rendering of a ticket takes about 10s in Chrome 16 (and even more in FF) when having about 45 custom fields that should be hidden or displayed depending on components and ticket types.

According to some quick tests, the performance issue might be caused by several nested $.each() in dynfields.html, one time in the function apply_rules() and in the jQuery(document).ready() part.

Do you see any chance that this part might be refactored to improve the performance?

Attachments (3)

ticket-custom.txt (23.9 KB) - added by Onur Baykal 4 years ago.
t9606-optimize-dom-operations.diff (13.5 KB) - added by Jun Omae 15 months ago.
t9606-optimize-dom-operations.1.diff (13.1 KB) - added by Ryan J Ollos 3 months ago.
Rebased patch.

Download all attachments as: .zip

Change History (37)

comment:1 Changed 8 years ago by Jan Beilicke

I've investigated the issue further:

  • Replaced jQuery.each() with native JavaScript for-loops: No performance gain.
  • Set some console.time() measuring points: It turned out that moving each field in the DOM triggered in layout.js causes the slow-down.

comment:2 Changed 8 years ago by Rob Guttman

jbeilicke, really appreciate you investigating performance issues here. I admit I have not concerned myself with performance oprimizations as I haven't personally seen problems - but then I have a lot less fields than you do it seems.

Let me know what you find in your profiling and I'll be happy to attempt fixes accordingly.

comment:3 Changed 4 years ago by Onur Baykal

Sorry to rise this ticket from the dead. I have the same 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. Even loading the new ticket page takes about 30 seconds on a modern machine. And when a user selects a different type of ticket, it takes close to 20 seconds to reload. I assume automatic ticket preview feature also effects this but I mainly think there are some inefficiencies in the js code. Disabling Rules doesn't seem to speed up the page loading. I'd be happy to provide more detail to solve this problem.

Also we used to have a 1.0.1 installed on a mac with DynamicFields 1.2.1 installed. Somehow the plugin works faster with that configuration (same field & ticket types). I'm suspecting the underlying DOM operations for this problem since on 1.2.1, fields disappear after the rendering of the page.

Last edited 4 years ago by Ryan J Ollos (previous) (diff)

comment:4 Changed 4 years ago by Ryan J Ollos

You might want to try installing from the trunk. The 0.11 branch is unlikely to be maintained going forward.

comment:5 Changed 4 years ago by Onur Baykal

Just compiled from trunk and installed. The problem still exists.

comment:6 Changed 4 years ago by Ryan J Ollos

I'm not sure that I'll have time to take a look at this. However, attaching your [ticket-custom] section would greatly help in reproducing the issue. It sounds like the section is large, so please attach it as a file rather than pasting in a comment.

Version 0, edited 4 years ago by Ryan J Ollos (next)

Changed 4 years ago by Onur Baykal

Attachment: ticket-custom.txt added

comment:7 Changed 4 years ago by Onur Baykal

I've attached the [ticket-custom] field. Also here are the ticket types:

Sorun               
Test                
EGGIM               
MeetingAI           
UcusTalimati        
YDI                 
UcusAI              
UretimITF           
ACGIM               
InfEGG              
CodeIssue           
LeasonsLearned      
IntegrationIssue    
ToolIssue           
TestIssue           
IsTakibi            
Gereksinim          
YGT                 
Task                
TestTalimati        
AYESAS_FIX          
AYESAS_PLAN         
ComponentDescription
CAWS                
KarayelUcusAI       
Açıklama            
SahaTalep           
SesliUyari          
Release   

comment:8 Changed 4 years ago by Ryan J Ollos

In 14895:

2.2.0dev: Trivial optimizations and code style changes suggested by linter. Refs #9606.

comment:9 Changed 4 years ago by Onur Baykal

Your latest commit somewhat improved rendering times but still locks the browser.

comment:10 Changed 4 years ago by Ryan J Ollos

In 14896:

2.2.0dev: Move functions inside document ready handler. Refs #9606.

No performance change is expected.

comment:11 Changed 4 years ago by Ryan J Ollos

In 14897:

2.2.0dev: Minor selector optimizations. Refs #9606.

comment:12 Changed 4 years ago by Ryan J Ollos

Looks like 90% of the time is being spent in order_fields. I'll try to look into improving that, but it will be at least a few days before I have time.

comment:13 Changed 4 years ago by Ryan J Ollos

In 14898:

2.2.0dev: Minor selector optimization. Refs #9606.

comment:14 Changed 4 years ago by Ryan J Ollos

In 14905:

2.2.0dev: Fix errors in [14896].

The functions get_selector, apply_rules and setup_triggers needs to be global scope.

Refs #9606.

comment:15 Changed 4 years ago by Ryan J Ollos

In 14906:

2.2.0dev: Use jQuery is to compare element type.

Refs #9606.

comment:16 Changed 3 years ago by anonymous

Hi @rjollos. I am using this plugin too and we have a serious issue same as @theaob. Rendering is too slow when having lots of custom fields and rules and I can't open 3 tabs at the same time in my Trac.

Are you still trying to improve the plugin's performance? I am really sorry to wake up this old thread. Good work.

comment:17 Changed 3 years ago by Ryan J Ollos

I haven't had time to work on the plugin much lately because I'm working on Trac, and my JavaScript knowledge is also fairly limited. I recall doing some profiling and finding where time was being spent, but didn't see a way to fix it.

I'd gladly test a patch from anyone that wishes to contribute.

comment:18 Changed 3 years ago by anonymous

Thanks.
Can you give us some specific target to look on? So we can try to figure out what is causing this problem.

comment:19 in reply to:  18 Changed 3 years ago by anonymous

Replying to anonymous:

Thanks.
Can you give us some specific target to look on? So we can try to figure out what is causing this problem.

OR Is there any alternative plugin etc. that we can count on?

comment:20 Changed 2 years ago by figaro

Keywords: performance added

comment:21 Changed 15 months ago by anonymous

Trac Release: 0.121.2

Hi all;

Same problem. Can any one fix this order_fields script? We need the plugin desperately but having it also makes you wait desperately to have the page loaded.

Last edited 15 months ago by Ryan J Ollos (previous) (diff)

comment:22 Changed 15 months ago by Jun Omae

Trac Release: 1.20.12

comment:23 Changed 15 months ago by Rob Guttman

Owner: changed from Rob Guttman to Ryan J Ollos
Status: newassigned

comment:24 Changed 15 months ago by Ryan J Ollos

Owner: Ryan J Ollos deleted
Status: assignednew

Changed 15 months ago by Jun Omae

comment:25 Changed 15 months ago by Jun Omae

I tried to optimize DOM operations: t9606-optimize-dom-operations.diff.

Timing of apply_rules reduces from 2500ms to 850ms with 100 custom fields.

comment:26 Changed 15 months ago by Ryan J Ollos

Thank you for the patch. I'll test the changes.

comment:27 Changed 3 months ago by Ryan J Ollos

Owner: set to Ryan J Ollos
Status: newaccepted

This issue was discussed on the MailingList in gdiscussion:trac-users:Mau5CQ0fGvY.

comment:28 Changed 3 months ago by Ryan J Ollos

The optimization in comment:25 patch are:

  • Add local variable to avoid repeatedly referencing objects, such as window or this
  • Use some pure JavaScript methods rather than jQuery (e.g. getElementById)
  • Use strict operators (e.g. === rather than ==)
  • Save field name in data-dynfields-name attribute (for faster retrieval?)
  • Refine/minimize-repeated selectors
  • Get attribute from JavaScript object rather than from jQuery object using jQuery method

Changed 3 months ago by Ryan J Ollos

Rebased patch.

comment:29 Changed 3 months ago by Ryan J Ollos

In 17521:

TracDynamicFields 2.3.0dev: Optimize code

Patch by Jun Omae.

Refs #9606

comment:30 in reply to:  12 ; Changed 3 months ago by Ryan J Ollos

Replying to Ryan J Ollos:

Looks like 90% of the time is being spent in order_fields. I'll try to look into improving that, but it will be at least a few days before I have time.

It appears that Layout.update (which calls Layout.order_fields) is being called repeatedly. It looks to me like the layout should be updated just once, after applying all the rules. As a further optimization, we could have the rules set a flag that determines if the layout needs to be updated, in order to avoid unnecessary updates.

I tried the following patch, which makes the update very fast, but the layout is not applied correctly. Some fields incorrectly end up with their own row. I will continue to investigate whether this is a valid approach. Jun, do you think this approach should work?

  • dynfields/htdocs/dynfields.js

     
    3636        spec.rule.complete(input, spec);
    3737      });
    3838    });
     39
     40    // update layout (see layout.js)
     41    inputs_layout.update();
     42    header_layout.update();
     43
    3944  };
    4045
    4146  window.setup_triggers = function () {
  • dynfields/htdocs/layout.js

     
    2222    return element !== null && element.tagName.toLowerCase() === 'textarea';
    2323  };
    2424
    25   // Update the field layout given a spec
    26   this.update = function (spec) {
     25  // Update the field layout
     26  this.update = function () {
    2727    var this_ = this;
    2828    var name = this.name;
    2929
  • dynfields/htdocs/rules.js

     
    222222hiderule.complete = function (input, spec) {
    223223  jQuery('#properties .dynfields-hide, #ticket .properties .dynfields-hide').hide();
    224224
    225   // update layout (see layout.js)
    226   inputs_layout.update(spec);
    227   header_layout.update(spec);
    228 
    229225  // add link to show hidden fields (that are enabled to be shown)
    230226  if (spec.link_to_show.toLowerCase() == 'true') {
    231227    if (jQuery('#dynfields-show-link').length == 0) {
Last edited 3 months ago by Ryan J Ollos (previous) (diff)

comment:31 Changed 3 months ago by Ryan J Ollos

In 17522:

TracDynamicFields 2.3.0dev: Fix fields not hidden in header during preview

Patch by Jun Omae.

Refs #12575, #9606.

comment:32 in reply to:  30 Changed 3 months ago by Ryan J Ollos

Replying to Ryan J Ollos:

I tried the following patch, which makes the update very fast, but the layout is not applied correctly. Some fields incorrectly end up with their own row. I will continue to investigate whether this is a valid approach. Jun, do you think this approach should work?

The patch seems to work correctly after r17522.

comment:33 Changed 3 months ago by Ryan J Ollos

In 17523:

TracDynamicFields 2.3.0dev: Update the layout once after rules applied

Refs #9606.

comment:34 Changed 3 months ago by Ryan J Ollos

Resolution: fixed
Status: acceptedclosed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.