Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FW][PERF] evaluation: (much) faster array formulas #4622

Closed
wants to merge 3 commits into from

Conversation

fw-bot
Copy link
Collaborator

@fw-bot fw-bot commented Jul 8, 2024

Description:

description of this task, what is implemented and why it is implemented that way.

Task: : TASK_ID

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

Forward-Port-Of: #4619
Forward-Port-Of: #4589

laa-odoo and others added 2 commits July 8, 2024 10:41
When spreading the result of an array formula, we invalidate formulas
depending on the spread zone. Currently, we look for the dependencies
one position in the spread zone at a time.

It's much more efficient to group all those individual positions in zones
and call `this.getCellsDependingOn(...)` with those zones.

This commit reduces the time to evaluate the large array formula
dataset from 60+ seconds to ~36 seconds.

Task: 4036899
X-original-commit: 565dbdc
@robodoo
Copy link
Collaborator

robodoo commented Jul 8, 2024

Pull request status dashboard

@fw-bot
Copy link
Collaborator Author

fw-bot commented Jul 8, 2024

This PR targets saas-17.3 and is part of the forward-port chain. Further PRs will be created up to master.

More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port

@fw-bot
Copy link
Collaborator Author

fw-bot commented Jul 8, 2024

@LucasLefevre @VincentSchippefilt ci/runbot failed on this forward-port PR

Let's say we have the following array formulas:

in B1: =transpose(transpose(A1:A2))
in C1: =transpose(transpose(B1:B2))
in D1: =transpose(transpose(C1:C2))

Each columns depends on the array formula in the previous column.

Initially, all cells are evaluated in the order B1,C1,D1

When B1 is evaluated, all cells that depends on its result (B1 and B2)
are marked to be re-evaluated in another iteration.
=> C1 and D1 are added to `this.nextPositionsToUpdate`

At the next iteration (which evaluated C1 and D1), when C1 is evaluated
it goes again: D1 is added to `this.nextPositionsToUpdate` for a third
iteration.

Coming back to the initial iteration: even if C1 and D1 are in
`this.nextPositionsToUpdate`, they are later computed in the *current*
iteration (remember the initial iteration evaluates B1,C1,D1).
It's useless to re-compute them since they are already computed (and
no other result invalidated their result)

The evaluation of the large array formula dataset goes from ~36s
to ~750ms

Task: 4036899
X-original-commit: de8b6b1
@LucasLefevre LucasLefevre force-pushed the saas-17.3-17.0-perf-spread-lul-x_Ir-fw branch from e145eaf to 635ac67 Compare July 8, 2024 09:06
@LucasLefevre
Copy link
Collaborator

robodoo r+

@fw-bot
Copy link
Collaborator Author

fw-bot commented Jul 8, 2024

@LucasLefevre @VincentSchippefilt this PR was modified / updated and has become a normal PR. It must be merged directly.

robodoo pushed a commit that referenced this pull request Jul 8, 2024
Task: 4036899
X-original-commit: d164e42
Part-of: #4622
Signed-off-by: Vincent Schippefilt (vsc) <vsc@odoo.com>
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
robodoo pushed a commit that referenced this pull request Jul 8, 2024
When spreading the result of an array formula, we invalidate formulas
depending on the spread zone. Currently, we look for the dependencies
one position in the spread zone at a time.

It's much more efficient to group all those individual positions in zones
and call `this.getCellsDependingOn(...)` with those zones.

This commit reduces the time to evaluate the large array formula
dataset from 60+ seconds to ~36 seconds.

Task: 4036899
X-original-commit: 565dbdc
Part-of: #4622
Signed-off-by: Vincent Schippefilt (vsc) <vsc@odoo.com>
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
@robodoo robodoo closed this in 4c0626b Jul 8, 2024
@fw-bot fw-bot deleted the saas-17.3-17.0-perf-spread-lul-x_Ir-fw branch July 22, 2024 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants