[PERF] mail: speed up partner suggestion in chatter/discuss
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 12152279a5 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) <aku@odoo.com>
This commit is contained in:
@@ -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");
|
||||
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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<import("services").Services>} services
|
||||
|
||||
@@ -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
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user