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
        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: