[IMP] BottomBarStatistics: faster computation of stats

[rrahir]

Description:

[IMP] BottomBarStatistics: Memoize cell filtering
To compute the statistics, we filter out the cells by type for each
function. However, there are only 2 different filters to apply for the
6 different functions.

This revision introduces a memoization of that filter to compute it 3
times less.

[IMP] BottomBarStatistics: Lazify statistic evaluation
Each time we fetch the statistics, we compute all 6 of them even though
we only need one of them to display (We need all of them when the
statistics selection menu is open).

This revision lazifies the computation of those statistics.

[IMP] BottomBarStatistics: limit redundant getter calls
While we iterate over the cells to check their visibility, we keep
checking the visibility of cells over which we already know that their
col (or row) is hidden.

This revision adds a memoization of limit the calls to the different
getters involved by memoizing them.

Overall Benchmark

On a demo sheet with makeLargeDataset of 260k cells

_computeStatisticFnResult:
Firefox: 200ms to 80ms
Chrome: 130ms to 42ms

Task: : 3827324

review checklist

• feature is organized in plugin, or UI components
• support of duplicate sheet (deep copy)
• in model/core: ranges are Range object, and can be adapted (adaptRanges)
• in model/UI: ranges are strings (to show the user)
• undo-able commands (uses this.history.update)
• multiuser-able commands (has inverse commands and transformations where needed)
• new/updated/removed commands are documented
• exportable in excel
• translations (_t("qmsdf %s", abc))
• unit tested
• clean commented code
• track breaking changes
• doc is rebuild (npm run doc)
• status is correct in Odoo

[robodoo]

Pull request status dashboard.

[LucasLefevre]

we could even make the cell iteration and filter part of the lazy computation

    const fnResult: Lazy<number| undefined> = lazy(() => {
        const evaluatedCells = getCells(fn.types.join(","));
        if (evaluatedCells.length) {
          return fn.compute(evaluatedCells, locale);
        }
        return undefined;
      });

[rrahir]

no we cannot. We check the presence of fnResult in the final object to know if we should display the statistics button or not. So we'd end up evaluating everything all the time

[LucasLefevre]

does it really makes a significant difference ?
I believe the benchmark in the commit message is not the correct one. It's suspiciously the same as the previous commit

[rrahir]

It's around 40ms for each improvement, I left out the commas afterwards, I guess I can make it again if you want ...

[hokolomopo]

And if this getter really cause performance issue (which I'm not sure how since it really does nothing), shouldn't it be memoized inside the plugin rather than here ? That way every call to this getter in other plugin/components is faster

[rrahir]

I don't know what to say. Don't believe me? then try it for yourself.
The reason I haven't set it everywhere right now is because I don't want to have to invalidate the cache all the time and it's pretty much the only place where it's heavily used.

[rrahir]

Sorry I'm a moron and can't make proper commit messages apparently. The gain was around 30° ms. I did another round of benchmark and will update the values accordingly

[VincentSchippefilt]

After the 2 first improvements, the time is mostly spent in isColFolder and isRowFolder (of header_grouping.ts).
I think it would be wise to store on this plugin the list of cols and rows that are folded by sheet, so these getters becomes immediate.