Hi Leo!

The main problem with this code was that 2 different viewId String might have 
the same hashCode. The Sun JVM is much better in this regard as other JVMs. But 
we e.g. saw lots of collisions in the IBM JVM and IcedTea.
Using the hashCode is perfectly fine for detection of non-equality, but not 
enough for equality check. And that's exactly what the code did. It stores the 
viewId plus the viewIdHash. 


If the viewIdHash is not equals -> we are sure the viewId is different
if the viewIdHash are the same -> we check if the viewId String is really the 
same.

If your main goal is to keep mem low, we also can get rid of the explicit 
viewIdHash in our code as String.hashCode() in the Sun JVM caches itself. This 
is not guaranteed by the langspec though afaik.
In any case it's faster to do the hashCode comparison upfront.

LieGrue,
strub




----- Original Message -----
> From: Leonardo Uribe <[email protected]>
> To: MyFaces Development <[email protected]>; Mark Struberg 
> <[email protected]>
> Cc: 
> Sent: Wednesday, November 14, 2012 12:59 AM
> Subject: Re: overusing of == ProjectStage.Production
> 
> Hi
> 
> Why do you think that the changes proposed has no reason behind? The
> change was not made for improve speed. Instead, the change was done to
> minimize the key size stored into session. There is always one key per
> view stored into session, so this part can quickly grow. I think the
> change has a lot of sense, and I have checked it using memory
> profilers.
> 
> The problem described in MYFACES-3638 can be solved just doing a null
> check before try to calculate the hash code. The inner classes are
> there and not outside the class, because only the state saving
> implementation needs to know that detail. I would like to keep the
> code in that way, even if it is a little bit verbose.
> 
> Please take a look at:
> 
> https://issues.apache.org/jira/browse/MYFACES-3563
> 
> You can see that the patches and the comments related to each design
> decision. If you want to propose something else, we can discuss it.
> 
> I would like to keep in sync 2.0.x and 2.1.x as much time as possible,
> so if we do some changes, the preferred way is provide a patch so we
> can apply it in both branches. I know it is something bothersome, but
> there are people who appreciate that.
> 
> Please reconsider the changes committed in MYFACES-3638.
> 
> regards,
> 
> Leonardo Uribe
> 
> 2012/11/13 Mark Struberg <[email protected]>:
>>  Hi folks!
>> 
>>  It is perfectly fine if we disable some debug and reload stuff if we are in 
> ProjectStage.Production.
>> 
>>  But imo we are currently switching around way too much functionality. E.g. 
> I see absolutely no reason for changing the caching key strategy dynamically. 
> That makes it almost impossible to get proper results and the performance 
> benefit is really negligible.
>> 
>>  Wdyt, should we revise all the usages of those cases?
>> 
>>  LieGrue,
>>  strub
>> 
> 

Reply via email to