[IMP] point_of_sale: improve OrderLine.findAttribute speed
This commit addresses slow performance of the JS `OrderLine.findAttribute` method when `attributes_by_ptal_id` is exceedingly large. Before this commit, `findAttribute` would loop over all the values of `this.pos.attributes_by_ptal_id`, and filter out only the attributes that have at least one of the passed in ptal IDs (from the `values` parameter) within the attribute's "values" list. It would also modify the attribute to include a `"valuesForOrderLine"` key mapping to a list of all of the found ptal IDs from that attribute's values. This method becomes slow when `this.pos.attributes_by_ptal_id` is very large. Since it needs to loop over every single attribute. This commit provides a workaround to this slowness, by caching the search for valid attributes that this method performs in a lookup table. First, we construct the lookup table in `_processData` method of the `PosStore` class. `_add_ptal_ids_by_ptav_id` does this, by looping over all the values of all the attributes, mapping the ptav ID of the attribute to all the ptal IDs we find. Now, inside `findAttribute`, we will instead loop over each of the passed in `values`. For each value, we will retrieve the all the cached ptal IDs from `this.pos.ptal_ids_by_ptav_id` for the given value. Now that we have the ptal IDs, we can get all the corresponding attributes from `this.pos.attributes_by_ptal_id`. For each of those attributes, we can do the same modification from the original method by adding the `"valuesForOrderLine"` key/value. Finally we return all the modified attributes. This new method has a nested for loop, which may seem like a problem. But I believe that both the things being looped over (`values` and `this.pos.ptal_ids_by_ptav_id[value]`) should be very small compared to the potential size of `this.pos.attributes_by_ptal_id`. (43,000 in this customer's DB) `findAttributes` is called many times whenever the POS's numpad buttons are pressed, so this commit has the overall effect of drastically reducing the input latency for a numpad press. However, I do believe that this is a workaround, and the real problem is that the entire `attributes_by_ptal_id` is always passed to the POS, regardless of what products are actually being ordered. `attributes_by_ptal_id` should instead be incrementally fetched as products are ordered (if this is possible). Benchmarks (in customer DB): Before commit, each keypad press took around 1-3 seconds per Order Line present in the POS. For 11 products this was ~15 second latency. After commit, each keypad press is almost instant. The `ptal_ids_by_ptav_id` lookup table will consume additional memory, in this customer's DB I've estimated it to be about 0.3MB more memory. Taking memory snapshots in profiler shows no significant different between versions, however this is inconsistent. Will attach profiler results to PR. opw-3788840 closes odoo/odoo#159402 Signed-off-by: David Monnom (moda) <moda@odoo.com>
This commit is contained in:
@@ -1095,22 +1095,19 @@ export class Orderline extends PosModel {
|
||||
}
|
||||
findAttribute(values) {
|
||||
const listOfAttributes = [];
|
||||
Object.values(this.pos.attributes_by_ptal_id).filter(
|
||||
(attribute) => {
|
||||
for (const value of values){
|
||||
for (const ptal_id of this.pos.ptal_ids_by_ptav_id[value]){
|
||||
const attribute = this.pos.attributes_by_ptal_id[ptal_id]
|
||||
const attFound = attribute.values.filter((target) => {
|
||||
return Object.values(values).includes(target.id);
|
||||
});
|
||||
if (attFound.length > 0) {
|
||||
const modifiedAttribute = {
|
||||
...attribute,
|
||||
valuesForOrderLine: attFound,
|
||||
};
|
||||
listOfAttributes.push(modifiedAttribute);
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
const modifiedAttribute = {
|
||||
...attribute,
|
||||
valuesForOrderLine: attFound,
|
||||
};
|
||||
listOfAttributes.push(modifiedAttribute);
|
||||
}
|
||||
);
|
||||
}
|
||||
return listOfAttributes;
|
||||
}
|
||||
getDisplayData() {
|
||||
|
||||
@@ -286,6 +286,7 @@ export class PosStore extends Reactive {
|
||||
this._loadProductProduct(loadedData["product.product"]);
|
||||
this.db.add_packagings(loadedData["product.packaging"]);
|
||||
this.attributes_by_ptal_id = loadedData["attributes_by_ptal_id"];
|
||||
this._add_ptal_ids_by_ptav_id(this.attributes_by_ptal_id);
|
||||
this.cash_rounding = loadedData["account.cash.rounding"];
|
||||
this.payment_methods = loadedData["pos.payment.method"];
|
||||
this._loadPosPaymentMethod();
|
||||
@@ -299,6 +300,23 @@ export class PosStore extends Reactive {
|
||||
await this._loadPosPrinters(loadedData["pos.printer"]);
|
||||
this.open_orders_json = loadedData["open_orders"];
|
||||
}
|
||||
_add_ptal_ids_by_ptav_id(attributes_by_ptal_id) {
|
||||
// Create a cache based on attributes_by_ptal_id
|
||||
// that enables faster lookup for all ptal_ids
|
||||
// inside a given attribute's "values" list,
|
||||
// given a specific ptav_id.
|
||||
this.ptal_ids_by_ptav_id = {};
|
||||
for (const [ptal_id, attribute] of Object.entries(attributes_by_ptal_id)) {
|
||||
for (const value of attribute["values"]) {
|
||||
const ptav_id = value["id"];
|
||||
if (ptav_id in this.ptal_ids_by_ptav_id) {
|
||||
this.ptal_ids_by_ptav_id[ptav_id].push(ptal_id);
|
||||
} else {
|
||||
this.ptal_ids_by_ptav_id[ptav_id] = [ptal_id];
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
_loadPosSession() {
|
||||
// We need to do it here, since only then the local storage has the correct uuid
|
||||
this.db.save("pos_session_id", this.pos_session.id);
|
||||
|
||||
Reference in New Issue
Block a user