[
https://issues.apache.org/jira/browse/MYFACES-2737?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12872100#action_12872100
]
Jan-Kees van Andel commented on MYFACES-2737:
---------------------------------------------
I agree with Martin. It looks good.
Normally I'm quite keen on concurrency bugs (like putting non-threadsafe
classes in the session), but this approach where you manually keep the
FacesContext instance request scoped, looks safe.
How many FacesContext.getCurrentInstance calls do you expect to save by this
approach?
> Cache FacesContext on UIComponentBase instances
> -----------------------------------------------
>
> Key: MYFACES-2737
> URL: https://issues.apache.org/jira/browse/MYFACES-2737
> Project: MyFaces Core
> Issue Type: Improvement
> Components: JSR-314
> Affects Versions: 2.0.0
> Reporter: Leonardo Uribe
> Assignee: Leonardo Uribe
> Attachments: MYFACES-2737-1.patch
>
>
> Right now, the implementation of UIComponentBase.getFacesContext() is this:
> @Override
> protected FacesContext getFacesContext()
> {
> return FacesContext.getCurrentInstance();
> }
> I think it is possible to create a variable like this:
> private transient FacesContext _facesContext;
> and change the current implementation to:
> void setCachedFacesContext(FacesContext facesContext)
> {
> _facesContext = facesContext;
> }
> @Override
> protected FacesContext getFacesContext()
> {
> if (_facesContext == null)
> {
> return FacesContext.getCurrentInstance();
> }
> else
> {
> return _facesContext;
> }
> }
> Then we do this on methods like processXXX, encodeXXX (not on encodeAll),
> visitTree and invokeOnComponent:
> @Override
> public void processDecodes(FacesContext context)
> {
> try
> {
> setCachedFacesContext(context);
>
> /*...... do what is required ........*/
> }
> finally
> {
> popComponentFromEL(context);
>
> setCachedFacesContext(null);
> }
> In few words, set and release temporally the variable while those operations
> are executed. This change will reduce the amount of calls to
> FacesContext.getCurrentInstance() without side effects, because we are
> caching only on safe places and enclosing everything in a try finally block.
> If no objections I'll commit this code soon.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
