From f56841381c14a2bf50f1321462074750a3a45a7e Mon Sep 17 00:00:00 2001
From: Samuel Degueldre <sad@odoo.com>
Date: Tue, 19 Sep 2023 09:57:56 +0200
Subject: [PATCH] [FIX] web: fix performance issues with large x2m fields
Previously, when a view contained an x2m field that had a few thousand
records, the view would take ages to load. This was caused by a few
things in the basic model where we would linearly scan lists of records
in a tight loop, and doing so for every single other record, causing
quadratic increase runtime with regards to the number of records. For
example, creating a new batch picking in stock with 4000 existing
pickings would take up to ~20 seconds to load the view.
This commit creates a few objects to use as lookup tables in such
places, which speeds up the loading of the view to less than a second.
It also makes the context of the DataPoint into a property that's
initialized lazily on first read, as creating a context and the
corresponding evalContext can be expensive, which becomes a problem when
creating thousands of DataPoints whose context will never be read.
opw-3465073
closes odoo/odoo#135860
Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>
---
.../src/legacy/js/views/basic/basic_model.js | 25 ++++++++++++-------
.../src/views/basic_relational_model.js | 23 ++++++++++++++++-
2 files changed, 38 insertions(+), 10 deletions(-)
diff --git a/addons/web/static/src/legacy/js/views/basic/basic_model.js b/addons/web/static/src/legacy/js/views/basic/basic_model.js
index bc1c6e7d04c8..0dac799b6d60 100644
--- a/addons/web/static/src/legacy/js/views/basic/basic_model.js
+++ b/addons/web/static/src/legacy/js/views/basic/basic_model.js
@@ -3425,10 +3425,12 @@ var BasicModel = AbstractModel.extend({
// least one id was updated
var didChange = false;
var changes, command, relRecord;
+ const addedById = Object.fromEntries(relRecordAdded.map(r => [r.res_id, r]));
+ const updatedById = Object.fromEntries(relRecordUpdated.map(r => [r.res_id, r]));
for (var i = 0; i < list.res_ids.length; i++) {
if (_.contains(keptIds, list.res_ids[i])) {
// this is an id that already existed
- relRecord = _.findWhere(relRecordUpdated, {res_id: list.res_ids[i]});
+ relRecord = updatedById[list.res_ids[i]];
changes = relRecord ? this._generateChanges(relRecord, options) : {};
if (!_.isEmpty(changes)) {
command = x2ManyCommands.update(relRecord.res_id, changes);
@@ -3439,7 +3441,7 @@ var BasicModel = AbstractModel.extend({
commands[fieldName].push(command);
} else if (_.contains(addedIds, list.res_ids[i])) {
// this is a new id (maybe existing in DB, but new in JS)
- relRecord = _.findWhere(relRecordAdded, {res_id: list.res_ids[i]});
+ relRecord = addedById[list.res_ids[i]];
if (!relRecord) {
commands[fieldName].push(x2ManyCommands.link_to(list.res_ids[i]));
continue;
@@ -4797,9 +4799,16 @@ var BasicModel = AbstractModel.extend({
}));
}
return def.then(function (records) {
+ const context = list.getContext();
+ const changeMap = Object.fromEntries(
+ (list._changes || [])
+ .filter((c) => c.operation === "ADD")
+ .map((c) => [c.resID, c])
+ );
+ const recordsById = Object.fromEntries(records.map(r => [r.id, r]));
_.each(resIDs, function (id) {
var dataPoint;
- var data = _.findWhere(records, {id: id});
+ var data = recordsById[id];
if (id in list._cache) {
dataPoint = self.localData[list._cache[id]];
if (data) {
@@ -4808,7 +4817,7 @@ var BasicModel = AbstractModel.extend({
}
} else {
dataPoint = self._makeDataPoint({
- context: list.getContext(),
+ context,
data: data,
fieldsInfo: list.fieldsInfo,
fields: list.fields,
@@ -4822,11 +4831,9 @@ var BasicModel = AbstractModel.extend({
list._cache[id] = dataPoint.id;
}
// set the dataPoint id in potential 'ADD' operation adding the current record
- _.each(list._changes, function (change) {
- if (change.operation === 'ADD' && !change.id && change.resID === id) {
- change.id = dataPoint.id;
- }
- });
+ if (changeMap[id] && !changeMap[id].id) {
+ changeMap[id].id = dataPoint.id;
+ }
});
return list;
});
diff --git a/addons/web/static/src/views/basic_relational_model.js b/addons/web/static/src/views/basic_relational_model.js
index ae373bcc38da..3fc3a0ff516b 100644
--- a/addons/web/static/src/views/basic_relational_model.js
+++ b/addons/web/static/src/views/basic_relational_model.js
@@ -41,7 +41,28 @@ class DataPoint {
} else if (params.handle) {
this.__bm_handle__ = params.handle;
info = this.model.__bm__.get(this.__bm_handle__);
- this.context = this.model.__bm__.localData[this.__bm_handle__].getContext();
+ // Create a lazy property that sets itself on first read, because creating the context
+ // is very expensive, and in some cases we are creating a lot of DataPoints whose
+ // context will never be read.
+ Object.defineProperty(this, "context", {
+ get() {
+ const context = this.model.__bm__.localData[this.__bm_handle__].getContext();
+ Object.defineProperty(this, "context", {
+ value: context,
+ configurable: true,
+ writable: true,
+ });
+ return context;
+ },
+ set(value) {
+ Object.defineProperty(this, "context", {
+ value,
+ configurable: true,
+ writable: true,
+ });
+ },
+ configurable: true,
+ });
} else {
throw new Error("Datapoint needs load params or handle");
}
--
GitLab