Skip to content

Commit

Permalink
[IMP] registry: multiple ormcache
Browse files Browse the repository at this point in the history
One of the main issue with ormcache is that the invalidation clears
everything, meaning that some value, slow to compute but with a long
lifetime, can be removed from the cache because an easy to invalidate
value is cleared, like after writting or creating a product has an
example.

Most example in the code will try to invalidate the cache of the models
doing something like `env['ir.qweb'].clear_caches()` but it is
finally equivalent to `env.registry.clear_cache()`, and cross worker.

The idea is to have multiple cache, maybe with specific sizes for a
specific purpose.

Having one per model is maybe a bad idea because it will be difficult
to size the LRU correcly, and it is too dynamic. Checking invalidation
may be expensive.

The proposed solution is closed allow a limited number of named caches,
using onse sequence per cache. This is actually close to the
cache_longterm.

We want to discourage using a specific cache for one use case in
the buisness code. Adding a cache shouldn't be something easy, doable
in stable.

Note that we could also change the invalisation mecanism using an
insert only table. We an check the sequence of this table, but also
fetch all invalidation messages.
Another possible improvement, especially if we have more than x cache is
to have a global sequence, checking signaling would mean to check the
main sequence, and only the other ones if the main one changed.

Note that this poc is inspired from the long term cache but not all
use case where applie yet.

Part-of: odoo#119813
  • Loading branch information
Xavier-Do committed Jul 18, 2023
1 parent dda0f64 commit 595aa24
Show file tree
Hide file tree
Showing 60 changed files with 415 additions and 381 deletions.
4 changes: 2 additions & 2 deletions addons/auth_totp/tests/test_totp.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ def totp_hook(self, secret=None):
return totp.generate(time.time() + 30).token
# because not preprocessed by ControllerType metaclass
totp_hook.routing_type = 'json'
self.env['ir.http']._clear_routing_map()
self.env.registry.clear_cache('routing')
# patch Home to add test endpoint
Home.totp_hook = http.route('/totphook', type='json', auth='none')(totp_hook)
# remove endpoint and destroy routing map
@self.addCleanup
def _cleanup():
del Home.totp_hook
self.env['ir.http']._clear_routing_map()
self.env.registry.clear_cache('routing')

def test_totp(self):
# 1. Enable 2FA
Expand Down
4 changes: 2 additions & 2 deletions addons/auth_totp_portal/tests/test_tour.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ def totp_hook(self, secret=None):
totp_hook.routing_type = 'json'
# patch Home to add test endpoint
Home.totp_hook = http.route('/totphook', type='json', auth='none')(totp_hook)
self.env['ir.http']._clear_routing_map()
self.env.registry.clear_cache('routing')
# remove endpoint and destroy routing map
@self.addCleanup
def _cleanup():
del Home.totp_hook
self.env['ir.http']._clear_routing_map()
self.env.registry.clear_cache('routing')

self.start_tour('/my/security', 'totportal_tour_setup', login='portal')
# also disables totp otherwise we can't re-login
Expand Down
4 changes: 3 additions & 1 deletion addons/base_import/models/base_import.py
Original file line number Diff line number Diff line change
Expand Up @@ -1339,7 +1339,9 @@ def execute_import(self, fields, columns, options, dryrun=False):
if dryrun:
self._cr.execute('ROLLBACK TO SAVEPOINT import')
# cancel all changes done to the registry/ormcache
self.pool.clear_caches()
# we need to clear the cache in case any created id was added to an ormcache and would be missing afterward
self.pool.clear_all_caches()
# don't propagate to other workers since it was rollbacked
self.pool.reset_changes()
else:
self._cr.execute('RELEASE SAVEPOINT import')
Expand Down
2 changes: 1 addition & 1 deletion addons/bus/tests/test_assetsbundle.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def test_bundle_sends_bus(self):
])
# start from a clean slate
self.env['ir.attachment'].search(domain).unlink()
self.env.registry._clear_cache()
self.env.registry.clear_cache()

