Comment 1 Cameron Bateman 2008-04-28 15:20:48 EDT

Is there any reason this can't be deferred to the maintenance release?  The noticeable problems with our performance are well understood and are not related to the framework.  How will this improve our performance?

Comment 2 Nitin Dahyabhai 2008-04-30 01:46:24 EDT

The startup time retrieving the structured models can only be reliably shared with the V2 extension using groups.

Comment 3 Cameron Bateman 2008-04-30 16:21:10 EDT

(In reply to comment #2)
> The startup time retrieving the structured models can only be reliably shared
> with the V2 extension using groups.

I'm not sure I understand.  Is this to do with loading and unloading shared models?  Is V2 holding them in memory until all threads complete?  What is the measured savings?  Is migration doc'd somewhere? 

Comment 4 Nitin Dahyabhai 2008-04-30 17:40:37 EDT

V2 doesn't do anything special about sharing models in memory, but validators that use SSE models each load and unload the model for a resource on their own, which takes time that's better spent actually validating.  V2 validators run in sequence over the same resource, and our listener (see bug 227452) can get the model before the validators run and release it after they're done for that one resource.  The net effect is that regardless of the number of validators (which for JSP files is >2), you only really build the model up once.  The validator code itself should still do its get/release as before, but declaring itself a member of the org.eclipse.wst.sse.core.structuredModelGroup group lets it benefit from those savings.  I don't have a link to the migration doc, but it's not a difficult change to make.

Comment 5 Cameron Bateman 2008-04-30 17:46:52 EDT

> I don't have a link to the migration doc, but it's
> not a difficult change to make.

Can you point me at an example of what needs to be done?  Is it simply a plugin.xml change or are there other code changes?

Comment 6 Raghunathan Srinivasan 2008-05-20 14:25:21 EDT

Deferred for 3.0. We will pick this up in the maintenenace stream.

Comment 7 Yury Kats 2008-07-18 22:39:36 EDT

Can you consider this for 3.0.1? We see a considerable hit performance, especially in memory being allocated, when validating workspaces with a large number of JSPs. Using V2 allows for creating only one in-memory model for each JSP, while V1 validators create a model each.

Adding a jsp validator to org.eclipse.wst.sse.core.structuredModelGroup group, as Nitin mentioned, represents a significant performance improvement for projects/workspaces with lots of JSPs.

Comment 8 Cameron Bateman 2008-07-21 13:37:05 EDT

Let's consider it tentatively.

Comment 9 Raghunathan Srinivasan 2008-07-23 10:39:19 EDT

We would like to consider this for RC2.

Comment 10 Xiaonan Jiang 2008-07-24 12:29:25 EDT

Created attachment 108370 [details]
Tentative fix

I tried to port the validators to v2 framework.
For JSF view validator, since it extends from JSP validator which has been ported to v2, I only updated plugin.xml - used v2 extension to replace the old extension and its source validation extension. Also, specify the "group" attribute so that it can share sse model with other validators.
For Facesconfig validator, I modified the extension in plugin.xml and also some code in the validator.

Comment 11 Cameron Bateman 2008-07-24 18:41:38 EDT

(In reply to comment #10)
> extension and its source validation extension. 

Thanks for the patch Xiaonan.  Does the V2 validator have built-in support for SSE validation?  I'm still learning about the new framework.

Comment 12 Nitin Dahyabhai 2008-07-28 10:16:17 EDT

Looks to me like it was taken out of plugin.xml, possibly by accident.

Comment 13 Gary Karasiuk 2008-07-28 11:58:59 EDT

(In reply to comment #12)
> Looks to me like it was taken out of plugin.xml, possibly by accident.
> 
Nitan, What do you mean?

Comment 14 Cameron Bateman 2008-07-28 14:14:10 EDT

(In reply to comment #13)
> (In reply to comment #12)
> > Looks to me like it was taken out of plugin.xml, possibly by accident.
> > 
> Nitan, What do you mean?

Assuming I add the source validator back in, does the change look like it will solve the problem?  I can't find any sort of migration or design doc to ensure that I understand what I'm doing here.  

Otherwise, if all my regressions run ok, I'm fine commit this change with the source validation added back.

Comment 15 Nitin Dahyabhai 2008-07-28 19:04:43 EDT

Cameron, check out the presentation attached to bug 212196 comment 1.  This pretty much follows the migration steps around slide 13.

Comment 16 Xiaonan Jiang 2008-07-29 11:07:33 EDT

(In reply to comment #11)
> (In reply to comment #10)
> > extension and its source validation extension. 
> 
> Thanks for the patch Xiaonan.  Does the V2 validator have built-in support for
> SSE validation?  I'm still learning about the new framework.
> 

My understanding from the V2 extension point document was that it support source validation. So I removed the source validation extension in plugin.xml.

Comment 17 Cameron Bateman 2008-07-29 13:25:02 EDT

Created attachment 108675 [details]
Updates 108370

-keeps source validation
-fixes marker id
-cleanup on AppConfigValidator
-fix validation test suite data

Comment 18 Cameron Bateman 2008-07-29 13:34:26 EDT

1. Explain why you believe this is a stop-ship defect. Or, if it is a "hotbug" 

Creates a performance improvement requested by adopters.

2. How has the fix been tested? 

Existing JUnit tests provide sufficient coverage since feature is not changing, just the framework being used.  Also performed manual functional regression test in sandbox to ensure expected routine use cases still works.  One test zip that provides project data for validation testing has been updated.

3. Give a brief technical overview. Who has reviewed this fix?

The AppConfig and JSF View validators have been updated to use the V2 Validation Framework.  This involves migrating the extension point used and updating AppConfigValidator to use the new AbstractValidator class.  The JSF View validator is based on the JSPValidator which has already been migrated.

4. What is the risk associated with this fix? 

Low.  The V2 validator framework has already been adopted by several other WTP components including the JSP framework that one of the effected validators is based on.

Comment 19 Raghunathan Srinivasan 2008-07-29 13:46:08 EDT

Approved and submitting for PMC review. 

Performance improvement bug. See comment #7 for the need for this fix.

This is significant code change but as explained in comment #18 ,  the V2 validation framework has been adopted and tested, the changes in this patch has been tested with junits and this mitigates any risk.

Comment 20 Raghunathan Srinivasan 2008-07-29 18:54:05 EDT

Approving this bug for checkin on my vote. We want to get this into the build. 

Comment 21 Cameron Bateman 2008-07-29 19:01:48 EDT

Patch 2 applied at HEAD (3.0.1RC2 target).

Comment 22 Xiaonan Jiang 2008-08-05 11:55:36 EDT

Why the fix is not picked up by wtp 0731 build?

Comment 23 Cameron Bateman 2008-08-05 12:04:32 EDT

Good catch!  It appears it wasn't labeled for release.  We will pick it up in RC3.