Modify

Opened 6 years ago

Closed 5 years ago

#3531 closed enhancement (fixed)

performance of gridmod is poor

Reported by: abbywinterscom Owned by: abbywinterscom
Priority: normal Component: GridModifyPlugin
Severity: normal Keywords: performance
Cc: Trac Release: 0.11

Description

Page load times for reports and queries with this plugin are very bad. The slowdown seems to come from the call to Genshi's Transformer. Page loads are 11-16 seconds (compared to approx 4-5 seconds with the plugin disabled).

It seems to be worse on my python2.3 server than on my python2.5 server. This is anecdotal only and may be due to different hardware rather than different python version.

Any tips or patches anyone has to improve the performance of this Transform (or to use a different method entirely to insert the select tags), perhaps by moving the DOM manipulation into JavaScript, would be greatly appreciated!

Attachments (1)

patch.zip (5.2 KB) - added by mmastrac 6 years ago.
Updated patch

Download all attachments as: .zip

Change History (8)

comment:1 Changed 6 years ago by abbywinterscom

  • Keywords performance added
  • Trac Release changed from 0.10 to 0.11
  • Type changed from defect to enhancement

comment:2 Changed 6 years ago by peter@…

You may want to consider the approach used by EstimationToolsPlugin, which uses jquery to dynamically add an editor to the table when the user clicks in the table cell. There doesn't appear to be any change to the text of the table HTML to accomplish this.

comment:3 Changed 6 years ago by mmastrac

This patch seems to improve things for us a bit. It reduces the HTML sent to the client and disables the selects until they are populated. I'm brainstorming some ways to make this better:

Index: gridmod/web_ui.py
===================================================================
--- gridmod/web_ui.py (revision 5160)
+++ gridmod/web_ui.py (working copy)
@@ -95,7 +95,7 @@

for field in TicketSystem(self.env).get_ticket_fields():

if fieldtype == 'select':

xpath = '*[contains(@class, "tickets")]td[contains(@class, "%s")]' % (fieldname?)

  • select = tag.select(name=fieldname?)

+ select = tag.select(name=fieldname?, class_="gridmod_form", disabled="disabled")

# HACK: For some reason custom fields that have a blank value
# as a valid option don't actually have that blank
# value among the options in fieldoptions? so

@@ -105,7 +105,7 @@

select.append(tag.option())

for option in fieldoptions?:

select.append(tag.option(option, value=option))

  • stream |= Transformer(xpath).wrap(tag.td(class_=fieldname?)).rename('div').attr('class', 'gridmod_default').attr('style', 'display: none').before(tag.form(select, class_='gridmod_form'))

+ stream |= Transformer(xpath).wrap(tag.td(class_=fieldname?)).rename('div').attr('class', 'gridmod_default').attr('style', 'display: none').before(select)

return stream


def _get_action_controllers(self, req, ticket, action):

Index: gridmod/htdocs/gridmod.js
===================================================================
--- gridmod/htdocs/gridmod.js (revision 5160)
+++ gridmod/htdocs/gridmod.js (working copy)
@@ -14,7 +14,8 @@

$(".gridmod_form").each(function(i) {

var gridmod_default = $.trim($(this).next('.gridmod_default').text());
$(this).find('option[value="'+gridmod_default+'"]').attr('selected', 'selected');

  • $(this).children('select').change(function() {

+ $(this).removeAttr('disabled');
+ $(this).change(function() {

var ticket_field = $(this).attr('name');
var ticket_number = $(this).parents('tr').find('[class="id"],[class="ticket"]').text();
ticket_number = ticket_number.replace(/[\d]/g, );

comment:4 Changed 6 years ago by mmastrac

Augh.. forgot to quote.

Index: gridmod/web_ui.py
===================================================================
--- gridmod/web_ui.py	(revision 5160)
+++ gridmod/web_ui.py	(working copy)
@@ -95,7 +95,7 @@
             for field in TicketSystem(self.env).get_ticket_fields():
                 if field['type'] == 'select':
                     xpath = '//*[contains(@class, "tickets")]//td[contains(@class, "%s")]' % (field['name'])
-                    select = tag.select(name=field['name'])
+                    select = tag.select(name=field['name'], class_="gridmod_form", disabled="disabled")
                     # HACK: For some reason custom fields that have a blank value
                     # as a valid option don't actually have that blank
                     # value among the options in field['options'] so
@@ -105,7 +105,7 @@
                         select.append(tag.option())
                     for option in field['options']:
                         select.append(tag.option(option, value=option))
-                    stream |= Transformer(xpath).wrap(tag.td(class_=field['name'])).rename('div').attr('class', 'gridmod_default').attr('style', 'display: none').before(tag.form(select, class_='gridmod_form'))
+                    stream |= Transformer(xpath).wrap(tag.td(class_=field['name'])).rename('div').attr('class', 'gridmod_default').attr('style', 'display: none').before(select)
         return stream
     
     def _get_action_controllers(self, req, ticket, action):
Index: gridmod/htdocs/gridmod.js
===================================================================
--- gridmod/htdocs/gridmod.js	(revision 5160)
+++ gridmod/htdocs/gridmod.js	(working copy)
@@ -14,7 +14,8 @@
     $(".gridmod_form").each(function(i) {
         var gridmod_default = $.trim($(this).next('.gridmod_default').text());
         $(this).find('option[value="'+gridmod_default+'"]').attr('selected', 'selected');
-        $(this).children('select').change(function() {
+        $(this).removeAttr('disabled');
+        $(this).change(function() {
             var ticket_field = $(this).attr('name');
             var ticket_number = $(this).parents('tr').find('[class="id"],[class="ticket"]').text();
             ticket_number = ticket_number.replace(/[^\d]/g, '');

Changed 6 years ago by mmastrac

Updated patch

comment:5 Changed 6 years ago by mmastrac

OK, I took a different tack here to improve performance. The real reason it's slow is that there are N xpath stream modifiers happening at the same time, where N is the number of fields that have a <select> box, irregardless of whether they are visible in the report.

Instead of refactoring that code, I modified the server side to generate a hidden div that holds a bunch of template <select> controls. The JS then inserts these <select> controls in the form after the page loads. You don't see this happening because of the point at which the replacement occurs.

In addition, I've added some images to show what's happening on the server.

One bonus advantage of this design is that the code degrades gracefully if JS isn't enabled. It also opens the door to enabling/disabling this mode through a button click.

comment:6 Changed 6 years ago by anonymous

Fantastic. This is exactly the approach I was planning to take when I got the time to work on this performance issue. I will review and integrate your patch next week.

Thanks for the submission!

comment:7 Changed 5 years ago by abbywinterscom

  • Resolution set to fixed
  • Status changed from new to closed

Patch applied in [6614]. Thanks!

Add Comment

Modify Ticket

Action
as closed .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from abbywinterscom. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
Author


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

 
Note: See TracTickets for help on using tickets.