performance issue with new resource registry

[davisagli]

The new resource registry code calls plone.app.theming.utils.getTheme to get settings from the theme's manifest.cfg file (for enabled bundles, cooked css and js paths). That is not cached so it ends up reading manifest.cfg from disk for all themes on each request, which is not ideal for performance. Anecdotally, I tried replacing this with hardcoded paths and bin/alltests ran several minutes faster.

This same function is also called again from PloneSettingsAdapter in patterns/init.py to get the content area CSS for tinymce.

We need to cache this info somehow...possibly when the theme is activated we should copy these values to registry settings so they can be looked up efficiently.

/cc @bloodbare @vangheem

[vangheem]

Yes, I think @bloodbare is aware of a few areas where we need to do some caching.

Caching in a v property might be a good way to go I think right?

[bloodbare]

Definitly yes, may we adapt getTheme to get the info from the registry ? and store on the IThemeSettings the manifest information as we are storing the rules file ?

[davisagli]

I don't think we should change what getTheme is responsible for doing, because I don't know all the places it's used. Also I don't think it makes sense to store the entire manifest in the registry, because it could contain custom keys.

But I think it could make sense to have applyTheme in plone.app.theming (which is called when the theme is applied) store the particular settings we need in the registry, and then make the resource registry and pattern settings view use these settings from the registry instead of calling getTheme. We would also need an upgrade step to add these settings for existing sites that already have a theme applied.

Caching the result of getTheme could also work. And yeah I think a _v_ attribute would be good since that will last across multiple requests as long as the object isn't invalidated. The cache would need to be invalidated if a new theme is applied or the current theme is edited.

[bloodbare]

What about using IRAMCache

themeObj = cache.query(
    'plone.currentTheme',
    key=dict(prefix='theme'),
    default=None)
if themeObj is None:
    theme = getCurrentTheme()
    themeObj = getTheme(theme)
    cache.set(
          themeObj,
          'plone.currentTheme',
          key=dict(prefix='theme'))

[bloodbare]

Pushed https://github.com/plone/Products.CMFPlone/commit/dc8158c7ee5eeaf31400ff87c73ade71b352252d

[pbauer]

As I seet it that improvement will break customizations where different themes are used for different parts of a site (e.g. using lineage.themeselection) since getTheme will return the cached value while getCurrentTheme would return a different Theme.

[bloodbare]

it would help to store a dict based on the theme name of getCurrentTheme ? The main problem is reading again the manifest.

[vangheem]

I believe this is fixed. @bloodbare we should verify with latest plone.app.theming changes.

[bloodbare]

This is solved on the last merge of lone.app.theming