> Really I prefer do the previous improvement and make vdl.buildView > faster and cheaper, than do a component tree thread safe, because the > later one has a lot more code and it is something really outside the > > spec. I think that > create a component tree is cheap, and in practice we should try to reduce > innecessary allocations of strings (like unique ids and others), and > try to make the component tree itself lighter. At the end, it will be more > effective to improve performance, and the side effects will be more limited. >
of course, 10 minor optimalizations = 1 major opt. and we can always find others 10 minor ;) (in every code) Leonardo Uribe píše v Út 21. 02. 2012 v 11:32 -0500: > Hi > > 2012/2/21 Martin Koci <[email protected]>: > > Leonardo Uribe píše v Po 20. 02. 2012 v 17:51 -0500: > >> Hi > >> > >> Unfortunately we can't cache facelets generated unique ids into > >> ComponentTagHandlerDelegate because a tag handler can be used in many > >> views because a view is the result of execute many "facelet > >> compositions". > >> > >> In facelets there are two "hierarchical counters", one for > >> MARK_CREATED (FaceletContext.generateUniqueId) and the other for > >> component unique ids > >> (FaceletCompositionContext.generateUniqueComponentId). In theory if > >> the view is "static", which means it generates the same component tree > >> each time, the list of generated ids will be the same. What we can do > >> is create a per-view cache, holding an ordered list with the generated > >> ids. In theory, when a dynamic part of a tree is about to begin, a new > >> level is started calling > >> FaceletCompositionContext.startComponentUniqueIdSection(), which > >> increase the level calling > >> SectionUniqueIdCounter.startUniqueIdSection() for both counters. So we > >> could add some logic into SectionUniqueIdCounter to store the values > >> of the counter on the base level and reuse it per view. It is not > >> going to be an easy trick but it seems reasonable, because it will > >> save a bunch of memory. > > Ahh, I see. The idea with an ordered list is very good, it is better > > store for ids. > > > > Semi-related question: Can you please take a look at MYFACES-3470, > > please? It can save the generateUniqueId now, but I'm not sure. > > > > The suggestion is feasible, but it only will work when > org.apache.myfaces.REFRESH_TRANSIENT_BUILD_ON_PSS > is set on "auto" mode. In this mode, the view is checked for dynamic > parts and if has one, it activates a flag for do refresh if the view does > not changes. It is also feasible to do not set those attributes if the > view is rendered for first time when PSS mode is active, because the > delta only contains the attributes that change, and on a later postback, > the attribute will be set if necessary. In practice, we are changing the > "initial state", so I need to check if our algorithm could "resist" that. > > Note what we are proposing here is something risky. In few words, we are > assuming facelets dynamic views will only be created using the known > methods (c:if, ui:include src="#{...}", ...). It is reasonable, because at > this > moment there is no evidence of somebody has some tag that changes the > view dinamically (adding or removing components) different to those ones. > > Additionally, there are some cases like when the view has metadata > (f:metadata, f:viewParam, ... ) that makes the view to be refreshed again > in that part, so the algorithm should be enabled there. > > >> > >> About @ResourceDependency annotations: The current code in > >> RequestViewContext check if the annotation was processed on the same > >> view, which means if the resource has been already added. That code is > >> ok. Note there is another cache that checks if the class has > >> @ResourceDependency annotations, to prevent scan it, but what we don't > >> have is one cache to store the classes that does not have any > >> @ResourceDependency and prevent any scanning at all. Which is more > >> expensive? a get() over a ConcurrentHashMap or a call to > >> inspectedClass.getAnnotation()? Only if the first is cheaper, the > >> cache has sense. > > > > This one was only a though what is cacheable in "static" views, current > > implementation is not a performance problem. > > > > Ok, so it is better let it as is. > > >> > >> Finally, I don't think we can do anything for clientId, because its > >> generation is more tied to UIComponentBase internals. If an UIData or > >> UIRepeat or any other iteration component is used, clientId changes > >> per row, and is reset using setId() call. I think we can pass the > >> clientId through component attribute map, using some special attribute > >> name, but it doesn't sound good to me. After all, clientId calculation > >> is responsibility of UIComponent and it could be changed on the > >> related Renderer. > > > > Yes, iteration components are problem. Generally many invocations of > > getCliendId() come from > > DefaultFaceletsStateManagementStrategy.restoreStateFromMap and thus is > > this memory allocation coming from getClientId related to state saving. > > But you have proposed solutions to state(ful, less) problem in email > > "discussion about stateless jsf implementation" and that can minimize > > String allocation in stateless/thread safe cases. > > > > Here we have a problem with the spec. I remember somewhere says that > clientId should be used as a key to store the state. > > Really I prefer do the previous improvement and make vdl.buildView faster > and cheaper, than do a component tree thread safe, because the later one > has a lot more code and it is something really outside the spec. I think that > create a component tree is cheap, and in practice we should try to reduce > innecessary allocations of strings (like unique ids and others), and > try to make the component tree itself lighter. At the end, it will be more > effective to improve performance, and the side effects will be more limited. > > regards, > > Leonardo Uribe > > > Regards, > > > > Kočičák > > > >> > >> regards, > >> > >> Leonardo Uribe > >> > >> 2012/2/20 Martin Koci <[email protected]>: > >> > Same situation for clientId: in "stable" component tree is clientId > >> > always the same (state saving depends on it) > >> > > >> > unfortunately has UIComponent no method setClientId. But we can try set > >> > it via reflexion and compare if it brings an improvement or not. > >> > Currently getClientId() uses lazy init and involves: > >> > > >> > findParentNamingContainer > >> > namingContainer.getContainerClientId > >> > stringBuilder appends > >> > renderer.convertClientId > >> > > >> > > >> > > >> > Martin Koci píše v Pá 17. 02. 2012 v 18:56 +0100: > >> >> Hi, > >> >> > >> >> in situation, where no build-time tags (c:if, co:foreach, ...) and no > >> >> ui:include src="#{}" are used (and no direct component.getChildren() > >> >> manipulation of course), builds VDL.buildView every time the same > >> >> component structure (same graph). > >> >> > >> >> In this case compute myfaces some informations again and again but every > >> >> time with same results. For example, creating of unique ids is very > >> >> expensive: > >> >> > >> >> FaceletContext.generateUniqueId > >> >> FaceletCompositionContext.generateUniqueComponentId > >> >> UniqueIdVendor.createUniqueId > >> >> > >> >> in ComponentTagHandlerDelegate.apply(FaceletContext, UIComponent) > >> >> > >> >> Can we cache this ids at ComponentTagHandlerDelegate in production in > >> >> case of view without build time modifications ? This is similar to [1]. > >> >> > >> >> Same situation with @ResourceDependency annotations: ApplicationImpl. > >> >> _handleAttachedResourceDependencyAnnotations computes the same result > >> >> with every request/response. > >> >> > >> >> Others ideas what can be cached? > >> >> > >> >> Regards, > >> >> > >> >> Kočičák > >> >> > >> >> > >> >> [1] https://issues.apache.org/jira/browse/MYFACES-3160 > >> >> > >> >> > >> >> > >> >> > >> >> > >> >> > >> >> > >> >> > >> > > >> > > > > >
