You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by "Martin Kočí (JIRA)" <de...@myfaces.apache.org> on 2012/06/04 22:01:24 UTC
[jira] [Created] (MYFACES-3562) [perf] Optimize
UIOutput.saveState(FacesContext)
Martin Kočí created MYFACES-3562:
------------------------------------
Summary: [perf] Optimize UIOutput.saveState(FacesContext)
Key: MYFACES-3562
URL: https://issues.apache.org/jira/browse/MYFACES-3562
Project: MyFaces Core
Issue Type: Improvement
Reporter: Martin Kočí
Priority: Trivial
1) use converterSaved as delta change check
2) remove nullDelta boolean
3) move new _AttachedDeltaWrapper into if (attachedState != null) statement
question: _AttachedDeltaWrapper used in this context provides Class of Converter, but that info is not used in restoreState - can we remove usage of that wrapper?
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MYFACES-3562) [perf] Optimize
UIOutput.saveState(FacesContext)
Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-3562?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13289395#comment-13289395 ]
Leonardo Uribe commented on MYFACES-3562:
-----------------------------------------
MK >> Ah, I see ... in that case new _AttachedDeltaWrapper(_converter.getClass(), /* null */ attachedState); represent the change setConverter(null), right?
No, the internal value represents the delta, the one who represent the null converter is the null blank in:
return new Object[]{parentSaved, converterSaved};
_isSetConverter() flag activates, _converter is null, so null value to store on the state, so we return a null blank in the array [parentSaved, null].
MK >> During saveView, for each converter a instance of _AttachedDeltaWrapper is created, but immediatelly GCed. In this case, _AttachedDeltaWrapper has attachedState null too. Is it possible to distinguish between these two cases?
Maybe the solution is add something like this:
//Delta
StateHolder holder = (StateHolder) _converter;
if (!holder.isTransient())
{
Object attachedState = holder.saveState(facesContext);
if (attachedState != null)
{
nullDelta = false;
}
// create it only when it has sense to do it.
if ( !(parentSaved == null && nullDelta) )
{
converterSaved = new _AttachedDeltaWrapper(_converter.getClass(),
attachedState);
}
}
else
{
converterSaved = null;
}
Other option is change the semantic of the algorithm and do not use an AttachedDeltaWrapper, instead an AttachedNullWrapper or something like that. I'm not sure how to do it.
> [perf] Optimize UIOutput.saveState(FacesContext)
> ------------------------------------------------
>
> Key: MYFACES-3562
> URL: https://issues.apache.org/jira/browse/MYFACES-3562
> Project: MyFaces Core
> Issue Type: Improvement
> Reporter: Martin Kočí
> Priority: Trivial
> Attachments: MYFACES-3562.patch
>
>
> 1) use converterSaved as delta change check
> 2) remove nullDelta boolean
> 3) move new _AttachedDeltaWrapper into if (attachedState != null) statement
> question: _AttachedDeltaWrapper used in this context provides Class of Converter, but that info is not used in restoreState - can we remove usage of that wrapper?
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MYFACES-3562) [perf] Optimize
UIOutput.saveState(FacesContext)
Posted by "Martin Kočí (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-3562?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13289328#comment-13289328 ]
Martin Kočí commented on MYFACES-3562:
--------------------------------------
Ah, I see ... in that case new _AttachedDeltaWrapper(_converter.getClass(), /* null */ attachedState); represent the change setConverter(null), right?
Ok, I created this issue, because in my view I have hundreds of UIOutput, each with a converter. During saveView, for each converter a instance of _AttachedDeltaWrapper is created, but immediatelly GCed. In this case, _AttachedDeltaWrapper has attachedState null too. Is it possible to distinguish between these two cases?
> [perf] Optimize UIOutput.saveState(FacesContext)
> ------------------------------------------------
>
> Key: MYFACES-3562
> URL: https://issues.apache.org/jira/browse/MYFACES-3562
> Project: MyFaces Core
> Issue Type: Improvement
> Reporter: Martin Kočí
> Priority: Trivial
> Attachments: MYFACES-3562.patch
>
>
> 1) use converterSaved as delta change check
> 2) remove nullDelta boolean
> 3) move new _AttachedDeltaWrapper into if (attachedState != null) statement
> question: _AttachedDeltaWrapper used in this context provides Class of Converter, but that info is not used in restoreState - can we remove usage of that wrapper?
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MYFACES-3562) [perf] Optimize
UIOutput.saveState(FacesContext)
Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-3562?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13397555#comment-13397555 ]
Leonardo Uribe commented on MYFACES-3562:
-----------------------------------------
By coincidence, I found one way to prevent create AttachedDeltaWrapper instances changing the array size. I remembered a similar trick in UIComponentBase. So, I have attached a patch with junit test cases, trying all possible combinations, and in the way I found one missing case with transient property and PartialStateHolder too.
If no objections I'll commit the proposed patch soon.
> [perf] Optimize UIOutput.saveState(FacesContext)
> ------------------------------------------------
>
> Key: MYFACES-3562
> URL: https://issues.apache.org/jira/browse/MYFACES-3562
> Project: MyFaces Core
> Issue Type: Improvement
> Reporter: Martin Kočí
> Priority: Trivial
> Attachments: MYFACES-3562-2.patch, MYFACES-3562.patch
>
>
> 1) use converterSaved as delta change check
> 2) remove nullDelta boolean
> 3) move new _AttachedDeltaWrapper into if (attachedState != null) statement
> question: _AttachedDeltaWrapper used in this context provides Class of Converter, but that info is not used in restoreState - can we remove usage of that wrapper?
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (MYFACES-3562) [perf] Optimize
UIOutput.saveState(FacesContext)
Posted by "Martin Kočí (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-3562?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Martin Kočí updated MYFACES-3562:
---------------------------------
Status: Patch Available (was: Open)
> [perf] Optimize UIOutput.saveState(FacesContext)
> ------------------------------------------------
>
> Key: MYFACES-3562
> URL: https://issues.apache.org/jira/browse/MYFACES-3562
> Project: MyFaces Core
> Issue Type: Improvement
> Reporter: Martin Kočí
> Priority: Trivial
> Attachments: MYFACES-3562.patch
>
>
> 1) use converterSaved as delta change check
> 2) remove nullDelta boolean
> 3) move new _AttachedDeltaWrapper into if (attachedState != null) statement
> question: _AttachedDeltaWrapper used in this context provides Class of Converter, but that info is not used in restoreState - can we remove usage of that wrapper?
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MYFACES-3562) [perf] Optimize
UIOutput.saveState(FacesContext)
Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-3562?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13289235#comment-13289235 ]
Leonardo Uribe commented on MYFACES-3562:
-----------------------------------------
The logic around nullDelta and _AttachedDeltaWrapper is intentional.
In simple words, the idea is to restore delta properly we need to differentiate between a "null delta" and a "null value". "null delta" means the component has not changes since the last markInitialState(). "null value" can happen in two scenarios:
1. If some code forcefully clear the property (converter).
2. If there is no value assigned to the property (or a converter is not set).
I can remember the nullDelta boolean is for case 1. Suppose a converter is removed after markInitialState(). In this case _isSetConverter() flag activates, but _converter is null, so saveAttachedState() will return null. We cannot return null, because to preserve the change, we need to indicate that the property was set to null. A check based in converterSaved will skip this case, but the check using nullDelta boolean will not.
I know the case proposed is rare, but please note the intention is make PSS algorithm 100% compatible with JSF 1.2 state saving. Note the same pattern has been used in other places too (for example in componentClass20.vm).
> [perf] Optimize UIOutput.saveState(FacesContext)
> ------------------------------------------------
>
> Key: MYFACES-3562
> URL: https://issues.apache.org/jira/browse/MYFACES-3562
> Project: MyFaces Core
> Issue Type: Improvement
> Reporter: Martin Kočí
> Priority: Trivial
> Attachments: MYFACES-3562.patch
>
>
> 1) use converterSaved as delta change check
> 2) remove nullDelta boolean
> 3) move new _AttachedDeltaWrapper into if (attachedState != null) statement
> question: _AttachedDeltaWrapper used in this context provides Class of Converter, but that info is not used in restoreState - can we remove usage of that wrapper?
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (MYFACES-3562) [perf] Optimize
UIOutput.saveState(FacesContext)
Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-3562?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Leonardo Uribe updated MYFACES-3562:
------------------------------------
Resolution: Fixed
Fix Version/s: 2.1.9
2.0.15
Assignee: Leonardo Uribe
Status: Resolved (was: Patch Available)
> [perf] Optimize UIOutput.saveState(FacesContext)
> ------------------------------------------------
>
> Key: MYFACES-3562
> URL: https://issues.apache.org/jira/browse/MYFACES-3562
> Project: MyFaces Core
> Issue Type: Improvement
> Reporter: Martin Kočí
> Assignee: Leonardo Uribe
> Fix For: 2.0.15, 2.1.9
>
> Attachments: MYFACES-3562-2.patch, MYFACES-3562.patch
>
>
> 1) use converterSaved as delta change check
> 2) remove nullDelta boolean
> 3) move new _AttachedDeltaWrapper into if (attachedState != null) statement
> question: _AttachedDeltaWrapper used in this context provides Class of Converter, but that info is not used in restoreState - can we remove usage of that wrapper?
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira