ICEfaces
  1. ICEfaces
  2. ICE-9532

ace:dataTable - Improve dynamic stylesheet performance by using cssText instead of insertRule

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.3
    • Fix Version/s: 4.0.BETA, 4.0
    • Component/s: ACE-Components
    • Labels:
      None
    • Environment:
      *
    • Assignee Priority:
      P2

      Description

      Currently using the styleSheet API insertRule method to add our rules as we calculate them likely is triggering multiple CSS recalculations. Possibly a significant amount of performance can be gained by appending the calculated rules to the cssText attribute in batches as infrequently as possible.

        Issue Links

          Activity

          Nils Lundquist created issue -
          Nils Lundquist made changes -
          Field Original Value New Value
          Assignee Nils Lundquist [ nils.lundquist ]
          Ken Fyten made changes -
          Assignee Nils Lundquist [ nils.lundquist ] Arturo Zambrano [ artzambrano ]
          Fix Version/s 3.4 [ 10770 ]
          Assignee Priority P2 [ 10011 ]
          Hide
          Ken Fyten added a comment -

          We believe the issue below may be resolved by implementing the improvement in this JIRA.

          Using Showcase (FF21):

          ace:dataTable > Grouping, Multi Row Header (When expanding/contracting the rows the table header columns become misaligned temporarily, then snap back to proper alignment. ) Reported in revision #37756.

          Show
          Ken Fyten added a comment - We believe the issue below may be resolved by implementing the improvement in this JIRA. Using Showcase (FF21): ace:dataTable > Grouping, Multi Row Header (When expanding/contracting the rows the table header columns become misaligned temporarily, then snap back to proper alignment. ) Reported in revision #37756.
          Hide
          Arturo Zambrano added a comment -

          A first attempt was made for this improvement.

          Here's an example of the original code, using insertRule:

                              selector =  this.jqId+' .ui-col-'+index+' > div';
                          if (styleSheet.insertRule)
                              styleSheet.insertRule(selector + ' { width:' + bodyColumnWidth + 'px; }', styleSheet.length);
                          else if (styleSheet.addRule)
                              styleSheet.addRule(selector, 'width:' + bodyColumnWidth + 'px;');
          

          This is the new code, using cssText:

          				var elements = ice.ace.jq(selector);
          				var size = elements.size();
          				for (var i = 0; i < size; i++) {
          					var element = elements.get(i);
          					element.style.cssText += ';width:' + bodyColumnWidth + 'px;';
          				}
          

          This actually causes the page to freeze (in the case of the Grouping and Scrolling demos in the showcase).

          The cssText string is too long for all the elements matching the selector, and this part of the code is executed once per column. It seems like the entire page styling is recalculated every time an element is updated.

          Show
          Arturo Zambrano added a comment - A first attempt was made for this improvement. Here's an example of the original code, using insertRule: selector = this.jqId+' .ui-col-'+index+' > div'; if (styleSheet.insertRule) styleSheet.insertRule(selector + ' { width:' + bodyColumnWidth + 'px; }', styleSheet.length); else if (styleSheet.addRule) styleSheet.addRule(selector, 'width:' + bodyColumnWidth + 'px;'); This is the new code, using cssText: var elements = ice.ace.jq(selector); var size = elements.size(); for (var i = 0; i < size; i++) { var element = elements.get(i); element.style.cssText += ';width:' + bodyColumnWidth + 'px;'; } This actually causes the page to freeze (in the case of the Grouping and Scrolling demos in the showcase). The cssText string is too long for all the elements matching the selector, and this part of the code is executed once per column. It seems like the entire page styling is recalculated every time an element is updated.
          Hide
          Arturo Zambrano added a comment -

          Committed improvement at revision 38600. Removed the use of styleSheet.insertRule (for inserting two rules per column) and replaced that approach with simply inserting/replacing a text node once in the <style> element, containing all the rules.

          Show
          Arturo Zambrano added a comment - Committed improvement at revision 38600. Removed the use of styleSheet.insertRule (for inserting two rules per column) and replaced that approach with simply inserting/replacing a text node once in the <style> element, containing all the rules.
          Arturo Zambrano made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Repository Revision Date User Message
          ICEsoft Public SVN Repository #38600 Tue Oct 15 11:41:56 MDT 2013 art.zambrano ICE-9532 improved table rendering performance in the client by removing use of styleSheet.insertRule for inserting two rules per column (causing recalculations) and replacing that approach with simply inserting/replacing a text node once in the <style> element, containing all the rules
          Files Changed
          Commit graph MODIFY /icefaces3/trunk/icefaces/ace/component/resources/icefaces.ace/datatable/datatable.js
          Hide
          Ken Fyten added a comment -

          New JS error noticed on IE8 (all IEs I think) when viewing the Scrolling ace:dataTable demo on showcase.

          Show
          Ken Fyten added a comment - New JS error noticed on IE8 (all IEs I think) when viewing the Scrolling ace:dataTable demo on showcase.
          Ken Fyten made changes -
          Resolution Fixed [ 1 ]
          Status Resolved [ 5 ] Reopened [ 4 ]
          Hide
          Ken Fyten added a comment -

          Attached image of IE JS error.

          Show
          Ken Fyten added a comment - Attached image of IE JS error.
          Ken Fyten made changes -
          Attachment IEJSError.png [ 16534 ]
          Hide
          Arturo Zambrano added a comment -

          Committed fixes for javascript errors at revision 38635.

          Show
          Arturo Zambrano added a comment - Committed fixes for javascript errors at revision 38635.
          Arturo Zambrano made changes -
          Status Reopened [ 4 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Repository Revision Date User Message
          ICEsoft Public SVN Repository #38635 Fri Oct 18 12:39:10 MDT 2013 art.zambrano ICE-9532 fixed some javascript errors
          Files Changed
          Commit graph MODIFY /icefaces3/trunk/icefaces/ace/component/resources/icefaces.ace/util/util.js
          Commit graph MODIFY /icefaces3/trunk/icefaces/ace/component/resources/icefaces.ace/datatable/datatable.js
          Hide
          Arturo Zambrano added a comment -

          The issue mentioned in the first comment cannot be solved by the fixes already committed for this JIRA. The cause of that issue is the fact that the entire table markup is updated/replaced by a newly-rendered table. The temporary misalignment and snap back is simply caused by rendering all the nodes by the browser. It only seems to happen on slow browsers like IE. If you observe carefully, when you first load the grouping and multi row header demos you can notice the same temporary misalignment in the header.

          The reasons why the entire table is updated when expanding/contracting rows could be that the row expansions don't have client ids and the corresponding <tr> elements are "inserted" in the original markup. So, a possible solution to this issue could be to always render the expanded rows with a client id, with style="display:none;", and with no content inside of them. Then, when a row is actually expanded, only such <tr> element would be updated, removing the "display:none;" and adding the content inside of it, thus avoiding a full table update.

          Show
          Arturo Zambrano added a comment - The issue mentioned in the first comment cannot be solved by the fixes already committed for this JIRA. The cause of that issue is the fact that the entire table markup is updated/replaced by a newly-rendered table. The temporary misalignment and snap back is simply caused by rendering all the nodes by the browser. It only seems to happen on slow browsers like IE. If you observe carefully, when you first load the grouping and multi row header demos you can notice the same temporary misalignment in the header. The reasons why the entire table is updated when expanding/contracting rows could be that the row expansions don't have client ids and the corresponding <tr> elements are "inserted" in the original markup. So, a possible solution to this issue could be to always render the expanded rows with a client id, with style="display:none;", and with no content inside of them. Then, when a row is actually expanded, only such <tr> element would be updated, removing the "display:none;" and adding the content inside of it, thus avoiding a full table update.
          Arturo Zambrano made changes -
          Link This issue blocks ICE-9674 [ ICE-9674 ]
          Hide
          Carmen Cristurean added a comment -

          Fixes verified with icefaces-ee-3.3.0_P01-Williams-tableConfigEnh-Build rev# 38805 in Firefox25, Chrome30, IE7/8/9/10.

          Show
          Carmen Cristurean added a comment - Fixes verified with icefaces-ee-3.3.0_P01-Williams-tableConfigEnh-Build rev# 38805 in Firefox25, Chrome30, IE7/8/9/10.
          Ken Fyten made changes -
          Summary improve dynamic stylesheet performance by using cssText instead of insertRule ace:dataTable - Improve dynamic stylesheet performance by using cssText instead of insertRule
          Ken Fyten made changes -
          Fix Version/s 4.0 [ 11382 ]
          Ken Fyten made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

            People

            • Assignee:
              Arturo Zambrano
              Reporter:
              Nils Lundquist
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: