Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PERF] point_of_sale: lazy reactive getter #174184

Open
wants to merge 2 commits into
base: saas-17.2
Choose a base branch
from

Conversation

caburj
Copy link
Contributor

@caburj caburj commented Jul 23, 2024

Problem:

A customer experiences a noticeable delay (1 to 2 sec) when adding product in
the cart because of lack of caching computations. It gets worse the more
orderlines are added in the cart.

Solution:

This commit introduces an implementation of lazy reactive computed value that is
pull-based -- it only recomputes when it's needed. Check the following PR in
odoo/owl for its origin: odoo/owl#1499

This can be used for smart-caching selected getters. In this PR, we made a
getter for the get_all_prices method and instead of normal getter access, we
get the result of the getter using the get method introduced in the wrapper
class.

This means that in one call stack, calling get('allPrices') will only run once
and it will keep returning the cached value until a dependency in its
dependency (calculation) graph changed.

With this patch, adding an orderline is now 300ms -- approximate 5 times faster
than without caching.

Related: https://github.com/odoo/enterprise/pull/67382

@robodoo
Copy link
Contributor

robodoo commented Jul 23, 2024

Pull request status dashboard

@caburj caburj changed the title [FIX] point_of_sale: lazy reactive getter [PERF] point_of_sale: lazy reactive getter Jul 23, 2024
@caburj caburj force-pushed the saas-17.2-lazy-reactive-getter-jcb branch 3 times, most recently from 08a2d26 to ced2369 Compare July 23, 2024 11:42
@C3POdoo C3POdoo added the RD research & development, internal work label Jul 23, 2024
@caburj caburj force-pushed the saas-17.2-lazy-reactive-getter-jcb branch from ced2369 to 4df4d91 Compare July 25, 2024 08:01
@caburj caburj marked this pull request as ready for review July 25, 2024 09:51
@C3POdoo C3POdoo requested review from a team and davidmonnom and removed request for a team July 25, 2024 09:55
Comment on lines +51 to +60
function registerGetters() {
for (const [name, func] of getAllGetters(WithLazyGetters.prototype)) {
if (name.startsWith("__") && name.endsWith("__")) {
continue;
}
const lazyName = `__lazy_${name}`;
getters.set(name, [lazyName, (obj) => func.call(obj)]);
}
gettersRegistered = true;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to change when this is called and the getAllGetters should be given the prototype of WithLazyGetters, not the passed Class.

This change is necessary so that patches of the getters from other modules are taken into account.

@caburj caburj force-pushed the saas-17.2-lazy-reactive-getter-jcb branch 3 times, most recently from 689cba7 to 774372c Compare July 26, 2024 06:53
caburj added 2 commits July 26, 2024 08:53
**Problem:**

A customer experiences a noticeable delay (1 to 2 sec) when adding product in
the cart because of lack of caching computations. It gets worse the more
orderlines are added in the cart.

**Solution:**

This commit introduces an implementation of lazy reactive computed value that is
pull-based -- it only recomputes when it's needed. Check the following PR in
odoo/owl for its origin: odoo/owl#1499

This can be used for smart-caching selected getters. In this PR, we made a
getter for the `get_all_prices` method and instead of normal getter access, we
get the result of the getter using the `get` method introduced in the wrapper
class.

This means that in one call stack, calling `get('allPrices')` will only run once
and it will keep returning the cached value until a dependency in its
dependency (calculation) graph changed.

With this patch, adding an orderline is now 300ms -- approximate 5 times faster
than without caching.
@caburj caburj force-pushed the saas-17.2-lazy-reactive-getter-jcb branch from 774372c to 8985aa7 Compare July 26, 2024 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants