[GH-DEV1530] Performance degrade from v12.x.x

Tags: #<Tag:0x00007efc6253fd30> #<Tag:0x00007efc6253fbf0> #<Tag:0x00007efc6253fa88>
#1

Hi, guy
As discussed at https://github.com/handsontable/handsontable/issues/5687#issuecomment-1788258011

I check the performance of mergeing cells and found it degrades sharply from v12.x.x. I paste my sample code at the link. When I go back to v11.x.x, it takes about 4s, but v12.x.x will takes about 60s.

Could anyone help me to check it?

Hope for your reply!
Thanks

#2

Hi @space.chang

Thank you for creating your own thread. That seems to be a better idea.

To clarify, let me format the code, as the unformatted version is tough to read.

<script>
import 'handsontable/dist/handsontable.full.css';
import * as Vue from 'vue';
import Handsontable from 'handsontable';
import { HotTable } from '@handsontable/vue3';

function getDebugInfo() {
  let debug = "Handsontable:";
  debug += ` v${Handsontable.version}`;
  debug += ` (${HotTable.version}`;
  debug += " Vue:";
  debug += ` v${Vue.version}`;
  return debug;
}

function generateData() {
  let _data = [];
  let _mergeData = [];
  for (let i = 0; i < 10000; i++) {
    let _i = parseInt(i / 5);
    if (i % 5 == 0) {
      _mergeData.push({ row: i, col: 0, rowspan: 5, colspan: 1 });
      _mergeData.push({ row: i, col: 3, rowspan: 5, colspan: 1 });
    }
    _data.push([
      `A${_i}`,
      `B${i}`,
      `C${i}`,
      `E${i}`,
      `E${i}`,
      `A1${i}`,
      `B1${i}`,
      `C1${i}`,
      `E1${i}`,
      `E1${i}`,
      `A${i}`,
      `B${i}`,
      `C${i}`,
      `E${i}`,
      `E${i}`,
      `A1${i}`,
      `B1${i}`,
      `C1${i}`,
      `E1${i}`,
      `E1${i}`,
    ]);
  }
  return { data: _data, mergeData: _mergeData };
}

console.log(getDebugInfo());
let _ret = generateData();

export default {
  name: "App",
  data: function () {
    return {};
  },
  setup() {
    Vue.onMounted(() => {
      let div = document.getElementById('example');
      console.time('HandsonTable');
      let table = new Handsontable(div, {
        data: _ret.data,
        colWidths: [
          80, 80, 80, 80, 80, 80, 80, 80, 80, 80, 80, 80, 80, 80, 80, 80, 80, 80, 80, 80
        ],
        colHeaders: false,
        rowHeaders: true,
        licenseKey: "non-commercial-and-evaluation",
        // mergeCells: _ret.mergeData,
        height: '100%',
        width: '100%',
        // dropdownMenu:['filter_by_value','filter_action_bar'],
        dropdownMenu: {
          items: {
            filter_by_value: {
              // hide the 'Filter by value' list from all columns but the first one
              hidden() {
                return this.getSelectedRangeLast().to.col > 0;
              },
            },
            filter_action_bar: {
              // hide the 'OK' and 'Cancel' buttons from all columns but the first one
              hidden() {
                return this.getSelectedRangeLast().to.col > 0;
              },
            }
          }
        },
      },
      filters: true,
      afterGetColHeader(col, TH) {
        // remove the column menu button from the 'Brand', 'Price', and 'Date' columns
        if (col > 0) {
          const button = TH.querySelector('.changeType');
          if (!button) {
            return;
          }
          button.parentElement.removeChild(button);
        }
      }
    });
    console.timeEnd('HandsonTable');
    table.batch(() => {
      console.time('B');
      table.updateSettings({ mergeCells: _ret.mergeData });
      console.timeEnd('B');
    });
  }
};
</script>

<style>
body {
  height: 100%;
  width: 100%;
  display: block;
}
</style>

Now, it will be easier to analyze what is done.

ps. didnā€™t you say that the

 _mergeData.push({ row: i, col: 0, rowspan: 5, colspan: 1 });
 _mergeData.push({ row: i, col: 3, rowspan: 5, colspan: 1 });

is already out of the generateData() function?

#3

Hi @aleksandra_budnik what do you mean? the code can run smoothly and the performance statistic of merging cells can be output at

table.batch(() => {
console.time(ā€˜Bā€™);
table.updateSettings({ mergeCells: _ret.mergeData });
console.timeEnd(ā€˜Bā€™);
});

Thanks!

#4

Hi @aleksandra_budnik how is it going?
if it is confirmed that performance degrades from v12.x.x, what is your plan to the improved version?
Due to your date-based license policy, we have to re-evaluate it and make a decision.

Hope for your reply!
Thanks!

#5

Hi @space.chang

None of our tests confirms that the v12 is noticeably slower than the previous version with mergeCells on.

Hereā€™s is one of the tests

https://jsfiddle.net/4qx8kgo0/1/ - 13.1.0 (newest version)
https://jsfiddle.net/4qx8kgo0/1/ - 11.0

And it seems that the loading time is similar, and the new versions are slightly better while scrolling.

If you wish I can schedule a full code review, but I would need to get a working demo (not a text) to do so. Then, the team may also suggest some changes to make the code more efficient.

#6

Hi @aleksandra_budnik, your test data only contains 3 merged blocks. Im my case, there are 4,000 merged blocks and each block contains 5 cells. you can check the ā€œ_mergeDataā€ in ā€œgenerateDataā€. I think it is related to that.

Hope for your further check and reply!
Thanks!

#7

You are right with 4000 merges for a huge colspan we can see a significant drop in performance

https://jsfiddle.net/jmxq0u8y/7/ 12.0. - 33 seconds to load
https://jsfiddle.net/2xLsdqr4/4/ 11.1. - 2.8 seconds to load

and calling

hot.batch( () => {
	hot.updateSettings({mergeCells: result})
})

does not seem to cut off more than 2 seconds.

Thank you for sharing this case. I will report it to the team and do my best to fix it soon.

#8

Hi @aleksandra_budnik Could you share the release plan for the improved version? As i said the license policy is date-based, i need to synchronize it with others and make further decission.

Hope for your reply!
Thanks!

#9

We have a sprint planning meeting on the 8th of November. Before the date I wonā€™t be able to tell anything.

We are also in the code freeze of a new major version (planned to be released around 15th of November). Our typical release cycle is 6-8 weeks, so the soonest possible option is the end of December (probably will be start of January as itā€™s a holiday season). However, as our tech team did not see this issue yet I am not able to tell if it will make it to the next release.

ps. Could you please message me at support@handsontable.com to share your current support and maintenance plan package? Sometimes that can have an impact on the priority of the issue.

#10

Sorry. I cannot send outside email due to some policies. And I hope your can prioritize this issue because as you said it is significant drop and it affects all of the users who are using/will use the versions starting from v12.x.x when merging enougg cells.

Looking forward to good news!
Thanks.

#11

No worries. I will keep you updated.

#12

Hi @aleksandra_budnik
How is it going? and is it appended to the release plan?

Thanks
BR

#13

Hi @space.chang

We did not release a version yet (only then we can plan another one) since we spoken.

I will surely update you when it happen.

#14

@aleksandra_budnik any word on this? We are facing the same serious user experience issue for one of our grids that have a lot of merges.

Thanks.

#15

Hi @dhaber

This issue just got a higher priority. I will keep you updated.

1 Like
#16

We are currently evaluating v14. Iā€™m having the same problem with an application that didnā€™t have any problems in v6.

#17

Hi @hiroshia

I will make sure to inform you when it is ready. We already have some ideas for the pull request so that should not take too much time.

1 Like
#18

Hi @hiroshia @dhaber @space.chang

I am happy to inform you that this issue was fixed in the newest release of Handsontable 14.5: https://github.com/handsontable/handsontable/pull/10995

1 Like
#19