sendones = []
def patched_sendone(self, channel, notificationType, message):
Expand Down
4 changes: 2 additions & 2 deletions addons/google_calendar/models/google_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class GoogleSync(models.AbstractModel):
def write(self, vals):
google_service = GoogleCalendarService(self.env['google.service'])
if 'google_id' in vals:
self._event_ids_from_google_ids.clear_cache(self)
self.env.registry.clear_cache() # _event_ids_from_google_ids
synced_fields = self._get_google_synced_fields()
if 'need_sync' not in vals and vals.keys() & synced_fields and not self.env.user.google_synchronization_stopped:
vals['need_sync'] = True
Expand All @@ -79,7 +79,7 @@ def write(self, vals):
@api.model_create_multi
def create(self, vals_list):
if any(vals.get('google_id') for vals in vals_list):
self._event_ids_from_google_ids.clear_cache(self)
self.env.registry.clear_cache() # _event_ids_from_google_ids
if self.env.user.google_synchronization_stopped:
for vals in vals_list:
vals.update({'need_sync': False})
Expand Down
3 changes: 1 addition & 2 deletions addons/hr_timesheet/tests/test_performance.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ def test_timesheet_preprocess(self):
'project_id': project.id,
} for i in range(17) for project in projects])
self.env.invalidate_all()
projects.clear_caches()
tasks.clear_caches()
self.env.registry.clear_cache()
with self.assertQueryCount(7):
self.env['account.analytic.line']._timesheet_preprocess([
{'task_id': task.id} for task in tasks for _i in range(10)
Expand Down
13 changes: 8 additions & 5 deletions addons/http_routing/models/ir_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,15 +185,15 @@ def url_for(url_from, lang_code=None, no_rewrite=False):
:param no_rewrite: don't try to match route with website.rewrite.
'''
new_url = False

rewrite = not no_rewrite
# don't try to match route if we know that no rewrite has been loaded.
routing = getattr(request, 'website_routing', None) # not modular, but not overridable
if not getattr(request.env['ir.http'], '_rewrite_len', {}).get(routing):
no_rewrite = True
if not request.env['ir.http']._rewrite_len(routing):
rewrite = False

path, _, qs = (url_from or '').partition('?')

if (not no_rewrite and path and (
if (rewrite and path and (
len(path) > 1
and path.startswith('/')
and '/static/' not in path
Expand Down Expand Up @@ -660,7 +660,7 @@ def _handle_error(cls, exception):
return response

@api.model
@tools.ormcache('path', 'query_args')
@tools.ormcache('path', 'query_args', cache='routing.rewrites')
def url_rewrite(self, path, query_args=None):
new_url = False
router = http.root.get_db_router(request.db).bind('')
Expand All @@ -676,3 +676,6 @@ def url_rewrite(self, path, query_args=None):
except werkzeug.exceptions.NotFound:
new_url = path
return new_url or path, endpoint and endpoint[0]

def _rewrite_len(self, website_id, rewrites=None):
return 0
6 changes: 3 additions & 3 deletions addons/mail/models/mail_message_subtype.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,15 @@ class MailMessageSubtype(models.Model):

@api.model_create_multi
def create(self, vals_list):
self.clear_caches()
self.env.registry.clear_cache() # _get_auto_subscription_subtypes
return super(MailMessageSubtype, self).create(vals_list)

def write(self, vals):
self.clear_caches()
self.env.registry.clear_cache() # _get_auto_subscription_subtypes
return super(MailMessageSubtype, self).write(vals)

def unlink(self):
self.clear_caches()
self.env.registry.clear_cache() # _get_auto_subscription_subtypes
return super(MailMessageSubtype, self).unlink()

@tools.ormcache('model_name')
Expand Down
8 changes: 4 additions & 4 deletions addons/product/models/product_product.py
Original file line number Diff line number Diff line change
Expand Up @@ -314,18 +314,18 @@ def create(self, vals_list):
self.product_tmpl_id._sanitize_vals(vals)
products = super(ProductProduct, self.with_context(create_product_product=True)).create(vals_list)
# `_get_variant_id_for_combination` depends on existing variants
self.clear_caches()
self.env.registry.clear_cache()
return products

def write(self, values):
self.product_tmpl_id._sanitize_vals(values)
res = super(ProductProduct, self).write(values)
if 'product_template_attribute_value_ids' in values:
# `_get_variant_id_for_combination` depends on `product_template_attribute_value_ids`
self.clear_caches()
self.env.registry.clear_cache()
elif 'active' in values:
# `_get_first_possible_variant_id` depends on variants active state
self.clear_caches()
self.env.registry.clear_cache()
return res

def unlink(self):
Expand All @@ -350,7 +350,7 @@ def unlink(self):
# products due to ondelete='cascade'
unlink_templates.unlink()
# `_get_variant_id_for_combination` depends on existing variants
self.clear_caches()
self.env.registry.clear_cache()
return res

def _filter_to_unlink(self, check_access=True):
Expand Down
2 changes: 1 addition & 1 deletion addons/test_mail/tests/test_message_track.py
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ def test_track_groups(self):

# test editing the record with user not in the group of the field
self.env.invalidate_all()
self.record.clear_caches()
self.env.registry.clear_cache()
record_form = Form(self.record.with_user(self.user_employee))
record_form.name = 'TestDoNoCrash'
# the employee user must be able to save the fields on which they can write
Expand Down
2 changes: 1 addition & 1 deletion addons/web/controllers/home.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ def switch_to_admin(self):
if request.env.user._is_system():
uid = request.session.uid = odoo.SUPERUSER_ID
# invalidate session token cache as we've changed the uid
request.env['res.users'].clear_caches()
request.env.registry.clear_cache()
request.session.session_token = security.compute_session_token(request.session, request.env)

return request.redirect(self._login_redirect(uid))
Expand Down
2 changes: 1 addition & 1 deletion addons/web/tests/test_assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class TestPregenerateTime(HttpCase):
def test_logs_pregenerate_time(self):
self.env['ir.qweb']._pregenerate_assets_bundles()
start = time.time()
self.env.registry.clear_caches()
self.env.registry.clear_cache()
self.env.cache.invalidate()
with self.profile(collectors=['sql', odoo.tools.profiler.PeriodicCollector(interval=0.01)], disable_gc=True):
self.env['ir.qweb']._pregenerate_assets_bundles()
Expand Down
2 changes: 1 addition & 1 deletion addons/web_editor/controllers/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,7 @@ def _update_svg_colors(self, options, svg):
if re.match('^o-color-([1-5])$', css_color_value):
if not bundle_css:
bundle = 'web.assets_frontend'
asset = request.env["ir.asset"]._get_asset_bundle(bundle)
asset = request.env["ir.qweb"]._get_asset_bundle(bundle)
bundle_css = asset.css().index_content
color_search = re.search(r'(?i)--%s:\s+(%s|%s)' % (css_color_value, regex_hex, regex_rgba), bundle_css)
if not color_search:
Expand Down
2 changes: 1 addition & 1 deletion addons/web_editor/models/assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ def save_asset(self, url, bundle, content, file_type):
# If it was already modified, simply override the corresponding
# attachment content
custom_attachment.write({"datas": datas})
self.env.registry.clear_cache('assets')
else:
# If not, create a new attachment to copy the original scss/js file
# content, with its modifications
Expand Down Expand Up @@ -93,7 +94,6 @@ def save_asset(self, url, bundle, content, file_type):
new_asset['bundle'] = IrAsset._get_related_bundle(url, bundle)
IrAsset.create(new_asset)

self.env["ir.qweb"].clear_caches()

@api.model
def _get_content_from_url(self, url, url_info=None, custom_attachments=None):
Expand Down
41 changes: 24 additions & 17 deletions addons/website/models/ir_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from functools import partial

import odoo
from odoo import api, models
from odoo import api, models, tools
from odoo import SUPERUSER_ID
from odoo.exceptions import AccessError
from odoo.http import request
Expand Down Expand Up @@ -62,16 +62,10 @@ def get_request_website():
class Http(models.AbstractModel):
_inherit = 'ir.http'

@classmethod
def routing_map(cls, key=None):
def routing_map(self, key=None):
if not key and request:
key = request.website_routing
return super(Http, cls).routing_map(key=key)

@classmethod
def clear_caches(cls):
super()._clear_routing_map()
return super().clear_caches()
return super().routing_map(key=key)

@classmethod
def _slug_matching(cls, adapter, endpoint, **kw):
Expand All @@ -81,18 +75,31 @@ def _slug_matching(cls, adapter, endpoint, **kw):
qs = request.httprequest.query_string.decode('utf-8')
return adapter.build(endpoint, kw) + (qs and '?%s' % qs or '')

@classmethod
def _generate_routing_rules(cls, modules, converters):
@tools.ormcache('website_id', cache='routing')
def _rewrite_len(self, website_id, rewrites=None):
# This little hack allow to store a _rewrite_len in cache if we have the value,
# (Populated during routing map computation)
# but also ensures that we will have the correct value if the cache is not populated.
# The cache key actually does not depends on "rewrites" because this is the information
# we are storing, rewrites actually depends on website_id
if rewrites is None:
rewrites = self._get_rewrites(website_id)
return len(rewrites)

def _get_rewrites(self, website_id):
domain = [('redirect_type', 'in', ('308', '404')), '|', ('website_id', '=', False), ('website_id', '=', website_id)]
rewrites = {x.url_from: x for x in self.env['website.rewrite'].sudo().search(domain)}
self._rewrite_len(website_id, rewrites) # optionnal, update value in cache
#self._rewrite_len.add_cache_value(website_id, value=rewrites)
return rewrites

def _generate_routing_rules(self, modules, converters):
if not request:
yield from super()._generate_routing_rules(modules, converters)
return

website_id = request.website_routing
logger.debug("_generate_routing_rules for website: %s", website_id)
domain = [('redirect_type', 'in', ('308', '404')), '|', ('website_id', '=', False), ('website_id', '=', website_id)]

rewrites = dict([(x.url_from, x) for x in request.env['website.rewrite'].sudo().search(domain)])
cls._rewrite_len[website_id] = len(rewrites)
rewrites = self._get_rewrites(website_id)

for url, endpoint in super()._generate_routing_rules(modules, converters):
if url in rewrites:
Expand All @@ -104,7 +111,7 @@ def _generate_routing_rules(cls, modules, converters):

if url != url_to:
logger.debug('Redirect from %s to %s for website %s' % (url, url_to, website_id))
_slug_matching = partial(cls._slug_matching, endpoint=endpoint)
_slug_matching = partial(self._slug_matching, endpoint=endpoint)
endpoint.routing['redirect_to'] = _slug_matching
yield url, endpoint # yield original redirected to new url
elif rewrite.redirect_type == '404':
Expand Down
8 changes: 4 additions & 4 deletions addons/website/models/ir_ui_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ def unlink(self):
specific_views += view._get_specific_views()

result = super(View, self + specific_views).unlink()
self.clear_caches()
self.env.registry.clear_cache('templates')
return result

def _create_website_specific_pages_for_view(self, new_view, website):
Expand Down Expand Up @@ -363,7 +363,7 @@ def _get_filter_xmlid_query(self):
"""

@api.model
@tools.ormcache_context('self.env.uid', 'self.env.su', 'xml_id', keys=('website_id',))
@tools.ormcache('self.env.uid', 'self.env.su', 'xml_id', 'self._context.get("website_id")', cache='templates')
def _get_view_id(self, xml_id):
"""If a website_id is in the context and the given xml_id is not an int
then try to get the id of the specific view for that website, but
Expand All @@ -374,7 +374,7 @@ def _get_view_id(self, xml_id):
method. `viewref` is probably more suitable.
Archived views are ignored (unless the active_test context is set, but
then the ormcache_context will not work as expected).
then the ormcache will not work as expected).
"""
website_id = self._context.get('website_id')
if website_id and not isinstance(xml_id, int):
Expand All @@ -388,7 +388,7 @@ def _get_view_id(self, xml_id):
return view.id
return super(View, self.sudo())._get_view_id(xml_id)

@tools.ormcache('self.id')
@tools.ormcache('self.id', cache='templates')
def _get_cached_visibility(self):
return self.visibility

Expand Down
8 changes: 4 additions & 4 deletions addons/website/models/website.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ def _compute_menu(self):
website.menu_id = top_menus and top_menus[0].id or False

# self.env.uid for ir.rule groups on menu
@tools.ormcache('self.env.uid', 'self.id')
@tools.ormcache('self.env.uid', 'self.id', cache='templates')
def _get_menu_ids(self):
return self.env['website.menu'].search([('website_id', '=', self.id)]).ids

Expand Down Expand Up @@ -216,7 +216,7 @@ def write(self, values):
original_company = self.company_id
self._handle_create_write(values)

self.clear_caches()
self.env.registry.clear_cache()

if 'company_id' in values and 'user_id' not in values:
public_user_to_change_websites = self.filtered(lambda w: w.sudo().user_id.company_id.id != values['company_id'])
Expand All @@ -228,7 +228,7 @@ def write(self, values):

if 'cdn_activated' in values or 'cdn_url' in values or 'cdn_filters' in values:
# invalidate the caches from static node at compile time
self.env['ir.qweb'].clear_caches()
self.env.registry.clear_cache()

# invalidate cache for `company.website_id` to be recomputed
if 'sequence' in values or 'company_id' in values:
Expand Down Expand Up @@ -1088,7 +1088,7 @@ def viewref(self, view_id, raise_if_not_found=True):
return view

@api.model
@tools.ormcache_context('key', keys=('website_id',))
@tools.ormcache('key', 'self._context.get("website_id")', cache='templates')
def is_view_active(self, key):
"""
Return True if active, False if not active, None if not found
Expand Down
6 changes: 3 additions & 3 deletions addons/website/models/website_menu.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def create(self, vals_list):
Be careful to return correct record for ir.model.data xml_id in case
of default main menus creation.
'''
self.clear_caches()
self.env.registry.clear_cache('templates')
# Only used when creating website_data.xml default menu
menus = self.env['website.menu']
for vals in vals_list:
Expand Down Expand Up @@ -103,11 +103,11 @@ def create(self, vals_list):
return menus

def write(self, values):
self.clear_caches()
self.env.registry.clear_cache('templates')
return super().write(values)

def unlink(self):
self.clear_caches()
self.env.registry.clear_cache('templates')
default_menu = self.env.ref('website.main_menu', raise_if_not_found=False)
menus_to_remove = self
for menu in self.filtered(lambda m: default_menu and m.parent_id.id == default_menu.id):
Expand Down
Loading

0 comments on commit 595aa24

Please sign in to comment.