Leonardo Uribe píše v Po 20. 02. 2012 v 19:04 -0500: > Hi > Hi,
I did check only the original source code from blog some time ago and it I have found practically same problems as you. > I have been studying the proposed code attached to MYFACES-3465. > Unfortunately, I have found some issues with the related code that > needs to be discussed under dev list. The code has the following > drawbacks: > > - It uses request uri to mark the views instead viewId. This causes > when multiple POST are chained with stateless views, a > ViewExpiredException is thrown. It is known there's a difference about > how myfaces and mojarra deals with viewId, which were clarified in JSF > 2.1, with the introduction of ViewHandler.deriveLogicalViewId(). So > instead use request uri as the marker, a combination of > viewId/deriveLogicalViewId() should be used. yes, and user can also expect something what JSF already have, like j.f.FULL_STATE_SAVING_VIEW_IDS id web.xml - other context parameter. > > - Instead use vdl.buildView() call, it replaces it with > com.rits.cloning.Cloner, to duplicate the view and use it on the > current request. I think in this case, vdl.buildView() will be faster > and more optimal from memory perspective, because we can cache > information at Facelet/TagHandler/TagAttribute level like > ValueExpressions, attribute values and others, and a deep clone will > create unnecessary objects each time. Comparing MyFaces and Mojarra, > our implementation of vdl.buildView() is very fast, and for cases when > the view is has no dynamic parts it is only built once, so from > MyFaces perspective the is not visible difference here (but from > Mojarra you can see a difference). This means we don't really need the > cache proposed at all. Sometimes less is more. cleary this is resposibility of vdl.buildView - cloning is slower and is not smart enough to do some optimalizations. > > - Some synchronized blocks can slow down performance without need (by > thread contention), and I have found other bugs when the code deals > with high concurrency. There were static synchronized methods and the pool was static map or something like that. those constructs were not nice for sure and also far from ideal in multi classloader scenario/shared myfaces/osgi. Pool are generally good idea for heavyweight object like Db connection, but pool itself causes thread waiting when number of threads is greater as max size of pool. I did profiling of commons-pool, c3p0, atomikos JTA pool and some others and the result aren't nice ... not suitable for myfaces - just for information. > > Anyway, the trick is good enough, in cases where you don't want to > deal with view state load, even if with the changes done over the time > in that part until 2.1.6 have made view state very small. It is an > easy way to say to the state manager "... don't save the state for > this view ...". Or StateHandlingStrategy? I think stateManager in @deprecated in 2.2. > > After looking the code, I think we could add this as an built-in > specific feature for MyFaces Core, rather than a subproject. > > Anyway, I have an idea that we can do for enhance performance in a > significant way. You idea is simply great! I did some analysis of views two year ago and also some prototypes, but I leaved it , while 99% of ours view is stateful. I have found following groups of views: 1) singleton-per-webapp view, readOnly, non-actionable only one instance exists per web application and server all request. It is no possible to input values with EditableValueHolder and it is not possible fire event with ActionSources. This view is something like old JSP (but component oriented) and use cases are reports, overviews usw. There was possible to navigate to other views with plain link (outputLink). All state per view must be available within ValueExpression 2) Singleton-per-webapp, readOnly, actionable similar as 1), but it is possible to fire event with ActionSource. This was done with custom ViewRoot where all events go and threadLocal event store. Use cases were main menus, dashboards ... in such views are no EditableValueHolders and user only clicks on a menu item. 3) Singleton-per-webapp, editable, actionable same as 2, but EditableValueHolders were capable to update model but without conversion and validation - only String values in model were permitted (no localValue in EVH). these three are similar to stateless JSF and thread safe components together, but I did analysis of ours view and that would be an overkill to do it for 10 views from two thousands ;) stateless JSf tries to preserve editableHolders functionality but clear localValues in separate thread ... 4) stateful normal views - view with at least one component with own state, like component.setRendered(false) or setValid(false)- every post request with viewState must obtain own component tree with own state. > > In theory, we can classify JSF views into two groups > > - static view: views that as an invariant always has the same > structure and state once vdl.buildView and render code is called for > first time. > - dynamic views: views that has some parts that change according to > expressions (c:if ....) in vdl.buildView or has programatically added > components based on conditions when the render code is called for > first time. > > Also we can classify JSF views into another two groups > > - stateful: Requires to store some data on the view to work after > markInitialState is called. > - stateless: Does not require to store anything on the state, because > we can duplicate it safely just calling vdl.buildView(). > in stateful case, should we differentiate data request-scoped (threadlocal) like events and viewscoped, like component.setRendered(false) ? Regards, Kočičák > There is a lot of views that on the first request, or in other words, > when the view is created by first time and then rendered does not > change. The rendered page could change, but the view tree itself do > not. Later, on further postbacks, the view state might change for both > stateful and stateless views. > > Here is the trick, if by some way we can make the component tree > "thread safe" making it immutable and moving some temporal variables > (clientId, rowIndex, dataModel, ...) from the component to > FacesContext attribute map, we could use the same view across multiple > requests. Then, on a postback (normal submit or ajax), we can build > the view as usual. This would involve some changes over > UIComponentBase/UIData/UIRepeat internals, to switch between > thread-safe or immutable mode and normal mode, and we could expect > more calls over FacesContext.getCurrentInstance() because we can't > cache FacesContext at UIComponent level, but it will work. A fast > proof-of-concept test done shows a big improvement, because we can > skip vdl.buildView in some cases and reducing memory usage. Obviously > there will be a lot of component that will not support that mode. > Anyway, it will not be an easy trick. > > Suggestions are welcome. > > regards, > > Leonardo Uribe
