Community
Participate
Working Groups
Tested on Eclipse 3.2, WTP 1.5.1 + Prerequisites + JSF 1.5.1. Verified the issue still exists with Eclipse 3.2.1 and WTP 1.5.2 Import the project I'll attach, clean it, and watch the progress view and task manager. Validation pegs the CPU and takes several minutes to finish validating the single JSP page in the project (which is about 1000 lines long). Also, after the validation finishes or you cancel it, the process is not actually cancelled and still pegs the CPU for a long time (but the progress view no longer shows it running).
Created attachment 53512 [details] File 'jsf1.zip' containing a WTP JSF project that shows the problem Unzip the attachement, jsf1.zip, and import it into a WTP / JSF installation, then add the second attachement, myfaces-all.jar, to its WebContent/META-INF/lib folder
Created attachment 53513 [details] File 'myfaces-all.jar' After importing attachment #1 [details], add this jar file to WebContent/META-INF/lib and clean the project to begin validation to show the issue. This was done a separate attachement due to the file sizes of the project and this library.
DIAGNOSTIC: This isn't entirely surprising, as the validation framework has never been optimized or even measured for performance. I will conduct some profiling soon. A few things that might be helpful if anyone wants to contribute some time: 1) Remove any f:loadBundle variables and see if performance improves dramatically more than proportional to the number of EL expressions are now invalid. 2) See how much turning off EL validation helps: Preferences->Web&XML->JSF Validation.
Some progress has been here. f:loadbundle property file loading has been greatly streamlined. On my PC using adhoc testing I was finding that each resource bundle was taking as much as 300ms to find and load (primarily because we were using JDT Search which is horrifyingly inefficient for this sort of thing). The method has reduced to the overhead to less than 1ms in my tests.
Improvements will be available in M6.
Even using M6 the jsp semantic validator (jsf) takes far too long to validate. I did some investigation: For my current project (a rather small real-world project without f:loadbundle and with many jsp includes) semantic validation takes 23 minutes (all other validators about 0.5 min.). The key factor seems to be resolving of bean property symbols: Most of the time is spent in JDTBeanIntrospector.getProperies(+methods called from there) (about 0.2s per call!) and in TypeUtil.resolveTypeRelative (about 0.01s per call, called quite often). If I cache property symbols / method symbols per owning type, validation time goes down to 2.5 minutes. If I also cache supertypes (which I must cache anyway in order to react to changes) and interfaces which are used to determine type compatibility, validation of the project takes about 1 minute.
Created attachment 63448 [details] patch implementing caching of property symbols/supertypes per type This patch implements a cache (TypeInfoCache) that can cache per IType: * all bean property symbols * all bean method symbols * all supertypes * all implemented interfaces, and it makes IJavaTypeDescriptor2Impl use this cache. This dramatically improves the performance of the jsp semantic validator (and of symbol resolving in general). TypeInfoCache listens to changes in the java model and invalidates all affected information when necessary. The only user-visible difference to current (uncached) behaviour is that changes in java source will update cached information not until the source file is saved. The logic that reacts on changes in the java model is quite similar to the one in jdt's TypeHierarchy (originally copied from there). TypeInfoCache handles added/changed/removed source files and class files by only invalidating the affected types. In all other cases it just throws away all cached information for all types. TypeInfoCache is a bit more complicated than I would have liked it, because it has to handle creation of new java types efficiently (i.e. not by throwing away everything) since jsp validation creates new java types all the time.
Thanks Matthias! This looks great and has been badly needed for some time. I will review this night. We'll try and get this in for RC1 if there aren't any issues.
Created attachment 64528 [details] caching of property symbols/supertypes per type, updated updated to follow "internal.provisional" refactoring and to use java5 generics. Fixed a "hidden catch block" problem (strange I have not noticed earlier...).
I would consider this change medium to high risk at this time of development cycle. It is rather global for jsf and critically depends on the update logic working correctly (otherwise it can cache wrong information for a very long time). There has been very little testing in presence of more than 1 open jsf project, and concerning "strange" classpath changes (adding new jars/classpath containers/...). On the other hand, without it validation is close to unusable for real projects, and it did not show problems on my own project so far.
Hi Matthias, Because of the size of the contribution, we have kicked off the IP review process. That is the first step for us to accept most contributions of this size and type. We are trying to get this change in for this release.
I have integrated to HEAD patch 64528 except for a small change to JDTBeanProperty due to a lot of change in that area from Java 5 language support being added. This and other performance optimizations will be added later as we increase our performance test activities. A simple performance test has also been added to org.eclipe.jst.jsf.validaton.el (available in CVS, not yet in the test suite) to track the current performance of affect features. Below are the results from before the patch: =================================================== Report for performance test: Stress Variable Resolver Number of iterations: 100000 =================================================== Max: 3,043,324,501, Max Index: 0 Min: 159,238, Min Index: 42,139 Avg: 258,328, StdDev: 9,648,846.939, StdDev Ignore Max/Min: 705,499.692 =================================================== =================================================== Report for performance test: Stress Property Resolver Number of iterations: 1000 =================================================== Max: 947,628,819, Max Index: 141 Min: 215,617,907, Min Index: 389 Avg: 242,454,079, StdDev: 49,118,889.039, StdDev Ignore Max/Min: 43,756,984.201 =================================================== =================================================== Report for performance test: Stress Simple Bean Property Validation Number of iterations: 1000 =================================================== Max: 1,562,307,500, Max Index: 719 Min: 460,512,719, Min Index: 303 Avg: 687,557,702, StdDev: 256,462,089.68, StdDev Ignore Max/Min: 254,864,801.041 =================================================== and here is a sample run with the patch: =================================================== Report for performance test: Stress Variable Resolver Number of iterations: 100000 =================================================== Max: 2,812,605,233, Max Index: 0 Min: 156,444, Min Index: 77,049 Avg: 257,688, StdDev: 8,915,112.194, StdDev Ignore Max/Min: 621,480.648 =================================================== =================================================== Report for performance test: Stress Property Resolver Number of iterations: 1000 =================================================== Max: 841,487,117, Max Index: 0 Min: 21,512, Min Index: 878 Avg: 900,743, StdDev: 26,596,172.057, StdDev Ignore Max/Min: 877,577.866 =================================================== =================================================== Report for performance test: Stress Simple Bean Property Validation Number of iterations: 1000 =================================================== Max: 1,488,545,890, Max Index: 0 Min: 468,216, Min Index: 799 Avg: 2,275,518, StdDev: 47,030,733.24, StdDev Ignore Max/Min: 1,699,112.729 =================================================== All values are nanoseconds per test executions using the System.nanoTime() call on the JDK1.5_06 platform, running in standard JUnit test plugin mode (non-headless). As you can see, worst case, average case, variability and worst case predictability have all improved substantially. Special thanks to Matthias Fuessel for his work to make this improvement possible. Change checked into HEAD targeted for WTP2.0 RC0 build.
An additional increase of performance is to add "getProperties(Set propertyIds)" and "getMethods(Set methodIds)" to JDTBeanIntrospector, ITypeDescriptor and IJavaTypeDescriptor2Impl and use these from DefaultDTMethodResolver and DefaultDTPropertyResolver instead of retrieving and instantiating ALL methods/properties on every inspection. That increased the validation time of my project from > 10 min to < 30 sec.
Can you provide a patch to demonstrate what you are proposing? (In reply to comment #13) > An additional increase of performance is to add "getProperties(Set > propertyIds)" and "getMethods(Set methodIds)" to JDTBeanIntrospector, > ITypeDescriptor and IJavaTypeDescriptor2Impl and use these from > DefaultDTMethodResolver and DefaultDTPropertyResolver instead of retrieving and > instantiating ALL methods/properties on every inspection. > That increased the validation time of my project from > 10 min to < 30 sec. >
Created attachment 69490 [details] Patch to use resolve only properties and methods really needed on evaluation (In reply to comment #14) > Can you provide a patch to demonstrate what you are proposing? For sure. See the new attachment. I had to patch v20070206 (WTP 1.5.3?) as I use MyEclipse 5.5. I don't really know which WTP version they use, as the call the plugin files org.eclipse.jst.jsf.common_1.0.0.zmyeclipse55020070514.jar, but the files I patched contained the same code before. So I don't know if your current version creates conflicts with that patches.
> For sure. See the new attachment. Thanks for the patch. I'm not able to find a patch point yet (this is against a WTP 1.5.x build) so I haven't applied it, but based on review of the code I have a few questions: 1) It appears you are optimizing the property/method lookup by creating a Set to cache the intermediate values? I see where you're passing that into the type descriptor, but I don't see where you are store the cached set. It appears as though it is being re-calculated everytime because null gets passed as the set. 2) How does this solution invalidate the cache when the underlying Java type changes? For example, if you have a bean called "Foo" cached in your Set and the programmer comes along and adds a new method called "getDates()" to Foo and re-compiles. How do we ensure that the set gets updated for these types of changes?
Created attachment 69580 [details] Applied patch (In reply to comment #16) > Thanks for the patch. I'm not able to find a patch point yet (this is against > a WTP 1.5.x build) so I haven't applied it, The code point is tag v20070206 on dev.eclipse.org/cvsroot/webtools (module org.eclipse.jsf) - are you using a different one? I attached a set of the source files that I have changed (relevant positions marked with CHANGED). > but based on review of the code I > have a few questions: > > 1) It appears you are optimizing the property/method lookup by creating a Set > to cache the intermediate values? I see where you're passing that into the > type descriptor, but I don't see where you are store the cached set. It > appears as though it is being re-calculated everytime because null gets passed > as the set. > I don't cache any values, I just "pass through" the information I have: the property/method name(s) I want to check - to resolve only those and not all the others, as the "resolving" is the time consuming task. The old "get all" methods are emulated by passing null as set of property/method names. (I think that makes question 2 obsolete).
Ah, I see. So rather than caching you are seeking to generate properties for only those ids we really need. This is a nice approach, particularly for its simplicity, however I think the type cache we have already integrated should produce equivalent or better results since it not only avoids redundant lookups on unneeded ids but also redundant lookups on ids we are interested in (unless a change invalidates it). I see value in improving the API on JDTBeanIntrospector to allow targeted lookups however. Time permitting I will integrate that part of the change for this release. Note that the type cache is integerated in WTP 2.0 only: set for release end of June.
Thinking about it, we may be able to optimize the type cache further by allowing it to selectively update it's cache based only those ids needed. This favors a validate-heavy usage pattern where we have a priori knowledge of what is needed. In the content-assist usage pattern it won't improve performance since we need to look for everything. However, I see where it could improve our worst-case numbers. If the user rarely uses content assist.
(In reply to comment #18) > Ah, I see. So rather than caching you are seeking to generate properties for > only those ids we really need. This is a nice approach, particularly for its > simplicity, however I think the type cache we have already integrated should > produce equivalent or better results since it not only avoids redundant lookups > on unneeded ids but also redundant lookups on ids we are interested in (unless > a change invalidates it). I didn't looked into your new cache implementation, but I noticed that already the resolving of all methods/properties for only one type took, in my version, about 10 seconds. So even if the cache gets only invalidated on every class change, it would take that 10 seconds to reevaluat the whole type. > > I see value in improving the API on JDTBeanIntrospector to allow targeted > lookups however. Time permitting I will integrate that part of the change for > this release. > That would decrease the 10 seconds. > > Note that the type cache is integerated in WTP 2.0 only: set for release end of > June. > As I don't know when this will be integrated into MyEclipse I stay with my patch for that while. I noticed that someone from genuitec is watching, I hope that at least that patch is integrated soon as it doesn't generate mutch risk.
My initial profiling when looking into validation performance indicated that when resolving properties/methods building the jdt ITypeHierarchy / doing the jdt lookup was the dominant problem (about 0.3s to 0.5s per call). In this case caching property/method symbols as it is done now is the best option, because in the long run most of the methods/properties will need to be resolved anyway, and every single uncached query needs to build that type hierarchy/do jdt lookup. That does not explain the drastic performance improvement Zacharias gets by that patch. I don't understand that right now. 10 seconds for looking up methods of only one type seems *way* too much time even for the 'old' implementation. Zacharias, do you use anything special in your application? How many properties, how many other public methods do you have roughly per backing bean? Do you use deep hierarchies in backing beans? My test applications only use a few properties/methods per bean and a rather flat hierarchy (which I think both is typical for backing beans, but maybe I'm wrong here). Are there any other performance changes since WTP 1.5 that would explain that? Using caching for only subsets of properties per type seems not very promising to me. It adds complexity, and in the long run most properties will need to be resolved anyway (unless you have backing beans with *much* code that is not called from web pages). Unless creating symbols from jdt objects turns out to be *very* costly, I would like to avoid this. The new API for looking up single named properties/methods I think is a useful thing, though: It's a typical usecase, it seems logical to have a function for that, and it will gain some performance improvement in the cached case (avoid creating list/map from array, only process half of the array on average). In the uncached case, I still think it's dominated by the jdt lookup. If nobody else does, I will look into this again and do some profiling (especially of uncached worst case scenario), but I will not have time for this until end of next week, it is not my top priority and it will definitely be too late for WTP 2.0.
(In reply to comment #21) > Zacharias, do you use anything special in your application? How > many properties, how many other public methods do you have roughly per backing > bean? Do you use deep hierarchies in backing beans? My test applications only > use a few properties/methods per bean and a rather flat hierarchy (which I > think both is typical for backing beans, but maybe I'm wrong here). > Thats the problem, I don't work on a test application :-) Your assumption "most of the methods/properties will need to be resolved anyway" is wrong in two ways: 1) an expression can contain more than the backing bean e.g. the one used here: rendered="#{bean.values.size > 0}" If values is something from the Collection API it may contain ~40 methods (including private ones, which are resolved, too) and we only need to test for one: getSize() (I know that the Collection interface does only define a method size(), but this is just an example). 2) in our application even the managed beans contain many methods not used in an expression (e.g. private ones used for retrieving the provided data, or the ones used for initialization of the bean). I know that this is not how the example applications look like, but hey, is eclipse only written for testing purposes?
Some excellent points have been made. In the short run, the most important thing I think is to get a set of benchmark JUnit cases together that give us a basis for comparative analysis of different approaches. If we can check these into CVS, then we can also each independently verify that what one person sees isn't just a quirk of their particular configuration, version of Eclipse, JVM etc. The comparative numbers I attached above were generated using a simple TestCase that can be found under the test folder in CVS as org.eclipse.jst.jsf.validation.el.tests plugin under the perf.StressTest class. I will be adding further tests here but if anyone wants to use it as the basis for further performance benchmarks, it will be very helpful in targeting our efforts in terms of continuing to improve performance.
Defer. We will continue to work on this in the post-2.0 stream.
(In reply to comment #24) > Defer. We will continue to work on this in the post-2.0 stream. We are in the shutdown mode for thew WTP 2.0 release and hence deferring the remaining work. If this is a show stopper for 2.0 release, please respond.
Ok, creating jsf symbols for methods/properties from jdt objects *does* take significant time (Unfortunately, I don't remember 'exact' figures, but it was considerably more time than getting all the jdt objects). So implementing Zacharias' version for creating only the symbols actually needed (and caching them) will probably improve performance noticeably in case there are many properties/methods not accessed by jsf, even though the lookup will be a bit more inefficient because caching is not guaranteed to be complete. This change, however, is too big for me to implement right now. But I did look at why creating jsf symbols is so costly, and found out that considerable time is spent in the various resolveSignature-/resolveType methods in TypeUtil, so I tried to improve that.
Created attachment 74137 [details] improve resolveType/resolveSignature This patch improves resolving of unresolved type signatures. First, it tries to avoid it where possible (by caching; when comparing with other types used within the same source file). Then, trying to resolve a signature of a primitive type will quickly return the signature instead of trying to resolve it (previously, I think resolving it was even failing, and certainly took time). Detecting resolved signatures will also work for resolved array signatures now. And finally, type signatures of methods will be resolved against the type the method is declared in, not against all supertypes of the type the method is encountered in. This is faster and even "correcter", since a subtype of the declaring type could import a type with the same simple name from another package. Overall, behaviour should be faster *and* correcter. This is probably not my "final" version (but close to it), since there are two details I still would like to improve, but I don't know when I will have time for it...
A change made within the last few days clearly improved memory consumption while validating, but slowed JSF Semantic Validator down by factor 2 to 3. Seems that DefaultDTVariableResolver.resolveVariable(DTFacesContext context, String name, IAdaptable externalContextKey) eats up all the time. No matter what the reason for the slowdown is, creating a map for all three scopes for every single occurrence of a managed been seems rather expensive. Does the result of DefaultDTVariableResolver.resolveVariable(...) change with the position within a given jsp file? (Don't know, where the variables introduced by tags like h:dataTable, which are valid only within this tag, come in.) If not, I think we should consider caching all symbols for a given jsp file while validating that file, since resolving a variable will be done very often.
Please review for a fix in 2.0.1.
> Does the result of DefaultDTVariableResolver.resolveVariable(...) change with > the position within a given jsp file? (Don't know, where the variables Unfortunately it does. Although we don't currently support it properly, we must provide for this situation where such a tag removes its variable when the tag ends.
Moving this to 3.0 since we will not be able to handle this change in the 2.0.1 time frame. We need to add tests, measure performance etc.
I have confirmed a degradation of EL validation performance in 2.0.1 (see below). I have narrowed the problem to a fix to dispose properly of held resources in the JSPModelProcessor. The result of the fix is that JSP documents are being re-parsed for tag-declared variables once for every EL variable lookup because the JSPModelProcessor is created and immediately disposed on each call. I am looking into several performance fixes for this, including changing the way the JSPModelProcessor's are cached as well as making a symbol resolver instance that can cache variables and properties between calls (this second one was suggested previously) when is known that these have not changed. =================================================== Report for performance test: Stress Variable Resolver Number of iterations: 100000 =================================================== Max: 2,824,050,822, Max Index: 0 Min: 2,814,883, Min Index: 59,547 Avg: 4,984,011, StdDev: 11,287,121.469, StdDev Ignore Max/Min: 6,922,982.534 =================================================== =================================================== Report for performance test: Stress Property Resolver Number of iterations: 1000 =================================================== Max: 1,824,049,705, Max Index: 0 Min: 37,994, Min Index: 781 Avg: 1,959,677, StdDev: 57,653,905.193, StdDev Ignore Max/Min: 1,989,226.244 =================================================== =================================================== Report for performance test: Stress Simple Bean Property Validation Number of iterations: 1000 =================================================== Max: 3,741,252,589, Max Index: 0 Min: 7,453,461, Min Index: 788 Avg: 17,062,705, StdDev: 118,066,942.526, StdDev Ignore Max/Min: 8,373,785.751 ===================================================
For last several months I was working on project with relatively high number of JSPs (about 250). All of them are mid-size with 30-50 ELs per page (up to 100). Full project JSF validation took about 2-3 minutes to complete with WTP 2.0 and that time is really acceptible from my point of view. I.e. problems it helps to diagnose do worth waiting several minutes. As of recent changes validation time increased to 40 minutes-1 hour. IMHO, it's too long to complete for operation that can be fired several times a day. Maybe WTP2.0 performance level should be taken as standard measure to deside wheather current JSF validation speed will be acceptible for end-user? Or it worth disabling that validation on full rebuild by default. P.S. You should take my opinion with a grain of salt, since it is *very* subjective.
We continue to try to address performance issues. Deferred post-3.0.
(In reply to comment #32) > I have confirmed a degradation of EL validation performance in 2.0.1 (see > below). I have narrowed the problem to a fix to dispose properly of held > resources in the JSPModelProcessor. The result of the fix is that JSP > documents are being re-parsed for tag-declared variables once for every EL > variable lookup because the JSPModelProcessor is created and immediately > disposed on each call. > > I am looking into several performance fixes for this, including changing the > way the JSPModelProcessor's are cached as well as making a symbol resolver > instance that can cache variables and properties between calls (this second one > was suggested previously) when is known that these have not changed. > > > =================================================== > Report for performance test: Stress Variable Resolver > Number of iterations: 100000 > =================================================== > Max: 2,824,050,822, Max Index: 0 > Min: 2,814,883, Min Index: 59,547 > Avg: 4,984,011, StdDev: 11,287,121.469, StdDev Ignore Max/Min: 6,922,982.534 > =================================================== > > =================================================== > Report for performance test: Stress Property Resolver > Number of iterations: 1000 > =================================================== > Max: 1,824,049,705, Max Index: 0 > Min: 37,994, Min Index: 781 > Avg: 1,959,677, StdDev: 57,653,905.193, StdDev Ignore Max/Min: 1,989,226.244 > =================================================== > > =================================================== > Report for performance test: Stress Simple Bean Property Validation > Number of iterations: 1000 > =================================================== > Max: 3,741,252,589, Max Index: 0 > Min: 7,453,461, Min Index: 788 > Avg: 17,062,705, StdDev: 118,066,942.526, StdDev Ignore Max/Min: 8,373,785.751 > =================================================== Any update on this problem? Is it still in R3.0.5/R3.1? As a WTP/JSF adopter, we received a lot complaints on the jsf view validator performance when validating a large project. Any chance to raise the importance of this bugzilla? Thanks.
We will review this for Helios. If possible we will make incremental improvements in the 3.1.2 time frame.
Thanks for scheduling this for some investigation. This has become important to us as more consumers of our product have more and more projects based on JSF. In one particular case a customer sees quite a dramatic increase in validation time when the JSF View Validator is enabled. They have a workspace with 17 JSF based projects (portlets) with roughly 25 jsp files per project. With all validators enabled, the time taken on initial validation of this workspaces is about 8:40. If just the JSF View Validator is disable this initial validation is about 3:30. Now on subsequent workspace validations the times are better, 4:35 vs. 2:35 but still the JSF View Validator is almost doubling the time for validation. The patch provided in 290412 did get us about a 16% improvement on the initial validation (down to 7:20) but there was no improvement for subsequent validations. Would providing a test project be helpful for working on this issue?
(In reply to comment #37) > Would providing a test project be helpful for working on this issue? Yes, it will help.
Created attachment 149562 [details] Performance Test projects (Part1) The zip file contains 10 projects. Each project has 40 jsp pages which contain 1800 EL expressions (600 resource bundle expressions and 600 var expressions). The jars are removed, so please copy the jsf-api.jar, jsf-impl.jar and tomahawk.jar to the WEB-INF/lib folders after import. I disabled all other validators except jsf view validator, restart the workbench, and validate the 10 projects. The first validation took 5 min 40 sec and the second took 3 min 28 sec. Due to the size limitation, I separate the zip file into two parts.
Created attachment 149563 [details] Performance Test projects (Part2)
Mass update of Helios bugs
