From 4892b879e68103d723cf179b6cd5ff488c003a8a Mon Sep 17 00:00:00 2001 From: "Victor Piryns (pivi)" Date: Tue, 17 Oct 2023 14:40:14 +0200 Subject: [PATCH] [PERF] mail: speed up partner suggestion in chatter/discuss MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: Typing '@' in a chatter and waiting for the partner suggestions can sometimes be really slow for some users, even freeze some users browsers. Analysis: Since 12152279a5d08739959d900bfd4c6936ca39d64b the sorting on partner suggestions has been generalized to take an arbitrary amount of functions to apply sequential filtering. One particular of those is `partnerCompareRegistry['discuss.recent-chats']` which makes a call to `getRecentChatPartnerIds` which makes multiple checks for each pair of partners in the thread to order them based on their `lastInterestDateTime`. This property is actually quite heavy, as it involves deserializing datetime objects and making calls to libs and regex matches, creating a lot of garbage memory in the process. The complexity of this machinery is atrocious: - First `sortPartnerSuggestions` loops over all the current partners we fetched from the backend, and in a `sort` callback we call *each* filtering method (we shortcut on the first filtering that succeed). So we call at worse each filter `O(n*log(n))` times. - Since `partnerCompareRegistry['discuss.recent-chats']` is the lowest sequence in the filtering array, we will *always* go through it for those partners `O(n*log(n))` times, inherently calling `getRecentChatPartnerIds`. - This function in question gets it's list of partner from the thread in the store, and sorts those partners with a callback again. So again we have `O(m*log(m))` callbacks. This callback makes 8 calls to `lastInterestDateTime`, which as mentioned previously is actually an expensive function if called frequently as it generates a lot of garbage. - All this garbage that is generated triggers multiple calls to the browser GC, and it's being frequently called, either stalling the browser for a few seconds, and for some users (as the problems scales over quadratic for the number of partners in the suggestions/thread) completely halting it. Solution: - Correct implementation of `getRecentChatPartnerIds` to not call `lastInterestDateTime` so frequently, it was unnecessary. - Dependency inject the call to `getRecentChatPartnerIds` outside of the initial sorting callback, as it's independent of the sorting parameter and can be hoisted outside of the routine and injected into the filter. Reference: task-3556235 closes odoo/odoo#138988 X-original-commit: ba31d252c0738811cfe80a70bbbd7d489a45cf03 Signed-off-by: Alexandre Kühn (aku) --- addons/mail/static/src/core/common/partner_compare.js | 2 +- addons/mail/static/src/core/common/persona_service.js | 9 +++++---- addons/mail/static/src/core/common/suggestion_service.js | 5 ++++- .../static/src/discuss/core/common/partner_compare.js | 4 ++-- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/addons/mail/static/src/core/common/partner_compare.js b/addons/mail/static/src/core/common/partner_compare.js index f494740a464..920aba90d3d 100644 --- a/addons/mail/static/src/core/common/partner_compare.js +++ b/addons/mail/static/src/core/common/partner_compare.js @@ -7,7 +7,7 @@ import { registry } from "@web/core/registry"; * Registry of functions to sort partner suggestions. * The expected value is a function with the following * signature: - * (partner1: Partner, partner2: Partner, { env: OdooEnv, searchTerm: string, thread?: Thread }) => number|undefined + * (partner1: Partner, partner2: Partner, { env: OdooEnv, searchTerm: string, thread?: Thread , context?: Object}) => number|undefined */ export const partnerCompareRegistry = registry.category("mail.partner_compare"); diff --git a/addons/mail/static/src/core/common/persona_service.js b/addons/mail/static/src/core/common/persona_service.js index 8acb9998e15..58b0abdb955 100644 --- a/addons/mail/static/src/core/common/persona_service.js +++ b/addons/mail/static/src/core/common/persona_service.js @@ -84,16 +84,17 @@ export class PersonaService { return Object.values(this.store.Thread.records) .filter((thread) => thread.type === "chat") .sort((a, b) => { - if (!a.lastInterestDateTime && !b.lastInterestDateTime) { + const [a_dt , b_dt] = [a.lastInterestDateTime, b.lastInterestDateTime] + if (!a_dt && !b_dt) { return 0; } - if (a.lastInterestDateTime && !b.lastInterestDateTime) { + if (a_dt && !b_dt) { return -1; } - if (!a.lastInterestDateTime && b.lastInterestDateTime) { + if (!a_dt && b_dt) { return 1; } - return b.lastInterestDateTime.ts - a.lastInterestDateTime.ts; + return b_dt.ts - a_dt.ts; }) .map((thread) => thread.chatPartner?.id); } diff --git a/addons/mail/static/src/core/common/suggestion_service.js b/addons/mail/static/src/core/common/suggestion_service.js index 4f69e8e603c..0777a64e83a 100644 --- a/addons/mail/static/src/core/common/suggestion_service.js +++ b/addons/mail/static/src/core/common/suggestion_service.js @@ -14,6 +14,7 @@ export class SuggestionService { this.env = env; this.orm = services.orm; this.store = services["mail.store"]; + this.personaService = services['mail.persona']; } getSupportedDelimiters(thread) { @@ -155,12 +156,14 @@ export class SuggestionService { sortPartnerSuggestions(partners, searchTerm = "", thread = undefined) { const cleanedSearchTerm = cleanTerm(searchTerm); const compareFunctions = partnerCompareRegistry.getAll(); + const context = { recentChatPartnerIds: this.personaService.getRecentChatPartnerIds() }; return partners.sort((p1, p2) => { for (const fn of compareFunctions) { const result = fn(p1, p2, { env: this.env, searchTerms: cleanedSearchTerm, thread, + context, }); if (result !== undefined) { return result; @@ -238,7 +241,7 @@ export class SuggestionService { } export const suggestionService = { - dependencies: ["orm", "mail.store"], + dependencies: ["orm", "mail.store", "mail.persona"], /** * @param {import("@web/env").OdooEnv} env * @param {Partial} services diff --git a/addons/mail/static/src/discuss/core/common/partner_compare.js b/addons/mail/static/src/discuss/core/common/partner_compare.js index 260a66f2512..f757bf1767b 100644 --- a/addons/mail/static/src/discuss/core/common/partner_compare.js +++ b/addons/mail/static/src/discuss/core/common/partner_compare.js @@ -4,8 +4,8 @@ import { partnerCompareRegistry } from "@mail/core/common/partner_compare"; partnerCompareRegistry.add( "discuss.recent-chats", - (p1, p2, { env }) => { - const recentChatPartnerIds = env.services["mail.persona"].getRecentChatPartnerIds(); + (p1, p2, { env, context }) => { + const recentChatPartnerIds = context.recentChatPartnerIds || env.services["mail.persona"].getRecentChatPartnerIds(); const recentChatIndex_p1 = recentChatPartnerIds.findIndex( (partnerId) => partnerId === p1.id );