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

          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.
          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.
          Hide
          Ken Fyten added a comment -

          Attached image of IE JS error.

          Show
          Ken Fyten added a comment - Attached image of IE JS error.
          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.
          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.
          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.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: