YUILibrary - Open source JavaScript and CSS for building richly interactive software.
Fork YUI on GitHub

YUI 2.x

Ticket #2528834 (accepted enhancement)

Reporter


Jonathan Morgan 		Opened: 03/10/10
Last modified: 05/5/10
Status: accepted
Type: enhancement

Owner


Jenny Han Donnelly 		Target Release: 2.9.0
Priority: P3 (normal)
Summary: DataTable.addRow() spending too much time in _setRowStripes()
Description:

After you have a fairly small number of rows in a datatable, at least half of the time spent in addRow() will be spent in _setRowStripes(). Almost all of this time was spent in YUI.DOM._replaceClass,
setting a new class for all of the rows in the table, not just for the newly added row. I suggest that _replaceClass be changed to only set the className if it is different from the current
className (which has already been obtained), since setting the class is likely to always be considerably more expensive than making the check. Something like the following code:



  var classNameStr = trim(className.join(EMPTY));                   

  if (classNameStr !== current) {

    Y.Dom.setAttribute(el, CLASS_NAME, classNameStr);

  }

Type: enhancement Observed in Version: 2.7.0
Component: DataTable Severity: S3 (normal)
Assigned To: Jenny Han Donnelly Target Release: 2.9.0
Location: Library Code Priority: P3 (normal)
Tags: datatable, replaceclass Relates To:
Browsers: All
URL:
Test Information:

Change History

Jonathan Morgan
Posted: 04/23/10

Looking at this again today, I found another possible solution. _setRowStripes() takes parameters to only restripe a range of rows rather than the entire table. This removes the need to even check the CSS class for all of the other rows and find that it hasn't changed. Changing the call in addRow() to the below seems a good idea (and I imagine similar could be done for addRows() and maybe other methods that call _setRowStripes).

this._setRowStripes(recIndex, 1);

I still think the change to YUI.Dom._replaceClass makes sense.

Jenny Han Donnelly

YUI Developer
Posted: 05/1/10
  • location changed to Library Code
  • milestone changed to 2.9.0
  • priority changed to P3 (normal)
  • status changed from new to accepted

Jonathan Morgan
Posted: 05/5/10

My comment above will only work if you are adding rows to the end of the table. It seems you can add rows to the middle as well, so you would want to restripe every row after the given index:

this._setRowStripes(recIndex);