search

plone / plone.i18n

Text normalization logic and language, country, cctld data.
8 stars 11 forks source link

Cache the registry lookup in LanguageUtility #34

Closed jaroel closed 3 years ago

jaroel commented 3 years ago

This changes the LanguageUtility.settings property to a zope.cachedescriptors.Lazy. .settings is called multiple times and shows up in py-spy for restapi calls. Making this cached on the utility increases req/s on my machine with ~100.

I do not know if and what negative side effect might be. Please handle with care.

Screenshot 2021-02-08 at 23 13 15

mister-roboto commented 3 years ago

@jaroel thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

jaroel commented 3 years ago

@jenkins-plone-org please run jobs

jaroel commented 3 years ago

@jensens what do you think?

jensens commented 3 years ago

Well, +1 for caching, but maybe a bit more careful. I doubt this works with i.e. lineage.registry. I would take the current context, like its path, into a cache key.

OTOH, more globally, I stumbled over frequent of these getRegistry calls already an different places. I tend to create a central place to lookup registry once per request and then cache it on the request.

jensens commented 3 years ago

that said, this does not solve the second's line slowness.

jaroel commented 3 years ago

I wish we'd didn't do utilities like this and just pass around the request with a registry on it. Anyhow, this change break stuff, so no-go.