ICEfaces
  1. ICEfaces
  2. ICE-11155

Performance analysis of radioButton and checkboxButton in dataTable and ui:repeat

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.0, EE-4.1.0.GA
    • Fix Version/s: 4.2.BETA, 4.2
    • Component/s: ACE-Components
    • Labels:
      None
    • Environment:
      jsf2 virtualVM, Google Chrome profiler
    • Assignee Priority:
      P2

      Description

      both checkboxButton and radioButton have 4 script tags.
      one for init, one for renderRest and the same script for onfocus and onhover.

      Decided to look at this in a vm profiler (used virtualVM) and the js profiler (used Google Chrome). Attaching the screenshots for these.

      Currently showcase has buttonGroup examples which show the buttons in columns of a dataTable as well as in rows for ui:repeat.
      these examples (now) show the ability to change the number of rows. Initially the number of rows is set to 10, but you can increase to 100, 500 and 1000. These screenshots show the results of 1000 for virtual-vm and the js console shows the example for by row (using ui:repeat) as it doesn't use the dataTable which may also show some performance lag.
      for example, the by row example is at showcase (myIP):-

      http://10.18.39.137:8080/showcase/showcase.jsf?grp=ace:buttonGroup&exp=Group%20In%20Row

      ALSO...if js is the issue--js could only toggle off the previously selected button instead of all the other buttons... (500 rows in dataTable takes about 5 seconds but only has 2 radioButtons and 2 checkboxButtons per row). 500 rows in row example takes about 8seconds but has 3 radioButtons and 3 checkboxButtons per row.
       
      1. buttonGroupByRow 1000 rows.png
        820 kB
      2. non-optimized.png
        271 kB
      3. optimized.png
        277 kB
      4. profiling js.png
        502 kB
      5. Screen Shot showcase buttonGroup 1000 rows CPU Column.png
        537 kB

        Activity

        Hide
        Judy Guglielmin added a comment -

        screen shots for showcase examples with 1000 rows.

        Show
        Judy Guglielmin added a comment - screen shots for showcase examples with 1000 rows.
        Hide
        Judy Guglielmin added a comment -

        Google js profiler for 1000 rows of the ui:repeat example for buttonGroup from the showcase link described above.

        Show
        Judy Guglielmin added a comment - Google js profiler for 1000 rows of the ui:repeat example for buttonGroup from the showcase link described above.
        Hide
        Mircea Toma added a comment -

        It turns out that the bottleneck is caused by the inline scripts rendered by radio and checkbox buttons, more precisely the <script> elements not the event handlers. This is mainly Mojarra's fault since their implementation of evaluating the scripts is slow, the Mojarra bridge adds the script element to the document to be evaluated and then it removes it.

        In the case of the checkbox button the ice.ace.checkboxbutton.register is invoked in the inline script. The function records which checkbox buttons are rendered so that later on it can implement the mutuallyExclusive feature. To avoid having the inline script we could invoke ice.ace.checkboxbutton.register function only in the initialization code (which is lazily executed).
        I have not found any cleanup code for the lists used ice.ace.checkboxbutton.register. This should be added since the list can change their content dynamically and any object accumulation along the time should be avoided.

        In the case tof the radio button I don't see lazy loading being implemented. Having the lazy loading implmented it would move the intialization code from the script element into the "onfocus" event handler, and thus workaround the bottleneck.

        Show
        Mircea Toma added a comment - It turns out that the bottleneck is caused by the inline scripts rendered by radio and checkbox buttons, more precisely the <script> elements not the event handlers. This is mainly Mojarra's fault since their implementation of evaluating the scripts is slow, the Mojarra bridge adds the script element to the document to be evaluated and then it removes it. In the case of the checkbox button the ice.ace.checkboxbutton.register is invoked in the inline script. The function records which checkbox buttons are rendered so that later on it can implement the mutuallyExclusive feature. To avoid having the inline script we could invoke ice.ace.checkboxbutton.register function only in the initialization code (which is lazily executed). I have not found any cleanup code for the lists used ice.ace.checkboxbutton.register . This should be added since the list can change their content dynamically and any object accumulation along the time should be avoided. In the case tof the radio button I don't see lazy loading being implemented. Having the lazy loading implmented it would move the intialization code from the script element into the "onfocus" event handler, and thus workaround the bottleneck.
        Hide
        Mircea Toma added a comment - - edited

        Modified ace:checkBoxButton to save initialization code in the data-init attribute of the component's root element to be used in multiple places (3 places to be exact) when widget needs to be lazy loaded. Factored out ice.ace.registerLazyComponent and ice.ace.evalInit utility functions to minimize rendered markup. Use JavaScriptRunner to render inline code to improve performance.

        Show
        Mircea Toma added a comment - - edited Modified ace:checkBoxButton to save initialization code in the data-init attribute of the component's root element to be used in multiple places (3 places to be exact) when widget needs to be lazy loaded. Factored out ice.ace.registerLazyComponent and ice.ace.evalInit utility functions to minimize rendered markup. Use JavaScriptRunner to render inline code to improve performance.
        Hide
        Mircea Toma added a comment -

        Modified ace:radioButton renderer to lazy initialize the client-side component code. Also implemented the same strategies for minimizing rendered inline code.

        Show
        Mircea Toma added a comment - Modified ace:radioButton renderer to lazy initialize the client-side component code. Also implemented the same strategies for minimizing rendered inline code.
        Hide
        Mircea Toma added a comment -

        Cleanup client IDs from the registered checkbox map when the components are removed from DOM.

        Show
        Mircea Toma added a comment - Cleanup client IDs from the registered checkbox map when the components are removed from DOM.
        Hide
        Liana Munroe added a comment -

        Tested with ICEfaces 4 trunk r49407, Tomcat 8, IE 11, FF 47 Chrome 54
        Issues found:
        1.) ace:radioButton apps that are not using a dataTable or repeat structure are not functioning properly. mutuallyExclusive = true but is not respected.
        AN example of this can be found in the showcase ace:radioButton > Overview and Custom Style apps.
        To reproduce:
        Open either of the radioButton showcase demos and select the unselected radioButton. Both buttons stay selected. After they are both selected you can select one to force it to deselect (not proper behaviour). After that they become mutually exclusive.

        2.) ace:radioButtons no longer retain their itemValue once selected. They return false instead. This can be seen in the showcase ace:radioButtons > Overview.
        When first loading the demo radioButton One is selected, the value shown is "The selected radios are: One". After selecting any other radioButton the value shown is "The selected radios are: false". This issue is seen whether or not the radioButtons are inside a dataTable or repeat structure.

        3.) Showcase ace:checkboxButtons > all demos fail. No buttons can be selected. There are console errors when loading the demo
        Chrome: coalesced.js.jsf?ln=ice.core&dgst=kbiglm:14029 Uncaught TypeError: Cannot read property 'init' of undefined(…)
        FF:
        TypeError: a.dataset is undefined

        Show
        Liana Munroe added a comment - Tested with ICEfaces 4 trunk r49407, Tomcat 8, IE 11, FF 47 Chrome 54 Issues found: 1.) ace:radioButton apps that are not using a dataTable or repeat structure are not functioning properly. mutuallyExclusive = true but is not respected. AN example of this can be found in the showcase ace:radioButton > Overview and Custom Style apps. To reproduce: Open either of the radioButton showcase demos and select the unselected radioButton. Both buttons stay selected. After they are both selected you can select one to force it to deselect (not proper behaviour). After that they become mutually exclusive. 2.) ace:radioButtons no longer retain their itemValue once selected. They return false instead. This can be seen in the showcase ace:radioButtons > Overview. When first loading the demo radioButton One is selected, the value shown is "The selected radios are: One". After selecting any other radioButton the value shown is "The selected radios are: false". This issue is seen whether or not the radioButtons are inside a dataTable or repeat structure. 3.) Showcase ace:checkboxButtons > all demos fail. No buttons can be selected. There are console errors when loading the demo Chrome: coalesced.js.jsf?ln=ice.core&dgst=kbiglm:14029 Uncaught TypeError: Cannot read property 'init' of undefined(…) FF: TypeError: a.dataset is undefined
        Hide
        Mircea Toma added a comment -

        1) Fixed ice.ace.registerLazyComponent to return the widget reference when just created.

        2) Modified CheckboxButtonsRenderer to replicate what scripts are rendered by CheckboxButtonRenderer after the recent changes. Same was done for RadioButtonsRenderer. I didn't realize how little code is reussed by the buttons renderers.

        3) Changed RadioButtonsRenderer to avoid using the inherited RadioButtonRenderer.getConvertedValue method since the value is always cast to a boolean. This issue was actually a regression introduced by ICE-11153 refactoring.

        Show
        Mircea Toma added a comment - 1) Fixed ice.ace.registerLazyComponent to return the widget reference when just created. 2) Modified CheckboxButtonsRenderer to replicate what scripts are rendered by CheckboxButtonRenderer after the recent changes. Same was done for RadioButtonsRenderer . I didn't realize how little code is reussed by the buttons renderers. 3) Changed RadioButtonsRenderer to avoid using the inherited RadioButtonRenderer.getConvertedValue method since the value is always cast to a boolean. This issue was actually a regression introduced by ICE-11153 refactoring.
        Hide
        Mircea Toma added a comment - - edited

        Here are the optimisation results. The profiling was run during the switching from 10 to 1000 rows in showcase > buttonGroup > Group In Column demo.

        Before optimization the measured interval (using visual clues) was 8 seconds. The profiling shows that our JS scripts took about 5 seconds.

        After optimization the measured interval (using visual cues) was 2 seconds. The profiling show that only 890 milliseconds was used by JS, 360 milliseconds was the actual time used by our code.

        Show
        Mircea Toma added a comment - - edited Here are the optimisation results. The profiling was run during the switching from 10 to 1000 rows in showcase > buttonGroup > Group In Column demo. Before optimization the measured interval (using visual clues) was 8 seconds. The profiling shows that our JS scripts took about 5 seconds. After optimization the measured interval (using visual cues) was 2 seconds. The profiling show that only 890 milliseconds was used by JS, 360 milliseconds was the actual time used by our code.
        Hide
        Mircea Toma added a comment - - edited

        1) I don't see any difference in behaviour from EE 4.1.0 GA release. One still needs to click on "Submit" button (after "Clear" button is clicked) to actually clear the outputText that renders the selected item. This is needed because the "Clear" button does not issue a submit, it just clears the radio buttons locally.

        2) This is again a regression caused by ICE-11153 commits. The fix restores RadioButtonsRenderer.renderResetSettings method to properly invoke the ice.ace.radionbutton.clear function for multiple selection (instead of relying on the inherited method meant for RadioButton).

        Show
        Mircea Toma added a comment - - edited 1) I don't see any difference in behaviour from EE 4.1.0 GA release. One still needs to click on "Submit" button (after "Clear" button is clicked) to actually clear the outputText that renders the selected item. This is needed because the "Clear" button does not issue a submit, it just clears the radio buttons locally. 2) This is again a regression caused by ICE-11153 commits. The fix restores RadioButtonsRenderer.renderResetSettings method to properly invoke the ice.ace.radionbutton.clear function for multiple selection (instead of relying on the inherited method meant for RadioButton ).

          People

          • Assignee:
            Mircea Toma
            Reporter:
            Judy Guglielmin
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: