Announcement

Collapse
No announcement yet.
X
  • Filter
  • Time
Clear All
new posts

    selectAllRecords is very slow

    SmartGWT Nightly build 2011-02-11
    GWT 2.1.1
    Browser: Chrome 9.0
    OS: Mac OS X 10.6.6, 2.4GHz Intel Core 2 Duo

    It takes 24,924ms to select/deselect all records from list grid of 5000 records.
    Here is the set up:
    Code:
        ListGrid grid = new ListGrid();
        grid.setWidth100();
        grid.setHeight100();
        grid.setFields(new ListGridField("name", "Name"));
        grid.setSelectionType(SelectionStyle.SIMPLE);
        grid.setSelectionAppearance(SelectionAppearance.CHECKBOX);
    
        ListGridRecord[] records = new ListGridRecord[5000];
        for (int i = 0 ; i < records.length; ++i) {
          BsdJs.Map map = BsdJs.Map.create();
          map.put("name", "Name " + i);
          records[i]= new ListGridRecord(map);
        }
    
        grid.setRecords(records);
        grid.draw();
    The culprit seems to be in ListGrid.GridBody.updateRowSelection.

    Based on my basic understanding from looking at SmartClient's code, when selectAllRecords is invoked, it traverses through each record item and invokes setSelected to mark the item as selected. It also marks the selection object as dirty so it can be redrawn later. There is an observer on setSelected that at the end of this call it invokes _rowSelectionChanged, which internally invokes updateRowSelection. Here is the definition for this method:
    Code:
     // override updateRowSelection to update selectionCanvas if necessary
        updateRowSelection : function (rowNum) {
            var lg = this.grid;
            if (!lg) return;
    
    
    
            if (lg.showSelectionCanvas) lg.updateSelectionCanvas();
            if (!lg._dontRefreshSelection) {
                this.invokeSuper(isc.GridBody, "updateRowSelection", rowNum);
            }
    
            if (lg.getCurrentCheckboxField() != null) {
                var cellNum = lg.getCheckboxFieldPosition();
                if (lg && !lg._dontRefreshSelection) lg.refreshCell(rowNum, cellNum);
                var validData = (isc.isAn.Array(lg.data) || (isc.isA.ResultSet(lg.data)
                            && lg.data.allMatchingRowsCached())),
                    selection = lg.getSelection() || [];
    
                if (validData) {
                    if (selection.length == lg.data.getLength()) {
                        lg._setCheckboxHeaderState(true);
                    } else {
                        lg._setCheckboxHeaderState(false);
                    }
                }
    
    	} else if (lg.getTreeFieldNum && lg.selectionAppearance == "checkbox") {
                var treeCellNum = lg.getTreeFieldNum();
                if (!lg._dontRefreshSelection) {
                    lg.refreshCell(rowNum, treeCellNum);
                }
            }
        },
    Every time this method is invoked (i.e. for each item), it calls lg.getSelection to recompute the selected records which is very expensive. I think most of the code in this method is unnecessary for select all/deselect all. Furthermore, if the comment stated above this method is true, it *only* needs to call lg.updateSelectionCanvas() anyway. Here is my proposed change:
    Code:
    // override updateRowSelection to update selectionCanvas if necessary
        updateRowSelection : function (rowNum) {
            var lg = this.grid;
            if (!lg) return;
    
            if (lg.showSelectionCanvas) lg.updateSelectionCanvas();
            if (lg._dontRefreshSelection) return;
    
            this.invokeSuper(isc.GridBody, "updateRowSelection", rowNum);
            
            if (lg.getCurrentCheckboxField() != null) {
                var cellNum = lg.getCheckboxFieldPosition();
                lg.refreshCell(rowNum, cellNum);
                var validData = (isc.isAn.Array(lg.data) || (isc.isA.ResultSet(lg.data)
                            && lg.data.allMatchingRowsCached())),
                    selection = lg.getSelection() || [];
    
                if (validData) {
                    if (selection.length == lg.data.getLength()) {
                        lg._setCheckboxHeaderState(true);
                    } else {
                        lg._setCheckboxHeaderState(false);
                    }
                }
    
    	} else if (lg.getTreeFieldNum && lg.selectionAppearance == "checkbox") {
                var treeCellNum = lg.getTreeFieldNum();
                lg.refreshCell(rowNum, treeCellNum);
            }
        },
    With this change, it only takes 254ms to select all/deselect all records: 99% performance improvement.

    This problem exacerbates when dealing with tens of thousands of records which is what we are running into in production. We are currently resisting making this change on our end because it involves making code change at implementation level in SmartClient javascript framework. Please make the fix ASAP so everyone can benefit from this.

    Thanks,
    Kevin
    Last edited by knguyen; 11 Feb 2011, 20:53.

    #2
    Hi Kevin
    Thanks for pointing this out and taking the time to dig into it.
    We've made this change internally - it will be present in builds from this point on.

    Regards
    Isomorphic Software

    Comment


      #3
      I too am working with large grids. I love the Live Grid implementation...but it seems to have a big big limitation.

      If you try to do a "select All" or a large selection of any sort I get the alert:

      "Can't select that many records at once.
      Please try working in smaller batches."

      From another post (and by testing) it seem as if this alert is coming up if you haven't loaded all the rows ...in my case:

      grid.setShowAllRecords(false);
      grid.setAutoFetchData(true);

      So in order to select every row, I have to scroll through the grid and load each page and then I can do a select-all.

      This is really a big big limitation.

      It seems like the "selection" should be maintained not as a list of selected rows but as some kind of meta-data where you can compute whether or not a row is selected or not...or you can compute the list of selected rows.

      Then when the grid is scrolled to a different range... you can compute which rows should be shown as "selected".

      I could bolt on something on the outside of ListGrid...using RowMouseDownEvent and DataArrivedEvent...but this seems rather silly...it should ought to work out of the box this way.

      That being said... I need to make this happen so I'm working on bolting this on outside of the SmartGWT code base. The question here is how can I tell if the shift-key is down when the user clicks on a row?

      Many thanks for all the help.

      - -Bob

      Comment


        #4
        We are having issues with large datasets as well:

        http://forums.smartclient.com/showthread.php?t=13901

        Comment


          #5
          Has this fix been reverted back in SmartGWT 2.5p LGPL nightly builds?
          I used the latest one as of today (2012-02-22) and it takes above 5.7 secs to select and above 4.8secs to deselect the example's 5000 records. I tested this on Linux and Windows, with Firefox 10, Chrome 19 and IE 9, compiled with GWT 2.4.0.

          I also tested it with 3.0p LGPL nightly build, same specs as mentioned above, and the times are greatly reduced ... around 0.6secs in all browsers (in some even slower). You, probably, will say, to use the 3.0p, but maybe some aren't ready to do the transition, just yet. Can you please verify that the fix is on in both versions?

          Comment

          Working...
          X