Performance Caching Current Locale #5096
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
markusweigelt
suggested changes
May 2, 2022
| private static final String ATTRIBUTE_NAME = "RequestScopeCacheHelper"; | ||
|
|
||
| @SuppressWarnings("unused") | ||
| private static final Logger logger = LogManager.getLogger(RequestScopeCacheHelper.class); |
There was a problem hiding this comment.
Do we need this logger? If not then I suggest to remove it.
There was a problem hiding this comment.
It's not needed. Removed
markusweigelt
approved these changes
May 2, 2022
Comment on lines
21
to
22
| import org.apache.logging.log4j.LogManager; | ||
| import org.apache.logging.log4j.Logger; |
There was a problem hiding this comment.
As there is no Logger anymore this imports are not needed anymore.
There was a problem hiding this comment.
I had overlooked thx
This was referenced Aug 30, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Contributes to:
On many occasions, e.g., when translating messages, the method LocaleHelper.getCurrentLocale is called. If a user is logged in, this request will trigger a database query for the current language setting of the user, see Line 84. Naturally, issuing a database query for each
#{msgs.something}will reduce performance significantly.In theory, this database query could be cached by Hibernate. Unfortunately, Hibernate caching is currently not working in Kitodo:
@org.hibernate.annotations.Cachefor each Bean, which are missing, see Documentation:Even with database caching enabled, one could argue that each
#{msgs.something}should still not require a full re-evaluation of the Hibernate query framework. In my opinion, the current user locale should be cached in memory for each request.The proposed pull request caches
LocaleHelper.getCurrentLocalevia a simple request scoped cache implementation usingFacesContext, meaning the cache is cleared after each HTTP request. Caching improves performance significantly (together with #5095):Drawbacks
Caching via
FacesContextcould be argued to not be a very clean way of implementing this. Ideally, such information should be cached via beans that are annotated with@RequestScope. However, I'm not aware of any other way to introduce request scope to low-level static service methods like this. I suppose this is a JSF architectural design issue. Any suggestions are welcome.