pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[FIX] mail: improve performance of JS write, especially for x2m

Browse Source 
- store `x2many` values as `Set`. This is conceptually more correct because
  duplicates were already not allowed, and using a `Set` allows for much faster
  operations such as checking the presence of a record in the relation, which is
  done at every write, for every linked record.

- `stringify` in `write` was slow and unnecessary because we can always assume
  that new non-primitive should be provided to `write`

- remove field values from store, in favor of just storing a counter, which is
  enough to trigger renders. This allows to get rid of one Proxy level around
  data, which was slower, but which also prevented from using `Set`.

- write in store exactly once per update cycle, to trigger one `useStore` call
  on each component. Records keep `__state` but as a local counter, to let
  `useStore` know which records have been updated (easier to compare a counter
  than comparing each and every field value).

Results
=======

The worse case test (using a lot of x2m) that was used to benchmark this PR
`form_renderer_tests.js` of `mail_enterprise` went from taking around 4s before
this commit, to around 2.5s after this commit.
And the worse `_setRelationLinkMany2Many` previously took around 250ms, and it
now takes around 50ms.

The whole `mail` QUnit suite took around 55s before, now around 42s.

Part of task-2308312

closes odoo/odoo#55248

X-original-commit: 4db0a10d85534ef896f6bcbf13bca4983a6e2003
Signed-off-by: Alexandre Kühn (aku) <aku@odoo.com>
This commit is contained in:
Sébastien Theys
2020-07-31 09:43:01 +00:00
parent 89bf21ba79
commit 78bef3e08c
5 changed files with 37 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/mail/static/src/env/test_env.js
Vendored
+3 -1
View File
@@ -34,7 +34,9 @@ function addMessagingToEnv(providedEnv = {}) {
*/
const store = new Store({
env,
state: {},
state: {
messagingRevNumber: 0,
},
});
/**
addons/mail/static/src/js/main.js
+3 -1
View File
@@ -13,7 +13,9 @@ const { EventBus } = owl.core;
*/
const store = new Store({
env,
state: {},
state: {
messagingRevNumber: 0,
},
});
/**
addons/mail/static/src/model/model_field.js
+20 -29
View File
@@ -276,13 +276,11 @@ class ModelField {
if (['one2one', 'many2one'].includes(this.relationType)) {
return OtherModel.get(this.read(record));
}
return this.read(record)
.map(localId => OtherModel.get(localId))
/**
* FIXME: Stored relation may still contain
* outdated records.
*/
.filter(record => !!record);
const res = [];
for (const localId of this.read(record)) {
res.push(OtherModel.get(localId));
}
return res;
}
throw new Error(`cannot get field with unsupported type ${this.fieldType}.`);
}
@@ -295,7 +293,7 @@ class ModelField {
* @returns {any}
*/
read(record) {
return record.__state[this.fieldName];
return record.__values[this.fieldName];
}
/**
@@ -357,18 +355,15 @@ class ModelField {
*/
write(record, newVal, { registerDependents = true } = {}) {
if (this.read(record) === newVal) {
return;
}
const prevStringified = JSON.stringify(this.read(record));
record.__state[this.fieldName] = newVal;
const newStringified = JSON.stringify(this.read(record));
if (this._containsRecords(newVal)) {
throw new Error("Forbidden write operation with records in the __state!!");
}
if (newStringified === prevStringified) {
// value unchanged, don't need to compute dependent fields
return;
}
if (this._containsRecords(newVal)) {
throw new Error("Forbidden write operation with records!!");
}
record.__values[this.fieldName] = newVal;
record.__state++;
if (!registerDependents) {
return;
}
@@ -656,7 +651,7 @@ class ModelField {
const prevLocalIds = this.read(record);
// other record already linked, avoid linking twice
if (prevLocalIds.includes(newLocalId)) {
if (prevLocalIds.has(newLocalId)) {
continue;
}
@@ -668,7 +663,7 @@ class ModelField {
}
// link other record to current record
this.write(record, prevLocalIds.concat([newLocalId]));
this.write(record, new Set([...prevLocalIds, newLocalId]));
// link current record to other record
newOtherRecord.update({
@@ -741,7 +736,7 @@ class ModelField {
const prevLocalIds = this.read(record);
// other record already linked, avoid linking twice
if (prevLocalIds.includes(newLocalId)) {
if (prevLocalIds.has(newLocalId)) {
continue;
}
@@ -753,7 +748,7 @@ class ModelField {
}
// link other record to current record
this.write(record, prevLocalIds.concat([newLocalId]));
this.write(record, new Set([...prevLocalIds, newLocalId]));
// link current record to other record
newOtherRecord.update({
@@ -861,14 +856,12 @@ class ModelField {
const prevLocalIds = this.read(record);
// other record already unlinked, avoid useless processing
if (!prevLocalIds.includes(otherLocalId)) {
if (!prevLocalIds.delete(otherLocalId)) {
continue;
}
// unlink other record from current record
this.write(record, prevLocalIds.filter(
localId => localId !== otherLocalId
));
this.write(record, new Set([...prevLocalIds]));
const otherRecord = OtherModel.get(otherLocalId);
// other record may be deleted due to causality, avoid useless
@@ -931,14 +924,12 @@ class ModelField {
const prevLocalIds = this.read(record);
// other record already unlinked, avoid useless processing
if (!prevLocalIds.includes(otherLocalId)) {
if (!prevLocalIds.delete(otherLocalId)) {
continue;
}
// unlink other record from current record
this.write(record, prevLocalIds.filter(
localId => localId !== otherLocalId
));
this.write(record, new Set([...prevLocalIds]));
const otherRecord = OtherModel.get(otherLocalId);
// other record may be deleted due to causality, avoid useless
addons/mail/static/src/model/model_manager.js
+8 -7
View File
@@ -122,11 +122,10 @@ class ModelManager {
const record = new Model({ valid: true });
Object.defineProperty(record, 'env', { get: () => Model.env });
record.localId = record._createRecordLocalId(data);
// Make state, which contain field values of record that have to
// be observed in store.
this.env.store.state[record.localId] = {};
record.__state = this.env.store.state[record.localId];
// Contains field values of record.
record.__values = {};
// Contains revNumber of record for checking record update in useStore.
record.__state = 0;
// Make proxified record, so that access to field redirects
// to field getter.
@@ -299,6 +298,8 @@ class ModelManager {
}
this._toComputeFields.clear();
this._isInUpdateCycle = false;
// trigger at most one useStore call per update cycle
this.env.store.state.messagingRevNumber++;
} else {
this._updateDirect(record, data);
if (this._isHandlingToUpdateAfters) {
@@ -689,8 +690,8 @@ class ModelManager {
}
if (field.fieldType === 'relation') {
if (['one2many', 'many2many'].includes(field.relationType)) {
// Ensure X2many relations are arrays by defaults.
field.write(record, [], { registerDependents: false });
// Ensure X2many relations are Set by defaults.
field.write(record, new Set(), { registerDependents: false });
} else {
field.write(record, undefined, { registerDependents: false });
}
addons/mail/static/tests/systray/systray_activity_menu_tests.js
+3
View File
@@ -228,6 +228,9 @@ QUnit.test('activity menu widget: close on messaging menu click', async function
const { widget } = await start({
hasMessagingMenu: true,
async mockRPC(route, args) {
if (args.method === 'message_fetch') {
return [];
}
if (args.method === 'systray_get_activities') {
return [];
}