myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Grant Smith <gr...@marathonpm.com>
Subject Re: StateSaving review
Date Thu, 15 Nov 2012 23:30:21 GMT
I have to say, as an observer of this conversation it makes sense to work
in 2.1.x-client-window, and let Leo proceed with the release process
without having to deal with vetting changes which could possible effect
stability. We have far more to lose by working in the 2.1.x branch.

-Grant


On Thu, Nov 15, 2012 at 3:11 PM, Leonardo Uribe <lu4242@gmail.com> wrote:

> Hi
>
> 2012/11/15 Mark Struberg <struberg@yahoo.de>:
> > Leo, it would be great if you reflect a bit.
> >
> >> I have checked the code. I know what SerializedViewKey does.
> >
> > good! no other arguments? Well, then all is fine it seems.
> >
> >
> >> it is YOU the one who are reverting code done by other people
> >
> > The only code I ever reverted in MyFaces was your revert of my code
> change. That's it.
>
> But you reverted the changes I did months ago. Didn't you? oh yes, you
> "refactor" it and
> get rid what you don't like.
>
> > But actually I'm not the only one who got code reverted by you, and
> those people are fed up as well and partly left the community already
> because of that.
> >
>
> That's unfair. I don't think that's true. I'm just exercising my right
> to participate, that's it. People can disagree from time to time.
> That's part of the life, if you can't deal with it, it's your problem.
> But I'm always willing to look for alternatives and resolve
> differences, like I'm doing right now, and that's what makes the
> difference.
>
> >
> >> I'm sorry to be so direct, but as a member of the community, I
> >> have the right to participate and I'm willing to exercise that right.
> >
> > That's perfectly fine as well. So please start your excercise with
> listening to other people sometimes.
> > * I listed 5++ flaws in the design.
> > * I removed a few classes which contained 99% copy & paste.
> > * I removed a few classes which are not used anymore since over a year
> > * I removed an old resources folder which was just lying around for no
> reason.
> > * I moved the remaining inner classes to toplevel and an own package to
> make it easier to maintain and overlook the whole complexity.
> > * I added the viewId to _one_ of the dozen key implementations. Actually
> we don't need it. We also don't need the viewId hashCode which was there
> before. So I don't care, let's move this back to your hashCode solution and
> fix it properly in the next release.
> > * I cleaned up your generics because it always only used the String type
> anyway. And this cannot change in the future because this fact comes
> directly from the JSF spec.
> > Otherwise there has been no fundamental change to your code. The mess
> you see is all yours so to say (sorry for the sarcasm).
> >
>
> That's your opinion, not mine. But I can see some advance. At last, there
> is
> one list of the changes proposed. Since the plan is do a release next
> week, could
> we revert the changes, and put them on hold while the necessary steps are
> done?
> Otherwise a release will not be possible for a long time (In fact, you
> have kidnapped
> the release). In that way, we can dedicate enough time and resources to get
> it out once for all. Do you agree with that? Could you let me complete
> the planned
> schedule?
>
> Anyway, what's the deal to put the changes in 2.1.x branch? why you are so
> desperate for that? You have not given to me any reason why you can't work
> with
> 2.1.x-client-window branch, which is best for what you are trying to
> do, or why you
> can wait just 2 weeks to commit the changes.
>
> Come on Mark, is not a big deal, right?
>
> >
> >> The concrete case is spring webflow does not support PSS algorithm, so
> >> we need to switch back and enable compatibility mode with JSF 1.2.
> >
> > Spring WebFlow uses it's own StateManager which natively extends
> javax.faces.StateManager. There is no MyFaces code involved anymore since
> ages!
> >
> http://static.springsource.org/spring-webflow/docs/2.0.x/javadoc-api/org/springframework/faces/webflow/FlowViewStateManager.html
> >
> > I also googled quite some time and fine nothing pointing to any problems
> with the MyFaces StateManager. Not even on stackoverflow.
> >
>
> Look this:
>
> https://jira.springsource.org/browse/SWF-1365
>
> Because that problem, I decided to preserve JspStateManagerImpl long time
> ago,
> specifically when MYFACES-3117 was solved, because the change over
> ResponseStateManager imposed a change over all state management in MyFaces.
> Is there a problem? don't care if exists or not, what's important is
> provide
> a way to go back to the old behavior. Be conservative, keep code stable.
>
> regards,
>
> Leonardo Uribe
>
> >
> >
> > LieGrue,
> > strub
> >
> >
> >
> > ----- Original Message -----
> >> From: Leonardo Uribe <lu4242@gmail.com>
> >> To: MyFaces Development <dev@myfaces.apache.org>; Mark Struberg <
> struberg@yahoo.de>
> >> Cc:
> >> Sent: Thursday, November 15, 2012 11:06 PM
> >> Subject: Re: StateSaving review
> >>
> >> Hi
> >>
> >> 2012/11/15 Mark Struberg <struberg@yahoo.de>:
> >>>
> >>>>  You did more than that. You removed the following classes:
> >>>>
> >>>>  IntIntSerializedViewKey
> >>>>  IntByteArraySerializedViewKey
> >>>>  ReferenceSerializedViewKey
> >>>
> >>>  Those have been almost 1:1 clones of other classes. Go check
> >> SerializedViewKey, it contains all the merged functionality. Without any
> >> performance overhead.
> >>>  Look at the code first and then you can complain. You revert code
> done by
> >> other people without even looking at the changes it seems.
> >>>
> >>>
> >>
> >> I have checked the code. I know what SerializedViewKey does.
> >> I'm pretty aware of what it does and its effects. I have to say it,
> >> it is YOU the one who are reverting code done by other people
> >> without even looking at the changes, just because you have
> >> the perception that the changes are wrong, but nothing more.
> >> Without any proof or any deep consideration.
> >>
> >> The changes I have done over the time has follow the same
> >> protocol, create an issue, provide a patch, wait for feedback of
> >> the community, if no objections after a prudent time commit them
> >> and close the issue. In this moment the changes done by me has
> >> the tacit aval of the community. Your changes do not have that,
> >> because they have objections done 72 hours after they were
> >> proposed. This is a meritocracy, and the rules of the community
> >> in this case are on my side.
> >>
> >> I'm sorry to be so direct, but as a member of the community, I
> >> have the right to participate and I'm willing to exercise that right.
> >> I will not look to other side and let some untested code go into
> >> myfaces core 2.1.x. I appreciate your efforts to make this part
> >> better, but I encourage you to respect and play with the rules
> >> of the community.
> >>
> >> Look the dates of the improvements:
> >>
> >> MYFACES-3563
> >> Created: 10/Jun/12 11:33
> >> Resolved: 25/Jun/12 09:05
> >>
> >> MYFACES-3117
> >> Created: 26/Apr/11 19:08
> >> Resolved: 14/May/11 02:27
> >>
> >> That code has been there for years!!!!!! , the latest changes were done
> >> 5 months ago !!!. Nobody complained at that time. Now you came here,
> >> complaining about the code and you want to fix it without any interest
> >> of what others think about that. That's not cool.
> >>
> >>>
> >>>>  and on top of that you removed JspStateManagerImpl, which is a class
> >>>>  that is there for compatibility with old state managers that relies
> on
> >>>>  MyFaces 1.2.x.
> >>>
> >>>  This is not JSF-1.x since a long time. APIs in the StateManager got
> changed
> >> for AJAX support, etc.
> >>>
> >>>  But I'm all fine with going back to the JspStateManagerImpl again as I
> >> already said.
> >>>
> >>>  Btw, may I quote your answer to my question if I can delete it?
> >>>  LU> "Remove that code is a valid option."
> >>>
> >>
> >> The concrete case is spring webflow does not support PSS algorithm, so
> >> we need to switch back and enable compatibility mode with JSF 1.2.
> >> JspStateManagerImpl is there because the state manager in that library
> >> relies on the old behavior. I think there are other cases too. But I
> hope to
> >> remove it in the future (JSF 2.2).
> >>
> >> regards,
> >>
> >> Leonardo Uribe
> >>
> >>>  LieGrue,
> >>>  strub
> >>>
> >>>
> >>>
> >>>
> >>>  ----- Original Message -----
> >>>>  From: Leonardo Uribe <lu4242@gmail.com>
> >>>>  To: MyFaces Development <dev@myfaces.apache.org>; Mark Struberg
> >> <struberg@yahoo.de>
> >>>>  Cc:
> >>>>  Sent: Thursday, November 15, 2012 10:20 PM
> >>>>  Subject: Re: StateSaving review
> >>>>
> >>>>  Hi
> >>>>
> >>>>  2012/11/15 Mark Struberg <struberg@yahoo.de>:
> >>>>>   Leo, the stuff which currently got changed is nothing more than
> >> just your
> >>>>  code. I only refactored it into an own package and moved the inner
> >> classes to
> >>>>  toplevel to show up the complexity of the solution. There was no code
> >> change
> >>>>  _yet_ other than the viewId. We can change back the viewId to the
> hash
> >> if you
> >>>>  feel better. It makes no difference anyway.
> >>>>>
> >>>>
> >>>>  You did more than that. You removed the following classes:
> >>>>
> >>>>  IntIntSerializedViewKey
> >>>>  IntByteArraySerializedViewKey
> >>>>  ReferenceSerializedViewKey
> >>>>
> >>>>  also you created this one (it was an abstract class before)
> >>>>
> >>>>  SerializedViewKey
> >>>>
> >>>>  and on top of that you removed JspStateManagerImpl, which is a class
> >>>>  that is there for compatibility with old state managers that relies
> on
> >>>>  MyFaces 1.2.x. Obviously you have not considered those cases, but I
> >> have
> >>>>  done that long time ago.
> >>>>
> >>>>  Without ask, without propose a patch, without let the people who care
> >> about
> >>>>  to participate. Just do it and let others do the clean up.
> >>>>
> >>>>  Mark, I have came here and proposed to you many possible alternatives
> >>>>  and you're letting me without choices. I beg you to move your
> >> changes to
> >>>>  2.1.x-client-window branch, and when we have certain about which
> >> changes
> >>>>  needs to be done, backport them to 2.1.x. In just one step, without
> >> this
> >>>>  pain and stress you are causing me. Please. The reason why it was
> >> created
> >>>>  2.1.x-client-window was precisely to try changes for client window
/
> >> state
> >>>>  manager and flash scope.
> >>>>
> >>>>  In this moment I can't do any release. You have tied my hands.
> >> I'm
> >>>>  trying to be
> >>>>  kind, please.
> >>>>
> >>>>>   In the long run this stuff needs _heavy_ cleanup.
> >>>>>   Again: Extracting out an abstraction over Server vs Client
> >> stateSaving is
> >>>>  perfectly fine, but the code is now way more complicated than it ever
> >> has been.
> >>>>  Premature abstraction is way worse than premature optimisation.
> >>>>>
> >>>>
> >>>>  It is a work in progress, I already told you many times. I know the
> >> abstraction
> >>>>  is not the best, but it works, and has helped a lot to me to
> understand
> >> how
> >>>>  this algorithm should work. It is like a worm that became chrysalis
> and
> >> in the
> >>>>  future will become a butterfly. "... the morphing step is not nice
> >>>>  ..."
> >>>>
> >>>>  regards,
> >>>>
> >>>>  Leonardo
> >>>>
> >>>>>   LieGrue,
> >>>>>   strub
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>   ----- Original Message -----
> >>>>>>   From: Leonardo Uribe <lu4242@gmail.com>
> >>>>>>   To: MyFaces Development <dev@myfaces.apache.org>; Mark
> >> Struberg
> >>>>  <struberg@yahoo.de>
> >>>>>>   Cc:
> >>>>>>   Sent: Thursday, November 15, 2012 9:30 PM
> >>>>>>   Subject: Re: StateSaving review
> >>>>>>
> >>>>>>   Hi
> >>>>>>
> >>>>>>   2012/11/15 Mark Struberg <struberg@yahoo.de>:
> >>>>>>>    Leo, I'm ok with a revert. But not to your version
> >> but to the
> >>>>  original
> >>>>>>   JspStateManagerImpl!
> >>>>>>>
> >>>>>>>    Really, this currently feels so overcomplicated without
> >> giving
> >>>>  much
> >>>>>>   benefit. It would have been perfectly enough to remove the
> >> viewId
> >>>>  String and
> >>>>>>   replace it with a XORShift random value if you don't trust
> >> the
> >>>>  sequencer.
> >>>>>>>
> >>>>>>>    I know one goal was to abstract out the state between
> >> client and
> >>>>  server.
> >>>>>>   But that doesn't mean that we end up with Object, Object
> >> and do
> >>>>  hardcoded
> >>>>>>   upcasts as done right now in the code. This is no improvement
> >> over the
> >>>>  original
> >>>>>>   code. The windowId change would have been 30 LOC btw.
> >>>>>>>
> >>>>>>
> >>>>>>   It is fine if you want to change the implementation to make
> >> something
> >>>>>>   better. What's not fine, is that you want to work in 2.1.x
> >> branch
> >>>>>>   directly, that is right now in maintenance stage. So, you
are
> >> doing
> >>>>>>   all kind of refactorings, and that makes me crazy. The last
> >> release
> >>>>>>   was done on 23/Sep/12 and I'm doing 1 release each 2
> >> months. Your
> >>>>>>   changes will take longer and I need time to review them,
> >> because I
> >>>>>>   will not do a release over changes that I have not seen.
> >>>>>>
> >>>>>>   My idea is start a release process next week. Why don't
> >> you take
> >>>>  your
> >>>>>>   time?, instead you try to do changes without any clear
> >> direction.
> >>>>>>
> >>>>>>   In this moments, we are starting the necessary steps to work
> >> on 2.2.x.
> >>>>>>   You should be working in 2.1.x-client-window branch !!!!!
> >>>>>>
> >>>>>>   regards,
> >>>>>>
> >>>>>>   Leonardo Uribe
> >>>>>>
> >>>>>>>    LieGrue,
> >>>>>>>    strub
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>    ----- Original Message -----
> >>>>>>>>    From: Leonardo Uribe <lu4242@gmail.com>
> >>>>>>>>    To: MyFaces Development
> >> <dev@myfaces.apache.org>; Mark
> >>>>  Struberg
> >>>>>>   <struberg@yahoo.de>
> >>>>>>>>    Cc:
> >>>>>>>>    Sent: Thursday, November 15, 2012 8:44 PM
> >>>>>>>>    Subject: Re: StateSaving review
> >>>>>>>>
> >>>>>>>>    Hi
> >>>>>>>>
> >>>>>>>>    2012/11/15 Mark Struberg <struberg@yahoo.de>:
> >>>>>>>>>     Btw, I still do not see where the trick for
> >> storing a
> >>>>  separate
> >>>>>>   state list
> >>>>>>>>    for each browser tab is hidden.
> >>>>>>>>>     That was the goal of all the refactoring
> >> initially. Where
> >>>>  is that
> >>>>>>   done?
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>>    Mark, months ago we created 2.1.x-client-window
> >> branch:
> >>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> http://svn.apache.org/repos/asf/myfaces/core/branches/2.1.x-client-window/
> >>>>>>>>
> >>>>>>>>    This branch is the same as 2.1.x, but only with the
> >> changes
> >>>>  proposed
> >>>>>>>>    long time ago for JSF 2.2 client window.
> >>>>>>>>
> >>>>>>>>    It has what you need to have a state list for each
> >> browser
> >>>>  tab.
> >>>>>>>>    The only thing you need to provide is a ClientWindow
> >>>>  implementation
> >>>>>>>>    and that's it. The state saving code already has
> >> included
> >>>>  the
> >>>>>>   client-window
> >>>>>>>>    concept. I also fixed Flash scope to use client
> >> window.
> >>>>>>>>    It is also available in a maver repo:
> >>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>
> https://repository.apache.org/content/repositories/snapshots/org/apache/myfaces/core/myfaces-bundle/
> >>>>>>>>
> >>>>>>>>    Let's make a deal. Move your changes to that
> >> branch, let
> >>>>  me apply
> >>>>>>   my fix
> >>>>>>>>    in trunk and do a release. When your changes are
> >> ready, we can
> >>>>  discuss
> >>>>>>>>    them and backport from 2.1.x-client-window to trunk.
> >> Does that
> >>>>  sound
> >>>>>>>>    good for you?
> >>>>>>>>
> >>>>>>>>    regards,
> >>>>>>>>
> >>>>>>>>    Leonardo Uribe
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>>     LieGrue,
> >>>>>>>>>     strub
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>     ----- Original Message -----
> >>>>>>>>>>     From: Mark Struberg
> >> <struberg@yahoo.de>
> >>>>>>>>>>     To: MyFaces Development
> >>>>  <dev@myfaces.apache.org>
> >>>>>>>>>>     Cc:
> >>>>>>>>>>     Sent: Thursday, November 15, 2012 6:32 PM
> >>>>>>>>>>     Subject: Re: StateSaving review
> >>>>>>>>>>
> >>>>>>>>>>     I did further reviews:
> >>>>>>>>>>
> >>>>>>>>>>     Currently the CounterKeyFactory needs some
> >> random to
> >>>>  be unique
> >>>>>>>>    (according to
> >>>>>>>>>>     Leo) and the RandomKeyFactory needs a
> >> counter to be
> >>>>  unique.
> >>>>>>>>>>
> >>>>>>>>>>     Does that ring a bell? That stuff is
> >> completely
> >>>>  useless!
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>     Foooooolks wake up, let's provide ONE
> >> factory
> >>>>  which is
> >>>>>>   waterproof.
> >>>>>>>>    That
> >>>>>>>>>>     would be much better than having 15++
> >> classes full of
> >>>>  half
> >>>>>>   baken/broken
> >>>>>>>>>>     algorithms.
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>     Proposal: Get rid of all that KeyFactory
> >> stuff again
> >>>>  and have
> >>>>>>   one GOOD
> >>>>>>>>>>     algorithm: Counter + XORShift Random.
> >>>>>>>>>>
> >>>>>>>>>>     The KeyFactory stuff got only introduced
in
> >> 2.1.9 and
> >>>>  is
> >>>>>>   crashing in
> >>>>>>>>    production.
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>     LieGrue,
> >>>>>>>>>>     strub
> >>>>>>>>>>
> >>>>>>>>>>     PS: don't take it personal, but from
> >> looking at
> >>>>  the code I
> >>>>>>   see
> >>>>>>>>    clearly what
> >>>>>>>>>>     happens if someone works on a stuff alone
> >> without
> >>>>  reviews.
> >>>>>>   This always
> >>>>>>>>    leads to
> >>>>>>>>>>     deficits, regardless how good one is.
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>     ----- Original Message -----
> >>>>>>>>>>>      From: Leonardo Uribe
> >> <lu4242@gmail.com>
> >>>>>>>>>>>      To: MyFaces Development
> >>>>  <dev@myfaces.apache.org>;
> >>>>>>   Mark
> >>>>>>>>    Struberg
> >>>>>>>>>>     <struberg@yahoo.de>
> >>>>>>>>>>>      Cc:
> >>>>>>>>>>>      Sent: Thursday, November 15, 2012 6:20
> >> PM
> >>>>>>>>>>>      Subject: Re: StateSaving review
> >>>>>>>>>>>
> >>>>>>>>>>>      Hi
> >>>>>>>>>>>
> >>>>>>>>>>>      2012/11/15 Mark Struberg
> >>>>  <struberg@yahoo.de>:
> >>>>>>>>>>>>       I'm currently reviewing all
> >> this area.
> >>>>  It seems
> >>>>>>   that we
> >>>>>>>>    have quite
> >>>>>>>>>>     some
> >>>>>>>>>>>      stuff to improve.
> >>>>>>>>>>>>
> >>>>>>>>>>>>       a.) just a gut feeling yet, but
my
> >> tummy
> >>>>  tells me
> >>>>>>   that we
> >>>>>>>>    have to
> >>>>>>>>>>     review
> >>>>>>>>>>>      our key generator/storage strategies.
> >> Too
> >>>>  complicated or
> >>>>>>   too badly
> >>>>>>>>>>     documented.
> >>>>>>>>>>>      At least they are not self describing.
> >>>>>>>>>>>
> >>>>>>>>>>>      It's a complex algorithm. No
> >> documentation
> >>>>  so far,
> >>>>>>   because it
> >>>>>>>>    is still
> >>>>>>>>>>>      work in progress.
> >>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>>       b.) candidate 1:
> >> CounterKeyFactory. If we
> >>>>  like to
> >>>>>>   prevent
> >>>>>>>>    reboot
> >>>>>>>>>>     clashes
> >>>>>>>>>>>      then we might add another int which
> >> contains a
> >>>>  random
> >>>>>>   value. Think
> >>>>>>>>    about
> >>>>>>>>>>     having
> >>>>>>>>>>>      a Server with a single page right now.
> >> Click on
> >>>>  it a few
> >>>>>>   times,
> >>>>>>>>    then
> >>>>>>>>>>     restart
> >>>>>>>>>>>      myfaces and issue a few requests to
the
> >> same
> >>>>  page and go
> >>>>>>   back in
> >>>>>>>>    your
> >>>>>>>>>>     browser
> >>>>>>>>>>>      history. Proposal: instead of the
> >> viewId we
> >>>>  should add a
> >>>>>>   random
> >>>>>>>>    number.
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>      Since the counter is also stored into
> >> session,
> >>>>  the last
> >>>>>>   counter
> >>>>>>>>    values
> >>>>>>>>>>>      is not lost.
> >>>>>>>>>>>
> >>>>>>>>>>>>       c.) a general one. We might
> >> introduce an
> >>>>  own Random
> >>>>>>   which
> >>>>>>>>    either uses
> >>>>>>>>>>>      java.util.concurrent.ThreadLocalRandom
> >> for java
> >>>>  7++ or
> >>>>>>   the old
> >>>>>>>>    Random impl.
> >>>>>>>>>>>>        ThreadLocalRandom has a _much_
> >> better
> >>>>  performance
> >>>>>>   on
> >>>>>>>>    servers! Or we
> >>>>>>>>>>     just
> >>>>>>>>>>>      use a simple XORShift which is surely
> >> good
> >>>>  enough for
> >>>>>>   most cases
> >>>>>>>>    and
> >>>>>>>>>>     performs
> >>>>>>>>>>>      like hell. The spreading of XORShift
is
> >> better
> >>>>  than the
> >>>>>>   standard
> >>>>>>>>    Java
> >>>>>>>>>>     algorithm
> >>>>>>>>>>>      even ...
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>      It could work.
> >>>>>>>>>>>
> >>>>>>>>>>>>       d.) KeyFactory looks a bit
> >> overengineered.
> >>>>  The
> >>>>>>   return type is
> >>>>>>>>    either
> >>>>>>>>>>>      Integer or byte [] but the encoded
> >> value is
> >>>>  always
> >>>>>>   represented as
> >>>>>>>>    String.
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>      The key is encrypted and base64
> >> encoded, but
> >>>>  that's
> >>>>>>   done in
> >>>>>>>>>>>
> >> HtmlResponseStateManager.writeViewStateField.
> >>>>  The change
> >>>>>>   only has
> >>>>>>>>>>>      sense if we move the responsibility
of
> >> encrypt
> >>>>  to
> >>>>>>>>>>>
> >>>>  ServerSideStateCacheImpl/ClientSideStateCacheImpl. My
> >>>>>>   opinion is
> >>>>>>>>    it is
> >>>>>>>>>>>      better to keep it as is.
> >>>>>>>>>>>
> >>>>>>>>>>>>       e.) Instead of trashing the
> >> Session with
> >>>>>>   setAttribute and
> >>>>>>>>    synchronized
> >>>>>>>>>>
> >>>>>>>>>>>      blocks we should rather store an
> >> AtomicInteger.
> >>>>  This is
> >>>>>>   perfectly
> >>>>>>>>    fine now
> >>>>>>>>>>     as we
> >>>>>>>>>>>      do not support java 1.4 any longer,
> >> right?
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>      The synchronized blocks are over the
> >>>>>>   SerializedViewCollection,
> >>>>>>>>    which
> >>>>>>>>>>>      is unique per session, so it does not
> >> suppose
> >>>>  any
> >>>>>>   overhead (tested
> >>>>>>>>>>>      with YourKit profiler). But performance
> >> tests
> >>>>  shows that
> >>>>>>   the
> >>>>>>>>>>>      additional calls to session map impose
> >> an small
> >>>>  overhead
> >>>>>>   (the
> >>>>>>>>    ideal is
> >>>>>>>>>>>      minimize those calls). Since the
> >> counter is
> >>>>  stored per
> >>>>>>   session,
> >>>>>>>>    there
> >>>>>>>>>>>      is no chance of key clashing. One
> >> option could
> >>>>  be
> >>>>>>   something like
> >>>>>>>>    the
> >>>>>>>>>>>      trick used in Flash Scope. An
> >> AtomicLong from a
> >>>>  random
> >>>>>>   seed, but
> >>>>>>>>>>>      anyway it is necessary to change the
> >> algorithm
> >>>>  for check
> >>>>>>   if there
> >>>>>>>>    is a
> >>>>>>>>>>>      key clashing, generates another key.
> >>>>>>>>>>>
> >>>>>>>>>>>>       Just a few random ideas...
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>      MS>> another one:
> >>>>>>>>>>>      MS>> All that stuff has nothing
> >> to do with
> >>>>  the
> >>>>>>   RenderKit and
> >>>>>>>>    should
> >>>>>>>>>>     go
> >>>>>>>>>>>      into
> >> org.apache.myfaces.application.viewstate.
> >>>>>>>>>>>      MS>> wdyt?
> >>>>>>>>>>>
> >>>>>>>>>>>      That stuff is used by
> >>>>>>>>>>>
> >>>>>>   org.apache.myfaces.renderkit.html.HtmlResponseStateManager,
so
> >> in
> >>>>>>>>>>>      theory is related to the renderkit
> >> (because
> >>>>>>   ResponseStateManager
> >>>>>>>>    is
> >>>>>>>>>>>      renderkit dependant), but it is an
> >> application
> >>>>  scope
> >>>>>>   concept.
> >>>>>>>>    Maybe
> >>>>>>>>>>>
> >>>>  org.apache.myfaces.renderkit.application.viewstate. has
> >>>>>>   more sense
> >>>>>>>>    to
> >>>>>>>>>>>      me.
> >>>>>>>>>>>
> >>>>>>>>>>>      MS>> Also the following classes
> >> are
> >>>>  related and
> >>>>>>   should imo
> >>>>>>>>    get moved
> >>>>>>>>>>>      to this new package:
> >>>>>>>>>>>      MS>> * StateCache
> >>>>>>>>>>>      MS>> * StateCacheFactory
> >>>>>>>>>>>
> >>>>>>>>>>>      Yes, a better name
> >>>>>>>>>>>
> >>>>>>>>>>>      MS>> * StateManagerImpl
> >>>>>>>>>>>
> >>>>>>>>>>>      No, StateManagerImpl deals with the
> >> logic
> >>>>  related to
> >>>>>>   calculate the
> >>>>>>>>>>>      state to a view, StateCache deals with
> >> the
> >>>>  storage of the
> >>>>>>   view
> >>>>>>>>    (maybe
> >>>>>>>>>>>      a better name is StateStorage, for
> >> example
> >>>>>>>>>>>
> >>>>  ServerSideStateStorageImpl/ClientSideStateStorageImpl?
> >>>>>>>>>>>
> >>>>>>>>>>>      regards,
> >>>>>>>>>>>
> >>>>>>>>>>>      Leonardo
> >>>>>>>>>>>
> >>>>>>>>>>>>       LieGrue,
> >>>>>>>>>>>>       strub
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>
>



-- 
Grant Smith - V.P. Information Technology
Marathon Computer Systems, LLC.

Mime
View raw message