[FW][FIX] xlsx: accelerate xlsx export #4018
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@rrahir @LucasLefevre cherrypicking of pull request #3949 failed. stderr: Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?). In the former case, you may want to edit this PR message as well. More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port |
`pushElement` was returning an object but only one of its properties was used. After this revision, the helper only returns the id inside the list. It also alleviates the memory print. Task: 3733743
The borders are already normalized at export time. The code was denormalizing the renormalizing it without reason. Task: 3733743
JSON.stringify is way slower than deepEquals to compare objects and
assuming that the keys of the object are not ordered the same way, will
yield false negatives.
e.g.
```javascript
a = {'a':2,'b':4}
b = {'b': 4, 'a':2}
JSON.stringify(a) === JSON.stringify(b) // false
deepEquals(a,b) // true
```
Benchmark of `getXLSX`
======================
(averaged on 10 runs)
- On a 30k random text cells (to force heavy use of pushElement):
Before: 153 seconds
After: 10 seconds
- On RNG heavy sheet (patch tested in master):
Before: 351 seconds
After : 15 seconds
Task: 3733743
After a first test of strict equalilty, `deepEquals` will make type comparisons before yielding the strict quality again. In the context of `pushElement`, we know that we compare same-type objects, which means we only need to evaluate the strict equality for primitive types. Benchmark of `getXLSX` ====================== (averaged on 10 runs) - On a 30k random text cells (to force heavy use of pushElement): Before: 10 seconds After: 6.7 seconds - On RNG heavy sheet (patch tested in master): Before: 15 seconds After : 9.7 seconds Task: 3733743
We would evaluate the strict equality of the two arguments twice in case of primitive (non-null) types but the outcome of that evaluation was already known.
rrahir
force-pushed
the
saas-16.3-15.0-fix-slow-xlsx-rar-Emvj-fw
branch
from
4a8c1dc
to
9ccd207
Compare
|
@robodoo r+ |
robodoo
pushed a commit
that referenced
this pull request
Apr 8, 2024
`pushElement` was returning an object but only one of its properties was used. After this revision, the helper only returns the id inside the list. It also alleviates the memory print. Task: 3733743 Part-of: #4018
robodoo
pushed a commit
that referenced
this pull request
Apr 8, 2024
The borders are already normalized at export time. The code was denormalizing the renormalizing it without reason. Task: 3733743 Part-of: #4018
robodoo
pushed a commit
that referenced
this pull request
Apr 8, 2024
JSON.stringify is way slower than deepEquals to compare objects and
assuming that the keys of the object are not ordered the same way, will
yield false negatives.
e.g.
```javascript
a = {'a':2,'b':4}
b = {'b': 4, 'a':2}
JSON.stringify(a) === JSON.stringify(b) // false
deepEquals(a,b) // true
```
Benchmark of `getXLSX`
======================
(averaged on 10 runs)
- On a 30k random text cells (to force heavy use of pushElement):
Before: 153 seconds
After: 10 seconds
- On RNG heavy sheet (patch tested in master):
Before: 351 seconds
After : 15 seconds
Task: 3733743
Part-of: #4018
robodoo
pushed a commit
that referenced
this pull request
Apr 8, 2024
After a first test of strict equalilty, `deepEquals` will make type comparisons before yielding the strict quality again. In the context of `pushElement`, we know that we compare same-type objects, which means we only need to evaluate the strict equality for primitive types. Benchmark of `getXLSX` ====================== (averaged on 10 runs) - On a 30k random text cells (to force heavy use of pushElement): Before: 10 seconds After: 6.7 seconds - On RNG heavy sheet (patch tested in master): Before: 15 seconds After : 9.7 seconds Task: 3733743 Part-of: #4018
robodoo
pushed a commit
that referenced
this pull request
Apr 8, 2024
We would evaluate the strict equality of the two arguments twice in case of primitive (non-null) types but the outcome of that evaluation was already known. closes #4018 Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com> Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>
14 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This Pull request addresses the slowness of the
pushElementhelper.The two main improvments concern the iteration over the array elements and the comparison operations.
Benchmark of
getXLSXfunctionOn a 30k random string cells (heavy use of
pushElement) averaged on 10 runsWith the same patch applied to the mater branch on RNG's super sheet
Description:
description of this task, what is implemented and why it is implemented that way.
Task: : 3733743
review checklist
Forward-Port-Of: #4015
Forward-Port-Of: #3949