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][IMP] history: reduce memory allocation #3187

Closed

Conversation

fw-bot
Copy link
Collaborator

@fw-bot fw-bot commented Nov 17, 2023

Backport of d8d943b

addChange is used a lot, like a lot, a lot, for every mutation in core plugins. Its performance is key.

Currently const [root, ...path] = args creates a new array for path, the array is allocated, and must be garbage collected later.

On the large formula dataset, with 20k rows (520k cells), and adding a column before A, then reload the spreadsheet:

(average over 5 runs)

minor GC	addChange

before 1609ms 1168ms
after 1094ms 753ms
-32% -35%

The memory usage also decreases: right after loading, memory was ~1725Mb and ~792Mb after running the GC. It's now ~1455Mb right after loading and ~670Mb after running the GC. (I'm not sure why it goes from 792Mb to 670Mb. Is it just thanks to the removed "root" key)

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: #3170

Backport of d8d943b

`addChange` is used *a lot*, like *a lot, a lot*, for every mutation in core
plugins. Its performance is key.

Currently `const [root, ...path] = args` creates a new array for `path`,
the array is allocated, and must be garbage collected later.

On the large formula dataset, with 20k rows (520k cells), and adding a column
before A, then reload the spreadsheet:

(average over 5 runs)

	minor GC	addChange
before	1609ms		1168ms
after	1094ms		753ms
	-32%		-35%

The memory usage also decreases: right after loading, memory was ~1725Mb and
~792Mb after running the GC. It's now ~1455Mb right after loading and ~670Mb
after running the GC. (I'm not sure why it goes from 792Mb to 670Mb. Is it
just thanks to the removed "root" key)

X-original-commit: 97ebec0
@robodoo
Copy link
Collaborator

robodoo commented Nov 17, 2023

@fw-bot
Copy link
Collaborator Author

fw-bot commented Nov 17, 2023

This PR targets saas-15.2 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

robodoo pushed a commit that referenced this pull request Nov 17, 2023
Backport of d8d943b

`addChange` is used *a lot*, like *a lot, a lot*, for every mutation in core
plugins. Its performance is key.

Currently `const [root, ...path] = args` creates a new array for `path`,
the array is allocated, and must be garbage collected later.

On the large formula dataset, with 20k rows (520k cells), and adding a column
before A, then reload the spreadsheet:

(average over 5 runs)

	minor GC	addChange
before	1609ms		1168ms
after	1094ms		753ms
	-32%		-35%

The memory usage also decreases: right after loading, memory was ~1725Mb and
~792Mb after running the GC. It's now ~1455Mb right after loading and ~670Mb
after running the GC. (I'm not sure why it goes from 792Mb to 670Mb. Is it
just thanks to the removed "root" key)

closes #3187

X-original-commit: 97ebec0
Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
@robodoo robodoo closed this Nov 17, 2023
@fw-bot fw-bot deleted the saas-15.2-15.0-faster-add-change-lul-7mpz-fw branch December 1, 2023 12: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.

None yet

3 participants