10:57 a.m.
>>>> On Tue, 19 Oct 2010 12:35:05 -0400, Andy Schwartz
<andy.schwartz(a)oracle.com> said:
EB> When the markInitialState() method is called, save the state of the
EB> direct children
AS> Is it just the direct children? Don't we need to save the state for
AS> non-direct descendents as well?
I see what you mean. Yes, we should, and I've updated the spec language.
AS> Should we mention that non-iterated facets of UIData are excluded
AS> from this?
I have mentioned that.
EB> without a dependency on any particular row value. For discussion, this
EB> rolled up state is called the "initial rolled up state".
AS> Minor issue, but for some reason the term "rolled up state" doesn't
seem
AS> to be clicking for me. I think of this as the "full initial state" -
AS> ie. we are doing a full state save to capture the initial state of these
AS> components so that we can get back to this later.
I'm going to stick with rolled up state. The word "initial" is used so
frequently in this sort of discussion that I don't want to use it any more.
EB> When setRowIndex(int) is called, the following additional actions must
EB> be taken before the usual call to setRowIndex(int).
AS> Instead of "before the usual call to setRowIndex()", maybe "before
any
AS> other work is performed"? It isn't clear what "the usual call"
means.
EB> Traverse the children as done with markInitialState(), saving the
EB> state of each child. Because the
EB> saveState(javax.faces.context.FacesContext) calls during this pass
EB> happen after the call to markInitialState(), the state saved is the so
EB> called "delta" state of each component.
AS> What happens is partial state saving is disabled? Do we still implement
AS> this behavior and just save/restore the full state, or do we bail on the
AS> preserveRowComponentState behavior?
I have clarified that the preserveRowComponentState is not impacted by
the application partial state saving behavior.
AS> BTW, we don't need to perform this traversal if the current row index
AS> (the row index at the point in time when setRowIndex() is called) is -1.
EB> Because this traversal happens during a per-row operation (in this
EB> case, setRowIndex(int)) the rolled up state
AS> Okay, so I guess "rolled up" state != "full initial state" -
more like
AS> the per-row saved state.
EB> must be saved in a row-aware data structure. One implementation choice
EB> would be to save the state from this pass in a Map keyed by the return
EB> from UIComponent.getContainerClientId(javax.faces.context.FacesContext).
AS> Though this might not work well if we need to save state for subtrees as
AS> opposed to only direct children - ie. might be better to use the full
AS> client id. Perhaps we should leave this implementation suggestion out?
I have done so.
EB> It is permissible for the rolled up state to be null or empty.
EB>
EB> If the current row index is not -1, traverse the children as in the
EB> previous step,
AS> Do we need to perform two separate traversals? Woudn't it be more
AS> efficient to consolidate these into a single traversal?
Perhaps, but that's how Leonardo first implemented it and I am not
inclined to try to optimize it here.
EB> but, instead of calling saveState(javax.faces.context.FacesContext),
EB> call UIComponent.saveTransientState(javax.faces.context.FacesContext).
EB> Save the rolled up state in a separate row-aware data structure from
EB> the one used in the preciding
AS> preceding (sp)
Thanks.
EB> step.
EB>
EB> Call DataModel.setRowIndex(int) on the model, as normal. Store the row
EB> data as request scope attributes, as normal.
EB>
EB> If the rolled up state saved during the call to markInitialState() is
EB> not null or empty,
AS> Since this is the "initial rolled up state" - ie. the full state,
AS> wouldn't this be non-null?
I'm not sure of the answer to this one. Leonardo?
EB> perform the reverse of the two save operations, as described next.
EB> This is necessary to indicate that transient state from the previous
EB> row must be discarded, in order to not pollute the state of the
EB> current row.
EB> If the per-row state saved in step a. above is null, traverse the
EB> children and restore the child state using the initial rolled up state.
AS> Does the "per-row state saved in step a. above" refer to the stave saved
AS> for the row that we are leaving? Or for the state that we saved the
AS> last time that visited the current row?
AS> That is, if we are on row 1 and setRowIndex(2) is called, does the
AS> "per-row state saved in step a" mean the state produced for row 1 before
AS> we adjusted the index, or the state that we previously saved away the
AS> last time we visited row 2?
AS> This is important to specify clearly.
It's the state before we adjusted the row index. I've clarified that.
EB> If the per-row state saved in step a. above is not null, traverse the
EB> children and restore the child state using the state saved during step
EB> a., using the initial rolled up state only as a backup in the case
EB> that per-row state is not available.
AS> Okay, so I guess that "per-row state saved in step a. above" refers to
AS> the previously saved state for the current row.
No, it's the state before we adjusted the row index. Leonardo, it is
vitally important that you and Andy and I agree on this.
AS> However, since this is delta state, before we apply this isn't it
AS> necessary to first restore the components to their initial state by
AS> restoring the full initial saved state? If we don't restore the
AS> components to their initial state before applying the delta state,
AS> won't we run the risk that state from the previous row might bleed
AS> over into the current row? (In the case where the state from the
AS> previous from was null, this won't be an issue.)
AS> Also, at some point is it necessary to tell the StateHelper that it
AS> needs to clear out any previously saved deltas so that it can start
AS> tracking deltas for the currently active row? Or does that happen
AS> implicitly at some point, eg. when we restore the row state?
EB> If the current row index is -1, traverse the children and pass null to
EB> UIComponent.restoreTransientState(javax.faces.context.FacesContext,
EB> java.lang.Object).
AS> Hopefully we can consolidate the restore-related traversals as well.
EB> If the current row index is not -1, take the following actions.
EB> If the per-row state saved in step b. above is null, traverse the
EB> children and restore the transient state by passing null to each
EB> child's
EB> UIComponent.restoreTransientState(javax.faces.context.FacesContext,
EB> java.lang.Object) method
EB>
EB> If the per-row state saved in step b. above is not null, traverse the
EB> children and restore the transient state from the state saved in step
EB> b. above, calling
EB> UIComponent.restoreTransientState(javax.faces.context.FacesContext,
EB> java.lang.Object) on each child, and passing the appropriate state.
AS> Sounds like we are doing the same thing whether or not the transient
AS> state is null, so perhaps we can simplify the wording.
Perhaps, but I'm concerned with getting it wrong. The current text was
approved by Leonardo so I'm inclined to go with it.
AS> Few other questions:
AS> 1. Leonardo raised an issue regarding the timing of when
AS> markInitialState() is called - ie. that markInitialState() needs to be
AS> called on the parent before the children - otherwise UIData won't be
AS> able to capture the full initial state of its children. How did we
AS> solve this problem? Are there spec changes relating to this? Did we
AS> find a way to do this that doesn't require introducing yet another full
AS> tree traversal?
I don't know.
AS> 2. Regarding the name of the new property...
AS> is/setPreserveRowComponentState() is a bit of a mouthful. Could we
AS> maybe shorten this to is/setRowStatePreserved()?
I like that a lot better.
AS> 3. Are there cases where it might be useful to enable per-row transient
AS> state saving without also enabling non-transient state saving (which
AS> seems more expensive)? I wonder whether it should be possible to enable
AS> these independently, in which case we may want to consider using an enum
AS> property instead of a boolean. Also wondering whether transient row
AS> state saving should just be on by default or possibly always on.
Leonardo?
Ed
--
| edward.burns(a)oracle.com | office: +1 407 458 0017
| homepage: | http://ridingthecrest.com/
| 5 work days until German Oracle User's Group Conference
| 12 work days until GlassFish 3.1 Hard Code Freeze
3 p.m.
New subject: [jsr-314-open-mirror] [jsr-314-open] [153-UIDataStateSaving] spec clarifications requested by Andy
Hi
2010/11/3 Ed Burns <edward.burns(a)oracle.com>
>>>>> On Tue, 19 Oct 2010 12:35:05 -0400, Andy
Schwartz <
andy.schwartz(a)oracle.com> said:
EB> When the markInitialState() method is called, save the state of the
EB> direct children
AS> Is it just the direct children? Don't we need to save the state for
AS> non-direct descendents as well?
I see what you mean. Yes, we should, and I've updated the spec language.
AS> Should we mention that non-iterated facets of UIData are excluded
AS> from this?
I have mentioned that.
EB> without a dependency on any particular row value. For discussion, this
EB> rolled up state is called the "initial rolled up state".
AS> Minor issue, but for some reason the term "rolled up state" doesn't
seem
AS> to be clicking for me. I think of this as the "full initial state" -
AS> ie. we are doing a full state save to capture the initial state of
these
AS> components so that we can get back to this later.
I'm going to stick with rolled up state. The word "initial" is used so
frequently in this sort of discussion that I don't want to use it any more.
EB> When setRowIndex(int) is called, the following additional actions must
EB> be taken before the usual call to setRowIndex(int).
AS> Instead of "before the usual call to setRowIndex()", maybe "before
any
AS> other work is performed"? It isn't clear what "the usual call"
means.
EB> Traverse the children as done with markInitialState(), saving the
EB> state of each child. Because the
EB> saveState(javax.faces.context.FacesContext) calls during this pass
EB> happen after the call to markInitialState(), the state saved is the so
EB> called "delta" state of each component.
AS> What happens is partial state saving is disabled? Do we still
implement
AS> this behavior and just save/restore the full state, or do we bail on
the
AS> preserveRowComponentState behavior?
I have clarified that the preserveRowComponentState is not impacted by
the application partial state saving behavior.
AS> BTW, we don't need to perform this traversal if the current row index
AS> (the row index at the point in time when setRowIndex() is called) is
-1.
EB> Because this traversal happens during a per-row operation (in this
EB> case, setRowIndex(int)) the rolled up state
AS> Okay, so I guess "rolled up" state != "full initial state" -
more like
AS> the per-row saved state.
EB> must be saved in a row-aware data structure. One implementation choice
EB> would be to save the state from this pass in a Map keyed by the return
EB> from
UIComponent.getContainerClientId(javax.faces.context.FacesContext).
AS> Though this might not work well if we need to save state for subtrees
as
AS> opposed to only direct children - ie. might be better to use the full
AS> client id. Perhaps we should leave this implementation suggestion out?
I have done so.
EB> It is permissible for the rolled up state to be null or empty.
EB>
EB> If the current row index is not -1, traverse the children as in the
EB> previous step,
AS> Do we need to perform two separate traversals? Woudn't it be more
AS> efficient to consolidate these into a single traversal?
Perhaps, but that's how Leonardo first implemented it and I am not
inclined to try to optimize it here.
Yes, it is possible. On the first implementation, I wanted to keep things
simple
and understandable, so I keep them on different methods, but now the
algorithm
is well understood.
EB> but, instead of calling
saveState(javax.faces.context.FacesContext),
EB> call UIComponent.saveTransientState(javax.faces.context.FacesContext).
EB> Save the rolled up state in a separate row-aware data structure from
EB> the one used in the preciding
AS> preceding (sp)
Thanks.
EB> step.
EB>
EB> Call DataModel.setRowIndex(int) on the model, as normal. Store the row
EB> data as request scope attributes, as normal.
EB>
EB> If the rolled up state saved during the call to markInitialState() is
EB> not null or empty,
AS> Since this is the "initial rolled up state" - ie. the full state,
AS> wouldn't this be non-null?
I'm not sure of the answer to this one. Leonardo?
In theory, if UIData.markInitialState is called correctly, the "initial
rolled up
state" is not null, so the check could be skipped. In the first versions of
the
algorithm, the check was used to fallback to the previous algorithm, but now
this is done using a check for the property ( isPreserveRowComponentState()
).
EB> perform the reverse of the two save operations, as described next.
EB> This is necessary to indicate that transient state from the
previous
EB> row must be discarded, in order to not pollute the state of the
EB> current row.
EB> If the per-row state saved in step a. above is null, traverse the
EB> children and restore the child state using the initial rolled up state.
AS> Does the "per-row state saved in step a. above" refer to the stave
saved
AS> for the row that we are leaving? Or for the state that we saved the
AS> last time that visited the current row?
AS> That is, if we are on row 1 and setRowIndex(2) is called, does the
AS> "per-row state saved in step a" mean the state produced for row 1
before
AS> we adjusted the index, or the state that we previously saved away the
AS> last time we visited row 2?
AS> This is important to specify clearly.
It's the state before we adjusted the row index. I've clarified that.
EB> If the per-row state saved in step a. above is not null, traverse the
EB> children and restore the child state using the state saved during step
EB> a., using the initial rolled up state only as a backup in the case
EB> that per-row state is not available.
AS> Okay, so I guess that "per-row state saved in step a. above" refers to
AS> the previously saved state for the current row.
No, it's the state before we adjusted the row index. Leonardo, it is
vitally important that you and Andy and I agree on this.
The "per-row state saved in step a. above" in other words means the
"delta"
state from the row that will be changed. If we are on row 1, to change to
row 2
we need to restore the state of row 2, and check if it is null or not, If it
is null
we use only the initial rolled up state, but if is not, we use both the
initial
rolled up state and the "per-row state saved".
AS> However, since this is delta state, before we apply this
isn't it
AS> necessary to first restore the components to their initial state by
AS> restoring the full initial saved state? If we don't restore the
AS> components to their initial state before applying the delta state,
AS> won't we run the risk that state from the previous row might bleed
AS> over into the current row? (In the case where the state from the
AS> previous from was null, this won't be an issue.)
AS> Also, at some point is it necessary to tell the StateHelper that it
AS> needs to clear out any previously saved deltas so that it can start
AS> tracking deltas for the currently active row? Or does that happen
AS> implicitly at some point, eg. when we restore the row state?
EB> If the current row index is -1, traverse the children and pass null to
EB> UIComponent.restoreTransientState(javax.faces.context.FacesContext,
EB> java.lang.Object).
AS> Hopefully we can consolidate the restore-related traversals as well.
EB> If the current row index is not -1, take the following actions.
EB> If the per-row state saved in step b. above is null, traverse the
EB> children and restore the transient state by passing null to each
EB> child's
EB> UIComponent.restoreTransientState(javax.faces.context.FacesContext,
EB> java.lang.Object) method
EB>
EB> If the per-row state saved in step b. above is not null, traverse the
EB> children and restore the transient state from the state saved in step
EB> b. above, calling
EB> UIComponent.restoreTransientState(javax.faces.context.FacesContext,
EB> java.lang.Object) on each child, and passing the appropriate state.
AS> Sounds like we are doing the same thing whether or not the transient
AS> state is null, so perhaps we can simplify the wording.
Perhaps, but I'm concerned with getting it wrong. The current text was
approved by Leonardo so I'm inclined to go with it.
In few words, it just say how to restore the "transient state". It is
possible in this
part I just forget to simplify things.
The time I wrote that part I was thinking if there was some kind of
manipulation
on transient state properties before build view time, those values should be
"propagated" to child rows, but the intention of this state is to have a
very
short life time (vdl build time or render time). If a value is necessary
to be stored more than the current request, use StateHelper. After thinking
about
it carefully, the conclusion was that is not necessary, so we can just
assume a
"null" or "empty" state for the transient map.
AS> Few other questions:
AS> 1. Leonardo raised an issue regarding the timing of when
AS> markInitialState() is called - ie. that markInitialState() needs to be
AS> called on the parent before the children - otherwise UIData won't be
AS> able to capture the full initial state of its children. How did we
AS> solve this problem? Are there spec changes relating to this? Did we
AS> find a way to do this that doesn't require introducing yet another full
AS> tree traversal?
I don't know.
It is possible to prevent the full tree traversal, but to do that it is
necessary to
change all code that uses PostAddToViewEvent to relocate components, and
that
means refactor composite component solution as is. This was done on MyFaces,
so there cc:insertChildren and cc:insertFacet does not use a Listener
attached to
that event there, but Mojarra uses it.
Anyway, users could add components on PostAddToViewEvent to the tree, so
in theory the call to markInitialState() should be done after that event is
handled.
AS> 2. Regarding the name of the new property...
AS> is/setPreserveRowComponentState() is a bit of a mouthful.
Could we
AS> maybe shorten this to is/setRowStatePreserved()?
I like that a lot better.
AS> 3. Are there cases where it might be useful to enable per-row
transient
AS> state saving without also enabling non-transient state saving (which
AS> seems more expensive)? I wonder whether it should be possible to
enable
AS> these independently, in which case we may want to consider using an
enum
AS> property instead of a boolean. Also wondering whether transient row
AS> state saving should just be on by default or possibly always on.
Leonardo?
It is possible, but shouldn't that condition just depends on the component
child itself ?.
For example, UIForm has submitted property that in the future will be stored
in transient
map, so this component requires transient state be always on.
best regards,
Leonardo Uribe
Ed
--
| edward.burns(a)oracle.com | office: +1 407 458 0017
| homepage: | http://ridingthecrest.com/
| 5 work days until German Oracle User's Group Conference
| 12 work days until GlassFish 3.1 Hard Code Freeze
10:57 a.m.
New subject: [jsr-314-open-mirror] [jsr-314-open] [153-UIDataStateSaving] spec clarifications requested by Andy
11:10 a.m.
New subject: [jsr-314-open-mirror] [jsr-314-open] [153-UIDataStateSaving] spec clarifications requested by Andy
(Re-sending, this time hopefully as plain/text, since the JBoss archive
scrubbed my previous text/html response.)
On 11/3/10 4:00 PM, Leonardo Uribe wrote:
Hi
2010/11/3 Ed Burns <edward.burns(a)oracle.com
<mailto:edward.burns@oracle.com>>
>>>>> On Tue, 19 Oct 2010 12:35:05 -0400, Andy Schwartz
<andy.schwartz(a)oracle.com <mailto:andy.schwartz@oracle.com>> said:
AS> What happens is partial state saving is disabled? Do we still
implement
AS> this behavior and just save/restore the full state, or do we
bail on the
AS> preserveRowComponentState behavior?
I have clarified that the preserveRowComponentState is not impacted by
the application partial state saving behavior.
I don't understand how this can be. The documentation for
preserveRowComponentState/rowStatePreserved is heavily dependent on
partial state saving - eg. see all of the references to
markInitialState(), which is only called when partial state saving is
enabled.
It is certainly possible to support rowStatePreserved-like behavior for
the full state saving case. However, the implementation would have
significant differences from behavior that is currently described in the
doc.
While I agree that it would make sense for rowStatePreserved to be
honored regardless of whether partial/full state saving is on, simply
stating that:
Note that this delta state is saved regardless of whether or not
partial state saving has been enabled or disabled for this application.
Is not nearly sufficient to cover the full state saving case.
Part of the problem is that the spec for rowStatePreserved is overly
detailed in what implementations are required to do. The spec is very
explicit about how to implement this feature for the partial state
saving case, but this same approach does not work for the full state
saving case. We either need to describe both cases in detail, or revise
the specification to provide a higher-level description of the expected
behavior, rather than explicit implementation instructions.
AS> Do we need to perform two separate traversals?
Woudn't it be more
AS> efficient to consolidate these into a single traversal?
Perhaps, but that's how Leonardo first implemented it and I am not
inclined to try to optimize it here.
Yes, it is possible. On the first implementation, I wanted to keep
things simple
and understandable, so I keep them on different methods, but now the
algorithm
is well understood.
Sure, that makes sense.
However, this is another case where the specification is being
overly-specific in dictating implementation details that are better left
unspecified. The specification must describe the requirements in a way
that allows implementors to make their own decisions about how best to
balance complexity/performance - ie. the specification must not get in
the way of an implementation that wants to implement this functionality
in a more efficient manner. The current wording is not appropriate.
This needs to be fixed for 2.1.
On a somewhat related note, the spec currently requires that we collect
non-transient state first, followed by transient state. This poses a
non-obvious dilemma for a use case that I care about. The use case is a
bit obscure, so I'll start with some background...
Trinidad provides an (optional) optimization for keeping the current
component tree around across requests. This is enabled by the
org.apache.myfaces.trinidad.CACHE_VIEW_ROOT [1] context parameter. One
potential pitfall with this optimization: without taking special steps
to free up transient state referenced by the component tree, this
transient state will survive across requests, which is a significant
difference from the normal state saving behavior.
One way to limit the impact of this is for components that hold onto
transient state to free up this state at the end of the request. The
most obvious time to do this is when saveState() is called. Now that we
are introducing a formal transient state solution, I would very much
like to see this transient state releasing behavior implemented
centrally - ie. it would very much help the view root caching
optimization if we could discard transient state when (non-transient)
state saving is performed.
This should be trivial to implement - eg. in Mojarra,
ComponentStateHelper.saveState() could call transientState.clear().
However, if UIData's rowStatePreserved behavior requires that
saveState() is called before saveTransientState(), this would prohibit
us from clearing transient state during state saving, since this would
break transient state saving in the UIData case. Swapping the order
around (save transient state first followed by non-transient state),
would at least allow the possibility of implementing this in a way that
would work well with Trinidad's view root caching.
It might make sense to consider whether we should formalize some of this
in 2.2 - eg. we may want to consider promoting the view root caching
optimization up to the JSF spec/implementations and standardizing the
requirements for what components need to do to be compatible with this.
In the meantime, I would ask that we avoid introducing any new spec
language that would thwart this functionality.
Oh, on yet another tangent... I noticed while looking at Mojarra's
ComponentStateHelper that we have the following implementations for
save/restoreTransientState():
public void restoreTransientState(FacesContext context, Object
state)
{
transientState = (Map<Object, Object>) state;
}
public Object saveTransientState(FacesContext context)
{
return transientState;
}
Doesn't this mean that we'll end up sharing the same transient state map
across all rows? I don't see how this can possibly work. Don't we need
to make a copy of the state on the save rather than providing a
reference to our internal transient state map?
EB> If the rolled up state saved during the call to
markInitialState() is
EB> not null or empty,
AS> Since this is the "initial rolled up state" - ie. the full state,
AS> wouldn't this be non-null?
I'm not sure of the answer to this one. Leonardo?
In theory, if UIData.markInitialState is called correctly, the
"initial rolled up
state" is not null, so the check could be skipped. In the first
versions of the
algorithm, the check was used to fallback to the previous algorithm,
but now
this is done using a check for the property (
isPreserveRowComponentState() ).
Hrm... so seems like this will never be null.
EB> If the per-row state saved in step a. above is null, traverse the
EB> children and restore the child state using the initial rolled
up state.
AS> Does the "per-row state saved in step a. above" refer to the
stave saved
AS> for the row that we are leaving? Or for the state that we
saved the
AS> last time that visited the current row?
AS> That is, if we are on row 1 and setRowIndex(2) is called, does the
AS> "per-row state saved in step a" mean the state produced for
row 1 before
AS> we adjusted the index, or the state that we previously saved
away the
AS> last time we visited row 2?
AS> This is important to specify clearly.
It's the state before we adjusted the row index. I've clarified that.
EB> If the per-row state saved in step a. above is not null,
traverse the
EB> children and restore the child state using the state saved
during step
EB> a., using the initial rolled up state only as a backup in the case
EB> that per-row state is not available.
AS> Okay, so I guess that "per-row state saved in step a. above"
refers to
AS> the previously saved state for the current row.
No, it's the state before we adjusted the row index. Leonardo, it is
vitally important that you and Andy and I agree on this.
The "per-row state saved in step a. above" in other words means the
"delta"
state from the row that will be changed. If we are on row 1, to change
to row 2
we need to restore the state of row 2, and check if it is null or not,
If it is null
we use only the initial rolled up state, but if is not, we use both
the initial
rolled up state and the "per-row state saved".
This makes sense. Any time we being operating on a new row, we need to:
1. Restore it to the initial state using the rolled up (full) state
that we saved during markInitialState(). And...
2. Apply delta state, if there is any.
Ed, this means that this clarification:
It's the state before we adjusted the row index. I've
clarified that.
Is incorrect and needs to be re-clarified. :-)
BTW, this is a case where full vs. partial state saving impacts the
implementation. If we implement rowStatePreserved behavior for the full
state saving case, we would never do #2 above - ie. there would never be
any delta state. Instead, we would always save off the full state for
each row on exit and restore the previously saved full state on re-entry.
AS> However, since this is delta state, before we apply this isn't it
AS> necessary to first restore the components to their initial
state by
AS> restoring the full initial saved state? If we don't restore the
AS> components to their initial state before applying the delta state,
AS> won't we run the risk that state from the previous row might bleed
AS> over into the current row? (In the case where the state from the
AS> previous from was null, this won't be an issue.)
AS> Also, at some point is it necessary to tell the StateHelper
that it
AS> needs to clear out any previously saved deltas so that it can
start
AS> tracking deltas for the currently active row? Or does that happen
AS> implicitly at some point, eg. when we restore the row state?
I answered my own question by taking a peek at the code. I see that in
Mojarra's UIData implementation that we explicitly call
markInitialState() after restoring the initial/rolled up state and
before applying deltas:
component.clearInitialState();
if (childInitialState != null)
{
component.restoreState(facesContext,
childInitialState);
component.markInitialState();
component.restoreState(facesContext, childState);
}
This seems reasonable.
EB> If the per-row state saved in step b. above is null,
traverse the
EB> children and restore the transient state by passing null to each
EB> child's
EB>
UIComponent.restoreTransientState(javax.faces.context.FacesContext,
EB> java.lang.Object) method
EB>
EB> If the per-row state saved in step b. above is not null,
traverse the
EB> children and restore the transient state from the state saved
in step
EB> b. above, calling
EB>
UIComponent.restoreTransientState(javax.faces.context.FacesContext,
EB> java.lang.Object) on each child, and passing the appropriate
state.
AS> Sounds like we are doing the same thing whether or not the
transient
AS> state is null, so perhaps we can simplify the wording.
Perhaps, but I'm concerned with getting it wrong. The current
text was
approved by Leonardo so I'm inclined to go with it.
In few words, it just say how to restore the "transient state". It is
possible in this
part I just forget to simplify things.
The time I wrote that part I was thinking if there was some kind of
manipulation
on transient state properties before build view time, those values
should be
"propagated" to child rows, but the intention of this state is to have
a very
short life time (vdl build time or render time). If a value is necessary
to be stored more than the current request, use StateHelper. After
thinking about
it carefully, the conclusion was that is not necessary, so we can just
assume a
"null" or "empty" state for the transient map.
Hrm... so does that mean that we can simplify the above wording? I
think we should if we can - no need to make the doc more complicated
than it already is.
AS> Few other questions:
AS> 1. Leonardo raised an issue regarding the timing of when
AS> markInitialState() is called - ie. that markInitialState()
needs to be
AS> called on the parent before the children - otherwise UIData
won't be
AS> able to capture the full initial state of its children. How
did we
AS> solve this problem? Are there spec changes relating to this?
Did we
AS> find a way to do this that doesn't require introducing yet
another full
AS> tree traversal?
I don't know.
I see that we have added an extra traversal, in FaceletViewHandlingStrategy:
private void markInitialState(final UIComponent component)
{
component.markInitialState();
for (Iterator<UIComponent> it =
component.getFacetsAndChildren() ; it.hasNext() ; ) {
UIComponent child = it.next();
if (!child.isTransient()) {
markInitialState(child);
}
}
}
This sort of work represents a hidden extra tax on the lifecycle,
particularly for views with large component trees. At this point I have
lost track of how many component tree traversals JSF requires.
Unfortunately we are still in the process of doing our 1.2 vs. 2.0
performance testing here, so I am not yet in a position to say how
significant the extra overhead that we have added in 2.0 (and now 2.1)
is - ie. I don't really know whether I should object to yet another
component tree traversal or not. All I can say is that anytime we add
additional processing that involves visiting every component, this makes
me nervous.
It is possible to prevent the full tree traversal, but to do that it
is necessary to
change all code that uses PostAddToViewEvent to relocate components,
and that
means refactor composite component solution as is. This was done on
MyFaces,
so there cc:insertChildren and cc:insertFacet does not use a Listener
attached to
that event there, but Mojarra uses it.
Anyway, users could add components on PostAddToViewEvent to the tree, so
in theory the call to markInitialState() should be done after that
event is handled.
For 2.2 I think we are going to need to take a closer look at how well
partial state saving/markInitialState works for dynamically added subtrees.
AS> 2. Regarding the name of the new property...
AS> is/setPreserveRowComponentState() is a bit of a mouthful.
Could we
AS> maybe shorten this to is/setRowStatePreserved()?
I like that a lot better.
AS> 3. Are there cases where it might be useful to enable per-row
transient
AS> state saving without also enabling non-transient state saving
(which
AS> seems more expensive)? I wonder whether it should be possible
to enable
AS> these independently, in which case we may want to consider
using an enum
AS> property instead of a boolean. Also wondering whether
transient row
AS> state saving should just be on by default or possibly always on.
Leonardo?
It is possible, but shouldn't that condition just depends on the
component child itself ?.
For example, UIForm has submitted property that in the future will be
stored in transient
map, so this component requires transient state be always on.
That's right. A problem that we have is that it won't necessarily be
clear to application developers when the are using a component that
happens to make use of transient state and thus requires
rowStatePreserved to be enabled. It would be good if we could find some
way to limit the chance that application developers will get this
wrong. Or, if not that, it would help if we could at least warn
application developers in the event that they have missed this.
Andy
[1]
http://myfaces.apache.org/trinidad/devguide/configuration.html#org.apache...
12:01 p.m.
New subject: [jsr-314-open-mirror] [jsr-314-open] [153-UIDataStateSaving] spec clarifications requested by Andy
Gang -
After thinking about this some more:
One way to limit the impact of this is for components that hold onto
transient state to free up this state at the end of the request. The
most obvious time to do this is when saveState() is called. Now that
we are introducing a formal transient state solution, I would very
much like to see this transient state releasing behavior implemented
centrally - ie. it would very much help the view root caching
optimization if we could discard transient state when (non-transient)
state saving is performed.
This should be trivial to implement - eg. in Mojarra,
ComponentStateHelper.saveState() could call transientState.clear().
However, if UIData's rowStatePreserved behavior requires that
saveState() is called before saveTransientState(), this would prohibit
us from clearing transient state during state saving, since this would
break transient state saving in the UIData case. Swapping the order
around (save transient state first followed by non-transient state),
would at least allow the possibility of implementing this in a way
that would work well with Trinidad's view root caching.
I realized that the solution is not as simple as I originally thought.
The problem is that once we introduce the new and improved UIData state
saving mechanism, UIComponent.saveState() will be called at multiple
points in time:
1. During state saving (the traditional case).
2. In response to UIData.markInitialState().
3. In response to UIData.setRowIndex().
Since Trinidad view root caching holds onto the component tree across
requests, we really want to blow away transient state for #1.
However, we definitely do not want to blow away transient state in
response to #2 - this state should survive the call to
UIData.markInitialState().
I am not quite sure what the right behavior is for #3.
It seems like we'll need some way to distinguish between these cases.
Andy
5118
days inactive
5119
days old
jsr-314-open-mirror@lists.jboss.org
4 comments
3 participants
participants (3)
-
Andy Schwartz -
Ed Burns -
Leonardo Uribe